Commit Graph

919 Commits

Author SHA1 Message Date
John Stiles
7bd7ea2c8e Prevent no-op statements in GLSL code generator.
This eliminates a handful of useless statements. They were harmless, but
presumably this saves optimization work for the GLSL compiler.

(Patterned after http://review.skia.org/496377 )

Change-Id: Ibe135750488a9917b982dcac67f22b3765412ee1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496599
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-01-19 17:36:54 +00:00
John Stiles
3dcaf4ac16 Prevent no-op statements in Metal code generator.
These are harmless, but they can cause the Metal compiler to emit a
warning, and these warnings are making some logs messy for Flutter.

Change-Id: I21b7a3c9ae661c55bbbe8bc13d097966180e977d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496377
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-01-19 17:34:59 +00:00
Julia Lavrova
55c215cfb6 Reland "Better Matrix/Scalar testing"
This reverts commit 455e580b9c.

Reason for revert: Fixing the build break

Original change's description:
> Revert "Better Matrix/Scalar testing"
>
> This reverts commit abb611550e.
>
> Reason for revert: Build break
> Original change's description:
> > Better Matrix/Scalar testing
> >
> > Adding tests for matrix math and comparison
> > bug: skia:12681
> >
> > Change-Id: Ia1537ee2e411383749456fd6ff938b7c9a2e1061
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493416
> > Reviewed-by: John Stiles <johnstiles@google.com>
> > Commit-Queue: Julia Lavrova <jlavrova@google.com>
>
> Change-Id: I70871b4b75c1f10e870dc5e884a42405a80fc0f9
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494816
> Reviewed-by: John Stiles <johnstiles@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Julia Lavrova <jlavrova@google.com>

Change-Id: Idef8dbcd6f5a5bbe84d3fd86888e3eab0f0521ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494817
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-14 19:12:00 +00:00
Julia Lavrova
455e580b9c Revert "Better Matrix/Scalar testing"
This reverts commit abb611550e.

Reason for revert: Build break
Original change's description:
> Better Matrix/Scalar testing
>
> Adding tests for matrix math and comparison
> bug: skia:12681
>
> Change-Id: Ia1537ee2e411383749456fd6ff938b7c9a2e1061
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493416
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Julia Lavrova <jlavrova@google.com>

Change-Id: I70871b4b75c1f10e870dc5e884a42405a80fc0f9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494816
Reviewed-by: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-13 18:52:04 +00:00
Julia Lavrova
abb611550e Better Matrix/Scalar testing
Adding tests for matrix math and comparison
bug: skia:12681

Change-Id: Ia1537ee2e411383749456fd6ff938b7c9a2e1061
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493416
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-13 15:35:12 +00:00
John Stiles
866db186fa Simplify control flow in switch test to unbreak iOS.
This looks like the GLSL driver in iOS generates wrong results when
returning a value from inside a switch.

Change-Id: I478a045c64c3dae9824f86f52e0c7f8f9685c9af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494476
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-01-13 14:49:06 +00:00
John Stiles
8522bf910a Add test for switch-case folding.
Change-Id: I3dabd77890a73ea054bb57d466a6ed8273eae3e8
Bug: skia:12811
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494196
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-01-12 22:24:55 +00:00
John Stiles
44c00ae64a Add test for vector constant folding.
Change-Id: Iecf1313af5f2938cb899f2a3e750ffc04554bae0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-01-12 17:23:20 +00:00
Julia Lavrova
a856f40086 Interface blocks no longer allow duplicate fields
bug: skia:12793
Change-Id: Idccdab5ee8f1c7792bdfe98efd379d0199a65377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492397
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-07 16:44:08 +00:00
Julia Lavrova
ae5f86ea1a Test: Interface blocks allow duplicate fields
bug: skia:12793
Change-Id: Ie47b1df381779ed24cab8b05b1ee6947bda82516
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492396
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-07 15:55:58 +00:00
Julia Lavrova
2f3891bf76 Duplicate field names in struct
bug: skia:12712
Change-Id: I61a4512c10bf91d6396c38f148a714492669a9e0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491820
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-06 22:44:16 +00:00
Julia Lavrova
ce49ff6520 Test: duplicate fields in the same struct allowed
As @johnstiles suggested I add the test first and the fix after.
bug: skia:12712

Change-Id: I9316cf40f71e756fc1730ee630bc0d0377f200d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
2022-01-06 19:40:50 +00:00
John Stiles
3c233c7f73 Update comments in FirstClassArrays test.
We used to reject ES3-style array declarations in strict-ES2 mode, so
this test originally expected two errors.

Change-Id: I17f71630076cda4b37b7723225dcff951eba9dcc
Bug: skia:12410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-01-06 18:53:34 +00:00
Ethan Nicholas
4cd3ae4009 Revert "Fixed SkSL error reporting on array types"
This reverts commit 6e686b8b8b.

Reason for revert: After internal discussion, we established that nobody was actually sure why this had needed to be an error in the old parser in the first place, so there does not appear to be a reason to carry the behavior forward.

Original change's description:
> Fixed SkSL error reporting on array types
>
> The DSLParser was not reporting errors when the array type appeared
> before the variable name (float[2] x) as opposed to after (float x[2])
> in strict ES2 mode.
>
> Bug: skia:12410
>
> Change-Id: Ia388aa150f65916dc3ccc58f7680dbde0a636c5f
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491819
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:12410
Change-Id: I355fd1ad89e2e64b0377be7672b7f3f824eebac8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491996
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-01-06 16:53:17 +00:00
Ethan Nicholas
6e686b8b8b Fixed SkSL error reporting on array types
The DSLParser was not reporting errors when the array type appeared
before the variable name (float[2] x) as opposed to after (float x[2])
in strict ES2 mode.

Bug: skia:12410

Change-Id: Ia388aa150f65916dc3ccc58f7680dbde0a636c5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491819
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-01-06 16:28:47 +00:00
Ethan Nicholas
f393067c37 Made SkSL type aliases into first-class objects
Previously, type aliases ('vec2') were just an additional name which
could be used to refer to a type ('float2'). This was simple and worked,
except that error messages would be wrong - any type-related error
message would refer to the type as 'float2' rather than the 'vec2' that
the user actually typed.

This CL adds an AliasType class so that we can track which name was
used to refer to an aliased type and report messages using the correct
type name.

Bug: skia:12737

Change-Id: I40e234239ab47557033e0695e4fbbd5f01da354e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/490256
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-01-05 16:20:19 +00:00
Brian Osman
735ff421bb Reject #extension in runtime-effect mode
Bug: oss-fuzz:43062
Change-Id: I10d8fa40c81c5b1595d30221d89c84f5cc3478fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/490857
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-01-04 16:12:50 +00:00
John Stiles
bab224e1c2 Prevent interface block members from using invalid names.
Interface blocks now guard against naming their member variables with
built-in type names like "float" or "bool".

Change-Id: Ia767542ace76fb8fbc2d50c81772b7f54b1bf973
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-28 20:57:18 +00:00
John Stiles
aa09c782a8 Add test for structs/interface-blocks with invalid member names.
Structs already handled this appropriately, but interface blocks did not
guard against naming their member variables built-in type names like
"float" or "bool".

Change-Id: I12ec054b3f158b83e35031449cf2a088ff8d0dc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489596
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-12-28 20:55:32 +00:00
John Stiles
f604efbeb4 Add support for anonymous function parameters in SkSL.
Anonymous function parameters are now automatically assigned a name,
"_skAnonymousParamN", where N is the parameter index.

Change-Id: I87adcd51ed025c76ae2b333317f21b523a4632b4
Bug: skia:12769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489538
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-28 20:47:05 +00:00
John Stiles
1c7d442b52 Add test containing anonymous function parameters.
SkSL will reject ES2-compatible code because function parameters always
require a name in SkSL. (A followup CL relaxes this restriction and
allows anonymous parameters in SkSL.)

Change-Id: Ifdcf0fcbe0f52d16007c018b545631ca4033a8c4
Bug: skia:12769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489537
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-12-28 20:08:56 +00:00
John Stiles
106776364d Prevent structs/interface-blocks from claiming builtin types.
Structs and interface blocks allow a trailing identifier which is added
to the symbol table. This identifier is now prohibited from
overlapping built-in types.

Change-Id: I33b9d6156a27ce017e6744a05979748c04a04767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489516
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-28 19:32:22 +00:00
John Stiles
48432e133e Add test demonstrating struct/interface-block name conflict.
Structs and interface blocks allow a trailing identifier which is added
to the symbol table. This identifier should be prohibited from
overlapping built-in types; at present, this is not checked. Add a test
demonstrating the issue.

Change-Id: I99aa915c1715c468cc369c97b7f12e031b86ea4a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/489496
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-12-28 19:26:17 +00:00
John Stiles
71f7880bb6 Emit trace_scope ops from SkVM code generation.
These will give the debugger enough information to discard variables as
they fall out of scope.

Change-Id: Ia400e82a3ca9cf0a51a72d819f897d346979226c
Bug: skia:12741
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484556
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-15 01:05:34 +00:00
Brian Osman
fde20db7ca Rename SkSL's 2D cross product builtin function
Note that the 2D cross product isn't defined. There are at least two
possible interpretations of what that might mean. This name makes it
clearer that we're asking for the length of the resulting vector, if
we computed the 3D cross product (assuming Z == 0 for both vectors).

It also eliminates name overlap between builtin functions and actual
intrinsics.

Change-Id: I24e8bc0ab2ec91aaace20f0dd3e8565c10bd44a0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/484440
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-12-14 22:00:26 +00:00
John Stiles
b1a97caf71 Revert "Trace function return values after function-exit."
This reverts commit 85cc1bece7.

Reason for revert: ends up not being useful after all

Original change's description:
> Trace function return values after function-exit.
>
> This will allow function return values to be easily seen when stepping
> "over." This has the unexpected side benefit of generating slightly
> fewer ops when a function has unoptimizable conditional returns.
>
> Change-Id: I48d23de635d3caaddff91aa595593d0371dfcdcb
> Bug: skia:12708
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481076
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>

Bug: skia:12708
Change-Id: I61ebb175b60d2060f6ad21b170238b37557b80be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482458
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-10 18:06:04 +00:00
John Stiles
83b5237b53 Fix assertion when debug-tracing a void-return function.
The debug-slot code didn't expect to encounter a void type.

Change-Id: Ied452b51e1cf90a0c0bc24770f82e711105b8e82
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482461
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-10 14:39:05 +00:00
John Stiles
3856a5854e Revert "Add SkVM op trace_done to indicate completion of debug tracing."
This reverts commit 062652067b.

Reason for revert: design change removes need for this op

Original change's description:
> Add SkVM op `trace_done` to indicate completion of debug tracing.
>
> This op can be used to invoke a callback function and dump the log to
> disk when it is ready. SkRuntimeEffect doesn't have any other viable
> mechanisms for detecting that a paint has completed, AFAIK. We can
> wait for ~SkRTShader to occur, but there's no guarantee that this will
> happen quickly, and the SkPaint with the SkRTShader shader can be reused
> over and over again.
>
> Unlike other trace ops, this only needs a trace mask, not an execution
> mask (we are unconditionally done at the end of main).
>
> Change-Id: I6f7ee41f2005b65940d36dee892279d4f245509f
> Bug: skia:12708
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479876
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: skia:12708
Change-Id: Ic4c4f5dd72541195f07ca32035267a20a82536e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481577
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-12-08 17:49:56 +00:00
Brian Osman
8c3d183cc2 Rename SkSL 'srgb_unpremul' to just 'color'
Added comments to explain the semantics (both what's expected when you
set the uniform, and what you see in the shader). The old name was
confusing, because it sounded like you got an sRGB color in the shader.
This is terse, but I think it's the cleanest syntax - and for embedding
clients, they can use C++ (etc.) API to require that color uniforms are
assigned from color types.

Bug: skia:10479
Change-Id: If00ea754060494aaa83001a5b357687953de8a5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480577
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-12-07 17:56:27 +00:00
John Stiles
85cc1bece7 Trace function return values after function-exit.
This will allow function return values to be easily seen when stepping
"over." This has the unexpected side benefit of generating slightly
fewer ops when a function has unoptimizable conditional returns.

Change-Id: I48d23de635d3caaddff91aa595593d0371dfcdcb
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481076
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-12-07 16:32:44 +00:00
Julia Lavrova
4388f16262 Fixing comparison for structs and arrays
Recursive comparison with tests

Bug: skia:12642
Change-Id: I344812a26f9af7b8a904c6faf6319df86ce57d03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472997
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-12-06 20:46:41 +00:00
John Stiles
062652067b Add SkVM op trace_done to indicate completion of debug tracing.
This op can be used to invoke a callback function and dump the log to
disk when it is ready. SkRuntimeEffect doesn't have any other viable
mechanisms for detecting that a paint has completed, AFAIK. We can
wait for ~SkRTShader to occur, but there's no guarantee that this will
happen quickly, and the SkPaint with the SkRTShader shader can be reused
over and over again.

Unlike other trace ops, this only needs a trace mask, not an execution
mask (we are unconditionally done at the end of main).

Change-Id: I6f7ee41f2005b65940d36dee892279d4f245509f
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479876
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-12-06 18:44:27 +00:00
Julia Lavrova
94f726ae62 Adding test files demonstrating type confusion for arrays/structs.
Change-Id: Iacd924471971c3c551648f75c1ecf0f1bfd3ed07
Bug: skia:12642
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479061
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-12-03 20:54:36 +00:00
John Stiles
d4713ad528 Move trace hooks into the skvm::Builder.
I ran into a snag while trying to hook up SkRuntimeEffect with debug
tracing. Runtime effects only have access to a skvm::Builder, and are
never exposed to the full skvm::Program. SkVMBlitter is responsible for
assembling the full skvm::Program, but is oblivious to runtime effects
and nested skvm sub-programs. Additionally, multiple runtime effects can
(and often do) coexist within a paint.

This CL changes how debug traces are enabled. skvm::Program no longer
has a `attachDebugTrace` method. Instead, this method lives on the
skvm::Builder. Calling `attachDebugTrace` generates a "trace-hook ID"
(which is actually an index into a vector of TraceHook pointers).
Every trace opcode now includes this trace hook ID. When the Builder
assembles a final Program, it copies the TraceHooks into the Program.
The skvm interpreter uses the trace hook ID on the op to dispatch a
trace command to its associated TraceHook.

From a user perspective, this doesn't change very much, but it does
mean that the SkVM Code Generator now supplies a TraceHook for us
(since it adds trace ops to the Builder and needs to know the proper
trace-hook ID).

Change-Id: I8bd5fea24f477f81470fae8ba41be45f76949407
Bug: skia:12708
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479597
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-12-03 20:09:22 +00:00
John Stiles
d7c3b21b79 Revert "Enable various switch tests in ES2 mode."
This reverts commit ffe365eb4d.

Reason for revert: crash inside IntelHD405 Vulkan driver
http://screen/6ugVDdjJpqDkxX6

Original change's description:
> Enable various switch tests in ES2 mode.
>
> switch is no longer an ES3-specific feature.
>
> Change-Id: Ic878a77268e517e17699c2e35a37da6b0a7765dd
> Bug: skia:12450
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452320
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: skia:12450
Change-Id: Id6f32a084f1bc7b2b3a1e5fb0b82d2011e4ba780
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/479059
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2021-12-02 19:03:11 +00:00
John Stiles
2756b0ee02 Reduce the number of extra bit_and ops caused by SkVM traces.
Previously, the trace opcodes took a single mask argument, which was
computed as `execution mask & trace mask`. This led to extra bit_ands in
the output, as this value would be need to be recalculated every time
the execution mask changed.

To reduce this cost on program size, the trace ops now take two mask
arguments and require that both must be true. We have four register
slots at our disposal in an Op, which is more than we need, so this
doesn't really cost us anything.

(As an extra minor optimization, if one of the masks is "always-on", we
optimize it away. This avoids burning a register just to hold a ~0
immediate value.)

Change-Id: I9eb71292a1983e71b03c7ac842534beb3d6bbf17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478456
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-12-02 18:15:34 +00:00
John Stiles
ffe365eb4d Enable various switch tests in ES2 mode.
switch is no longer an ES3-specific feature.

Change-Id: Ic878a77268e517e17699c2e35a37da6b0a7765dd
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452320
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-12-02 17:17:43 +00:00
John Stiles
dfe33f49f5 Add a setting to disable SkVM variable traces.
This will allow the visualizer to see line-number information without
incurring the code bloat caused by trace_var.

For reference, Commutative.skvm with no debug traces:
https://osscs.corp.google.com/skia/skia/+/main:tests/sksl/runtime/Commutative.skvm;drc=c809e5ba9fc9bc67291d088c448ed0057371b760

Change-Id: If98b5e102571d7fe06a1653d199e3ae035b3cb78
Bug: skia:12692
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478417
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-12-01 17:25:59 +00:00
John Stiles
030f8260a7 Add debug traces to an existing test.
This demonstrates how much additional non-trace code can be generated
when debug traces are turned on. A followup CL adds a setting to disable
`trace_var`, which eliminates a significant percentage of the additional
ops (at the cost of removing valuable debug info).

Change-Id: I238e28e6f6531f1dbccfef8f1dcd24a1e8481669
Bug: skia:12692
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/478416
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Julia Lavrova <jlavrova@google.com>
2021-12-01 17:25:59 +00:00
John Stiles
5588bdd72d Put function return values in a slot.
This shouldn't change code generation when debug traces are disabled.
When they are enabled, we now get trace_var opcodes emitted for every
return statement. Internally, this required a fair amount of refactoring
around how return values are passed around, but it should all be
functionally equivalent.

Change-Id: Ieb9d9c75399109186f905e0499d8fe6e2fc2067c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477981
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-30 19:25:31 +00:00
John Stiles
f57140768e Allow setting a trace coordinate without a skvm::Builder.
SkRuntimeEffect users will be insulated from skvm and the Builder, so
they won't be able to make an skvm::Coord directly. Replace this with
an SkIPoint, and do the conversion inside the SkVM code generator.

This also removes the requirement of assigning a trace coordinate; if
no trace coordinate is set, (0, 0) will be used.

Change-Id: I97be025d0ea146a86655cc3cdb76726f3bbc2324
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474556
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-29 21:47:05 +00:00
John Stiles
eeb5b224c4 Separate enter and exit into two opcodes.
Reusing the same opcode for enter and exit didn't have any real upside,
and forced us to deal with a fake immediate-value.

Change-Id: I35cab4d196f55b6f5e3040074c731b298720d62b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477157
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-11-29 15:55:58 +00:00
Brian Osman
00aa1a9259 Prevent 'binding' and 'set' on struct/interface block fields
I should have realized the fuzzer would find this assert when I added
it. Now the front-end rejects these layout qualifiers on both struct
fields and interface block fields. LayoutInInterfaceBlock.sksl is a
reformatted version of the fuzzer input. LayoutInStruct is hand-crafted
to trigger the same failure on a different code path. Both would
previously assert in the SPIRV generator. Now, neither one gets that
far.

Bug: oss-fuzz:41347
Change-Id: Iff69d8f5482da7b772e9331c4fd2d58e89813c46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476396
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-11-29 15:53:57 +00:00
Brian Osman
4fa40eb741 Revert "Update some skvm test outputs"
This reverts commit b1f450bb2b.

Reason for revert: Hmm, bot disagrees.

Original change's description:
> Update some skvm test outputs
>
> Not sure how these were wrong in the repo for so long - these all should
> have changed like this when the optimization of commutative operations
> landed: reviews.skia.org/473239
>
> Change-Id: I3035d4f29c208096f83184b4393023666e10ab92
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476316
> Auto-Submit: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Change-Id: I9dc808927c73a979f4cde5c7035b109d8db808f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476397
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-11-24 19:18:09 +00:00
Brian Osman
b1f450bb2b Update some skvm test outputs
Not sure how these were wrong in the repo for so long - these all should
have changed like this when the optimization of commutative operations
landed: reviews.skia.org/473239

Change-Id: I3035d4f29c208096f83184b4393023666e10ab92
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476316
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-11-24 16:45:39 +00:00
Brian Osman
10c77dbce3 Reland "Restrict where 'binding' and 'set' can appear"
This is a reland of 9372ef0228

Original change's description:
> Restrict where 'binding' and 'set' can appear
>
> In SPIRV, these are an error when applied to struct members. Some of our
> tests were triggering that because we had free-floating uniforms
> decorated this way (and we coalesce those into members of an interface
> block).
>
> Now, we only allow those layout qualifiers on variable types that will
> remain top-level constructs in the back-end.
>
> Bug: skia:12670
> Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:12670
Change-Id: I01c0323bba7ce0bddea5f9fb907e2b60e6b812d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475156
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-11-23 18:03:24 +00:00
Brian Osman
d7ebf8604e Revert "Restrict where 'binding' and 'set' can appear"
This reverts commit 9372ef0228.

Reason for revert: Unhappy bots

Original change's description:
> Restrict where 'binding' and 'set' can appear
>
> In SPIRV, these are an error when applied to struct members. Some of our
> tests were triggering that because we had free-floating uniforms
> decorated this way (and we coalesce those into members of an interface
> block).
>
> Now, we only allow those layout qualifiers on variable types that will
> remain top-level constructs in the back-end.
>
> Bug: skia:12670
> Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:12670
Change-Id: Ie518192d9a52fc896e615ec08ce0674ad683ec61
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/475099
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2021-11-22 19:52:23 +00:00
Brian Osman
9372ef0228 Restrict where 'binding' and 'set' can appear
In SPIRV, these are an error when applied to struct members. Some of our
tests were triggering that because we had free-floating uniforms
decorated this way (and we coalesce those into members of an interface
block).

Now, we only allow those layout qualifiers on variable types that will
remain top-level constructs in the back-end.

Bug: skia:12670
Change-Id: I73e69cecf6237a1c1180ad38d9b5d52ea80316fb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474218
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-11-22 18:28:41 +00:00
John Stiles
fb0fa24e31 Remove VarType from trace_var opcode.
This is redundant information now that we have SkVMSlotInfo.

Change-Id: Ia05b1eaa722023e719042c83255708aa9debed61
Bug: skia:12614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/473777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-20 01:52:40 +00:00
Brian Osman
f72c919a9a Roll SPIRV-Headers and SPIRV-Tools
This adds new validation rules that we were breaking.
Binding and DescriptorSet can't be applied to push constants, nor to
struct members.

Bug: skia:12670
Bug: chromium:1270328
Change-Id: I332f77717b08d9945c8e5b79c5bf649a8f5f2043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474056
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-11-19 19:22:54 +00:00