GLSL does not support assigning to ternaries, and will fail to compile
and/or generate non-functional shaders if we pass in a shader that tries
to assign into a ternary expression.
If SkSL is able to completely eliminate the ternary (e.g. if it boils
down to a simple `true ? x : y` or `false ? x : y`), SkSL can strip out
the ternary entirely and generate valid GLSL. This case is harmless and
so it is still allowed.
Change-Id: I960f119fb9934f998697634e6c4e519cd77d3780
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319679
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I452e52a87d89cefb5c21a0d9d57e9771f3038d73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319783
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Was checking scale+translate but that was overly strict.
Bug: chromium:1087705
Change-Id: I19c54eab9baffac21cfaf4ef781a60566a39f666
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319779
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
When reverse mapping bounds through SkDisplacementMapEffect, we need to
consider both the color input and the displacement input because they
could both directly, or indirectly, reference the source.
Bug: chromium:1128962
Change-Id: I03489fd2391f2ea595945961bf7e940a0e200cab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319656
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This was unused and did not work on non-GLSL backends.
Change-Id: I6bd314d43cfefa64871b5c0e964b5ae52e494164
Bug: skia:10757
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319778
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Note: I intend to make shaping.html live in demos.skia.org, but
that is hard to verify it works until the APIs land.
Change-Id: I3af1cf8ded316f616d05ee60198393ed9c9a284f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319130
Reviewed-by: Nathaniel Nifong <nifong@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This will allow the inliner to use IsAssignable.
Change-Id: Ic94f71002779b53d0b3dc97f37fbe4bb98b026d8
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319414
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL implements and exercises the preference for gpu-generated content.
This CL also switches to drawing a rect (vs. an arrow) since drawing
a concave path on the gpu can be fraught.
Bug: 1108408
Change-Id: Ieec1619b5357ffb31aa74b471ea09c061bd8f74e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319416
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
These tests will also be used for Metal and SPIR-V testing. A small
handful of GLSL-specific stragglers (#version-specific or type-precision
related) will remain in /glsl/.
Change-Id: I7f2b2bd92825c327922c8ce74e438d2daa440dff
Bug: skia:10649
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319408
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Now uses a GN template to avoid copy-pasting the same logic for each
type of test we want to perform, and the same file to be compiled in
more than one way at a time via an extra flag to compile_sksl_tests.py.
Change-Id: I8aadedeb140d78d58a345a2bac0da3d9c77e9a19
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319347
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We had lots of checks just checking defined but we always define
GR_TEST_UTILS
Change-Id: I588c50ddd91f71618a96ab6c9eda2050b423f611
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319682
Commit-Queue: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The absence of the FieldAccess::fBase traversal appears to be a simple
oversight. This doesn't appear to affect any tests.
Change-Id: I82a5828acedd00f62bf177bd2cf70d67071a83fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319413
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We will soon be re-targeting variable references, and this is going to
be much easier (and cheaper) than replacing the entire VariableReference
itself.
Change-Id: I8febc44a1c06e99251153f038a4f5f693cd30231
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319344
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Just noticed in passing that this was being used more broadly than warranted
Change-Id: I233060a272c4ed6b7fde7b830cbb5b16cad4cb29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319350
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is a reland of 2bded27a96
Original change's description:
> Allow rect and circle blur fast cases to be used with rotation matrices.
>
> For circles this is trivial. The existing shader works as is.
>
> For rects this requires back projecting from device space.
>
> Adds a GM for rotated rect blurs and modifies a circle blur GM to add
> rotation.
>
> Bug: chromium:1087705
>
> Change-Id: I6b969552fbcc9f9997cfa061b3a312a5a71e8841
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318757
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: chromium:1087705
Change-Id: Ie3ef96c40461665f6207225a15e19b4cf5bcba94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319338
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Change-Id: I93ff7e5f1062c6a85152c587fcedc34e9257dd27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319345
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
I've been thinking and rethinking and rethinking how best to use 16-bit
values like Q14 fixed-point in SkVM. Here's some ways:
A) don't... just use 32-bit values instead
B) use 16x2-bit pairs to match the narrower 32-bit lane count
C) double-pump 32-bit values to match the wider 16-bit lane count
D) use native 16- and 32-bit values and let the backends sort it out
A) is how things work today, and C) is how SkRasterPipeline's lowp mode
works. Having tried out B) and C) both for a good fair shake, they were
both already awkward to work with after writing just a few functions. I
would not give up on them entirely, but they're no longer my favorites.
D) is subtle and my new favorite. It's easiest to program with SkVM
when the values we're holding represent single values and the backend
handles any parallelism for us. That suggests we add a simple 16-bit
Q14 to the existing 32-bit I32 and F32 types, where they can be actively
converted between as normal, but not freely no-op bit punned. D) says
we people shouldn't have to choose between A-C) up front... each backend
can handle it themselves.
Under strategy D), it's entirely the backend's job to decide how to
represent each value, and how to to vectorize them. We don't need to
know as a user, and the backends can use the program itself to inform
how they vectorize. 16-bit values could live in xmm registers and
32-bit values in ymm, or the 16-bit values could go in the low half of a
ymm, or the even lanes of a ymm, or a full ymm and use two for 32-bit
values, etc. etc. This all is a backend choice, not something we should
have to know about when writing a program using Q14/I32/F32.
My next steps are to get Q14 operations tested and plumbed through the
JIT again, and to build out a blitter and a few effects using Q14 color
channels. Then, independently, we can look at each backend and how to
vectorize them. Some ideas:
1) keep running at current vectorization, with half rate 16-bit ops
2) pump up to 2x wider vectorization unconditionally to favor 16-bit
3) pump up to 2x wider vectorization only when any 16-bit op is used
These choices can be made independently for each backend (JIT, LLVM,
interp), and I wouldn't be surprised to find that we'll want to do them
differently. For instance, the interpreter is already running at 32x
vectorization... might be pumping it higher won't help anything.
Change-Id: Ib8ad2b1bf790e8c4e3acfb4818d4032f7628e8f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319321
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Also adds GalaxyS20 to the mix, which wasn't running skpbench
previously.
Also removes the skpbench logic to fail if we don't recognize the
hardware or have specific scripts for it. We don't have time to reverse
engineer every new piece of hardware we want to run on and the general
android script is quite helpful already.
Bug: skia:10419
Change-Id: I0e139cdd4bc2e7ca0e2e14c715d319664fa8c949
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319143
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Records programInfos for the stroke ops and for the stencil portions
of the path ops.
We can't prePrepare programInfos for the fill portions yet because it
would require multiple GrPipelines that all reference the same
GrProcessorSet. And GrProcessorSet is currently designed to be
std::moved into one single GrPipeline.
Bug: skia:10419
Change-Id: I3b8c061da181e20d3ff68746cf4b9c61f6d73a88
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319256
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: I5b4fe40847112a11d6057ee7acd208879a71722f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319190
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 reverts commit b61c3a9a01.
Change-Id: I42d93bdc6455c8ef941a6cbe1339df2ba916bb3c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318697
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
We don't want to be polluting the global namespace with external values,
especially when the typical/recommended way to use the Compiler is with
a single long-lived instance. Force client code to manage ownership (the
only non-unit-test case was already doing this), and pass external
values to convertProgram, so they can be added to the Program's symbol
table.
Change-Id: If4c1db5e48a62e2cf4333b8d80420f2dfede27ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319125
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 2bded27a96.
Reason for revert: Seems to be blocking the Chrome roll
Original change's description:
> Allow rect and circle blur fast cases to be used with rotation matrices.
>
> For circles this is trivial. The existing shader works as is.
>
> For rects this requires back projecting from device space.
>
> Adds a GM for rotated rect blurs and modifies a circle blur GM to add
> rotation.
>
> Bug: chromium:1087705
>
> Change-Id: I6b969552fbcc9f9997cfa061b3a312a5a71e8841
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318757
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com
Change-Id: Iafb479f3b3561e226678a3020254c6e76d4ce284
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1087705
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319186
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Adds base class GrD3DAlloc and GrD3DMemoryAllocator, and a reference
to a GrD3DMemoryAllocator in GrBackendContext and a reference to a
GrD3DAlloc in GrD3DTextureResourceInfo. Internally, we override this
base class to define the AMD memory allocator.
Change-Id: I033924b0247ea330969b1398f25985e7a84aec11
Bug: skia:9935
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317243
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
If the blob is empty, then try to regenerate it. Using
this method caused a slowdown in Skia perf, so we
added an extra check to allow some empty blobs through
for perf performance. The perf problem was caused by
SKPs generate empty blobs because of font mismatches.
Flutter has shown that scaling from very small to
normal size is not correctly handled by the existing
check. This CL favors correctness over optimizing empty
text blob and always regenerates empty blobs.
https://github.com/flutter/flutter/issues/64936
Change-Id: Ib18ecb684b0af5cf6dce274b6dc09a9c61b17c77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319031
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Many calls to `setRefKind` failed to check the return value; if it's
false, an error has occurred and the program is in a bad state.
Specifically, there is an assignment to a variable that's not marked as
"written-to." If we continue processing the program, we're likely to
assert.
Change-Id: I2dd5d1f41aa5ca0d30f8d638f05fe2e838216d78
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Adds various optimizations to GrWangsFormula as well as "pow4" variants
of the formula that are quicker than the standard and/or log2 versions.
Uses the pow4 variants in GrStrokePatchBuilder.
Bug: skia:10419
Change-Id: I8478582df5296b088d25808bcaeb93107ff20797
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318954
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
These cover:
- Properly configured out-params
- Invalid/non-lvalue out-params, which currently cause an SkSL crash
- Interactions between the inliner and variable swizzles
Change-Id: I4874101236084f273e704d8717149b431d813883
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319036
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
For circles this is trivial. The existing shader works as is.
For rects this requires back projecting from device space.
Adds a GM for rotated rect blurs and modifies a circle blur GM to add
rotation.
Bug: chromium:1087705
Change-Id: I6b969552fbcc9f9997cfa061b3a312a5a71e8841
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318757
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
- Remove a spurious symbol table inserted by convertProgram. start()
already pushes a symbol table, and this was pushing a second one,
which didn't seem necessary. (The Parser can inject symbols for types
it discovers, but I can't justify those needing to be in a different
table than the rest of the program elements?)
- The convertProgram one had a comment indicating that it was popped by
the Compiler. That wasn't true, so this gets us one step closer to
balance.
- The one in start() is meant to be balanced by a pop in finish(), but
no one ever called finish(). Add that call in, and also rearrange
things so that the base symbol table is a parameter to start(), rather
than just setting it on the IR generator. (There's more of this
pattern around, but I wanted to limit the scope of this CL).
- When dehydrating the include files, we had logic to work around the
extra symbol table (absorbing the symbols) - that's not needed now.
- Simplify some other logic in processIncludeFile (no need to make so
many string copies). Always just put the incoming include file strings
into the root table, also. It's largely irrelevant where they go.
Change-Id: I18d897af3d5fa6506e11024beb9bb70e6cc5b538
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319038
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
