Commit Graph

401 Commits

Author SHA1 Message Date
Ethan Nicholas
cb13c892af Added range highlighting to SkSL error reports
SkSL errors now identify the specific range of code they are describing,
rather than just the line number.

Change-Id: Ifabb3148476f9b4cd8e532f23e5b38e1cf33a87e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528039
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-04-07 13:29:48 +00:00
Ethan Nicholas
b47a67ab75 Fixed SkSL positioning error with double negation
We were not propagating the position into a double-negated expression,
leading to an assertion failure in PrefixExpression.

Change-Id: I1970ff1a06d9631582626c68e151f12f6b3ef278
Bug: oss-fuzz:46381
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527507
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-04-05 17:10:23 +00:00
Brian Salomon
863bc82c62 Reland "Always apply mipmap sharpening on GPU"
This is a reland of commit 1aedd5dc11

Original change's description:
> Always apply mipmap sharpening on GPU
>
> Bug: skia:13078
>
> Change-Id: If459a96eba09fb10e967bc364435f79b83fdc1ec
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522099
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>

Bug: skia:13078
Change-Id: Ic05b38fc07566f090d609431f2738d64dfdc8a66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524218
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2022-04-01 17:53:28 +00:00
Brian Salomon
dcd21712d8 Reland "Change GPU LOD bias to be just shy of -.5."
This is a reland of commit 355f0f9fa2

Original change's description:
> Change GPU LOD bias to be just shy of -.5.
>
> We want to ensure that when a MIP level is 1:1 with device space
> that kNearest picks that level instead of a larger level.
>
> Bug: skia:13078
>
> Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>

Bug: skia:13078
Change-Id: I7fc765a8718d770ebdac68adf9c59ff15d8c8451
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526517
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2022-04-01 13:29:38 +00:00
Michael Ludwig
fa88291aa0 Revert "Change GPU LOD bias to be just shy of -.5."
This reverts commit 355f0f9fa2.

Reason for revert: blocking chrome roll, should be #if defined(...) for the guard

Original change's description:
> Change GPU LOD bias to be just shy of -.5.
>
> We want to ensure that when a MIP level is 1:1 with device space
> that kNearest picks that level instead of a larger level.
>
> Bug: skia:13078
>
> Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>

Bug: skia:13078
Change-Id: I42d6e99509a87f0354f104f2c0177e78cf0d0e21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526462
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-04-01 00:51:07 +00:00
Brian Salomon
355f0f9fa2 Change GPU LOD bias to be just shy of -.5.
We want to ensure that when a MIP level is 1:1 with device space
that kNearest picks that level instead of a larger level.

Bug: skia:13078

Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2022-03-31 16:58:30 +00:00
John Stiles
674eb326f4 Improve distinct-out-param test cases.
Added new aliasing tests inspired by syoussefi@'s ANGLE changes at
http://go/crrv/c/3561278.

Change-Id: Ifa312faa9503b211b7c09edd2abd5087ead35e5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526018
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-30 18:49:15 +00:00
John Stiles
4449ca6cd4 Allow builtin code to reference builtin variables.
The builtin variable scanner did not check builtin code for the presence
of sk_FragColor, etc. We currently get away with this because none of
the existing builtin code uses a builtin variable.

Now FindAndDeclareBuiltinVariables checks shared program elements too.

Change-Id: Ifb3ee3857ef73b18d9e4f406970f0f67681dd4be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525042
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-28 15:37:20 +00:00
Arman Uguray
5f16ed4c58 [sksl] Special-case unary negation on matrix types in MSL/SPIR-V
MSL does not support the unary "-" operator on matrix types. Similarly
the SPIR-V OpFNegate/OpSNegate operations only work on scalar and vector
type.

* An expression such as "-<mat>" is now transformed to "-1.0 * <mat>" when
generating MSL.
* The same expression now generates a component-wise negation in SPIR-V,
matching what glslang outputs for GLSL.
* A unary "+" is now treated as NOP for MSL, matching the SPIR-V backend.
An expression such as "+<expr>" is now evaluated as "<expr>".
* The shared/Negation.sksl has been moved to folding/ as much of its
contents exercise constant-folding of comparison expressions.
* The shared/UnaryPositiveNegative.sksl test has been extended to
exercise scalar and matrix types.

NOTE: The SPIR-V backend changes have caused a minor re-ordering of SSA
IDs generated when writing out a prefix-expression. The affected gold
files have been updated.

Bug: skia:12627, skia:12992
Change-Id: Iec5cdafc591aed7e49b3b52bda42a02661380bab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513976
Auto-Submit: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-01 20:14:46 +00:00
John Stiles
1380d2c02c Use unordered comparisons for != in SPIR-V.
On modern hardware, this will give the correct result for `NaN != x`
(true).

Change-Id: I9683f74756da5da5f34ccacec02c1f2449791f26
Bug: skia:12977
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513317
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-02-25 22:22:43 +00:00
John Stiles
c3d8062555 Fix up SkSL test on Wembley.
Test "InlinerHonorsGLSLOutParamSemantics" was failing on Wembley devices
and is now disabled on that GPU.

Also, it turns out that the inliner has ignored functions with out
params for a long time now, but our test names haven't been updated to
account for this. So, did some additional cleanup:
- "InlinerHonorsGLSLOutParamSemantics" (the test in question) has been
  moved to shared/ and renamed to "OutParamsAreDistinct."
- Removed test "OutParamsNoInline" as it is functionally the same as
  "OutParams".

Change-Id: I1431ed197b9216cb482eee4f5e4eb2579a5303f7
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502303
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-01-31 21:17:40 +00:00
John Stiles
15fd5e99f4 Update out-param semantics in SPIR-V.
Removed a special case from `writeFunctionCallArgument` which avoided
using a scratch variable for out params; now we always use the scratch
variable and copy it back to the original variable at the end.

Change-Id: I0e446a3fde6d19554943384210bd911f6f9c8cfa
Bug: skia:11052, skia:11919, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-01-31 16:07:37 +00:00
John Stiles
fad0a051c3 Remove old test outputs.
The test input was removed at http://review.skia.org/497742.

Change-Id: I7b30f2f70cd0812b900c9c67b70e742b3d96930a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499574
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-01-25 22:42:56 +00:00
John Stiles
493a9c0cbc Fix up test SkSLInlineWithInoutArgument.
The test has been moved to shared/, since it's a valid test, but it is
no longer related to inlining, as the inliner no longer attempts to
inline functions with inouts at all.

Also, one function here (outParameterIgnore) actually invoked undefined
behavior and has been removed. According to the GLSL ES2 docs: "If a
function does not write to an out parameter, the value of the actual
parameter is undefined when the function returns." SkVM leaves the value
unchanged, so SKSL_TEST_CPU would pass, but a GPU might clear it (and in
fact, my GPU does).

Change-Id: I77c77ed1354bc980344ec5c406992bd62015f5e5
Bug: skia:11919
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499752
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-01-25 21:33:45 +00:00
John Stiles
a9f7a8b617 Implement constant-folding for matrix-op-scalar and scalar-op-matrix.
GLSL supports adding, subtracting, multiplying, and dividing matrices
with scalars. This works by splatting the scalar across every matrix
component and then performing the op componentwise. Our constant folder
now knows how to fold out these simplifications.

Change-Id: Idb8751ec16135e1b61da0d58cfd0505ab31ac087
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497738
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2022-01-25 17:52:31 +00:00
John Stiles
b8f05d6f63 Revert "[skslc] Generate .hlsl test output files"
This reverts commit a97a6769b5.

Reason for revert: breaking bot Housekeeper-PerCommit-CheckGeneratedFiles -- see http://screen/5FN75fF9tvcQFKR

Based on the diff, I think this just needs to be synced up to latest code and a rebuild should fix it.


Original change's description:
> [skslc] Generate .hlsl test output files
>
> - The build now generates HLSL output when `skia_compile_sksl_tests` is
> enabled.
> - The "blend" and "shared" tests have been enabled for HLSL with the
> exception of 6 tests that exercise intrinsic inverse hyperbolic
> functions, which don't have HLSL equivalents.
>
> Bug: skia:12691, skia:12352
> Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: Arman Uguray <armansito@google.com>
> Commit-Queue: Arman Uguray <armansito@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

Bug: skia:12691, skia:12352
Change-Id: Iaad607d48edd136eee2b60e48c0643b6e90179e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499216
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>
2022-01-25 04:57:15 +00:00
Arman Uguray
a97a6769b5 [skslc] Generate .hlsl test output files
- The build now generates HLSL output when `skia_compile_sksl_tests` is
enabled.
- The "blend" and "shared" tests have been enabled for HLSL with the
exception of 6 tests that exercise intrinsic inverse hyperbolic
functions, which don't have HLSL equivalents.

Bug: skia:12691, skia:12352
Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-01-24 21:50:30 +00:00
John Stiles
d68069019b Fix whitespace when commas are used in a binary-expression.
Previously, any code which emitted a binary expression would always emit
a leading and trailing space. This caused comma expressions to look
goofy: `foo() , bar();` instead of `foo(), bar();`.

Operator::operatorName() now returns the operator token with appropriate
whitespace around it, and tightOperatorName() is a new method which
omits the whitespace. Functions which assemble binary expressions
should now concatenate `x + operatorName() + y` instead of hard-coding
`x + " " + operatorName() + " " + y`. Prefix/postfix expressions should
use `tightOperatorName()` because otherwise negation looks bad (` - 123`
instead of `-123`).

Super low priority, but it was easy to fix.

Change-Id: I3c92832207293a310fb1070b3b5e72455757b0ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497776
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-01-24 16:21:43 +00:00
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
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
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
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
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
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
John Stiles
e10839f998 Add basic unit test for octal parsing support.
This is required by the ES2 standard: http://screen/Qysv4fPW5r5LA9e
This actually already worked fine because `strtoull` natively recognizes
octal values without any work on our part. However, we lacked a test.

Change-Id: I3033de899918abe99c63a9b7b79bd4c3374ee315
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471716
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-16 13:49:41 +00:00
John Stiles
14a487fd54 Replace getConstantSubexpression with getConstantValue.
The only type of expressions that getConstantSubexpression could ever
return are Literal and nullptr. getConstantValue now returns an
optional<double>; nullopt indicates a non-constant value in the slot.
This simplifies most use cases, and allows us to get rid of some extra
"zero" and "one" Literal objects in some of our Constructor classes.

This change fixes a recent fuzzer issue. The fuzzer had discovered that
calling `getConstantSubexpression` on a ConstructorCompoundCast that
contained a compile-time-constant value would return literals of the
wrong type (the cast was not applied). By nesting repeated matrix casts,
this type confusion could be turned into an assertion.

Change-Id: Icee69219e6db2822ffdfab4e5ccdaff54584a4b6
Bug: oss-fuzz:41000
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471376
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-11-15 14:46:21 +00:00
John Stiles
6460541ee4 Reland "Fix Metal codegen error with structs containing compound types."
This reverts commit 3aaed99930.

Reason for revert: removing changes to PrecisionQualifiers

Original change's description:
> Revert "Fix Metal codegen error with structs containing compound types."
>
> This reverts commit 2a6c41571b.
>
> Reason for revert: causing Mali G7x failures on tree
>
> Original change's description:
> > Fix Metal codegen error with structs containing compound types.
> >
> > While working on an unrelated test, I accidentally triggered a bug in
> > Metal code generation. Our struct-equality helper functions did not
> > properly handle vector fields. Wrapping each comparison in `all(...)`
> > fixes the problem. (all() on a scalar is allowed and does nothing.)
> >
> > Our struct comparison tests now include a vector and a matrix.
> >
> > Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: Ieb5d5a1839978fb82525863488e9d54fdf44adbd
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471097
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Reviewed-by: Chris Dalton <csmartdalton@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Change-Id: I8ee90df3de075cf82c0fcf3b4787577b09bb1a70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471156
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-11-15 13:29:06 +00:00
John Stiles
3aaed99930 Revert "Fix Metal codegen error with structs containing compound types."
This reverts commit 2a6c41571b.

Reason for revert: causing Mali G7x failures on tree

Original change's description:
> Fix Metal codegen error with structs containing compound types.
>
> While working on an unrelated test, I accidentally triggered a bug in
> Metal code generation. Our struct-equality helper functions did not
> properly handle vector fields. Wrapping each comparison in `all(...)`
> fixes the problem. (all() on a scalar is allowed and does nothing.)
>
> Our struct comparison tests now include a vector and a matrix.
>
> Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com

Change-Id: Ieb5d5a1839978fb82525863488e9d54fdf44adbd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471097
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-11-12 23:09:34 +00:00
John Stiles
2a6c41571b Fix Metal codegen error with structs containing compound types.
While working on an unrelated test, I accidentally triggered a bug in
Metal code generation. Our struct-equality helper functions did not
properly handle vector fields. Wrapping each comparison in `all(...)`
fixes the problem. (all() on a scalar is allowed and does nothing.)

Our struct comparison tests now include a vector and a matrix.

Change-Id: I59061ae9c3c3ab2c2dbdcb5257bc23e2257152af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/470399
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-12 15:54:41 +00:00
John Stiles
76c1ff1566 Optimize indexing into an array with a constant-expression.
Previously, SkSL was unable to resolve the constant expression `x[y]`
for a constant-array `x` and a constant-integer-scalar `y`. Now, if `x`
and `y` are known, we can replace `x[y]` with the indexed array element.

Note that we need to be careful here, as it's not a valid optimization
to eliminate array elements that have side effects. We preserve side-
effecting expressions using the comma operator.

Change-Id: I5721337eb42b48c0b05f919c1cadfae19dd3b84f
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469839
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-10 19:43:58 +00:00
John Stiles
e7a9535247 Enforce basic limits on global size in SkSL.
Much like http://review.skia.org/467759, this CL defensively guards
against programs which consume more space than is reasonable. Globals
exist outside of functions, so they wouldn't be caught by the stack size
checks.

Change-Id: I035f27d57bc329508820a729a1e367ecaadfe156
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467760
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-11-04 18:34:19 +00:00
John Stiles
7cde28909f Enforce basic limits on function stack size in SkSL.
Functions that declare variables totaling more than 100,000 slots will
now generate an error.

This is only a partial mitigation to the problem, as a sophisticated
attack could still chain/nest multiple functions together to consume
extremely large amounts of stack. However, this mitigation is still more
sophisticated than our peers; both WebGL and glslang are susceptible to
similar problems, and in the general case (ES3+ with full flow control)
it's intractable.

Change-Id: I153c75267c017a23f59fe9e59f6e391197ee6101
Bug: oss-fuzz:40304, oss-fuzz:40694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467759
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-04 18:17:44 +00:00
John Stiles
293bb46b2d Enable more ES3-specific SkSL tests.
Updated ReturnsValueOnEveryPathES3 to remove overlap with the ES2 tests,
and fixed some broken cases. Disabled the ReturnValueOnEveryPathES3 test
on Intel + Windows because switch statements on Intel + Windows are
pretty broken.

Change-Id: Id93e8af1ef7bf11fd74ef12a464c77d56cc032a0
Bug: skia:11209, skia:12465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467078
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-03 14:02:05 +00:00
John Stiles
9b9805959d Reland "Add support for half-precision types in Metal."
This reverts commit 9d24b02c2f.

Reason for revert: needs premul/unpremul conversion fix (http://review.skia.org/465798)

Original change's description:
> Revert "Add support for half-precision types in Metal."
>
> This reverts commit d90e09b1ae.
>
> Reason for revert: MacMini failing CompressedBackendAllocationTest
>
> Original change's description:
> > Add support for half-precision types in Metal.
> >
> > This will hopefully improve performance on lower-end GPUs.
> >
> > Change-Id: I9c2ee6dc31acd08bec0bfb5f59edc3cf90163f9e
> > Bug: skia:12339
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465078
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> Bug: skia:12339
> Change-Id: Ic5aa4bef454ca67f5ce26c600444d9565e0158cb
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465796
> 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>

Bug: skia:12339
Change-Id: I53a8a6fef299da15d206d884ba7029820ffcff43
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465799
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-10-30 14:33:52 +00:00
Brian Osman
9d24b02c2f Revert "Add support for half-precision types in Metal."
This reverts commit d90e09b1ae.

Reason for revert: MacMini failing CompressedBackendAllocationTest

Original change's description:
> Add support for half-precision types in Metal.
>
> This will hopefully improve performance on lower-end GPUs.
>
> Change-Id: I9c2ee6dc31acd08bec0bfb5f59edc3cf90163f9e
> Bug: skia:12339
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465078
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Bug: skia:12339
Change-Id: Ic5aa4bef454ca67f5ce26c600444d9565e0158cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465796
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-10-29 23:04:09 +00:00
John Stiles
d90e09b1ae Add support for half-precision types in Metal.
This will hopefully improve performance on lower-end GPUs.

Change-Id: I9c2ee6dc31acd08bec0bfb5f59edc3cf90163f9e
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/465078
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-10-29 19:47:02 +00:00
John Stiles
94b1f0dfa1 Add overflow protection to vector-arithmetic folding.
This was causing errors in UBSAN when compiling some of our existing
SkSL tests.

Change-Id: I66f22607094df77d47ff70948a139c77feae8624
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464118
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-10-27 16:49:02 +00:00
John Stiles
8ed23eb917 Implement array casts in Metal.
These are not very interesting right now, because the in and out types
boil down to the same thing (int/int, float/float). When half-
precision types are enabled, these helpers will be more useful. They
will return an array which casts each element from int-to-short or
float-to-half (or vice versa).

Change-Id: Ida716ddd27d370ba33fd23f17a1b07fa5a201e40
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463337
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-10-26 15:26:43 +00:00
John Stiles
9f43ceefa3 Allow vector operator~ in SkSL.
This is supported in GLSL ES3. (Strangely, vector operator! isn't.)
Previously, this was flagged as an error: http://review.skia.org/459885

Change-Id: I2c4299159fff58fefe8bd131c8d317cd82974a62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459886
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-10-18 15:03:33 +00:00
John Stiles
21fe518fbb Revert "Disallow matrix ctors which overflow a column."
This reverts commit eb68973c2f.

Reason for revert: ES2 conformance test checks this

Original change's description:
> Disallow matrix ctors which overflow a column.
>
> The GLSL spec allows matrix constructors containing vectors that would
> split between multiple columns of the matrix. However, in practice, this
> does not actually work well on a lot of GPUs!
>
> - "cast not allowed", "internal error":
> 	Tegra 3
> 	Quadro P400
> 	GTX 660
> 	GTX 960
> - Compiles, but generates wrong result:
> 	RadeonR9M470X
> 	RadeonHD7770
>
> Since this isn't a pattern we expect to see in user code, we now report
> it as an error at compile time. mat2(vec4) is treated as an exceptional
> case and still allowed.
>
> Change-Id: Id6925984a2d1ec948aec4defcc790a197a96cf86
> Bug: skia:12443
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449518
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

Bug: skia:12443
Change-Id: I5a32744c88b9b830ad657488824c8c7dd0b0a652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
2021-10-14 01:30:08 +00:00
John Stiles
5420cbcf65 Match GLSL scoping rules more closely in SkSL.
GLSL treats builtin types and user-defined types differently; `int` and
`float` are keywords and cannot be used to name variables. However, it's
fine for a user type like `struct xyz` to be hidden by a variable
`int xyz` or even `xyz xyz` (i.e., a variable of type `struct xyz` named
`xyz`).

We now honor that distinction and include tests for it. This will fix
several ES2 conformance tests (local_struct_variable_hides_struct_type,
local_int_variable_hides_struct_type, etc.).

Change-Id: I7a45c70707087f9f355ce5b06b032fed16683f3e
Bug: skia:12527
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458721
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-10-12 21:53:28 +00:00
John Stiles
6d0234673a Optimize away empty for loops.
The fuzzer has found that it can get timeouts in SkVM by nesting loops
very very deeply, then at the bottom of the chain, making an inside-out
loop that runs for zero iterations. This has a calculated unrolled-size
of zero, but SkVM would still think hard about unrolling the (ultimately
empty) outer loops.

SkSL now optimizes away unrollable loops that run for zero iteratinons,
as well as empty unrollable loops. This should eliminate the fuzzer's
troublesome construct entirely.

Change-Id: Ic3ef7b7a6a9fc7ee7fb13eb7bd7f34c9bff57448
Bug: oss-fuzz:39661
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456469
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-10-06 17:56:26 +00:00
John Stiles
e5d4c43561 Add SkSL test for uniform arrays.
These weren't used anywhere in our test suite.

Change-Id: I35e8607ad2dbddf8f403668bd2b2636a8964d304
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/455777
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-10-05 13:25:00 +00:00