This includes compile-time optimization and tests.
The unit test is disabled in a followup CL
(http://review.skia.org/447057) because it exposes a Radeon 5300M bug
in OpenGL.
Change-Id: I8b2f0411358aeb68c4edfeb0bd7a2814c4be1f40
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447056
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:10412
Bug: skia:12437
Change-Id: I93077bbd2ed40252966305df1b93ceb813218828
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446181
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
It turned out that Metal had equivalent intrinsics/casts all along; we
just needed to emit them.
Tests will be improved in a followup CL which adds the ES3-compatible
packing intrinsics into sksl_public.
Change-Id: Iec8a20b9f9fe9b1badea2944eb0b1f0a17c74560
Bug: skia:12351
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446744
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>
The test has been improved and now covers a variety of values. Because
this intrinsic has side-effects (an out-param), we do not support
optimizing it or treating it as a constant-expression.
The modf documentation doesn't mention anything about constant-
expression support or lack thereof. Experimentally, modf is also not
treated as a constant-expression by Apple GLSL or glslang:
http://screen/4RWwYKr6vCjxCPQhttp://screen/45ttDTVAFGDRyxP
Change-Id: I15bb1de80e90fa97ddf8e9d3803352603b9608d0
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446396
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The code in writeFunctionCall which supported swizzled out-params has
been factored out into a pair of helper functions. The code for emitting
intrinsics now relies on these helpers when emitting out-params.
Change-Id: I4436185ae107d70b529e7e1ea0dd89f844c6a673
Bug: skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446719
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 0f4304e6e7.
Reason for revert: breaks Adreno 6xx
Original change's description:
> Add RelaxedPrecision decoration to function-call temp vars.
>
> This is really same basic issue as http://review.skia.org/446640. We
> were creating a temp variable but ignoring its type's precision.
>
> Change-Id: I9a5fedd7ada864d36757fc196f42ff95bac7d706
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446718
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I6ae4e264b60f7f38a1abb5f1d0324461a33c896d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446742
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>
An unused child that used its coordinates would triggers asserts in the
GP - we expect that all FPs are sampled/invoked. This generates slightly
sub-optimal code (we compute and interpolate the local coordinates for
that child FP), but it's not likely to happen in real code.
Bug: skia:12429
Change-Id: Ic2ddc65d16a7e1f47af8c4192e5ff9ea329bf335
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446836
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is really same basic issue as http://review.skia.org/446640. We
were creating a temp variable but ignoring its type's precision.
Change-Id: I9a5fedd7ada864d36757fc196f42ff95bac7d706
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446718
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
skgpu::v2::Device will also need both of these so move them somewhere accessible
Bug: skia:11837
Change-Id: Iacaf0332da933515451056eeda62949b049378b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446180
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We don't need to initialize input values for `out` params. (We were
treating them the same as `inout`.)
Change-Id: Ib447d15de237a6a03740ad012180691dc60a50bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446717
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously we were not honoring the variable precision at all (passing
null to nextId).
Change-Id: I0e217e6c0d3d701dc0540d4d4069a3597abdad11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446640
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:9282
Change-Id: I04afd352ad8c0c167993f29f4293d857f5af7170
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445619
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
DSL was improperly allowing interface blocks in runtime shaders, which
caused PipelineStageGenerator to get upset.
Bug: oss-fuzz:38131
Change-Id: I593e68f2cab3db9151d606e65e2826ffa9c494e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446324
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Two minor changes:
- Converting a Block with bad statements will now generate a partial
block instead of nullptr.
The change mirrors how DSL behaves; functions containing invalid
statements will now be created and added to the program. Previously, we
would discard a function definition with any invalid statements inside;
this prevented duplicate-function-definition errors from appearing.
- Converting a return with a bad expression will now generate a
poisoned return instead of nullptr.
This change improves diagnostics for functions with invalid return
statements. If we eliminate the return statements (by returning null),
we report bad return statements as "function can exit without returning
a value" (which is confusing).
Change-Id: I6d998d5c50585f8d96bb7e3cb7f59b63125d6a62
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446325
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
To do this with a clean conscience, I needed to convert the unroll-
counting logic from a linear time algorithm to constant-time. Getting
all the edge cases correct requires a lot of care, and there are now
plenty of unit tests.
Change-Id: I620909d069ac425b7310e345bf80ec844fe035f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445643
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This allows us to get rid of a lot of .c_str()'s.
Change-Id: I09102f90d69620614dc5a7a2ebc64bd3e9b1c437
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445816
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Several fuzzer issues, and one Chromium issue that's blocking the roll.
Bug: chromium:1246795
Bug: skia:12423
Change-Id: I00370b74569b447e543d9a1f22c588eb493063da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445960
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
An ES2-compatible for loop supports six separate rel-ops:
< <= > >= != ==
Each rel-op, in addition to its expected usage, is also able to
represent a loop which never terminates, as well as a loop which
terminates instantly. Since SkVM unrolls these loops, we should make
sure we do it properly. We now have unit tests for all of these cases.
Change-Id: Icae04d48bc158bf8c0c98db97f76756a1a29110c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445756
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Intrinsic-call optimization can be triggered during inlining. In this
case, inlining turned `normalize(x)` into `normalize(constant-value)`.
DSL is used to implement optimizations for a handful of intrinsic calls,
including `normalize`, which internally relies on `length`.
The DSL expects that it can use the IRGenerator to handle function
calls. This was not working because we were finished with the initial
compilation pass, and the IRGenerator's symbol table is removed when
finish() was called.
We now temporarily give a symbol table back to the IRGenerator while
the inliner runs. We remove it again as soon as inlining is complete.
Change-Id: I6da98788d93749ffeb008c1f4c3f72b436e8ceeb
Bug: oss-fuzz:37994
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Id894eb70273454716eb33c85dff2056333e90cdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445281
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12302
Change-Id: I4ff394f1f9d93d2def19a9f9d49cb208651aff10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445639
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also update RELEASE_NOTES to describe new syntax.
Change-Id: I2666551b98f80b61ae3a48c92a9e306cdc7242b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444735
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This analysis pass assumes we have a program with a valid structure--all
loops must be ES2-compliant, and all function-calls must reference real
functions that exist. If we detected an error during compilation, our
program might not meet these criteria.
Change-Id: I4c7aefb3221438643614f1e0cbc2bad40b94b161
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444982
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This moves the swizzle domain test into the path used by the DSL so that
the error check benefits both sides. To make this possible we need to
be able to distinguish between equivalent swizzle components like x and
r, so they aren't collapsed down to the same component until the very
end.
Change-Id: I48f2582886391eabd7ce6eae949babdeead6051e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445280
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Previously, deeply-nested loops could lead to stack overflow during SkVM
dead-code elimination. The DCE algorithm in SkVM now stores its worklist
on the heap and does not rely on recursion. Since we now enforce an
upper bound on maximum program size within SkSL, this worklist's size
should always be reasonable and bounded.
Change-Id: I8b7955298b2540a9f600fc5d94d3bc804c68632c
Bug: skia:12396, oss-fuzz:37827, oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445597
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Our program-size analysis pass needs to recurse into called functions;
depending on the exact order of functions in the program, this recursion
can hypothetically be as deep as the deepest function-call chain. Set an
upper bound on recursion here, so we don't overflow the stack while
trying to check the program size. In practice, 50 frames is far deeper
than a regular shader should ever go.
Change-Id: I733ee48dad6f8053facdfd9f6d8a2b9b2a4af188
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445279
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This allows us to remove the static-recursion analysis pass entirely,
while still providing the same results.
Change-Id: If1564cd4df55be86ca4e0bf53ecc094ba76007df
Bug: skia:12396
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445296
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The fuzzer is currently learning to make unboundedly-large programs by
nesting medium-size loops repeatedly. SkVM doesn't have a mechanism to
limit the ensuing explosion of code and ends up making unreasonably deep
stacks and/or unreasonably large programs.
SkSL now enforces an upper bound of approximately 100,000 IR nodes on a
fully-flattened, fully-inlined strict-ES2 program. The limit is picked
out of thin air, but this should be enough to prevent SkVM from going
haywire while still being large enough to handle any reasonable program.
We can definitely tune this value if we find that it is too large
(admitting dangerous code) or too small (rejecting good code).
Change-Id: I11735636175721fbc79460b4e194d8e4b42dc47d
Bug: skia:12396, oss-fuzz:37827, oss-fuzz:37837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444358
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I06ba9dd9ed8af8555233ddfa10d3e0ec6babc2ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444759
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Most of the logic in IRGenerator::finish has moved to
Compiler::finalize. The @if/@switch pass has been combined with the pass
that verifies no dangling FunctionReference/TypeReference expressions,
saving one walk through the IR tree. Most program-finalization logic now
exists in Compiler and Analysis.
This change reorders our error generation logic slightly, and manages to
squeeze a few extra (valid) errors out of one of our fuzzer-generated
tests, but is not really intended to affect results in any significant
way.
Change-Id: I461de7c31f3980dedf74424e7826c032b1f40fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444757
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>
The fuzzer discovered that, when we attempt to verify that an array
doesn't contain any literal values that are out-of-range for its base
type, we pay a linear-time cost based on the size of the array. This
happens even when the array value isn't known at compile time; we still
iterate over its slot count and diligently discover that every single
constant-subexpression slot in the expression is "null".
We now have a helper function on Expression,
`allowsConstantSubexpressions`, which only returns true for expression
kinds that can contain constant subexpressions. We use this helper to
skip over this linear-per-subexpression check when the expression
cannot possibly contain a constant subexpression. In particular,
`AnyConstructor::compareConstant` and `Type::checkForOutOfRangeLiteral`
will now early-out for expressions that can't possibly contain a
constant subexpression.
Change-Id: Ia34e422afa67b478a8616acb0a0e9cd211b29698
Bug: oss-fuzz:37900
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444136
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>
This reverts commit 9a0302cd9d.
Reason for revert: layout tests suppressed
Original change's description:
> Revert "Simplify SkRasterClip now that it's only intersect/diff"
>
> This reverts commit 75bab9249d.
>
> Reason for revert: Experiment to see if blocking Chrome roll
>
> Original change's description:
> > Simplify SkRasterClip now that it's only intersect/diff
> >
> > Bug: skia:10205
> > Change-Id: Id29a63783bd38e5977e94bf8e8d7fbb4fe16cb51
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442279
> > Reviewed-by: Mike Reed <reed@google.com>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=reed@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: I1ceeccf880a139dc9236b7df17e3599f8cae611f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10205
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443414
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:10205
Change-Id: I13a49190c67e5e32431ea268312ede60c55798b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443897
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 75bab9249d.
Reason for revert: Experiment to see if blocking Chrome roll
Original change's description:
> Simplify SkRasterClip now that it's only intersect/diff
>
> Bug: skia:10205
> Change-Id: Id29a63783bd38e5977e94bf8e8d7fbb4fe16cb51
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442279
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=reed@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I1ceeccf880a139dc9236b7df17e3599f8cae611f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10205
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443414
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We had a logic bug when attempting to optimize the following code:
const vecN x = vecN(a, b, c);
-x;
The goal was to replace `-x` with `vecN(-a, -b, -c)` but we accidentally
tried to cast the `x` VariableReference to a Constructor. We
unfortunately didn't cover this in any of our test cases, but the fuzzer
managed to synthesize it by mixing and matching elements from its new
corpus.
This affected several different constructor types: splat, diagonal-
matrix, compound and array.
Change-Id: I10dd2460ab26ba3e820b0cff5db091368fb7e648
Bug: oss-fuzz:37764, oss-fuzz:37861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443407
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:10205
Change-Id: Id29a63783bd38e5977e94bf8e8d7fbb4fe16cb51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442279
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:11837
Change-Id: I3dde13940e57763d5a8224cb1a4b555e904351d7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442716
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:11837
Change-Id: Ie99b1c512885404351d6917bbea751d99a2ca23e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442797
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously the magic surrounding sk_RTAdjust was inaccessible to the
DSL, meaning that the DSLParser would not work properly when the
sk_RTAdjust field was present in an interface block. This refactoring
means all interface blocks are processed via the same path.
Change-Id: I99a2fe6875dfcbccc53f7a44f0fb1912cb2722ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442456
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
These only existed for geometry shader interface blocks.
Change-Id: Ie82252715fe5e6babb85e3b437c6edd811fab955
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442695
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Our analysis pass for checking if an expression is a constant-expression
would assert if the expression contained a TypeReference or a
FunctionReference. This could happen if you passed in an expression that
had not yet been type-coerced. This check seemed overly strict, so the
assertion has been removed (although such an expression will be reported
as 'not a constant expression').
This bit us in global-variable declaration, where we checked if a
global variable's initial-value expression was constant before coercing
it to the variable's type. This has also been reordered so the type-
coercion happens first. (Either order is now valid, but the type-
coercion related errors tend to be more detailed.)
Change-Id: I5104cf817767d65fd84421243d9530734ba624a9
Bug: oss-fuzz:37710
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442693
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:8451 skia:10827
Change-Id: I5b38a1d72cd4558f8e2a92aaf9b12f05efce0923
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442683
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is a first step towards replacing `finalizeFunction` with a
`FunctionDefinition::Convert` method living outside of the IRGenerator.
Previously this code would assert that we had no early returns from a
vertex-program main() method; this has been turned into an error.
(The original assertion was also tied to fRTFlip, because the *problem*
with early-returns in main is tied to the lack of RTFlip fixups, but
we fundamentally don't allow early returns, so it makes more sense to
just universally disallow it.)
Change-Id: Iba0742f7ef3cbc83995ea130fec1eb1ef2556c44
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442691
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
SkRectPriv::Subtract had assumed that Sk[I]Rect::Intersects returns
false if either argument is empty. However, when an SkIRect with
overflowing dimensions is intersected with a non-overflowing SkIRect,
then the intersection can be valid and the remainder of the impl's
expectations are violated because dimensions are "negative".
Since these overflowing rects are considered empty anyways, this just
explicitly checks for that.
Bug: chromium:1243206
Change-Id: I8b69731e8d8ae467cf98c906da3aaa657dfe7994
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442277
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The fuzzer invented a much more elaborate example, but I was able to
winnow it down to a simple otherwise-normal test case. This also fixes
a latent DSL bug; DSL functions were not updating the list of referenced
intrinsics, so the compiler might emit finished programs that called
built-in functions that didn't exist in the code.
Change-Id: I095bb566b9db9f87cbe9460732c300b7973eb112
Bug: oss-fuzz:37659
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442325
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The fuzzer managed to trigger an assertion by returning an invalid type
from a void function. We were neglecting to clear out the expression
when reporting it as invalid, leaving it for `checkValid` to find later.
Change-Id: Icc152c867a3316fe994967e192601fb4d10da98f
Bug: oss-fuzz:37704
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442678
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>
No-op arithmetic simplification will convert expressions like `x += 0`
to `x`. When making this simplification, we will also downgrade the ref-
kind of `x` from "write" to "read" since the new expression is no longer
an assignment.
The fuzzer discovered that the ref-kind downgrade was too aggressive,
and would also traverse into nested subexpressions and downgrade them
as well. That is, for `x[y=z] += 0` would convert both `x` and `y`
into "read" references, which is incorrect; `y` is still being written
to.
The fuzzer managed to turn this mistake into an assertion by leveraging
a separate optimization. It added a leading, side-effect-less comma
expression for us to detect as worthless and eliminate. In doing so, we
clone the expression with the busted ref-kind, triggering an assertion.
Change-Id: I42fc31f6932f679ae875e2b49db2ad2f4e89e2cb
Bug: oss-fuzz:37677
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442536
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 9155b338bb.
Reason for revert: disable test for GLSL + Adreno 6xx
Original change's description:
> Revert "Add ES3 intrinsics isinf/isnan to public SkSL ES3."
>
> This reverts commit e43714f490.
>
> Reason for revert: Several Pixel (Adreno) devices failing the test
>
> Original change's description:
> > Add ES3 intrinsics isinf/isnan to public SkSL ES3.
> >
> > The ES3 spec doesn't mandate that `isnan` actually has to do anything,
> > so the Isnan test is not enabled. (It doesn't work on my personal
> > machine unless I make the NaN detectable at compile-time.)
> >
> > We do not support these functions in constant-expressions, as we
> > currently avoid optimizing anything into a non-finite value; we leave
> > expressions alone if we calculate a NaN/inf result for their value.
> >
> > Change-Id: Ibfdfb47b6e6134165c8780db570de04a916d2bfa
> > Bug: skia:12022
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441581
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: I89899ed391aa870350d0452bab4a0fb75bd7be38
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:12022
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441716
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12022, skia:12377
Change-Id: Ib149dbc1138feb3ee2bf6f7e31e9e8a9414560bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441884
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I4a825c0d191f3f24d558c4331e07fe2a55832f76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441838
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Our SPIR-V code generator did not implement support for negating a uint.
However, this is something that GLSL allows (as does the rest of SkSL).
I checked glslang and it uses OpSNegate here. The SPIR-V docs indicate
that OpSNegate allows any type of integer, and the validator lets it
pass, so we now use OpSNegate here as well.
http://screen/33mkq92uxAT5Xu8http://screen/4YBTh3gCWz8eZx7http://screen/388HtXyytcN5vLZ
Change-Id: I8c142018fd5e162dcd051abe1bc5d69a6e034794
Bug: oss-fuzz:37627
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441880
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, a struct containing a vardecl with multiple declarations
would interpret arrays incorrectly. An array would be applied to ALL
variables in the decl after its initial appearance. That is,
`int w, x[10], y, z;` would be interpreted as
`int w, x[10], y[10], z[10];`.
This is now fixed and our test case runs as expected.
Change-Id: I5b4a617c58cdfb83face651effd42770a1f68638
Bug: oss-fuzz:37622
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441879
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>
The fuzzer detected a serious parsing error; a struct containing a
vardecl with multiple declarations would interpret arrays incorrectly.
An array would be applied to ALL variables in the decl after its initial
appearance. That is, `int w, x[10], y, z;` would be interpreted as
`int w, x[10], y[10], z[10];`. The fuzzer caught this by putting two
arrayed variables in a row; the second variable was interpreted as a
nested array, which led to an assertion.
This CL contains a simple hand-written test case demonstrating the bug,
with the fix coming in a followup.
Change-Id: I42d7372ba77fa1528ae24eb8c29a2e5903784139
Bug: oss-fuzz:37622
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441878
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We weren't coercing the expression because we don't care about its type,
but that allowed intermediate-expressions to pass through without
reporting an error. Now we coerce the expression to its present type,
which will always fail if the type is disallowed and succeed otherwise.
Change-Id: Ic0de0d17f0f5d56360575efe992ce4d74dec2a5a
Bug: oss-fuzz:37620
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441876
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
src/gpu/GrBlockAllocator -> src/core/SkBlockAllocator
src/gpu/GrTBlockList -> src/core/SkTBlockList
Tests and references also renamed.
Bug: skia:12330
Change-Id: I5fad05faa3dcecd89a0a478dcf30c090ea7589f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441477
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit e43714f490.
Reason for revert: Several Pixel (Adreno) devices failing the test
Original change's description:
> Add ES3 intrinsics isinf/isnan to public SkSL ES3.
>
> The ES3 spec doesn't mandate that `isnan` actually has to do anything,
> so the Isnan test is not enabled. (It doesn't work on my personal
> machine unless I make the NaN detectable at compile-time.)
>
> We do not support these functions in constant-expressions, as we
> currently avoid optimizing anything into a non-finite value; we leave
> expressions alone if we calculate a NaN/inf result for their value.
>
> Change-Id: Ibfdfb47b6e6134165c8780db570de04a916d2bfa
> Bug: skia:12022
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441581
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I89899ed391aa870350d0452bab4a0fb75bd7be38
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12022
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441716
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12302
Change-Id: I8cf958acf9214d0de903a4097647afd74f2a659e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441541
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The ES3 spec doesn't mandate that `isnan` actually has to do anything,
so the Isnan test is not enabled. (It doesn't work on my personal
machine unless I make the NaN detectable at compile-time.)
We do not support these functions in constant-expressions, as we
currently avoid optimizing anything into a non-finite value; we leave
expressions alone if we calculate a NaN/inf result for their value.
Change-Id: Ibfdfb47b6e6134165c8780db570de04a916d2bfa
Bug: skia:12022
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441581
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
If x is a known compile-time constant value, it can already be optimized
to a final value.
If x is not known, it could be zero, and 0/0 should result in a NaN.
Change-Id: I643a7c6da0a43ec366235c4df39fc78d3b361de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441580
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I26754745aa26313a2f76a86bd41699c7ac5b8a46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441596
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
After further discussion, using intrinsics with signatures similar to
sample keeps us looking like GLSL. However, using "sample" is still
misleading, so this adds explicit "shade", "filter", and "blend"
intrinsics. After migrating clients, the "sample" versions will be
removed.
Bug: skia:12302
Change-Id: Ia03e4b3794fc1fc5ae3c3099a7a350343ec7702e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441457
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
The additional tests from http://review.skia.org/441238 uncovered a gap
in the constant folder's abilities; it was not able to fold away
boolean vector comparisons even when they were constant. These are ES2
constant-expressions, so folding them properly is a requirement.
Change-Id: Ia0b4d5d1215c5fc2b247ac3f0dec4c8747d2153e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441579
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>
Much easier to maintain, especially with an upcoming change to the
sampling syntax.
Change-Id: I378811b7be0afcce5b7e68a942e7b46d96568155
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441518
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The inliner contained a type error when attempting to inline a function
that takes an array as input. The scratch copy of the array was created
as `float[123] var;` instead of `float var[123];`. This led to an
assertion in VarDeclaration::Make.
Change-Id: I5128fe71462bb59a015a7b4e59c1a74800828b16
Bug: oss-fuzz:37466
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441576
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>
This is a reland of 0f7c10ef56
Original change's description:
> Add sRGB 8888 colortype
>
> A color type that linearizes just after loading, and re-encodes to sRGB
> just before storing, mimicking the GPU formats that work the same way.
>
> Notes:
> - No mipmap support
> - No SkPngEncoder support (HashAndEncode's .pngs are ok, though?)
> - Needs better testing
>
> This is a re-creation of reviews.skia.org/392990
>
> Change-Id: I4739c2280211e7176aae98ba0a8476a7fe5efa72
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438219
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I5b6bb28c4c1faa6c97fcad7552d12c331535714d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441402
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This fixes an assertion failure uncovered by the fuzzer.
Bug: oss-fuzz:37469
Change-Id: I626c003cfa8a0bc65851899df3a7695dbe29200b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441311
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
During constant-folding, we baked in an assertion stating that any
const-typed variable reference ought to have an initial value, because
you can't declare a const variable without assigning a value. However,
function parameters are an exception to this rule! They are variable
references and are allowed to be const, but will not have an initial
value. (In this case, `const` just means you can't alter the value.)
In this case, all we needed to do was remove the assertion; we already
treated this case defensively and with the appropriate care.
Change-Id: I61242c6d08c59886c6992898f195771e6334f2b4
Bug: oss-fuzz:37465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441239
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 0f7c10ef56.
Reason for revert: Unhappy rollers
Original change's description:
> Add sRGB 8888 colortype
>
> A color type that linearizes just after loading, and re-encodes to sRGB
> just before storing, mimicking the GPU formats that work the same way.
>
> Notes:
> - No mipmap support
> - No SkPngEncoder support (HashAndEncode's .pngs are ok, though?)
> - Needs better testing
>
> This is a re-creation of reviews.skia.org/392990
>
> Change-Id: I4739c2280211e7176aae98ba0a8476a7fe5efa72
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438219
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com,reed@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ie199535b9b65ec7c7fef3c773452ea06bdbd2d9c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441376
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This was another place where we needed to use
`getConstantSubexpression` to rebuild vectors/matrices; it is a more
robust approach than trying handle each ctor type individually. The
fuzzer found an edge case with double-casting matrices to vectors that
fell through the cracks with the original approach.
In adding additional tests, I also found a case that the constant-folder
seems to ignore, `bool4(x,x,x,x) == bool4(x)`. This does fold for ints
and floats, so this ought to be fixable in a followup, but it's not a
big deal either way; this is very unlikely to occer in real code.
Change-Id: I4d577c87ef7049306685ca95250ecdf93b1dbc06
Bug: oss-fuzz:37464
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441238
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>
Improved tests caught a longstanding bug in the compile-time
optimization logic for round/roundEven. These would *always* round to an
even number even when it didn't make sense to do so. (e.g. 3.1 would
round to 4.)
RoundEven isn't available in lower shader models of Direct3D;
SPIRV-Cross throws if it's unavailable. We may need a caps bit for this.
Change-Id: I3cc50238a2116b8d4e2c4059730d8b5cfb2bb056
Bug: skia:12022, skia:12352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441078
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
A color type that linearizes just after loading, and re-encodes to sRGB
just before storing, mimicking the GPU formats that work the same way.
Notes:
- No mipmap support
- No SkPngEncoder support (HashAndEncode's .pngs are ok, though?)
- Needs better testing
This is a re-creation of reviews.skia.org/392990
Change-Id: I4739c2280211e7176aae98ba0a8476a7fe5efa72
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438219
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
The fuzzer noticed insufficient guards in IndexExpression::Convert when
converting an array size from an IntLiteral to a SKSL_INT. We had code
in IRGenerator which did this properly, so I moved our array-size
conversion logic into SkSLType and had IndexExpression share it.
Also, a variety of tests around similar error conditions were added.
Change-Id: I51529dea25f9029f81ae236511610069d66be29f
Bug: oss-fuzz:37462
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441236
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We now stop processing a var-declaration if its array-size expression is
invalid. Previously, we'd pass a null array-size expression into
convertVar, which would assert (but would fail cleanly afterwards).
Change-Id: I976f3326e32afbc7045a86d73c0dcb28f418a6f4
Bug: oss-fuzz:37457
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441079
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 879b2f2e6e
Now includes a test that demonstrates the bug found by Chrome's fuzzers,
and a different (safer) implementation.
Original change's description:
> In SkCanvas destructor, discard (rather than blit) unbalanced layers
>
> Bug: skia:12267
> Change-Id: I6808f62b2385a3466b1a93db905041a6529f58cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433360
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Florin Malita <fmalita@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
Bug: skia:12267
Change-Id: Ide7dc61b054761826faa5bca3eec6be2fc63c83a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440977
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These now have proper testing and compile-time optimization support.
Change-Id: I7978161ec126e1c3096b9ca9dfbb2be7d8ea02f5
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440859
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>
The no-op-arithmetic simplifier was written before we allowed casting a
mat2x2 to a float4, and did not expect a matrix inside a vector ctor.
The expression `float4(myMat2) * float4(anything)` would assert when we
tried to determine if `myMat2` was a constant zero or constant one.
The code has been rewritten to use getConstantSubexpression and now
allows matrices inside.
Change-Id: Id625141256bf89d816c57d2d21f16b0ec252c158
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440858
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The prototype has been added sksl_public, compile-time optimization is
implemented, and test code has been improved.
Change-Id: I536d6bd7fcae437a03744941b008940bf2a3b1c1
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440524
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
When compiling test shaders, we were setting SK_FRAGCOORD_BUILTIN on the
`coords` parameter to main() instead of SK_MAIN_COORDS_BUILTIN. These
two built-ins don't have the same type (float2 vs. float4) and don't
mean quite the same thing.
The SPIR-V code generator saw a variable with the SK_FRAGCOORD_BUILTIN
builtin value and assumed the presence of a global variable named
`sk_FragCoord`, which didn't exist (because it was never referenced in
the code, so it was never cloned in from the sksl_frag module).
This is only a concern when compiling test shaders with skslc; real
shaders don't hit these code paths. The generated code here is still
imperfect; if you look closely, you'll see the GLSL and Metal code is
referencing the `coords` variable but it's never declared anywhere.
Change-Id: I3ad249469927ff35eb1e75d6536f95317502708f
Bug: skia:12340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440520
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 lets us set the minimum bar for SkSL ES3 support in one place,
rather than checking this set of bits in multiple spots.
Change-Id: Icba58d8b6a93626ce2ffbe3c4b846cad4749cab5
Bug: skia:12347
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440518
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11837
Change-Id: I42dfaf795ec6afe9d648b0715457a3a38ef8c7a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439943
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I9ddb80b8886827250e243dc9174bb3679e70df9b
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440262
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit fdde20d3ec.
Reason for revert: generation() can promise things that it can't deliver
Original change's description:
> Use generation() to detect ES3 support.
>
> Desktop GLSL 1.30 supports the things we currently consider as "ES3
> only"--nonsquare matrices, derivatives, and unsigned integers.
>
> Change-Id: I4b5a844cf3aabee6b6d2c562e78859a29efc36fc
> Bug: skia:12347
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439937
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I118e08da078090f404a4fdf33c3a16a48c702753
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12347
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440457
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
If the passed-in shader references RTFlip (i.e., sk_FragCoord is used),
the settings must contain RTFlip layout info; otherwise, an error
occurs. Originally, the fuzzer detected this as a problem because the
error was being delivered via SK_ABORT, but it's failing more cleanly
now that Ethan's new error handling code is in place (causing the fuzzer
to report that the bug was "fixed"). With this CL, the oss-fuzz shader
will actually compile successfully in SPIR-V instead of leading to an
error.
Change-Id: I3268e84bd8e01c95a25ed0845a37324e98033c4b
Bug: oss-fuzz:35916
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439779
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11837
Change-Id: Ifa1da88aafcaa96e0e885facaeb849cc9963bcfe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439938
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
We don't have compile-time optimization for this intrinsic yet, but
otherwise everything is working as expected.
Change-Id: Id9c678699baa1d9867848459bf680cc40f29b4bd
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440257
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This intrinsic uses non-square matrices, so it will be useful in
confirming that we can use ES3 types in sksl_public intrinsics. Bulking
up this test (which we don't run in SkSLTest today) is a good first
step.
Change-Id: I8178f13d5ca376d7cae3d1a4350b2bc0397efb1f
Bug: skia:12348
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440256
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is the other half of making everything in gpu/ops be v1-only.
Bug: skia:11837
Change-Id: I5d77a499ef02eba69208d5bd634650433d02f6fb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440216
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, we hid non-ES2 numeric types from Runtime Effect code by
only including them in the private symbol table. Now, they are present
in the root symbol table, but marked with a new flag that identifies
them as disallowed in ES2.
The IR generator now enforces that strict-ES2 code doesn't contain types
that aren't allowed. This has two benefits:
- Intrinsic functions in sksl_public can now reference these types
- Error reporting is nicer
Change-Id: I32375de4efdcb57b74a8a1692fb2ee315a003336
Bug: skia:12348, skia:11115
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Desktop GLSL 1.30 supports the things we currently consider as "ES3
only"--nonsquare matrices, derivatives, and unsigned integers.
Change-Id: I4b5a844cf3aabee6b6d2c562e78859a29efc36fc
Bug: skia:12347
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439937
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11837
Change-Id: Ia34f1840aaa7360ea9a3ca40fef5cb96b68c6ca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439781
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Surprisingly, we didn't actually have a preexisting test covering this.
Error reporting is lackluster in this CL but will be improved in the
followup.
Change-Id: I0b1cdb5a82f066af6b9d3fd9c39748080c2e18c0
Bug: skia:12348
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439996
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I04da5871c668b1ed9ca8db7fdcbfe06a70ee227f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440079
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:11837
Change-Id: Ib00ccc78d6394ad85781dc1de090c2fe0e83c01f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439736
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, you would declare child objects (shaders, colorFilters, etc.)
and "sample" them like this:
uniform shader input;
uniform colorFilter filter;
half4 main(float2 coord) {
half4 inColor = sample(input, coord);
return sample(filter, inColor);
}
With the new syntax, those child objects become directly callable,
reflecting the way that Skia assembles all parts of the paint (as functions)
in the overall fragment shader:
uniform shader input;
uniform colorFilter filter;
half4 main(float2 coord) {
half4 inColor = input(coord);
return filter(inColor);
}
Bug: skia:12302
Change-Id: Ia12351964dc5d2300660187933188e738671cd83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436517
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Right now, Metal forces types to full precision. The matrix helper
functions previously baked in that assumption by hard-coding "floatX".
Now, they honor the component type; if this->typeName() started
returning "half", our helper functions would be named with "halfX". This
would allow half-precision and full-precision helpers to coexist.
Change-Id: I1679e6e76d2cf3c27fd69c42a92fb24bff6b69ec
Bug: skia:12339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439396
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit d90777ada3.
Reason for revert: relanding with fix to GrBackendTexture
Original change's description:
> Revert "Remove GrBackendFormat's textureType use from isFormatTexturable call."
>
> This reverts commit 832c817bc8.
>
> Reason for revert: uninitialized value in GrBackendTexture
>
> Original change's description:
> > Remove GrBackendFormat's textureType use from isFormatTexturable call.
> >
> > The goal of this change was to remove the use of GrBackendFormat::textureType()
> > from GrCaps::isFormatTexturable call. Instead we will always pass in a
> > GrTextureType into this call.
> >
> > To do this a lot of plumbing of GrTextureType was added to various call
> > sites. However, this CL halts the plubming up at the proxy level where we
> > get it from the GrBackendFormat still. Future CLs will continue removing
> > these call sites and others that use GrBackendFormat::textureType().
> >
> > Bug: skia:12342
> > Change-Id: Ic0f02b9c7f7402405623b8aa31aa32a9a7c22297
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439277
> > Commit-Queue: Greg Daniel <egdaniel@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> TBR=egdaniel@google.com,bsalomon@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: I354bbbf00be7a86c480009f3e7b36a8777a6bf3a
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:12342
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439338
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:12342
Change-Id: I151196f149f9e191d2975b8fe81334f4f8720744
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439339
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This eliminates the SkSL ErrorReporter class and funnels everything
through the DSL ErrorHandler. Since the DSL error handler can be
changed, this required a number of updates to ensure that things work
properly in the face of custom error handlers. There is probably more
work to be done in that area, but this at least passes all existing
tests.
Change-Id: Iaee27b79fc4ed426c484ccab257c09d28619ead5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438116
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This adds sinh, cosh, tanh, asinh, acosh, and atanh. We now also support
compile-time optimization for the arc functions.
Change-Id: I688f579b50403db534622b82926aa20d1f445341
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439319
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>
This reverts commit 832c817bc8.
Reason for revert: uninitialized value in GrBackendTexture
Original change's description:
> Remove GrBackendFormat's textureType use from isFormatTexturable call.
>
> The goal of this change was to remove the use of GrBackendFormat::textureType()
> from GrCaps::isFormatTexturable call. Instead we will always pass in a
> GrTextureType into this call.
>
> To do this a lot of plumbing of GrTextureType was added to various call
> sites. However, this CL halts the plubming up at the proxy level where we
> get it from the GrBackendFormat still. Future CLs will continue removing
> these call sites and others that use GrBackendFormat::textureType().
>
> Bug: skia:12342
> Change-Id: Ic0f02b9c7f7402405623b8aa31aa32a9a7c22297
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439277
> Commit-Queue: Greg Daniel <egdaniel@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I354bbbf00be7a86c480009f3e7b36a8777a6bf3a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439338
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The goal of this change was to remove the use of GrBackendFormat::textureType()
from GrCaps::isFormatTexturable call. Instead we will always pass in a
GrTextureType into this call.
To do this a lot of plumbing of GrTextureType was added to various call
sites. However, this CL halts the plubming up at the proxy level where we
get it from the GrBackendFormat still. Future CLs will continue removing
these call sites and others that use GrBackendFormat::textureType().
Bug: skia:12342
Change-Id: Ic0f02b9c7f7402405623b8aa31aa32a9a7c22297
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439277
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
We can now add functions to sksl_public.sksl with an $es3 prefix. These
will be allowed in a Runtime Effect when strict-ES2 mode is disabled.
Note that the CPU backend still doesn't have support for these calls,
and will fail ungracefully (assertion, nonsense result) if these
intrinsics are used.
The testing here is limited, due to an unrelated bug in SPIR-V
(skia:12340)
Change-Id: I9c911bc2b77f5051e80844607e7fd08ad386ee56
Bug: skia:12202, skia:12340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439058
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
Change-Id: Ia92c86a902acc6864f3252431faeb157628b509e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439156
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Pulls the matrix and layer bounds from the captured skp that was
originally clipped, and asserts that both SkMatrix and SkM44's
separate mapRect functions don't regress.
Bug: skia:12335
Change-Id: I092d60b8a38fcd3cb70a8123802d730644cffda1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439276
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
This modifier is currently allowed on built-in functions only.
The presence of this modifier will be used to indicate intrinsics which
are ES3-specific (and therefore, not allowed in user code under typical
circumstances).
Change-Id: Ice6be8d9d1b2bf0c8f07f2a89f335bb2f90f6681
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439057
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The value of 32 was causing errors when other uniforms were present, as
the SPIR-V code generator would detect overlapping uniform offsets and
fail.
Change-Id: I7bb1cf1244e54c39596c3a39e9f6972c6a47899c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439059
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
Change-Id: Ib8b92275c5da71c4ee48540d434f3afdc45f4067
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438819
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Basically, ensure all the headers about to become V1-only only appear in contexts that are currently or will soon be V1-only.
This is almost all fallout from retracting some of the moving headers from other headers i.e.:
GrMeshDrawOp.h from GrOpFlushState.h
GrDrawOp.h from GrOpsRenderPass.h
GrDrawOp.h from GrOpsTask.h
GrSimpleMeshDrawOpHelper.h from GrTessellationShader.h
Bug: skia:11837
Change-Id: I939f5c82c3042e9ab00571b5796ab82dbe968085
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438677
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 879b2f2e6e.
Reason for revert: UAF
Original change's description:
> In SkCanvas destructor, discard (rather than blit) unbalanced layers
>
> Bug: skia:12267
> Change-Id: I6808f62b2385a3466b1a93db905041a6529f58cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433360
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Florin Malita <fmalita@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:12267
Change-Id: I616e6c65c1eb3b6d3f67b2dde6be3db4de6db18a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438739
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
Change-Id: I571931e01b0b95d85a7b566fc785ac219e8e731f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438697
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This function was already divided into several different tests, but each
part relied on various bits and pieces from the others. This CL splits
it apart into smaller functions which should be easier to reason about.
This CL also fixes a handful of variable shadowing issues in the code.
Change-Id: Icfe189ae5fdf29913c1d94a6b5df9432442d648c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438658
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Move ProgramImpl function definitions into Processor subclass cpp files.
Delete separate h/cpp files.
Modify GrGLSLVaryingHandler::addPassThroughAttribute to break #include
cycle. It now takes a ShaderVar rather than a GP::Attribute, making
it slightly more flexible as the input can be any variable defined
in the vertex shader.
Bug: skia:11358
Change-Id: I94ee8cd44d29e194216592ecae5fd28de785c975
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438596
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Named "Impl" and nested in GP or makeProgramImpl definition.
Remove unused INHERITED typedefs.
Remove GenKey pattern.
Bug: skia:11358
Change-Id: Icb4d5a0844184f51d92135de6cee6f6e77f57a5e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438478
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This is a reland of ea17e2499d
Original change's description:
> Reland "uniform Ptr (UPtr) is a sub class of Ptr"
>
> This is a reland of cef047a490
>
> Fix strides in SkVMTest to be the right size.
>
> Original change's description:
> > uniform Ptr (UPtr) is a sub class of Ptr
> >
> > A pointer for a Uniform (UPtr) is a sub type of a Ptr. Everywhere you
> > can use a Ptr a UPtr will work, but you can't use Ptr where you need
> > a UPtr. All the UPtr instructions uniformF, gather32, etc are expected
> > to be hoisted and therefore loaded only once. While the varyings
> > instructions like load32, etc. are expected to remain in the body
> > of the loop, and be evaluated each time through the loop.
> >
> > Change-Id: I4fe6458c2a4614872ed67cda1e81b05ea8a9e69e
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436297
> > Commit-Queue: Herb Derby <herb@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> Change-Id: I858fa1224452ec801b6186fede353849edc895b5
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436564
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I774e710724c99a41d0d160e88a2b723f66e03861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436821
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Non-substantive changes that didn't fit any earlier CL in the chain.
Bug: skia:11837
Change-Id: Ic9b7c05c014b03ab5b30532098fb8cd2da846d9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/428958
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Remove GL or GLSL from names of subclasses. Make nearly all subclasses
nested either in FP class or its onMakeProgramImpl() function.
Make onSetData private rather than protected.
Remove unused INHERITED typedefs.
Embrace idea that Impl is part of FP private implementation:
direct member access rather than getters and no GenKey pattern.
Other random consistency updates, modernatizations, stylistic changes.
Bug: skia:11358
Change-Id: I0d94e00a146abdd38e094ca550fc3b9608bd433d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438056
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
An assignment like `mediump int a[2] = myHighpIntArray;` should succeed
now that the previous CLs have landed; originally, this would have
caused a type-mismatch error.
Change-Id: I86ffe6a21d0c7fbe289eef95aebc2605412566aa
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437740
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Compiling a program with "allow narrowing conversions" actually fixes up
narrowing casts in the program by inserting casts wherever they would be
needed for type-correctness. For instance, compiling the statement
`half h = myFloat;`
inserts an appropriate narrowing cast:
`half h = half(myFloat);`.
The Pipeline stage code generator relies on this behavior, as when it
re-emits a runtime effect into a complete SkSL program, the narrowing-
conversions flag will no longer be set, but that is okay, because the
emitted code now contains typecasts anywhere they would be necessary.
Logically, this implies that anything which supports narrowing
conversions must be castable between high and low precision. In GLSL and
SPIR-V, such a cast is trivial, because the types are the same and the
precision qualifiers are treated as individual hints on each variable.
In Metal, we dodge the issue by only emitting full-precision types. But
we also need to emit raw SkSL from an SkSL program (that is what the
Pipeline stage generator does).
SkSL already supported every typical cast, but GLSL lacked any syntax
for casting an array to a different type. This meant SkSL had no array
casting syntax as well. SkSL now has array-cast syntax, but it is only
allowed for casting low/high-precision arrays to the same base type.
(You can't cast an int array to float, or a signed array to unsigned.)
Change-Id: Ia20933541c3bd4a946c1ea38209f93008acdb9cb
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437687
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Previously, there was no way to create a forward declaration for a DSL
function. To avoid introducing new API and make this work in an
intuitive fashion, we now create prototypes for all DSL functions and
remove them when the function is promptly defined.
Change-Id: Ief36164ceb303a3d76a57dc073f2e9b8409bb45f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436562
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 23d8f94535
Original change's description:
> Fix array-of-matrix/struct comparisons in Metal.
>
> Metal needs helper functions in order to compare arrays, structs, and
> matrices. Depending on the input code, it was possible for the
> array-comparison helper to be emitted before a matrix-comparison
> or struct-comparison helper. If this occurred, array comparisons of that
> matrix or struct type would fail, because the operator== for the array's
> inner type was defined after array==, and Metal (like C++) parses
> top-to-bottom and only considers functions declared above the current
> function.
>
> We now emit prototypes for all the array, struct and matrix helper
> function. These prototypes are emitted above any helper functions. This
> ensures visibility no matter how your comparisons are organized.
>
> Change-Id: Ib3d8828c301fd0fa6c209788f9ea60800371edbe
> Bug: skia:12326
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437739
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12326
Change-Id: Ife68020f6b01fae973b97f76099c6d5e8215636c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438296
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Almost entirely mechanical.
Bug: skia:11837
Change-Id: I984339097fdeeae2eccb6c1d790d510020511961
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438177
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit ef9a1b66d0.
Reason for revert: not broken after all
Original change's description:
> Revert "Fix array-of-vector comparisons in Metal."
>
> This reverts commit 130338c9e1.
>
> Reason for revert: SkSL_ArrayComparison test causes Adreno 630/640 to crash in Vulkan
>
> Original change's description:
> > Fix array-of-vector comparisons in Metal.
> >
> > Comparing `vec1 == vec2` returns a bvec in Metal, so the result must be
> > wrapped in `all()` in order to boil it down to a single boolean result.
> > Our array-comparison helper function did not do this. Fortunately,
> > `all(scalar)` is a no-op, so we can just wrap the result unilaterally.
> >
> > Change-Id: I4f1f09a6832164ae2e6577d53b317f561332d581
> > Bug: skia:12324
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437736
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: Ic76a5527a8339c8201f52df08d43041d7dcbeb61
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:12324
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438077
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:12324
Change-Id: I3da699b8d1113800efb27e162d0c6315f0aeaa49
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438176
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The Metal code generator has historically avoided low-precision types in
the final output in order to dodge a variety of type-coercion issues.
However, the workaround was only coded for half/float. Extended the
workaround to cover int/short and uint/ushort as well.
Change-Id: I16e3a387ba2baef1ef2de7742e1b0d27786fee0e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437688
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Previously, SPIR-V would generate two separate SpvIds for array types if
they differed in SkSL, even if they matched in SPIR-V. For instance,
`half[10]` and `float[10]` are the same SPIR-V type, so they should
reuse the same SpvId. (The RelaxedPrecision decoration doesn't go on the
type.)
This is important because OpLoad and OpStore require the same SpvId on a
variable; you can't OpLoad from one type SpvId and OpStore to a
different type SpvId, even if the underlying type behind the SpvId is
the same.
(A slightly simpler fix exists at http://review.skia.org/437237, but
this triggered a memory pooling bug that I can't properly debug from
this machine.)
Change-Id: I7669a95a2c946dde1eeff73474a3a0fb9d180512
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437683
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 14b1d56a2b.
Reason for revert: fix emplace_back() usage
Original change's description:
> Revert "Remove SkTLList"
>
> This reverts commit e1d523d70f.
>
> Reason for revert: breaking old stdlib versions (< c17)
>
> Original change's description:
> > Remove SkTLList
> >
> > Change-Id: I198678b5cb298cf51872fbb8d4fd5d705a6b684e
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437339
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=brianosman@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: I8e02e4cd2f293e7530f842be783de10f69be2ef4
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438078
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
# Not skipping CQ checks because this is a reland.
Change-Id: Ied33ce81a8312963ff0713c4660cdb8541a02180
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438080
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit e1d523d70f.
Reason for revert: breaking old stdlib versions (< c17)
Original change's description:
> Remove SkTLList
>
> Change-Id: I198678b5cb298cf51872fbb8d4fd5d705a6b684e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437339
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=brianosman@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I8e02e4cd2f293e7530f842be783de10f69be2ef4
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438078
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
return from GrGP by unique_ptr, rename factory function to
makeProgramImpl()
Bug: skia:11358
Change-Id: I61dd36f770d2fc0b54de0e0e7b78ac4d3fbd119a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437741
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Change-Id: I198678b5cb298cf51872fbb8d4fd5d705a6b684e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437339
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 130338c9e1.
Reason for revert: SkSL_ArrayComparison test causes Adreno 630/640 to crash in Vulkan
Original change's description:
> Fix array-of-vector comparisons in Metal.
>
> Comparing `vec1 == vec2` returns a bvec in Metal, so the result must be
> wrapped in `all()` in order to boil it down to a single boolean result.
> Our array-comparison helper function did not do this. Fortunately,
> `all(scalar)` is a no-op, so we can just wrap the result unilaterally.
>
> Change-Id: I4f1f09a6832164ae2e6577d53b317f561332d581
> Bug: skia:12324
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437736
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ic76a5527a8339c8201f52df08d43041d7dcbeb61
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12324
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438077
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 23d8f94535.
Reason for revert: SkSL_ArrayComparison test causes Adreno 630/640 to crash in Vulkan
Original change's description:
> Fix array-of-matrix/struct comparisons in Metal.
>
> Metal needs helper functions in order to compare arrays, structs, and
> matrices. Depending on the input code, it was possible for the
> array-comparison helper to be emitted before a matrix-comparison
> or struct-comparison helper. If this occurred, array comparisons of that
> matrix or struct type would fail, because the operator== for the array's
> inner type was defined after array==, and Metal (like C++) parses
> top-to-bottom and only considers functions declared above the current
> function.
>
> We now emit prototypes for all the array, struct and matrix helper
> function. These prototypes are emitted above any helper functions. This
> ensures visibility no matter how your comparisons are organized.
>
> Change-Id: Ib3d8828c301fd0fa6c209788f9ea60800371edbe
> Bug: skia:12326
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437739
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I9e0fc69c46e1b4f63133e21e130e527ca4f0b31a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438076
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Metal needs helper functions in order to compare arrays, structs, and
matrices. Depending on the input code, it was possible for the
array-comparison helper to be emitted before a matrix-comparison
or struct-comparison helper. If this occurred, array comparisons of that
matrix or struct type would fail, because the operator== for the array's
inner type was defined after array==, and Metal (like C++) parses
top-to-bottom and only considers functions declared above the current
function.
We now emit prototypes for all the array, struct and matrix helper
function. These prototypes are emitted above any helper functions. This
ensures visibility no matter how your comparisons are organized.
Change-Id: Ib3d8828c301fd0fa6c209788f9ea60800371edbe
Bug: skia:12326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437739
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Comparing `vec1 == vec2` returns a bvec in Metal, so the result must be
wrapped in `all()` in order to boil it down to a single boolean result.
Our array-comparison helper function did not do this. Fortunately,
`all(scalar)` is a no-op, so we can just wrap the result unilaterally.
Change-Id: I4f1f09a6832164ae2e6577d53b317f561332d581
Bug: skia:12324
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437736
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
GrFragmentProcessor now provides an (explicit) copy constructor which
clones all child processors and flags from the passed-in FP. Since we no
longer have flags which propagate up to the root node of the FP tree,
all flags are now safe to copy, since a cloned FP also clones all of its
children.
Change-Id: Ia9f80e0ec540ed1056d25dbb1861a174a1d55f4b
Bug: skia:12299
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437836
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>
Bug: skia:10205
Change-Id: Iafee804751d69e98241a7825664f3be04b20eb14
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436566
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:11358
Change-Id: Ic179ddd9d52dca3fc0bd85b61db49097390b7f58
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437681
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 717ef9472b.
Reason for revert: Blocking the G3 roll
Original change's description:
> Clean up unflattening paints
>
> Just removing legacy cruft for fonts, and also change to return by
> value (cleaner).
>
> Change-Id: If120931119c32542e06801da2b6d60ba84ba2186
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437676
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Mike Reed <reed@google.com>
TBR=bungeman@google.com,fmalita@chromium.org,reed@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: If735d0212412c0262a6ccb52e7dd224a255906cf
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437678
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:11358
Change-Id: Ie70e45b18c12126c8e86700ad1040bc319be385a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436998
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Just removing legacy cruft for fonts, and also change to return by
value (cleaner).
Change-Id: If120931119c32542e06801da2b6d60ba84ba2186
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437676
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Bug: skia:10209
Change-Id: I72639b7e768742dcdec810a5a714ce21ff0f6e0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436565
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We were previously using a mix of pass-by-value and pass-by-pointer (to
allow for explicitly null PositionInfo). Being able to pass a null
PositionInfo didn't really add much, since we can just use a nullary-
constructor PositionInfo instead, so these have all been migrated to
by-value.
Change-Id: Ia31e252cac94f64c4b38c29a54e6e7f752e70672
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437276
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit b942d4b436.
Reason for revert: dependent cl no longer blocking android roll, so
this can be relanded.
Original change's description:
> Revert "Avoid expanding clip ops in tests that will remain after feature removal"
>
> This reverts commit d1c51b2572.
>
> Reason for revert: blocking revert that might be breaking android
>
> Original change's description:
> > Avoid expanding clip ops in tests that will remain after feature removal
> >
> > Bug: skia:10208
> > Change-Id: I4fb2c8181bfb8cac3c8ab95c833094c98f8ee6fc
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436159
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,reed@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: Ib62cc03f99793f8f1cb0180145b7557101a23ead
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10208
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436957
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:10208
Change-Id: Iff6e5b2b245426a76b92e895434613fe16ba717c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437277
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 8ba1e71a1f.
Reason for revert: had missed a few places where GrReducedClip needed to
use the equivalent region op, not skClipOp + replace bool.
TBR=robertphillips@google.com,brianosman@google.com,csmartdalton@google.com
Original change's description:
> Revert "Add SkClipStack::replaceClip() separate from deprecated clip op"
>
> This reverts commit 68587ae274.
>
> Reason for revert: breaking path clipping tests in Android?
>
> Original change's description:
> > Add SkClipStack::replaceClip() separate from deprecated clip op
> >
> > The replaceClip functionality was added to allow Android to move off of
> > generalized expanding clips. At the time, SkClipStack simply used the
> > kReplace_SkClipOp to handle it. In order to remove those expanding ops,
> > SkClipStack will need a proper implementation of replaceClip().
> >
> > The clip elements have an additional field to mark if
> > it's a replace (and it's op will be kIntersect). Adds a temporary
> > getRegionOp() function to unify elements that use this field vs.
> > elements that use the deprecated clip op (i.e. if they were deserialized
> > from an SKP that recorded an expanding op).
> >
> > Clients of SkClipOp that checked for replace ops use the new function
> > instead of referring to the enum value directly.
> >
> > Bug: skia:10209
> > Change-Id: I1c16c87fadb2becfe181db717c05e240ac87fd34
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436158
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
> > Reviewed-by: Chris Dalton <csmartdalton@google.com>
>
> TBR=robertphillips@google.com,brianosman@google.com,csmartdalton@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: If3f99a7d2f2df99c2b99d431d494ca28da66b1d8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10209
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436956
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
# Not skipping CQ checks because this is a reland.
Bug: skia:10209
Change-Id: I9feb0f3571ec26580bcdf0fe541f43f2ee8cf8d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436959
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is a "legacy" field in SkPath, and only needed for editing the
path (in funny cases, such as a relative verb or missing moveto).
When we finally make SkPath immutable, we won't need this field at all.
Note: this CL "fixes" the last 2 columns in path_append_extend gm.
They should appear the same as the previous 2 columns.
Change-Id: Ia5f2e8ec586b5f5189fc3ac2cd513fe89d31cd22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436958
Reviewed-by: Mike Reed <reed@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
Most of the code generated by the fuzzer is nonsense, but there is a
method to its madness. The crash is only triggered under specific
conditions:
- The runtime effect has enough helper functions to mostly fill up the
call graph hash-map. It won't rehash until it gets close to capacity.
- There must be several calls to built-in functions, in order to add
elements to the call graph to force a rehash.
The fuzzer-generated code manages to satisfy both these requirements.
Change-Id: I9a1d7535557fedd4e9bfece3930ac86ede291ffe
Bug: oss-fuzz:36655
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437118
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
GLSL allows an array of `lowp float` to be compared against `highp
float` seamlessly because the types are considered to be the same. SkSL,
however, treats these as different types, so we need to coerce the types
to allow this comparison to work.
In other words, these comparisons can cause an array to be implicitly
casted. The expression `myHalf2Array == float[2](a, b)` should be
allowed when narrowing conversions are enabled. To allow this to work,
we need a dedicated IR node representing this type coercion.
We now allow implicit coercion of array types when the array's component
types would be implicitly coercible, and have a new IR node representing
that implicit conversion.
This CL fixes array comparisons, but array assignment needs additional
fixes. It currently results in:
"type mismatch: '=' cannot operate on (types)".
Bug: skia:12248
Change-Id: I99062486c081f748f65be4b36a3a52e95b559812
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436571
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The optimization logic for swizzling a constructor assumed that every
argument to the constructor was a scalar or vector. When it was written,
this assumption was true. However, we recently added support for casting
mat2x2 to float4 which violates the assumption.
We now check every argument and do not attempt to optimize if a
non-scalar, non-vector arg is found.
Change-Id: Ia2b297bd62dfdf4af56712164fbc80c29c9611eb
Bug: oss-fuzz:36852
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437017
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
OSSFuzz discovered a minor variation of oss-fuzz:36770 which tickled a
different bug in SPIR-V RTFlip handling; we did not properly handle the
case where the InterfaceBlock is an array. SPIR-V does not support this
at all, but the IRGenerator allows it, and we don't detect it an an
error until later in the compilation process.
Change-Id: I80bd67a13dad878717dc122462132a2ed675532d
Bug: oss-fuzz:36850
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437018
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
Change-Id: Ic582c59b9f7dfee2d7e90e50bfb36c57e958c673
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436641
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 68587ae274.
Reason for revert: breaking path clipping tests in Android?
Original change's description:
> Add SkClipStack::replaceClip() separate from deprecated clip op
>
> The replaceClip functionality was added to allow Android to move off of
> generalized expanding clips. At the time, SkClipStack simply used the
> kReplace_SkClipOp to handle it. In order to remove those expanding ops,
> SkClipStack will need a proper implementation of replaceClip().
>
> The clip elements have an additional field to mark if
> it's a replace (and it's op will be kIntersect). Adds a temporary
> getRegionOp() function to unify elements that use this field vs.
> elements that use the deprecated clip op (i.e. if they were deserialized
> from an SKP that recorded an expanding op).
>
> Clients of SkClipOp that checked for replace ops use the new function
> instead of referring to the enum value directly.
>
> Bug: skia:10209
> Change-Id: I1c16c87fadb2becfe181db717c05e240ac87fd34
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436158
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Chris Dalton <csmartdalton@google.com>
TBR=robertphillips@google.com,brianosman@google.com,csmartdalton@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: If3f99a7d2f2df99c2b99d431d494ca28da66b1d8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436956
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This reverts commit d1c51b2572.
Reason for revert: blocking revert that might be breaking android
Original change's description:
> Avoid expanding clip ops in tests that will remain after feature removal
>
> Bug: skia:10208
> Change-Id: I4fb2c8181bfb8cac3c8ab95c833094c98f8ee6fc
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436159
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=robertphillips@google.com,reed@google.com,michaelludwig@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ib62cc03f99793f8f1cb0180145b7557101a23ead
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10208
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436957
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit ea17e2499d.
Reason for revert: blocking the android roll on a build failure
Original change's description:
> Reland "uniform Ptr (UPtr) is a sub class of Ptr"
>
> This is a reland of cef047a490
>
> Fix strides in SkVMTest to be the right size.
>
> Original change's description:
> > uniform Ptr (UPtr) is a sub class of Ptr
> >
> > A pointer for a Uniform (UPtr) is a sub type of a Ptr. Everywhere you
> > can use a Ptr a UPtr will work, but you can't use Ptr where you need
> > a UPtr. All the UPtr instructions uniformF, gather32, etc are expected
> > to be hoisted and therefore loaded only once. While the varyings
> > instructions like load32, etc. are expected to remain in the body
> > of the loop, and be evaluated each time through the loop.
> >
> > Change-Id: I4fe6458c2a4614872ed67cda1e81b05ea8a9e69e
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436297
> > Commit-Queue: Herb Derby <herb@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> Change-Id: I858fa1224452ec801b6186fede353849edc895b5
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436564
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
TBR=herb@google.com,brianosman@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I0ffc93a04f5329838d422ad9e42aba09b9ba0064
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436639
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
This CL does not update the DSLParser to honor these precision
qualifiers; that will be done in a followup.
Change-Id: Ib629bc99c0e6c7afb550a381d4e3b6ccc26aa64e
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436337
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
These parse into new modifier bits; the IR generator does not yet
support these bits. That's coming in a followup CL.
Change-Id: I362e9227694f9b862eaad100f6afca45a9b62a01
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436336
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is a reland of cef047a490
Fix strides in SkVMTest to be the right size.
Original change's description:
> uniform Ptr (UPtr) is a sub class of Ptr
>
> A pointer for a Uniform (UPtr) is a sub type of a Ptr. Everywhere you
> can use a Ptr a UPtr will work, but you can't use Ptr where you need
> a UPtr. All the UPtr instructions uniformF, gather32, etc are expected
> to be hoisted and therefore loaded only once. While the varyings
> instructions like load32, etc. are expected to remain in the body
> of the loop, and be evaluated each time through the loop.
>
> Change-Id: I4fe6458c2a4614872ed67cda1e81b05ea8a9e69e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436297
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I858fa1224452ec801b6186fede353849edc895b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436564
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
The only really interesting parts are:
src/gpu/SurfaceFillContext.*
src/gpu/v1/SurfaceFillContext.*
Everything else is mostly mechanical.
TBR=michaelludwig@google.com
Bug: skia:11837
Change-Id: Ia208e9a73d1529804c06d4f805d8ca3674851496
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436558
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:10208
Change-Id: I4fb2c8181bfb8cac3c8ab95c833094c98f8ee6fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436159
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This moves SkImageFilter's API closer to SkColorFilter's, and also
updates SkRecordDraw's PaintMayAffectTransparentBlack to be less
conservative.
Originally:
-canComputeFastBounds() handled aggregating behavior of the entire graph
but it's a public API
-affectsTransparentBlack() was the private virtual for a specific node
Now:
-affectsTransparentBlack() handles aggregating behavior of the graph and
is part of SkImageFilter_Base (mirroring SkColorFilter_Base).
-added onAffectsTransparentBlack() to clarify the per-node virtual that
they can override.
-canComputeFastBounds() simply returns !affectsTransparentBlack().
There are some usages in our code that I kept using canComputeFastBounds
(e.g. SkPaint's computefastBounds) because it kept naming consistent.
In other places I updated to use affectsTransparentBlack since I thought
that made it clearer why we were checking that behavior.
Bug: skia:12282
Change-Id: I7b58372c127b4d8d9097d6c0de64486e822d2342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436296
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We don't currently support this. There's no explicit syntax to cast an
array's type, but it can be implicitly required in some situations, like
`halfArray == floatArray` (when fAllowNarrowingConversions is on).
Change-Id: I00fe0ddd4f2682b2950e828dd78bb941d5f0430e
Bug: skia:12248
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436560
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>
The replaceClip functionality was added to allow Android to move off of
generalized expanding clips. At the time, SkClipStack simply used the
kReplace_SkClipOp to handle it. In order to remove those expanding ops,
SkClipStack will need a proper implementation of replaceClip().
The clip elements have an additional field to mark if
it's a replace (and it's op will be kIntersect). Adds a temporary
getRegionOp() function to unify elements that use this field vs.
elements that use the deprecated clip op (i.e. if they were deserialized
from an SKP that recorded an expanding op).
Clients of SkClipOp that checked for replace ops use the new function
instead of referring to the enum value directly.
Bug: skia:10209
Change-Id: I1c16c87fadb2becfe181db717c05e240ac87fd34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436158
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
This reverts commit cef047a490.
Reason for revert: DM test failures on some Windows/Linux devices
Original change's description:
> uniform Ptr (UPtr) is a sub class of Ptr
>
> A pointer for a Uniform (UPtr) is a sub type of a Ptr. Everywhere you
> can use a Ptr a UPtr will work, but you can't use Ptr where you need
> a UPtr. All the UPtr instructions uniformF, gather32, etc are expected
> to be hoisted and therefore loaded only once. While the varyings
> instructions like load32, etc. are expected to remain in the body
> of the loop, and be evaluated each time through the loop.
>
> Change-Id: I4fe6458c2a4614872ed67cda1e81b05ea8a9e69e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436297
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=herb@google.com,brianosman@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I785973be1493643e7d5a3da482bc4ab07d186865
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436559
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
A pointer for a Uniform (UPtr) is a sub type of a Ptr. Everywhere you
can use a Ptr a UPtr will work, but you can't use Ptr where you need
a UPtr. All the UPtr instructions uniformF, gather32, etc are expected
to be hoisted and therefore loaded only once. While the varyings
instructions like load32, etc. are expected to remain in the body
of the loop, and be evaluated each time through the loop.
Change-Id: I4fe6458c2a4614872ed67cda1e81b05ea8a9e69e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436297
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The only really interesting parts are:
src/gpu/SurfaceFillContext.*
src/gpu/v1/SurfaceFillContext.*
Everything else is mostly mechanical.
Bug: skia:11837
Change-Id: If2945f30dadd6ad0cccf6ff2b53e4a92b1dc6cbc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436099
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.
Change-Id: I169ff3bac8517869132297c8e98bd9bd4248a349
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436100
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The SkClipOpPriv.h header will be going away soon, but a number of
places still use its kIntersect_SkClipOp definitions instead of the
equivalent SkClipOp::kIntersect. Besides updating these references,
a number of unnecessary includes to SkClipOpPriv.h are removed and
some test cases exercising expanding clip ops are deleted since they
will be unnecessary shortly.
Bug: skia:10208
Change-Id: I2bbdd6bb39869134c6a80ef2b4482e6cabdeb0b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436157
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This code intentionally mixes half4s and float4s everywhere. Before
http://review.skia.org/435916 landed, this resulted in a compile error.
Change-Id: I852fef6ee99a8b78623e0e9ddeee2ad84a8c0504
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436058
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:11837
Change-Id: I60112e370c95e6f9d9bc12cb9b05d40dd2220bc9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435279
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Instead expand the map that is computed by GrGLSLGP to include a bool
that indicates whether each FP requires coords or not.
Now GrGLSLGP is solely responsible for determining which FPs take coords
and inserting any varyings. The rest of the system follows its lead when
generating FP functions and call sites.
Bug: skia:12198
Change-Id: I3471867fb64e94c7775c0b6209998159abeb6714
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435018
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SPIR-V code generation synthesizes some extra variables that don't
actually exist in the Program. Checking the ProgramUsage of these
variables would fail; ProgramUsage::get doesn't know about these
variables, so it asserts (and would consider them as dead even if it
didn't assert). We now track our SPIR-V bonus variables in a separate
set, and always report them as live.
Change-Id: If2f681470654025abf7ca4b3ec8126de2eb01297
Bug: oss-fuzz:36770
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435625
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>
In the new GrSurfaceContext class hierarchy we can get either a v1 or v2 SFC/SDC depending on the context options setting.
Bug: skia:11837
Change-Id: Ia25bc10b58bbbaf65a484b323d9d0eee471bb7ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435276
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This is a reland of 62d42db282
Check for PBO support before binding PBO 0.
Original change's description:
> Fix check in GrGLGpu for whether PBO 0 is bound
>
> Found in OOP-R canvas in Chrome. Most likely async read followed by
> sync read with an intervening resetContext().
>
> Bug: chromium:1208212
>
> Change-Id: Ibf85f070d0a4cd389f67e9b249655b0ef667c66e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435277
> 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>
> Reviewed-by: John Stiles <johnstiles@google.com>
Bug: chromium:1208212
Change-Id: I0da48fa9a1f31ec15827f9bef980e9d5a7d70d26
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435622
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 62d42db282.
Reason for revert: d3d9 angle tests failing
Original change's description:
> Fix check in GrGLGpu for whether PBO 0 is bound
>
> Found in OOP-R canvas in Chrome. Most likely async read followed by
> sync read with an intervening resetContext().
>
> Bug: chromium:1208212
>
> Change-Id: Ibf85f070d0a4cd389f67e9b249655b0ef667c66e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435277
> 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>
> Reviewed-by: John Stiles <johnstiles@google.com>
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: chromium:1208212
Change-Id: I85c89a4d0dd63aaafd80d7572c5643cfa547498b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435617
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Found in OOP-R canvas in Chrome. Most likely async read followed by
sync read with an intervening resetContext().
Bug: chromium:1208212
Change-Id: Ibf85f070d0a4cd389f67e9b249655b0ef667c66e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435277
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>
Reviewed-by: John Stiles <johnstiles@google.com>
Runtime shaders, color filters, and blenders are all able to sample from
a blender. These use the blend-function signature; both a src-color and
dst-color must be passed to sample. i.e.: sample(blender, s, d)
Change-Id: I3738e6b0b4af6d1d79e62ca1815c80d6a1ae9d6f
Bug: skia:12257
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11837
Change-Id: If8dd864d6cd8bc5ab9569fbab40866e1810dbc27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/434162
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I7b53df1eae83a596c4d1f3620e7f9bd146f68af2
Bug: oss-fuzz:36655
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/434465
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Id3e4d10456be6e0e0329600f05641034f3ffdb8b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/434336
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This CL is mostly mechanical. It:
replaces "src/gpu/GrSurfaceDrawContext.h" #includes with
"src/gpu/v1/SurfaceDrawContext_v1.h" and reorders
replaces "class GrSurfaceDrawContext;" with
"namespace skgpu { namespace v1 { class SurfaceDrawContext; }}"
replaces "GrSurfaceDrawContext*" with "auto" where possible
replaces "rtc" with "sdc"
replaces "surfaceDrawContext" with "sdc"
replaces GrSurfaceDrawContext with skgpu::v1::SurfaceDrawContext
reflows parameters as needed
This CL does not try to:
make skgpu::v1::SurfaceDrawContext V1-only
minimize the skgpu and/or skgpu::v1 prefixes
Those two tasks will be accomplished in follow up CLs. This CL is just trying to get the bulk of the mechanical changes comprehensibly landed.
Bug: skia:11837
Change-Id: I6fe59080249d585df8f5d27c6b67569cdc35842f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433156
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Note: I have changed from using a byte offset for index to
an ordinary array index at the builder interface, but the
builder converts this to a byte offset for the instruction.
This makes the API easier to use.
I've added pushArray, and pushArrayF to the Uniforms, and
convenience methods on the builder to take Uniforms.
I've expanded the tests to use the new API.
Change-Id: Id538e826a96d4d242ae6482acc711d84c9041239
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432036
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Forgot to load the pointer to the array in the arm64 case.
This is a reland of fe2506f3ca
Bug=skia:11822
Original change's description:
> Reland "add op array32 for indirect uniform access"
>
> This is a reland of ac2d053ccf
>
> The original CL was reverted because of a bug in the hash
> function.
>
> Bug=skia:11822
>
> Original change's description:
> > add op array32 for indirect uniform access
> >
> > Change-Id: I6249594a2348c7b24e4f057cce2f4e8a6a2c4409
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431676
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Herb Derby <herb@google.com>
>
> Change-Id: I94604f5589c72d342c39cad44540d810ed7f31a1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432797
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I185fc9554dcb6aa7ce367814ce2c69603074c434
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433356
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Includes unit test that reproduces the original bug.
Bug: chromium:1232834
Change-Id: Iae2f52b42d35f0774c4cf9fa686df7aaf57c130a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433279
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit fe2506f3ca.
Reason for revert: Breaks unit test on M1 mac
Original change's description:
> Reland "add op array32 for indirect uniform access"
>
> This is a reland of ac2d053ccf
>
> The original CL was reverted because of a bug in the hash
> function.
>
> Bug=skia:11822
>
> Original change's description:
> > add op array32 for indirect uniform access
> >
> > Change-Id: I6249594a2348c7b24e4f057cce2f4e8a6a2c4409
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431676
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Herb Derby <herb@google.com>
>
> Change-Id: I94604f5589c72d342c39cad44540d810ed7f31a1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432797
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
TBR=herb@google.com,brianosman@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: I3dccc04dc5a867bb45a99044991056b22f5b6fe3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433278
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This is a reland of ac2d053ccf
The original CL was reverted because of a bug in the hash
function.
Bug=skia:11822
Original change's description:
> add op array32 for indirect uniform access
>
> Change-Id: I6249594a2348c7b24e4f057cce2f4e8a6a2c4409
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/431676
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I94604f5589c72d342c39cad44540d810ed7f31a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/432797
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
