Commit Graph

24 Commits

Author SHA1 Message Date
Brian Osman
e12939f0d8 Unit tests for runtime effect SkSL errors found later in compilation
Forgot to add these when I fixed the bug. Verified that these tests
failed without the fix.

Bug: skia:10593
Change-Id: Ia20fad3cd8e5b0f63ca19946b8314eed49bec2bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309716
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-12 19:40:21 +00:00
Brian Osman
a4b9169fb6 Remove 'in' variables from SkRuntimeEffect
Runtime effects previously allowed two kinds of global input variables:
'in' variables could be bool, int, or float. 'uniform' could be float,
vector, or matrix. Uniform variables worked like you'd expect, but 'in'
variables were baked into the program statically. There was a large
amount of machinery to make this work, and it meant that 'in' variables
needed to have values before we could make decisions about program
caching, and before we could catch some errors. It was also essentially
syntactic sugar over the client just inserting the value into their SkSL
as a string. Finally: No one was using the feature.

To simplify the mental model, and make the API much more predictable,
this CL removes 'in' variables entirely. We no longer need to
"specialize" runtime effect programs, which means we can catch more
errors up front (those not detected until optimization). All of the API
that referred to "inputs" (the previous term that unified 'in' and
'uniform') now just refers to "uniforms".

Bug: skia:10593
Change-Id: I971f620d868b259e652b3114f0b497c2620f4b0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309050
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-08-10 22:00:44 +00:00
Brian Osman
269b21c501 SkRuntimeEffect: Apply uniform transforms in color filters, too
We were only respecting this feature in runtime shaders. Note that use
of any tagged matrices will cause color filter creation to fail, but
color transformation is a totally sensible thing to want in a color
filter.

Change-Id: I482226b287ab794cb341367fce453381cb581966
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308507
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-06 18:46:17 +00:00
Brian Osman
92aac1e1b3 Disallow runtime effect color filters that depend on position
Some of these checks are currently redundant (we don't allow color
filters to have children right now). But the next CL will re-add that
capability, and the unit tests here will ensure we don't re-break things
by allowing child-sampling to violate the color filter invariant.

Change-Id: I54c10d8b1d1e376c13347296765185d42b9f644a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308285
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-06 16:06:47 +00:00
Brian Osman
82329004d0 SkSL: Disallow fragmentProcessor variables in several places
We only want SkSL to contain fp variables at global scope, and to sample
directly from references to those global variables. Anything else will
confuse our sample-usage analysis, and often leads to asserts in the CPP
or pipeline-stage generators.

Bug: skia:10514
Change-Id: Ie1ef10821c1b2a946a92d050fea45d95569bc934
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304599
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-07-31 20:09:14 +00:00
Robert Phillips
e94b4e1b49 Update more tests to use the GrDirectContext/GrRecordingContext pair
This should be the last batch of tests. All the remaining uses of
GrContext should be resolved when SkImage no longer requires a context.

Change-Id: I47eeb3b74c28f483c20d9bec4daecbdb6d2cb982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305541
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2020-07-27 15:08:31 +00:00
Brian Osman
b5f0f52831 SkSL: Salt inline identifiers in PipelineStage and FragmentProcessor passes
Fixes a bug with name conflicts in the final SkSL.

Bug: skia:10526
Change-Id: Ic238f89dd778c186e775ecbaabfbaed9e426274f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2020-07-24 19:23:20 +00:00
Brian Osman
6241961a32 Add several more unit tests of runtime effects
Did some cleanup to remove repetition that was distracting from the
thing being tested.

Change-Id: Ie385c6ec2d1325a1bd0ba5c2270e7f2ddd1d24b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305076
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2020-07-22 21:12:21 +00:00
Brian Osman
182c92ebb3 Catch SkRuntimeEffects w/o "main" earlier
Previously, these would produce a "valid" effect object, but it wouldn't
draw anything.

Bug: oss-fuzz:24070
Change-Id: I17d0ed1710196853da0694cac9f4c260312700a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304064
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-07-20 20:48:04 +00:00
Brian Osman
e64ae86d27 Rearrange logic in SkRuntimeEffect::Make
To extract metadata and validate the shader, we've added several
iterations over all program elements. This CL rearranges things
to iterate once (*). Variable conversion is moved to a separate
loop later, to help with nesting and readability.

Removes hard-to-read asserts. These were validating things enforced
by both the IR generator and unit tests.

*: Technically, there are additional implicit iterations when we call
   SkSL::Analysis functions. Folding all of this into a single pass
   would be even better, but much more complicated.

Change-Id: I4f5aa649e74094e94c365ad20ef2ac96082285cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303924
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2020-07-20 19:33:33 +00:00
Robert Phillips
6d344c3069 Update unit tests to accept GrDirectContext
This CL makes it explicit that the unit tests always get a direct context.

It is mainly a mechanical CL.

Change-Id: I49e0628851d9c81eb47386ef978edf905c6469d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/299866
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2020-07-06 15:45:12 +00:00
Brian Osman
5aaaeea4da Supply device and local coords to SkShader_Base::onProgram
Use that to add support for sk_FragCoord in SkRuntimeEffect.

Change-Id: I587ad97057c13ec8a4052c7c20f655eae88786ba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/298504
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-06-23 22:00:20 +00:00
Michael Ludwig
22534f2098 Expose sk_FragCoord for runtime effects
This is needed for the inline dither effect to use device coordinates
instead of local coords for dithering.

The builtin is not "in float4" because that trips up the runtime effect
assertions about what types of in variables are allowed.

Change-Id: I580fc461fdc9cbd812592b2571f51868a7a3ea4b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292262
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-05-27 23:06:09 +00:00
Michael Ludwig
5e6b3cd259 Store float value for settings in SkSL v2
If you used 'in float foo' in a runtime effect it would always have the
value cast to an int. I don't think we saw this in .fp files because
the cpp generation handled those values directly (if I remember correctly)

Just uses a union of float and int, differentiated by fKind, so that it
compiles in standalone mode (vs. using SkFloat2Bits, etc.).

Also updated to add a unit test.

Change-Id: I420f43d1b54638883af0b8df6ccba2416c587868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292315
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2020-05-27 22:15:19 +00:00
Brian Osman
d9bde07f1e Add SkRuntimeShaderBuilder, clean up SkRuntimeEffect API a bit
Utility class for getting named access to uniforms and children of an
SkRuntimeEffect (also functions as an example of using the
SkRuntimeEffect public API).

Moved several internal SkRuntimeEffect functions to private, and added
findInput/findChild helpers.

Change-Id: I8c2e7745ea81670a49b7ab2f51ce44a8d8169278
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/286516
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2020-05-01 20:02:35 +00:00
Brian Osman
5f6b41e6d8 Runtime effects: Detect calls to undefined functions
Previously, the CPU backend would catch this later (ByteCodeGenerator),
and the GPU backend would assert in expandFormatArgs (out-of-range
function index).

Change-Id: Ib84bf7c477ddcc9fd3091c5b106ebdd654e9a30b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/276000
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-03-10 20:27:10 +00:00
Brian Osman
7353dc5490 Change SkSL main() from (float x, float y) to (float2 p)
Change-Id: Id046199edd63535ef07e1dfa65fbc7c0f8cefd00
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269371
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-02-07 20:19:13 +00:00
Brian Osman
6f5e94089e Clamp GrSkSLFP output to valid premul
Added this as an option to GrSaturateProcessor (also renamed it to
be more generic and end with FragmentProcessor).

Added a tweak to the unit test to check the new behavior.
(Raster was already doing the clamp).

Change-Id: Ic49fa5cd72b6c63430fb773baf8121546bf2b80d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/265580
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-01-22 16:10:04 +00:00
Brian Osman
504032e575 SkRuntimeEffect: Fix 'in' variables in CPU backend
We were never calling specialize() to bake in the values of ins,
so do that. Add uniformSize() to get the size of just the uniform
values. (The interpreter asserts that the size of the uniforms
being passed in matches the expected size from the ByteCode,
so these need to match up).

Added a unit test that uses both 'in' and 'uniform'.

Change-Id: I595822171211d35a17d5977fa790de0d1bbd6c78
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/263519
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-01-10 15:46:22 +00:00
Brian Osman
f72deddb89 Expose the Input variable and Child name collections in SkRuntimeEffect
Add framework for unit tests that draw (CPU and GPU) with a runtime
shader, as well as couple example tests.

Change-Id: I43b3b39e86634ec55521a2689a4c55c21939dce5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/262809
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-01-08 19:23:13 +00:00
Brian Osman
8783b78b38 Add a few more SkRuntimeEffect unit tests
Validating that only certain types are allowed as in or uniform.

Change-Id: I941da448bffec06158e67fbf653833846d9fe3db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/262222
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-01-06 20:32:54 +00:00
Brian Osman
ee426f223f Move SkRuntimeEffect.h to include/effects
Change-Id: I0b11d4210c6e663cfb4854fc33e1396fd79fe9a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261780
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-01-02 17:37:46 +00:00
Brian Osman
82d49704ce More SkRuntimeEffect tests
- 'in' variables can't be arrays (applies to all SkSL)
- 'in' and 'uniform' variables are restricted to a fixed
  list of types.

Change-Id: I957ce1ad33aaf6b5970ca7204c568bb533bc86d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261436
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-12-30 19:26:06 +00:00
Brian Osman
088913a63b Start adding unit tests of SkRuntimeEffect
- Change SkRuntimeEffect::Make so *it* can fail, and returns
  [Effect, ErrorText].
- Initial tests just test for expected failure conditions.
  Next steps are to add tests for effects that should work,
  and to validate results on CPU and GPU.

Change-Id: Ibac8c3046104577434034263e9e4a4b177e89129
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261095
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2019-12-20 14:15:06 +00:00