Commit Graph

1001 Commits

Author SHA1 Message Date
John Stiles
44832b292d Revert "Fix color fringes on blend_hue and blend_saturation."
This reverts commit 2e6f60f423.

Reason for revert: draws black incorrectly in various iPhone 8 tests

Original change's description:
> Fix color fringes on blend_hue and blend_saturation.
>
> Previously, we checked for division against zero, but didn't do anything
> to prevent division against extraordinarily small values. Now, we only
> saturate if the delta between max and min is greater than 0.00001.
>
> Change-Id: I7d1df3430941c7e1a7f94e597d5449f9259612d6
> Bug: skia:9320
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527498
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

Bug: skia:9320
Change-Id: Id83376080eed684577b3592c5e1bee3c80fc3fc9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528038
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
2022-04-06 17:07:06 +00:00
Ethan Nicholas
a98e077508 Fixed remaining SkSL positions
This addresses (hopefully) all of the remaining suboptimal positions in
SkSL error reporting.

Change-Id: I5bc977b03d51153b841a89fa687e54e3e9cb6ec3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527976
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-04-06 17:03:22 +00:00
Ethan Nicholas
b69a292dca Improved position reporting for various SkSL constructs
This cleans up a lot of positions produced by DSLParser to make them
actually match the ranges of the elements being parsed.

Change-Id: Ic3a9d62c99c4b5f92b84a597a2ceba386bbcc334
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527501
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-04-06 16:20:45 +00:00
John Stiles
cfa9938936 Simplify -1 * x into -x.
We already performed a matching simplification for `0 - x`, so this
seemed like a straightforward improvement.

Performing this simplification causes the expressions in the test code
to match on both sides (e.g. `-one == -one`) which allows them to fold
away.

Change-Id: Idf87a98024dd6831b45d0384285ead2e2e039493
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527656
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-04-05 20:27:15 +00:00
John Stiles
72d4c6b574 Collapse darken/lighten blend functions into one.
We use multiplication by 1 or -1 to branchlessly choose one of `min` and
`max` in the same function.

Change-Id: I44cf747feeae75a9c3e00f36e112e0a429871e86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527596
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-04-05 20:27:09 +00:00
John Stiles
dc28d75792 Collapse overlay/hard-light blend functions into one.
Hard-light is just overlay with the parameters reversed.

Change-Id: I6cf5963b1252cba3a7b71a56f4094a070188f8b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527503
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-04-05 20:21:47 +00:00
John Stiles
10e21844a7 Collapse hue/saturation/luminosity/color blend functions into one.
These functions were functionally almost identical, except:
- Sometimes sda/dsa are flipped
- Sometimes the saturation is not updated

We now have one method (blend_hslc) which can do all four blend
operations. It takes two new parameters ("flip" and "saturate") to
handle these four variations.

This reduces our shader count on some of our most shader-heavy slides
(e.g. aaxfermodes, xfermodeimagefilter) at a pretty reasonable cost.

Change-Id: Ifa8a48399851a9badb5d50038de1e25e60d44ebd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527281
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-04-05 20:16:54 +00:00
John Stiles
d0242346b8 Allow prefix-expressions in IsSameExpressionTree.
This will allow expressions like `-x == -x` or `!y == !y` to be detected
as matching expressions (which enables various constant-folding paths).

(Also, migrated the analysis code into a separate cpp.)

Change-Id: I3e317fdaed3762f8fa19e684a5ed557fc9348c7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527617
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-04-05 20:16:35 +00:00
John Stiles
0e3a248cbe Add new folding tests involving prefix expressions.
Followup CLs will improve their output.

Change-Id: I07059348f68cd6cd3154c31a41f81018b26a44e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527616
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-04-05 19:52:07 +00:00
John Stiles
2e6f60f423 Fix color fringes on blend_hue and blend_saturation.
Previously, we checked for division against zero, but didn't do anything
to prevent division against extraordinarily small values. Now, we only
saturate if the delta between max and min is greater than 0.00001.

Change-Id: I7d1df3430941c7e1a7f94e597d5449f9259612d6
Bug: skia:9320
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527498
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2022-04-05 19:46:58 +00:00
Ethan Nicholas
e0b6115a04 Added position tracking for SkSL struct fields
Change-Id: Ibff49d1928d7f82d04930c8cfd9d574780732c0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527497
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2022-04-05 19:46:52 +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
John Stiles
e1eec147f4 Add test for fuzzer-discovered error with ^^ operator.
We didn't have any existing tests that exercised this path; it is
separate from most operators since it has no C++ equivalent.

Change-Id: I95b538dad01f8c8b122954fb5f66337371a398a8
Bug: oss-fuzz:46289
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527196
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-04-04 15:41:08 +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
John Stiles
2ac7682b53 Implement constant-folding for vector/matrix multiplication.
This closes one of the last gaps in SkSL's constant-folding abilities.

Change-Id: I65c0f2e5fe11a7d47ab2069b2992403fca78b8a7
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524761
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-25 21:42:47 +00:00
John Stiles
ae5984082b Optimize SkVM selects.
The expression `!x ? y : z` can be optimized to `x ? z : y`, saving a
bit-not. SkVM now supports this optimization.

Change-Id: I06a0d2a716947de1021ba66b054b92e25568c641
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524226
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-24 21:17:41 +00:00
John Stiles
059d34594e Optimize SkVM bit-clears.
SkVM has a `bit_clear` opcode dedicated to the operation `x & ~y`, but
the optimizer was not smart enough to combine a bit-and with a bit-not
and replace it with a bit-clear. Now, it can.

Change-Id: Ida5345c3def0a4bf7afa08bb7f7835e1e2e37677
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524225
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
2022-03-24 21:04:23 +00:00
John Stiles
8318cc9928 Update opcode canonicalization logic in SkVM.
Previously, our ID canonicalization was simply "lower ID numbers before
higher ID numbers" and was done separately at every opcode by taking
the min and max of (x.id, y.id).

Now, this logic is factored out into a helper function
`canonicalizeIdOrder` and has two rules:
- Immediate values go last; that is, "x + 1" instead of "1 + x".
- If both/neither are immediate, lower IDs before higher IDs (as
  before)

This change lets us remove a lot of simplification logic. We no longer
need to check for both `x + 0` and `0 + x` when removing no-op
arithmetic; now we can be certain that the immediate will always come
last, so just checking for `x + 0` is sufficient.

Change-Id: I66cc5c23bba414041c0bc556521d3db57fac504d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524222
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-24 19:14:51 +00:00
Ethan Nicholas
7183f29125 Added position tracking for SkSL Modifiers
This is needed for accurate error reporting when we start reporting
ranges rather than line numbers.

Change-Id: If465317e04685e91ab7c408d29e82028b5d59d1a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523425
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-23 20:45:39 +00:00
John Stiles
c29e37ad4c Rename ES2 error tests 'T' through 'Z' to .rts.
Change-Id: I528d2b7a53748077f2dd7e7e04927d2a6b78ac8f
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523429
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-22 19:50:29 +00:00
John Stiles
04f8ee3f39 Rename ES2 error tests starting with 'R' to .rts.
Change-Id: Ibd39b45ef57c4e79e444a70aee1901cb33bcfa6a
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523378
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-22 17:03:48 +00:00
John Stiles
141f2873a9 Rename ES2 error tests 'O' through 'P' to .rts.
A few tests were divided into a Runtime Effect-compatible .rts test, and
a Runtime Effect-incompatible .sksl test.

Change-Id: Ib52554892685bdc44fe3622ab314960ee0962b90
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523377
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-22 17:03:07 +00:00
John Stiles
c5e6515e4a Rename ES2 error tests from 'H' through 'M' to .rts.
In a few cases, this involved splitting a test into two (an ES2-
compatible portion and a ES3+ portion).

Change-Id: Ie6f18f787cf7c10696a2841ff538bbe2b95bf50d
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523187
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-22 16:54:32 +00:00
John Stiles
2fdcca2777 Rename ES2 error tests from 'C' through 'G' to .rts.
A few tests received minor tweaks to make them Runtime Effect-friendly.

Change-Id: I9b4f66b0974c41d38324dfbb31ac9849338f600a
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523186
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-21 22:50:58 +00:00
John Stiles
1c5020d295 Fix error reporting position of repeated var-declarations.
These are wrapped in an unscoped Block. Previously, we didn't assign any
position to the block, so it was implicitly given the position of its
enclosing statement.

Change-Id: Id320eb1db583acd6ae42deba2fbb0b61033c3936
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522922
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-21 21:52:04 +00:00
John Stiles
0d226afee9 Rename ES2 error tests starting with 'B' to .rts.
A few tests received minor tweaks to make them Runtime Effect-friendly.

Change-Id: Icbcedb84b7882e42f21425b2d40d7819705c359e
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522918
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-21 16:49:17 +00:00
Ethan Nicholas
df40c7eae8 Fixed a past-the-end error location in SPIR-V output
The error was being reported at the position of the var declaration,
rather than the position of the reference. And since the declaration
was in a module, its position was both incorrect (with respect to the
program source) and could be past the end.

Change-Id: I443b9fbbe016c43b93d457abfefd17025e451d8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521522
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-16 21:54:21 +00:00
Ethan Nicholas
c9e9131f44 Switched SkSL positions from int to Position
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.

Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-14 17:06:17 +00:00
Arman Uguray
e70f2e0e0f [sksl][test] Add more folding tests for side-effecting expressions
Moved the MatrixFoldingES2.sksl test case for matrix construction with
side-effects into a new PreserveSideEffects.sksl test and added new test
cases for various vector and matrix types and constructors. The new test
is written such that none of its contents should be folded away.

Note: This test does not pass on NVIDIA GPUs when using OpenGL as
discussed in skia:13035. Notably, NONE of the increments are executed on
those GPUs as ALL increment expressions seemingly get subjected to
constant-folding. The test is disabled on NVIDIA GPU bots.

This also means that the remaining MatrixFoldingES2.sksl tests now work
on NVIDIA GPUs when using OpenGL (with the exception of Tegra3 + OpenGL
ES).

Bug: skia:13035, skia:11919
Change-Id: I561bb62fe2b6b814ba80fbc492d3885bbcd6b65b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518278
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-10 18:06:12 +00:00
John Stiles
6a10556775 Reject equality operators on opaque types.
ES2 disallows opaque types in expressions (other than passing them to
their associated builtin functions). We now enforce a similar
restriction on SkSL opaque types.

While I was there, I added several other cases to the invalid-shader
test to make sure that they were all caught.

I needed to reorder some code to make sure that ternary expression error
messages didn't change. Ternary expressions now check for opaque types
before checking that the left-side type and right-side type are
compatible. This is because we check for "compatible" ternary
expressions by checking if `leftSide == rightSide` would be accepted.
`shader1 == shader2` used to be considered a valid expression for the
purposes of this test, but not anymore.

Change-Id: I62a0a31feca9dadd428da7d1b48d7693c4b6434d
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516802
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-03-07 15:19:46 +00:00
John Stiles
b8cee7dab3 Reject operators =, ==, and != on void types.
The fuzzer discovered that we allow == on void types (confusing the SkVM
backend).

Change-Id: Ia9494642faf67f3f86e3a365807be8bd4a7062e4
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-07 14:10:29 +00:00
John Stiles
1c2b4492b8 Only run vector-folding logic on vectors.
Previously, we would take the vector-folding path for all types. This
didn't cause any problem for scalars, but failed for "zero-size" types
like void. It isn't valid to compare zero-size values, but we currently
don't reject such code (see skia:13026), and the fuzzer noticed this.
It's safest to only run the vector-folding code when we actually have
multiple slots that need to be folded into one result.

Change-Id: I0bc88043d9a4aeea38ae24dc1a6d1a7430d3d7b0
Bug: oss-fuzz:45279, skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516676
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-03-07 13:58:17 +00:00
John Stiles
7b40626ca9 Fix debugger return-value display with repeated function calls.
The slot-assignment logic has been changed to associate slots with
function calls, instead of function definitions. In our test case, you
can now see that the calls to `get` are now mapped to $15, $17 and $18.
This change also jiggles some existing tests and improves their
register allocation slightly (!).

One minor hitch here is that there's no FunctionCall node associated
with main() (it's never explicitly called). However, our slot map key
can be any IRNode, and we know main() can't be called by anyone else,
so it's harmless to use the function definition as the key in this case.
(This entry could probably stay out the map entirely if it made a
difference, but I don't think it matters.)

Change-Id: I68a6ff24cbd3a2db77f24126502bd3d11e8c0962
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514578
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-03 20:55:25 +00:00
John Stiles
d9f9f1813c Add test demonstrating issue with function calls.
If a function is called multiple times on one line, stepping over that
line does not show all of the function-call results. It only shows the
last result.

e.g. in this example, I have just stepped over the first line which
calls "get" three times. We should see three results, but we only see
one: http://screen/3WfJoZWm77cSexM

In this test you can see that all three calls to `get` are assigned to
the same slot, $15.

Change-Id: Id0c486ef349a1e527001efbcee2ed2b836f56e83
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514577
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2022-03-03 20:55:25 +00:00
John Stiles
60c5837632 Fix Housekeeper-PerCommit-CheckGeneratedFiles.
I added a comment and didn't rebuild; this shifted line numbers around,
which is reflected in the debug trace opcodes.

Change-Id: I1b8e00ff65557a03483e8d7ff0c4dbec9852866f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-02 16:42:10 +00:00
John Stiles
e11a14d17f Add workaround for LLVM crash in macOS 12 on Intel or M1.
The `break_loop` test causes LLVM to get confused and crash when
compiled on some GPUs. The crash goes away if we pass a literal 5
instead of a 5 that is computed at runtime. This also results in a
simpler test for SkVM, for better or worse, but we still have
coverage for dynamic loop exits in other tests.

LLVM crash: https://paste.googleplex.com/4718583155261440
Dangerous shader: https://paste.googleplex.com/4776089520963584

Change-Id: Ic6cbd55a36d2de58e5dd3459d4dfd74acdbc9f91
Bug: skia:13005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514538
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-02 15:20:10 +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
Ethan Nicholas
f6dc205b14 Improved SkSL array size parsing
We previously had no way to signal a parse error from arraySize,
resulting in a cascade of additional errors downstream. This tightens
up the behavior and allows us to fail more gracefully.

Bug: skia:12416
Change-Id: I83d3d5bc1dc63395edb325297375a6eb52415817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512952
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-01 16:44:49 +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
1542db1a9b Add a test runtime effect for child effects.
I wasn't able to find any other test which exercised child color-filters
or child blenders. (SampleWithExplicitCoord evaluates from a shader.)

Change-Id: I58ecee3beca2d3dc11ded5de0eea031e1d7c3e1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507922
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-02-12 01:06:57 +00:00
John Stiles
9c111af64b Add more fuzzer-discovered programs to error tests.
These all stemmed from the same root cause, but are interesting and
distinct enough to include in our error tests.

Bug: oss-fuzz:44555, oss-fuzz:44557, oss-fuzz:44559, oss-fuzz:44561, oss-fuzz:44565
Change-Id: I22c1798809754b4b38c77ffbe369a97c64a2f60e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-02-11 14:42:43 +00:00
John Stiles
683ae40560 Fix for fuzzer-discovered error with deeply-nested expressions.
The fuzzer constructs a long, valid nonsense expression
(x+x+x-x+x-x, etc.) which exceeds parse depth. At that point, the token
stream points to a `+` token. The parser attempts to consume a new
statement but stops in `unaryExpression`; this fails again, due to the
max parse-depth, but doesn't consume a token. The parser continues
trying to parse the statement, but stopping in `unaryExpression`, making
no forward progress in an infinite loop.

I've made a couple of changes as a result.
- Exceeding the max parse depth now sets `fEncounteredFatalError`.
- Encountering a fatal error causes block() to immediately halt. This
  actually undoes a few of the arbitrary changes from
  http://review.skia.org/506463 but not in a bad way.
- `unaryExpression()` now consumes a token before checking parse-depth.
- `structDeclaration()` had a similar issue where it could potentially
  fail without consuming any tokens; this is fixed as well.
- Some unnecessarily-nested logic in ternaryExpression() was flattened
  while I tried to ensure that it always consumes a token.

Change-Id: I52c2161965ffbcef1185761ca6897ec1cba5df89
Bug: oss-fuzz:44551
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507436
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-02-11 14:25:34 +00:00
John Stiles
6dda78ac7b Add SkSL error tests for runtime_errors directory.
This enables the SkSL error testing logic for runtime effects. The core
logic is identical, only the ProgramKind differs.

(Error creation scripts: http://go/paste/6413797460803584 with some
light post-processing)

Change-Id: I877205b3cc1014b50ccccf6037a2f4034c07543e
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506538
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-02-10 14:24:39 +00:00
John Stiles
fd36683fe6 Improve parser behavior with invalid statements inside a Block.
Previously, when the parser found a bad statement inside a Block, it
would stop processing that Block entirely. This caused our brace
matching to fall out of balance. block() would normally only return once
the Block's closing brace was consumed, but in this case, the closing
brace would still be in the parse stream awaiting consumption even
though block() had returned.

Now, when a bad statement is found inside a Block, we just ignore it and
continue processing. (I tried injecting a poisoned statement as well,
to see if it would affect the test results, but they were identical.)
This seems to generate somewhat better errors.

Change-Id: I8dc781d5602bf99d7610f8280cde8b7c1925cb65
Bug: skia:12868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506463
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-02-10 14:22:38 +00:00
John Stiles
a424b619bc Fix OverflowFloatLiteral test in OS X 10.12.
std::stringstream has a subtle bug in OS X 10.12. Reading in a too-large
floating point value returns INFINITY but does not set failbit. This
caused SkSL to report a different error message than expected
("floating point value is infinite" instead of "floating-point value
is too large: NNNNN"). We now guard against this case in SkSL::stod by
adding an explicit `isfinite` check.

Bug: skia:12928
Change-Id: I9996e64b69512ea5710e6fc3ff00ad1ad83c247b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505939
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-02-09 15:13:39 +00:00