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>
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>
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>
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>
MetalCodeGen would incorrectly identify `(someMatrix, someScalar)` as a
math operation between `someMatrix` and `someScalar` and attempt to
convert `someScalar` to a matrix. If `someScalar` was a Boolean type,
this would lead to an assertion.
The binary expression is now checked more thoroughly before converting
the scalar into a matrix.
Change-Id: Id7e104d5533d8c43375927d4815b83e1a3c36be1
Bug: skia:11125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/410682
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 90508f02dc.
Reason for revert: avoiding driver bugs this time
Original change's description:
> Revert "The Matrices test now verifies its results."
>
> This reverts commit 86121f6c0e.
>
> Reason for revert: tree sad
>
> Original change's description:
> > The Matrices test now verifies its results.
> >
> > Previously, this test did a bunch of matrix math but never actually
> > checked its results for correctness.
> >
> > Change-Id: I353be58049286266c2d561b0939b3874d2684403
> > Bug: skia:11985
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
> > 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>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I1335f01c14ee955426e02efaa3c30421cd41aa34
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11985
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407617
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:11985
Change-Id: I214375d74977f324973da72c440d7ff5ff179016
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/408157
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 86121f6c0e.
Reason for revert: tree sad
Original change's description:
> The Matrices test now verifies its results.
>
> Previously, this test did a bunch of matrix math but never actually
> checked its results for correctness.
>
> Change-Id: I353be58049286266c2d561b0939b3874d2684403
> Bug: skia:11985
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
> 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>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I1335f01c14ee955426e02efaa3c30421cd41aa34
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407617
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, this test did a bunch of matrix math but never actually
checked its results for correctness.
Change-Id: I353be58049286266c2d561b0939b3874d2684403
Bug: skia:11985
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/407360
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>
This will be implemented in Metal and SPIR-V in followup CLs.
Change-Id: I397b4db40b15dd54cf1d8a17f414c3fe184b48d2
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387638
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This can eliminate const variables which have been completely folded
away, unnecessary synthetic variables created during codegen/inlining,
or code that simply didn't need to exist at all.
Change-Id: I37a65e455e6527a6a6c2f4dde918f48d84dc2638
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383496
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 92748af1a5.
Reason for revert: SkSLCommaSideEffects_GPU crashing on Android
Original change's description:
> Inline functions of the form 'return (expr)' only.
>
> This drastically reduces the number of functions which we allow to be
> inlined. If this change does not hurt our performance, it will allow us
> to trivially remove hundreds of LOC. All current data leads us to
> believe that it may affect the Mali 400 but is highly unlikely to change
> results on any other device in the tree.
>
> More info: http://go/optimization-in-sksl-inliner
>
> Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I6a670dacaa58fe3386ff50375ac6d1cac4fd7f2c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385161
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This drastically reduces the number of functions which we allow to be
inlined. If this change does not hurt our performance, it will allow us
to trivially remove hundreds of LOC. All current data leads us to
believe that it may affect the Mali 400 but is highly unlikely to change
results on any other device in the tree.
More info: http://go/optimization-in-sksl-inliner
Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
I ran into an issue in an upcoming CL which generated a particularly
ugly switch statement:
switch (x) {
default:
discard;}
So I cleaned this up, and while resolving this issue, managed to improve
a bunch of existing codegen as well. The formatting change has been
split out to a separate CL since it impacts so many golden outputs.
Change-Id: I7a6be29903c47560dcc7f6acd3ef15fd0c5c3c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384179
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This CL will be used to test for potential performance regressions (or
improvements) that we might cause by disabling this optimization pass.
It will be reverted in ~1 day.
Change-Id: I26b7687c341eb6d81231406381c39869cfccf6d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381259
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 50b1b2b90d.
Reason for revert: ending experiment
Original change's description:
> Disable control-flow analysis in SkSL. (Performance experiment)
>
> This CL will be used to test for potential performance regressions (or
> improvements?) that we might incur by disabling this optimization pass.
>
> It will be reverted in ~1 day.
>
> Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ie385a82db237ff5651348d82b9651f8ba09375b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379581
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL will be used to test for potential performance regressions (or
improvements?) that we might incur by disabling this optimization pass.
It will be reverted in ~1 day.
Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
These variables were later being eliminated by the dead-code-elimination
pass, so you can't see them directly in the final output, but removing
them affects the name mangling off all future symbols, so it causes an
enormous ripple effect in the diff. And of course, it's a waste of time
and memory to synthesize IRNodes just to destroy them later.
If we disable control-flow analysis, we lose the dead-code-elimination
pass entirely; this change is also beneficial for emitting better code
when optimizations are turned off.
Change-Id: I882b3be4f3fd99b77d99b6abe128f26bb9252c89
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375776
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit c501857188.
Reason for revert: breaking many bots
Original change's description:
> Add support for matrix == and != in Metal shaders.
>
> We need to polyfill an operator== and != when these are first
> encountered in the code.
>
> Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
> Bug: skia:11306
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Id583109a0d167c2c58a57644b14cd5f49d670737
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368801
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
We need to polyfill an operator== and != when these are first
encountered in the code.
Change-Id: I539c838ee1871bcb0c4b66abb8a4a0f91146cd4f
Bug: skia:11306
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368496
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This uncovered a bug in Metal code generation of `matX *= matY` which is
now fixed. (It was emitting the helper function more than once.)
Change-Id: I0aeb0efe7ab5fbf5592a8ca6f4f5b50354d3d7f4
Bug: skia:11262
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/365489
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>
