Commit Graph

9971 Commits

Author SHA1 Message Date
John Stiles
64560c7cd5 Re-reland "Rewrite switch statements in GLSL strict-ES2 mode.""
This reverts commit c4c355940c.

Reason for revert: disabled offending tests

Original change's description:
> Revert "Reland "Rewrite switch statements in GLSL strict-ES2 mode.""
>
> This reverts commit c81edd0e8c.
>
> Reason for revert: ANGLE still unhappy
> Original change's description:
> > Reland "Rewrite switch statements in GLSL strict-ES2 mode."
> >
> > This reverts commit d26d0e6a47.
> >
> > Reason for revert: uses dedicated caps bit
> >
> > Original change's description:
> > > Revert "Rewrite switch statements in GLSL strict-ES2 mode."
> > >
> > > This reverts commit 45e3838006.
> > >
> > > Reason for revert: Also need to rewrite them in actual ES2 mode.
> > >
> > > Original change's description:
> > > > Rewrite switch statements in GLSL strict-ES2 mode.
> > > >
> > > > Once this lands, switch statements will work everywhere--Metal, SPIR-V,
> > > > GLSL, and SkVM.
> > > >
> > > > Change-Id: I2797d0a872de8be77bb9f7aa6acb93421d571d70
> > > > Bug: skia:12450
> > > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452356
> > > > Commit-Queue: John Stiles <johnstiles@google.com>
> > > > Auto-Submit: John Stiles <johnstiles@google.com>
> > > > Reviewed-by: Brian Osman <brianosman@google.com>
> > >
> > > Bug: skia:12450
> > > Change-Id: I92656ed40289872405c0873f2c56a52b04e35b1d
> > > No-Presubmit: true
> > > No-Tree-Checks: true
> > > No-Try: true
> > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452556
> > > 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:12450
> > Change-Id: I0d3b0969d2040dbb4ee808132146687767c97442
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452560
> > 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>
>
> Bug: skia:12450
> Change-Id: I869cf3e2c47fe94981aa2ffa6c1f0b3e4d6e6862
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452717
> 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:12450
Change-Id: Ia0e23e20794ea707e54be50123b5323369354a03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452721
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-09-24 20:50:20 +00:00
John Stiles
b05bbd03f9 Fix for fuzzer-discovered error with invalid var-initializers.
If a VarDeclaration line contained multiple variables, and the first
variable had an illegal initializer-expression, the Declare() would
return a Nop. AddVarDeclaration did not expect to see a Nop and would
assert once we tried to process the second var-declaration. Now, we
allow adding var declarations to a Nop.

Bulked up some tests to cover local and global variables (since those
are parsed in separate functions) and to check both the first
initializer as well as follow-on initializers (since those are parsed in
separate parts of the var-decl handler).

Change-Id: I66341191698175b490a659715cb8edaafe2f75ae
Bug: oss-fuzz:39032
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452696
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-24 20:15:31 +00:00
Brian Osman
fcc0ef1ee4 Remove all clamping to "valid" premultiplied colors
Previously, we were abundantly cautious about producing premultiplied
colors with RGB > A, when the color-type was normalized fixed-point.

I'm not seeing any evidence that code cares about that. If it does, I
suggest we fix the issue at the API border, or via dedicated pass. In
the common case, this makes Skia less opinionated, which (in general)
makes it more versatile.

Change-Id: Iae524ff61f6c81073c34d0f2dced973c229cdfb7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452558
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-09-24 20:14:39 +00:00
Brian Osman
a909dd6b8d Revert "Use SkSL "offset" to actually mean "line""
This reverts commit 47f76853c6.

Reason for revert: Test failures

Original change's description:
> Use SkSL "offset" to actually mean "line"
>
> SkSL internally tracks token offsets, but only ever reports errors using
> line numbers. With the introduction of the DSL, which (being embedded in
> C++ source) only has access to line numbers in the first place, tracking
> offsets went from merely providing little benefit to actively making
> life more difficult.
>
> We are changing SkSL's position tracking from handling offsets to
> handling line numbers, but to simplify the review process the change is
> split up into two main steps. The first step (this CL) starts using
> line numbers everywhere, but avoids the thousand-line churn of actually
> renaming "offset", so most "offset" fields, variables, and parameters
> will be briefly misnamed and will actually contain a line number.
>
> The followup CL will complete the process by renaming all of the
> now-misnamed fields, variables, and parameters, but will not make any
> behavioral changes.
>
> Bug: skia:12459
> Change-Id: I30dc87cf4b816c5ddd7b8ae1be32586388962085
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/451419
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>

Bug: skia:12459
Change-Id: I562d9980cd43a2fc5108e562155fe731a1761dca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452720
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-09-24 19:03:24 +00:00
Brian Osman
cc91452f0a Revert "Renamed SkSL "offset" to "line""
This reverts commit 58d47fa1ec.

Reason for revert: Tree broken

Original change's description:
> Renamed SkSL "offset" to "line"
>
> https://skia-review.googlesource.com/c/skia/+/451419 changed the meaning
> of "offset" throughout SkSL, so that it was actually tracking line
> numbers rather than offsets (and thus had a misleading name). This
> completes the transition by renaming all of the now-misnamed "offset"
> fields, parameters, and variables to "line'.
>
> Bug: skia:12459
> Change-Id: I394e6441f6ddfaad6d4098352ba9b1bfeaf273be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450644
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>

Bug: skia:12459
Change-Id: Idcec3b65cb81d51c8b860c4388578700030b40a9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452718
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2021-09-24 18:59:18 +00:00
Brian Osman
c4c355940c Revert "Reland "Rewrite switch statements in GLSL strict-ES2 mode.""
This reverts commit c81edd0e8c.

Reason for revert: ANGLE still unhappy
Original change's description:
> Reland "Rewrite switch statements in GLSL strict-ES2 mode."
>
> This reverts commit d26d0e6a47.
>
> Reason for revert: uses dedicated caps bit
>
> Original change's description:
> > Revert "Rewrite switch statements in GLSL strict-ES2 mode."
> >
> > This reverts commit 45e3838006.
> >
> > Reason for revert: Also need to rewrite them in actual ES2 mode.
> >
> > Original change's description:
> > > Rewrite switch statements in GLSL strict-ES2 mode.
> > >
> > > Once this lands, switch statements will work everywhere--Metal, SPIR-V,
> > > GLSL, and SkVM.
> > >
> > > Change-Id: I2797d0a872de8be77bb9f7aa6acb93421d571d70
> > > Bug: skia:12450
> > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452356
> > > Commit-Queue: John Stiles <johnstiles@google.com>
> > > Auto-Submit: John Stiles <johnstiles@google.com>
> > > Reviewed-by: Brian Osman <brianosman@google.com>
> >
> > Bug: skia:12450
> > Change-Id: I92656ed40289872405c0873f2c56a52b04e35b1d
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452556
> > 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:12450
> Change-Id: I0d3b0969d2040dbb4ee808132146687767c97442
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452560
> 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>

Bug: skia:12450
Change-Id: I869cf3e2c47fe94981aa2ffa6c1f0b3e4d6e6862
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452717
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-09-24 18:58:15 +00:00
John Stiles
c81edd0e8c Reland "Rewrite switch statements in GLSL strict-ES2 mode."
This reverts commit d26d0e6a47.

Reason for revert: uses dedicated caps bit

Original change's description:
> Revert "Rewrite switch statements in GLSL strict-ES2 mode."
>
> This reverts commit 45e3838006.
>
> Reason for revert: Also need to rewrite them in actual ES2 mode.
>
> Original change's description:
> > Rewrite switch statements in GLSL strict-ES2 mode.
> >
> > Once this lands, switch statements will work everywhere--Metal, SPIR-V,
> > GLSL, and SkVM.
> >
> > Change-Id: I2797d0a872de8be77bb9f7aa6acb93421d571d70
> > Bug: skia:12450
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452356
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> Bug: skia:12450
> Change-Id: I92656ed40289872405c0873f2c56a52b04e35b1d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452556
> 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:12450
Change-Id: I0d3b0969d2040dbb4ee808132146687767c97442
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452560
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-09-24 16:43:42 +00:00
Ethan Nicholas
58d47fa1ec Renamed SkSL "offset" to "line"
https://skia-review.googlesource.com/c/skia/+/451419 changed the meaning
of "offset" throughout SkSL, so that it was actually tracking line
numbers rather than offsets (and thus had a misleading name). This
completes the transition by renaming all of the now-misnamed "offset"
fields, parameters, and variables to "line'.

Bug: skia:12459
Change-Id: I394e6441f6ddfaad6d4098352ba9b1bfeaf273be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450644
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-24 14:23:15 +00:00
Ethan Nicholas
47f76853c6 Use SkSL "offset" to actually mean "line"
SkSL internally tracks token offsets, but only ever reports errors using
line numbers. With the introduction of the DSL, which (being embedded in
C++ source) only has access to line numbers in the first place, tracking
offsets went from merely providing little benefit to actively making
life more difficult.

We are changing SkSL's position tracking from handling offsets to
handling line numbers, but to simplify the review process the change is
split up into two main steps. The first step (this CL) starts using
line numbers everywhere, but avoids the thousand-line churn of actually
renaming "offset", so most "offset" fields, variables, and parameters
will be briefly misnamed and will actually contain a line number.

The followup CL will complete the process by renaming all of the
now-misnamed fields, variables, and parameters, but will not make any
behavioral changes.

Bug: skia:12459
Change-Id: I30dc87cf4b816c5ddd7b8ae1be32586388962085
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/451419
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-24 13:16:11 +00:00
Brian Osman
d26d0e6a47 Revert "Rewrite switch statements in GLSL strict-ES2 mode."
This reverts commit 45e3838006.

Reason for revert: Also need to rewrite them in actual ES2 mode.

Original change's description:
> Rewrite switch statements in GLSL strict-ES2 mode.
>
> Once this lands, switch statements will work everywhere--Metal, SPIR-V,
> GLSL, and SkVM.
>
> Change-Id: I2797d0a872de8be77bb9f7aa6acb93421d571d70
> Bug: skia:12450
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452356
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: skia:12450
Change-Id: I92656ed40289872405c0873f2c56a52b04e35b1d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452556
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-09-24 13:11:05 +00:00
John Stiles
b8310aefc4 Run conditional-assignment loop forwards.
This makes for a slightly more easier-to-read disassembly; register
numbering no longer goes in reverse for vector assignment. Of course, it
makes no difference in the actual execution.

Change-Id: I86c5024bae1f73b1cd98252e4831207e47dc11eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452323
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-24 13:04:42 +00:00
John Stiles
45e3838006 Rewrite switch statements in GLSL strict-ES2 mode.
Once this lands, switch statements will work everywhere--Metal, SPIR-V,
GLSL, and SkVM.

Change-Id: I2797d0a872de8be77bb9f7aa6acb93421d571d70
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452356
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-23 21:22:09 +00:00
John Stiles
cb40008b43 Implement switch in SkVM.
SkVM implements switches as a pseudo-loop; breaks are handled with the
condition mask just like a for loop. Fallthrough is handled via a
scratch Value in a temporary slot. `writeStore` neeeded to be refactored
to support writing into slot(s) without an associated Variable.

At IR generation time, SwitchStatements are now emitted without error
even in strict-ES2 mode. The GLSL code generator currently reports these
as an error in strict-ES2 mode, but this will be fixed in a followup
coming shortly (the switch will be rewritten as ifs inside a one-shot
loop, similar to our IR-rewrite strategy).

Change-Id: I5507257246c42a35d2f46b4b9a89492a5ffeff9b
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/451421
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-23 19:58:55 +00:00
Brian Osman
448b2d5795 Add GetConstantValue, fix const-variable ES2 loop bounds
Prior to this fix, the new test cases would report that the various loop
terms needed to be constant expressions.

Bug: skia:12472
Change-Id: Ic377ed0c4598136ae38fb2b65c93b6d8609d54cb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452276
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-23 16:30:12 +00:00
John Stiles
d668d4da68 Fix SwitchWithFallthrough test on iOS.
It looks like returning from inside a switch on iOS gives wrong results
in GLSL.

Change-Id: I9d6d8971a7a54600268e27443815444fca6f3c61
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450994
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-21 20:15:32 +00:00
John Stiles
1c5eb4b371 Disallow continue inside a switch.
This fails on several platforms in practice, and is of very limited
real-world utility.

Change-Id: Ib476396fc33cb51af6bbcf7fe822d30703ed995d
Bug: skia:12467
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450993
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-09-21 19:26:40 +00:00
John Stiles
35bd92638f Add tests for switch + loop constructs.
Change-Id: I17b5e21a28140b8e9313d87af9b1145674214fdb
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450989
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-21 17:23:47 +00:00
John Stiles
e32309d771 Update switch-fallthrough test to run in dm.
Also, removed "switch containing dead code" test. This wasn't testing
anything meaningful. (When we had full CFG analysis, we could have
eliminated some of the assignments inside the switch body, but this is
not something we do anymore.)

Change-Id: Iaeb74ebee41a7f368113ede9a4e30c033b9de8ac
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450985
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-09-21 15:41:37 +00:00
John Stiles
b8f1651f9b Add workaround for switch() containing only a default case.
The Mac Radeon GLSL driver crashes when given a switch statement that
only contains a default case and returns a value. Adding a case works
around the crash, and doesn't affect the meaning of the switch.

Change-Id: Iabbd267e0e31e8df7d3b7e747a7204d50931d0be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450977
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-21 14:22:17 +00:00
Ethan Nicholas
2816dcfc67 Fixed fuzzer-discovered bug with interface blocks
While I was in this code, I realized that the setVariable method of
InterfaceBlock was unused and there was therefore no reason to be
storing a pointer instead of a reference.

Bug: oss-fuzz:39000
Change-Id: If7505ba87f4060370cfd32ca2e30c76648965101
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450446
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-21 14:12:47 +00:00
John Stiles
ed2babaf94 Reland "Add switch statement support to PipelineStage."
This is a reland of be056f4f62

The Switch test has been restructured to dodge an iOS bug.

Original change's description:
> Add switch statement support to PipelineStage.
>
> This allows us to write SKSL_TEST_ES3 tests in SkSLTest and have them
> run properly. Previously, such a test would assert inside the pipeline-
> stage generator. In ES2 mode, we will rewrite switches as chained ifs,
> but in ES3 mode we will want to continue emitting them as-is (they will
> be faster than chained ifs on a modern GPU).
>
> `writeSwitchStatement` is adapted from GLSLCodeGenerator.
>
> Change-Id: I532ea5ed49869e7cdffced0cdcd0e353af8d4d79
> Bug: skia:12450
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450478
> 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>

Bug: skia:12450
Change-Id: I5102081c636ef09cd23f5bc894e6c96e92a4c121
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450757
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-21 13:07:50 +00:00
John Stiles
8e369b0a5a Replace break with continue inside empty (post-optimization) loop.
This fixes a driver bug with the Nexus 7 while retaining the meaningful
part of the test.

Change-Id: I98edab32132f0c52a1f69b03efd403fae43c336b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450482
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-21 13:06:50 +00:00
John Stiles
d288d8bc84 Revert "Add switch statement support to PipelineStage."
This reverts commit be056f4f62.

Reason for revert: apparently switch on iOS GLSL is extremely broken

Original change's description:
> Add switch statement support to PipelineStage.
>
> This allows us to write SKSL_TEST_ES3 tests in SkSLTest and have them
> run properly. Previously, such a test would assert inside the pipeline-
> stage generator. In ES2 mode, we will rewrite switches as chained ifs,
> but in ES3 mode we will want to continue emitting them as-is (they will
> be faster than chained ifs on a modern GPU).
>
> `writeSwitchStatement` is adapted from GLSLCodeGenerator.
>
> Change-Id: I532ea5ed49869e7cdffced0cdcd0e353af8d4d79
> Bug: skia:12450
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450478
> 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>

Bug: skia:12450
Change-Id: If40c90023a64c608181285f6470b3e75303cc3cc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450756
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-09-20 21:57:09 +00:00
Greg Daniel
84261653c5 Reland "Add new GrSurfaceInfo class and related backend structs."
This is a reland of 6aac1193a7

Original change's description:
> Add new GrSurfaceInfo class and related backend structs.
>
> Bug: skia:12402
> Change-Id: I45b2f71dcfa5843e2a19a8de7d34196a4d552905
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445176
> Commit-Queue: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Bug: skia:12402
Change-Id: Id540bea408d72ceba43ec4245c3748d630121926
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450277
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-09-20 20:44:53 +00:00
Michael Ludwig
8c9c185a92 Simplify SkAAClip to use SkClipOp, not SkRegion::Op
Bug: skia:10205
Change-Id: I19039f72db1052db27f5819fafdc5ba8eb8af909
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445618
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2021-09-20 20:12:38 +00:00
John Stiles
be056f4f62 Add switch statement support to PipelineStage.
This allows us to write SKSL_TEST_ES3 tests in SkSLTest and have them
run properly. Previously, such a test would assert inside the pipeline-
stage generator. In ES2 mode, we will rewrite switches as chained ifs,
but in ES3 mode we will want to continue emitting them as-is (they will
be faster than chained ifs on a modern GPU).

`writeSwitchStatement` is adapted from GLSLCodeGenerator.

Change-Id: I532ea5ed49869e7cdffced0cdcd0e353af8d4d79
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450478
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-09-20 18:55:09 +00:00
Ethan Nicholas
f62934b85a Fixed DSLParser error uncovered by ossfuzz
Missed one more case of Optional<Wrapper<Expression>>. This should be
the last one.

Bug: oss-fuzz:38944
Change-Id: Ic7f790cd99e2a3ee1c3874cc767a4702265d1723
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450476
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-20 17:57:14 +00:00
John Stiles
498bfa4a85 Add test case for 'loop over switch with continue inside.'
We didn't have a test case for this particular construct, but we will
emit special code to handle it when rewriting switch statements.

Change-Id: I7ac632f7bee348194940812c956c8a7df51ffaff
Bug: skia:12450
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450477
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-20 16:47:02 +00:00
John Stiles
809707671a Reorder optimizations to eliminate more code.
Unreachable code might contain the only reference to a variable or
function. We can eliminate those variables/functions if we remove the
unreachable code first.

(Are there counterexamples where this order leads to worse results? I
couldn't think of any, and pragmatically it didn't show up in any of
our existing tests.)

Change-Id: Ic9f0222851269e0c37eb9570547307998f882b6c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450156
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-20 13:57:46 +00:00
John Stiles
b42b926513 Add additional examples to UnusedVariables test.
`increment` and `float a` could be eliminated, but are not.
This is fixed in a followup CL.

Change-Id: I7a5c3ab7341f40020f84f157b08a7152bc067af0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450276
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-20 13:57:39 +00:00
Ethan Nicholas
c973d26854 Fixed DSLParser assertion error uncovered by fuzzer
Bug: oss-fuzz:38108
Change-Id: I0e055d837923f00b982bc395dbf29b6ff59a3b21
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448896
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-20 13:48:41 +00:00
Jim Van Verth
2f7ee02577 Revert "Add new GrSurfaceInfo class and related backend structs."
This reverts commit 6aac1193a7.

Reason for revert: Breaking Mac bots in Chromium roll. Looks like
they depend on getVkImageInfo for an unknown reason, and it's 
hidden behind SK_VULKAN.

Original change's description:
> Add new GrSurfaceInfo class and related backend structs.
>
> Bug: skia:12402
> Change-Id: I45b2f71dcfa5843e2a19a8de7d34196a4d552905
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445176
> Commit-Queue: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Bug: skia:12402
Change-Id: I3c9642354dae8c955bc58d281700536393f84519
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450199
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
2021-09-18 16:35:38 +00:00
Greg Daniel
6aac1193a7 Add new GrSurfaceInfo class and related backend structs.
Bug: skia:12402
Change-Id: I45b2f71dcfa5843e2a19a8de7d34196a4d552905
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445176
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-09-18 00:41:07 +00:00
Ethan Nicholas
0dc1e0f797 Fixed DSLParser unary expression error handling
Missed a case when eliminating optional/wrapper in an earlier CL.

Change-Id: If7f80ea6e2172acadf7b0087fe1a05853ccae445
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449838
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-17 17:55:13 +00:00
John Stiles
d0665d9a04 Fix fuzzer-discovered null deref with empty interface blocks.
Change-Id: I95bfa33f674bce0098cc6d4469c8c85ee8b051f5
Fixes: oss-fuzz:38875
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449817
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-17 14:19:14 +00:00
John Stiles
eb68973c2f 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>
2021-09-16 22:14:49 +00:00
John Stiles
2dda4b6563 Allow Mangler to be used outside of Inliner.
It now lives in the Compiler instead of the Inliner, and we have a
pointer to it inside the Context. I also took the opportunity to remove
a few unnecessary #includes from Context and fixed up the fallout from
this.

POTENTIALLY IMPORTANT FOOTNOTE: DSLWriter has its own Mangler object
inside of it. I experimented with removing this and having it share the
Mangler object inside the Context, but this caused failures in
"testThreading" and "testpersistentcache" CQ bots; the Expected and
Actual images would mismatch. (The Expected would be missing some
draws.) This particularly affected some blur-related tests which are
likely to be GaussianConvolution--i.e. it uses DSL.
This might be a canary in the coalmine for some quirky DSL threading
issue? e.g. using the same Context on two threads at once?

Change-Id: I2290b810d9487c40a3dbef119c95d1572b14a5ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449357
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-16 21:53:19 +00:00
Ethan Nicholas
2280058446 Reenable DSLParser
Change-Id: I1819b2c40902611d7e86245bff73ad8c2bd7629c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449060
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-09-16 15:28:19 +00:00
John Stiles
25868511fd Add fwidth intrinsic to public SkSL in ES3.
Change-Id: Icd55cc62db41ba6c7008161512e5584927f21757
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449190
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-09-16 01:56:32 +00:00
John Stiles
0dc09b190a Update SkSLTest to check a 2x2 surface.
Derivative tests don't really make intuitive sense in a 1x1 surface.
This should give the derivatives a better workout.

Change-Id: I09e5ff2b7815d42d1874e5deef360e9bc2e83aff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449195
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-16 01:56:02 +00:00
John Stiles
c915da9b20 Revert "Enable MatrixConstructors test, outside of Quadro P400."
This reverts commit c6ce617a8a.

Reason for revert: more Golo machines with old drivers

Original change's description:
> Enable MatrixConstructors test, outside of Quadro P400.
>
> Change-Id: Ie228befa747351445981093f6c57308e338c9684
> Bug: skia:12443
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449181
> 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>

Bug: skia:12443
Change-Id: Ica465810257b2f949a0cf0def62e699f7c2877cc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449258
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2021-09-15 20:08:13 +00:00
John Stiles
c6ce617a8a Enable MatrixConstructors test, outside of Quadro P400.
Change-Id: Ie228befa747351445981093f6c57308e338c9684
Bug: skia:12443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449181
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-09-15 19:23:20 +00:00
John Stiles
4f1593eaee Use swizzles in Metal matrix helper functions.
Not a big deal necessarily, but considering using this logic in GLSL as
well, and I'm less confident that your average GLSL ES driver will
optimize away the separate array loads.

Change-Id: I6a9f0d18c0fac138f64ad6426670f615e17f3492
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449099
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-09-15 17:32:45 +00:00
John Stiles
a047e8bf4d Improve overflow checks in CheckProgramUnrolledSize.
The fuzzer discovered that it could overflow the program-size value.
Rewrote the logic to use SkSafeMath everywhere, and to early-exit as
soon as a statement manages to exceed the program size.

Change-Id: I01511b2201173c95ebc1ac602901410ac9d74d73
Bug: oss-fuzz:38697
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449098
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-09-15 16:42:45 +00:00
Brian Osman
a81e7e2e71 Report an error if sk_LastFragColor is referenced without fbFetchSupport
Adjusted default caps in skslc to be consistent with runtime behavior,
and added optional settings mode to enable the feature. Tests for both
scenarios. (The error test crashed prior to the fix).

Bug: oss-fuzz:38726
Change-Id: I5270d4837ac982085d7baf5abd4b361f7bfb8562
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449062
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-09-15 15:41:49 +00:00
Ethan Nicholas
fd1332f5a7 Reland "Fixed DSL assertion error on source files containing nulls"
This is a reland of db38ad7b14

Original change's description:
> Fixed DSL assertion error on source files containing nulls
>
> The assertion was there to make sure we weren't running off the end of
> the source, but naturally fails in the presence of legitimate embedded
> nulls.
>
> Change-Id: I3b80499e9b182c9ea046c479f35d7a965d548401
> Bug: oss-fuzz:38107
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447182
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: oss-fuzz:38107
Change-Id: Idb1a6b7c64d2bb954edadae828d6de808158fd3f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448660
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-09-15 15:00:23 +00:00
John Stiles
78b84cc76a Fix size calculation of blockless for-loops.
Change-Id: Id6e1d1be276af01ce05777682dde8b58d803aedc
Bug: oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449097
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-15 14:29:45 +00:00
John Stiles
857c7f9dc5 Add program-too-large test using for loops without blocks.
The fuzzer has discovered a bug in our program size-checking logic; for
loops that immediately contain another for loop (with no block) were not
counting the inner loop's iterations. This allowed it to exceed our
maximum program-size threshold (and time out during SkVM compilation).

This test demonstrates the issue. A followup will fix it.

Change-Id: I3b7d4c8a4f0ed04cf0aba3f1a32fdad7d6d784e7
Bug: oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449096
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>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-09-15 14:19:00 +00:00
John Stiles
b701fa0ac0 Add non-square MatrixCompMult support to public SkSL in ES3 mode.
We already had a test case here, but it wasn't actually in operation.
The test has been split into ES2 (square) and ES3 (non-square) halves,
returns the color like a proper runtime effect, and it's now running in
dm.

Also, Metal doesn't natively support matrixCompMult, so it injects a
helper function; I tweaked the helper so it no longer requires an extra
result variable.

Change-Id: Ie79242768966fcbe879ad73461d17b4fb8e55670
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448117
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-14 18:15:23 +00:00
John Stiles
143e850237 Fix Metal row/column confusion, again.
We didn't have any tests which exercised the non-square matrix case
(because such a test requires ES3), so it was silently broken. It's
now fixed. The tests exposed a DIFFERENT Quadro P400 bug which will be
fixed separately.

Change-Id: Icf24acad5ea6f18aea3d8aa5a903e7bea41a5c23
Bug: skia:12443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/448379
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-09-14 17:06:48 +00:00