GPU was recently busted when switching alpha-color type swizzles from
aaaa to 000a.
There was no GM that exercised SkPerlinNoiseShader::MakeImprovedNoise.
It draws wrong before and after this change with the CPU backend.
Bug: skia:10536
Change-Id: I514e304d022fcccae80699a99facafa8ce947e9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350916
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Continue to test everything with the ByteCode interpreter, and run most
tests with the new SkSL-to-SkVM utilities, as well. A few tests rely on
features that aren't yet implemented (function calls, looping), and some
of the bespoke tests (that don't use the test() helpers) use even more
exotic features that need to be implemented or disallowed in the IR
generator. This is getting us closer to not needing ByteCode at all,
though.
Refactored a bunch of the helper code to reduce copy-paste among the
many different 'test' functions.
Change-Id: I138d4a24266f2d862742245c5ee895d86c01018e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350560
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:11134
Change-Id: I0b0a3f530452d5bb5c08da7f247728298c234dd7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350583
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Avoids the polyfill that uses looping instanced draws. This has led to
perf regressions on android.
Bug: skia:11138 skia:11139 chromium:1163441
Change-Id: I129bf96c6d8a3eaadc79bfca496c7d50189f737e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350738
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
We've been assuming that all Ops with the same arguments produce the
same value and deduplicating them, which results in a simple common
subexpression eliminator.
But we can't soundly dedup two identical loads with a store between;
that store could change the memory those loads read, producing different
values, as demonstrated by the first new unit test.
Then, by similar reasoning, it may first seem fine to deduplicate
stores, e.g.
store32 arg(0), v1
store32 arg(0), v1
That second store certainly does look redundant. But if we slot a
different store between, it's no longer redundant:
store32 arg(0), v1
store32 arg(0), v2
store32 arg(0), v1
If we dedup those two v1 stores, we'll skip the second and be left with
v2 in our buffer instead of v1. This is the second new unit test.
Now, uniform32 and gather ops also touch memory... are they safe to
dedup? Surprisingly, yes! Uniforms are easy: they're read-only. No
way to store to uniforms, so no intervening store can invalidate them.
Gathers are a little fuzzier, in that the buffer we gather from is
uniform in practice, but not strictly required to be... it's not
impossible to construct a program that gathers from a buffer that the
program also stores to, but you'd have to go out of your way to do it,
and it's not a pattern we use today, and SkVM does not provide the
synchronization primitives you'd need to make attempting that even
vaguely sensible. So gathers in practice can also be deduplicated.
In general it's safe to dedup an operation unless it touches _varying
memory_, i.e. loads and stores. uniform32 and gathers touch
non-varying memory, so they're safe, and while index is varying, it
doesn't touch memory.
Change-Id: Ia275f0ab2708d3f71e783164b419436b90f103a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350608
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I noticed is_always_varying() is a little wrong, and this new test demos
how. This isn't terribly important: in most practical situations
gathers will indeed be varying.
Change-Id: I456d4c7287147726c49ebb5af5af347c65cd21d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350602
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Many of our shaders generate the same vector constant dozens of times,
e.g. Gaussian blur uses float4(1) repeatedly. This change avoids
re-emitting redundant vector constants.
Change-Id: I22a71cd8b2783fb997f52d485b49031f64ca6d96
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350701
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Also explicitly default all special methods of SkStrikeCache to get
nicer warnings.
Change-Id: I8856a905c6075ca52d4ad900ccd0dacc581cd485
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350637
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This makes them reusable for the edge-AA code when we move it.
Bug: skia:10419
Change-Id: I20042a419617717214535d45fc92a8cae986fb33
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349356
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
and update GrGLGpu::flushScissorRect's signature to match it.
Change-Id: I9082003934168306833f86d310b942a8d7527384
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350612
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, we had constant-value deduplication, based on the SkSL type
of the constant. However, we were still generating redundant constants,
because we would emit a separate constant for Float(n) and Half(n), or
Int(n) and Short(n), even though we generate the exact same instruction
for these constants. We now deduplicate based on the type's number-kind,
separating constant literals into three categories: floats, signed ints,
and unsigned ints. This better matches our type-handling in
getActualType.
Change-Id: I5777d4b3d567839b7aa72dc8de76908c18fc387e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350031
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL adds additional variants of some IRGenerator functions to enable
them to be more easily called from upcoming DSL code. This should not
change functionality (other than a slightly different error message in
one case).
Change-Id: Ic727c194fca5111d4283007f4ec23653598a853b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350017
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
I plan to add at least one more gr_* class, so I figure it makes sense
to just keep all this similar things in one place.
Bug: skia:11136
Change-Id: I96d24dd094731e9694201e012fec37926cce564d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350639
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The pattern appears to be that the variable that stores the uploaded
value has a "Prev" suffix.
Change-Id: I78ea449d75f5fd091c113cf08af919331467eb8d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350638
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:10979
Change-Id: I81b8692b0bb1582b60a9324492a782d04a09538f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344963
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
IntLiteral and FloatLiteral each had a pair of public constructors: one
which takes a Type (what type of Int/Float is this?), and another
constructor which takes a Context (assume the generic Float/Int type).
BoolLiteral had a public Context-based constructor, but its Type-based
constructor was private. The Type-based constructor is now public to
match its siblings. This allows us to create Booleans without needing
a Context&, if we have an fBool_Type, and fixes make_unique (which
doesn't like private constructors).
Change-Id: Iffb4980947a73b3a89aa1452cc90dce63620bd67
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350636
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>
Change-Id: Ia4a1c38161046b94dc56a1a76704766f1e14aab7
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350019
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, the IR generator had code which could simplify conversion
constructors like `int(1.23)`. Separately, the optimizer's constant
propagation pass had its own separate implementation of these
simplifications as well.
This CL unifies the two implementations. Previously, the constant-
propagation pass version of the code only supported integer literals, so
this change also improves our code generation slightly.
Change-Id: I32c70a5f2aed210d03bef3166b1178a2d40cdabd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350024
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 237911a4d8.
Reason for revert: timeouts
Original change's description:
> Add more comprehensive test for GPU write pixels.
>
> Similar to existing SurfaceContextReadPixels but for writes. Tries all
> combinations of src/dst color type and alpha type for write pixels.
> Always reads back pixels for verification using the ImageInfo of the
> tested surface context.
>
> Bug: skia:8862
> Bug: skia:11130
>
> Change-Id: Id01f6aa511f00c4be47c32746dca872368cd5d82
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348886
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,bsalomon@google.com
Change-Id: I5498be0b20604e520ad887898695a81ca82936ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:8862
Bug: skia:11130
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350559
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
If using discardable msaa surfaces in vulkan, when we break up a
render pass for an inline upload, we must do a load msaa subpass for
the second render pass. However, if the original render pass did not
have this load subpass (e.g. clear or discard load op), then all the
GrProgramInfos for draws that end up in the second render pass will
have been recorded thinking they will be in a render pass with only 1
subpass. Thus we add an override flag to the makeDesc call to force
the actually VkPipeline that gets created to be created using a render
pass with 2 subpasses. We do lose the pre-compile abilities with this
approach, but inline uploads are very rare and already slow.
Ideally we would find a way to pull inline uploads into their own
GrRenderTasks so all this could be known ahead of time. But currently
we allow uploads to happen between draws of the same Op so it makes it
difficult to break up and even know when they'll occur at record time.
Bug: skia:10979
Change-Id: I247b007813dd0be280ce8a72fc6af70fe21f082d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344962
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
We rely on exact output from SkStringAppendScalar in our
Housekeeper "Generated Files" bot. However, we can't trust snprintf(%g)
to emit the same string for infinite and NaN values on every platform.
For instance, the bits 0xFFFFFFFF as a float are `-nan` on Linux and
`nan` on OS X. Infinity is represented as `inf` on Linux/Mac and
`1.#INF00` in Visual C++.
This CL standardizes on the strings `inf`, `-inf` and `nan` across all
platforms. This solves a GeneratedFiles issue in the followup CL:
http://screen/5RVdSnLmBupzpja
Change-Id: I648fd32571f8300998ec427dcb3d1e7d7215dbdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350496
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Similar to existing SurfaceContextReadPixels but for writes. Tries all
combinations of src/dst color type and alpha type for write pixels.
Always reads back pixels for verification using the ImageInfo of the
tested surface context.
Bug: skia:8862
Bug: skia:11130
Change-Id: Id01f6aa511f00c4be47c32746dca872368cd5d82
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348886
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This simplifies the argument lists for functions and will also allow us
to extract the AA logic into its own subclass.
Bug: skia:10419
Change-Id: If51e86a7633da7a3ee9352c0236258a0a21f2ebe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347976
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: oss-fuzz:29183
Change-Id: I60475e2bda42e98b79a4291839249158d2738c9c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350028
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously `number(boolean)` casts were converted to a ternary during
IR generation, and `boolean(number)` casts caused an error.
Metal and GLSL should support this cast as written. SPIR-V needed a
little bit of logic to handle converting the boolean to a number via
OpSelect.
Change-Id: I0069781e2b5a26a25c8625ab41c2392342bfd10d
Bug: skia:11131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349066
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The new unit test demonstrates load/store reordering is error-prone.
At head we're allowing loads from a given pointer to reorder later than
a store to that same pointer, and boy, that's just not sound. In the
scenario constructed by the test we reorder this swap,
x = load32 X
y = load32 Y
store32 X y
store32 Y x
using schedule() (following Op argument data dependencies) into
y = load32 Y
store32 X y
x = load32 X
store32 Y x
which moves `x = load32 X` illegally past `store X y`.
We write `y` twice instead of swapping `x` and `y`.
It's not impossible to implement that extra reordering constraint: I
think it's easiest to think about by adding implicit use edges in
schedule() from stores to prior loads of the same pointer. But that'd
be a little complicated to implement, and doesn't handle aliasing at
all, so I decided to ponder on other approaches that handle a wider
range of programs or would have a simpler implementation to reason
about. I ended up walking through this rough chain of ideas:
0) reorder using only Op argument data dependencies (HEAD)
1) don't let load(ptr) pass store(ptr) (above)
2) don't let any load pass any store (allows aliasing)
3) don't reorder any Op that touches memory
4) don't reorder any Op, period.
This CL is 4). It's certainly the easiest and cheapest implementation.
It's not clear to me that we need this scheduling, and should we find we
really want it I'll come back and work back through the list until we
find something that meets our needs.
(Hoisting of uniforms is unaffected here.)
Change-Id: I7765b1d16202e0645b11295f7e30c5e09f2b7339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Fixes terminology inside the generator to correctly use 'arguments'
rather than 'parameters'. Retains the caller-supplied argument span, so
that generateCode can push changes to 'out' and 'inout' parameters back.
Adds a useful helper function for getting a named function from a
Program.
All of this is used more extensively in upcoming CLs that migrate
interpreter unit tests (and particles) to SkVM.
Change-Id: I9d9aef4b7c4daf93d822e22f813162f4ed2385be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350023
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The code didn't take into account that x and y might be different types.
(This bug was not actually harmful; type coercion allowed the code to
compile even with the wrong type. The float would be silently splatted
into a vec and the rest of the code would work as-is.)
Change-Id: Ib76bc733f76304e451ef9197421b4bc22e29e49c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348888
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ieb7698d357c9be05ca1f17de84215add54553f84
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349065
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I2c39df532803d827d7cad876021f2ead81145f1d
Bug: skia:10902
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349064
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 6ea387e7c7
Original change's description:
> Enable GrTessellationPathRenderer by default
>
> Moves GrTessellationPathRenderer to the end of the chain and enables
> it by default.
>
> Also updates nvpr to not draw volatile paths. The tessellator is much
> faster at these.
>
> Bug: skia:10419
> Change-Id: I97ca7d4d1dff65fc9d4040c267f9808c8c33b548
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344377
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:10419
Change-Id: I53b50d6aa01ae1bea697e05c3f60d57d132d6aad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345759
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:7650
Change-Id: I5bae4dec2fbbd57b2cebc4324f5344cc8769b141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349398
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Mike Reed <reed@google.com>
While it is not often that moving an SkAutoDescriptor saves much, it
being non-movable prevents other classes which use it (like SkStrikeSpec)
from being default movable.
In general types with a copy constructor but an explicitly deleted move
constructor are generally not a good idea in C++.
Change-Id: I7557d38ba5e47b4eb5bdf36245590e717f64e12b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349057
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Previously, the declaration didn't link back to function definition.
This makes the function appear to be undefined, which inhibits inlining
and also makes it difficult for us to validate the presence of a
definition for every called function.
Change-Id: I220ab502634cb3e1d337c23bac150af9aa6370b1
Bug: skia:10902
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349063
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>
Previously, we did very little to distinguish between a built-in
intrinsic and a user-defined function whose name matches an intrinsic.
This could lead to all sorts of surprising outcomes, as our intrinsic-
rewriting code is able to make assumptions that might not hold true for
arbitrary user-defined functions.
Change-Id: I4180e0c5becdeb6a0a162534eaecfc90dda3392c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349062
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>
Previously, we had three separate implementations of intrinsics:
- "Special" intrinsics (the common case)
- "Metal" intrinsics (the GLSL bvec comparison functions)
- A hardcoded list of if-then workarounds in `writeFunctionCall`
There doesn't seem to be any particular rhyme or reason for why things
were organized in this way. These all been merged into the "Special"
intrinsic handling.
Change-Id: I10e4eed00a2dd2c7cb1f79a4b96460fc43a68ebb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349059
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fills out the GrVkOpsRenderPass and connects all the wires to have
things hooked up correctly. This CL won't actually enable things as the
GrVkCap is still disabled and will be landed separately.
Bug: skia:10979
Change-Id: I249fea4cbab3ba636c794986de080ceaf2d4769b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341381
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I6c0a6192a78ce60be60a71ed75350ca1bc256d57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348890
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I954af70f545a2258babd82af0d43d509201fdc59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348889
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This will make it easier to use GrVkPipeline when we make pipelines
for the discardable msaa loads.
Bug: skia:10979
Change-Id: I17c6864dad62c9ee2b406fb68e7ac4ab62546e57
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348893
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This reverts commit 7689319271.
Reason for revert: Sticking with this despite regression
Original change's description:
> Revert "Yank out old reduceOpsTaskSplitting code"
>
> This reverts commit dfc880bd9b.
>
> Reason for revert: chromium:1146701
>
> Original change's description:
> > Yank out old reduceOpsTaskSplitting code
> >
> > The behavior previously triggered by this flag is reimplemented
> > in review.skia.org/345168 . The current implementation isn't used
> > and can't really be used safely because it may go over the
> > GPU memory budget.
> >
> > Bug: skia:10877
> > Change-Id: I2759c688aa60a618ef76dfec0a49ef5e5f0a9afc
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345477
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Auto-Submit: Adlai Holler <adlai@google.com>
>
> TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:10877 chromium:1146701
> Change-Id: I90cdd16eaf033b816f2d1830fd0ee72fdc0ec74b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346777
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:10877 chromium:1146701
Change-Id: Idb734bebe4d0baac9d47539791b0e6240d6be2da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348883
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
This reverts commit 6e9bf51dd8.
Reason for revert: We're sticking with this despite regression
Original change's description:
> Revert "Have DDLs honor the reduceOpsTaskSplittingFlag"
>
> This reverts commit d7b59c091d.
>
> Reason for revert: Regression in chrome:1146701
>
> Original change's description:
> > Have DDLs honor the reduceOpsTaskSplittingFlag
> >
> > Especially while I'm fiddling with the implementation, we don't
> > want the user to be surprised when using DDLs also triggers
> > this other codepath.
> >
> > Bug: skia:10877
> > Change-Id: I660ea08189fff45acd7a45df12e15c45f607758a
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332720
> > Commit-Queue: Brian Salomon <bsalomon@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
> > Auto-Submit: Adlai Holler <adlai@google.com>
>
> TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: skia:10877 chromium:1146701
> Change-Id: Ieab2853b54663e7633bec6d71929db9885251ed2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346659
> Commit-Queue: Adlai Holler <adlai@google.com>
> Reviewed-by: Adlai Holler <adlai@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:10877 chromium:1146701
Change-Id: I754436755829d25da3fdb37f634669ae33419eb4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348882
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
This reverts commit 8f924ac0ce.
Reason for revert: suppressions landed for fuchsia images to rebaseline
Original change's description:
> Revert "Add new virts, hide old ones"
>
> This reverts commit c56e2e5aa6.
>
> Reason for revert: suspected of breaking chrome roll
>
> Original change's description:
> > Add new virts, hide old ones
> >
> > Add virtuals for the draw methods that now take sampling/filtermode.
> >
> > drawImage
> > drawImageRect
> > drawImageLattice
> > drawAtlas
> >
> > Add a flag that can remove the older virtuals, once each client has
> > stopped overriding them. In that situation, the older public methods
> > will simplify extract the sampling from the paint, and call the new
> > public methods.
> >
> > Bug: skia:11105, skia:7650
> > Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
> > Commit-Queue: Mike Reed <reed@google.com>
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> TBR=bsalomon@google.com,fmalita@chromium.org,reed@google.com
>
> Change-Id: I0a90952c11a180d918126ea06a630f4a0bf9b49b
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11105
> Bug: skia:7650
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348194
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
TBR=djsollen@google.com,bsalomon@google.com,fmalita@chromium.org,reed@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:11105
Bug: skia:7650
Change-Id: Ia2b4537a2d330460b7554278d2c05075cf27162a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348876
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This logic looks to have been dropped when we moved to the auto
generated interfaces
Bug: skia:11083
Change-Id: I5069d81c872ed2c084b8ddf773f63e5202b123eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348190
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: skia:10913
Change-Id: I430e5eb3fecb0f15775db03699819194d44271b6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347958
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit c56e2e5aa6.
Reason for revert: suspected of breaking chrome roll
Original change's description:
> Add new virts, hide old ones
>
> Add virtuals for the draw methods that now take sampling/filtermode.
>
> drawImage
> drawImageRect
> drawImageLattice
> drawAtlas
>
> Add a flag that can remove the older virtuals, once each client has
> stopped overriding them. In that situation, the older public methods
> will simplify extract the sampling from the paint, and call the new
> public methods.
>
> Bug: skia:11105, skia:7650
> Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
> Commit-Queue: Mike Reed <reed@google.com>
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,fmalita@chromium.org,reed@google.com
Change-Id: I0a90952c11a180d918126ea06a630f4a0bf9b49b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11105
Bug: skia:7650
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348194
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Add virtuals for the draw methods that now take sampling/filtermode.
drawImage
drawImageRect
drawImageLattice
drawAtlas
Add a flag that can remove the older virtuals, once each client has
stopped overriding them. In that situation, the older public methods
will simplify extract the sampling from the paint, and call the new
public methods.
Bug: skia:11105, skia:7650
Change-Id: I8b0029727295caa983e8148fc743a55cfbecd043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347022
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This reverts commit d7b59c091d.
Reason for revert: Regression in chrome:1146701
Original change's description:
> Have DDLs honor the reduceOpsTaskSplittingFlag
>
> Especially while I'm fiddling with the implementation, we don't
> want the user to be surprised when using DDLs also triggers
> this other codepath.
>
> Bug: skia:10877
> Change-Id: I660ea08189fff45acd7a45df12e15c45f607758a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332720
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Auto-Submit: Adlai Holler <adlai@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:10877 chromium:1146701
Change-Id: Ieab2853b54663e7633bec6d71929db9885251ed2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346659
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
This code was not using typeName() to emit its types, inadvertently
generating Metal code containing the `half` type.
We didn't have any unit tests which synthesized a matrix-construct
helper with half types, so Matrices.sksl was cloned into two separate
test files--MatricesFloat and MatricesHalf. These should be equivalent
except for float vs half types.
Change-Id: I19ecea994b8bc45594bb3f69e596896a3bcefe4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348180
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In Metal, matrix *= matrix is not natively supported and needs to be
injected via a helper function. This helper function now properly
converts `halfNxM` types to `floatNxM` types (as Metal does not support
half types). It also returns the result by reference instead of by
value to avoid an unnecessary copy.
Matrices.sksl now includes tests for operators += -= *=. Previously we
did not have any coverage for `matrix *= matrix` at all.
Change-Id: I7dfe468ced67eaf7c2405960e8c5efe6f2acf9e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348178
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>
4x4 was dividing a matrix by a scalar - this isn't allowed, multiply by
the scalar's inverse instead.
The types in the signature were derived from type.name(), which wasn't
applying the half->float re-mapping.
Finally, use raw strings so the resulting shader code isn't all crammed
on one line.
Bug: skia:10913
Change-Id: Ie28373fc138445b8c195dbd37687e4ad4504e918
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348177
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change readPixels contract to allow unknown->unknown AT reads, but
fail if one side is unknown and the other isn't (and update GPU read
pixels test accordingly).
Also, ProxyUtils::MakeTextureProxyViewFromData takes GrPixmap.
Bug: skia:8862
Change-Id: I771c154833408e666f860413c1a711714696326d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347196
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Previously, SKSL_INT was limited to an int32_t, so we couldn't
differentiate between -1 and 4294967295. We could paper over the
difference in some cases by relying on the expression's type, but this
was imperfect and left us unable to differentiate between an overflow
and valid results. SKSL_INT is now an int64_t; the code has been
updated to fix bugs that shook out as a result of the change.
This isn't a complete solution for overflow handling. There are still
lots of obvious places for improvement--e.g. constant folding can
easily overflow, and statements like `byte x = 1000;` are still
happily accepted.
Change-Id: I30d1f56b6f264543f3aa83046f43c2eb56d5fce4
Bug: skia:10932
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345173
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10913
Change-Id: I59f5b0fb2d015f8543b4038c2c5b18ce24c194a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347956
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Also, add control over kMedium mapping
Seems like a loss that GrInterpret throws away the cubic coefficients...
A step towards https://skia-review.googlesource.com/c/skia/+/347022
Bug: skia:11105, skia:7650
Change-Id: I2564c29b0eeffd4df92c0baa78e2c04f6348d7d7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347636
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
channels in shader.
This matches how CPU backend and 3D APIs do things.
Change-Id: I455e89e5782f6653ccd4e29e0f23ec087687aac0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317763
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Will be used to simplify current and forthcoming code that needs
GrImageInfo, void* pixels, size_t rowbytes. The motivation here is
to make GrSurfaceContext::writePixels support mip levels via an array
of GrPixmap. The goal of that is to remove special purpose
updateBackendTexture code. That code path doesn't support the three
GrColorType paradigm used by GrSuraceContext::writePixele: src color
type, dst color type, intermediate color type used for upload.
Bug: skia:8862
Change-Id: I1f09e1ee017c8a2637ac9f630a13a1ed8040d891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345175
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change SkTypeface::onCreateScalerContext to return std::unique_ptr
instead of a bare pointer. The public SkTypeface::createScalerContext
implementation already returns std::unique_ptr, so this is an internal
change only.
Move SkTypeface::createScalerContext implementation from
SkScalerContext.cpp to SkTypeface.cpp for consistency.
Also change the return type of SkScalerContext::MakeEmptyContext to
std::unique_ptr and rename to SkScalerContext::MakeEmpty.
Change-Id: I965308e8f9c78b887811e428f0de873dc6196479
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346658
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: Ben Wagner <bungeman@google.com>
Previously, some types of overflow were detected, but most would assert
or silently generate invalid code. Now, the parser will properly report
an error if it encounters any integer that exceeds UINT_MAX or any float
that exceeds FLT_MAX.
This fixes test OverflowUintLiteral.sksl. Added a test for floats as
well, OverflowFloatLiteral.sksl.
OverflowIntLiteral.sksl does not fail yet, because its values are larger
than INT_MAX, not UINT_MAX. These are legal from the perspective of the
parser. This must be caught later at IR generation time.
Change-Id: Ia5a904d01427cdc9f2ab5f4174154418737835e6
Bug: skia:10932
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347176
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These are part of the GLSL ES 1.0 spec.
Bug: skia:10913
Change-Id: I61bfcadc92209fb3a6aa07b3d6ad579460ad2ecd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347049
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This was only used for nullable fragment processors, which are gone.
Change-Id: I1ea805c683995367a7525b787c9113ae6d2d0ae0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347051
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This fix is overly conservative in some situations (identity conversions
among vectors with the same component type), but fixes errors in two
existing unit test cases.
Bug: skia:11116
Change-Id: If852f8591fb26817528fdc37191c49129e17d6b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347053
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This feature had devolved to just an assert, and one that isn't really
necessary - all of Ganesh is built to handle any child processor being
null. The next step is to remove nullable types entirely -- a large
amount of code.
Change-Id: I612a5867f8690400b405aa1f5c929e76cf5918fd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347050
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Not used.
Change-Id: I01426b3ed6aa05fd1749fa132c7e92ab09f90fb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344973
Auto-Submit: Ben Wagner <bungeman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This CL updates `compareConstant` to fail gracefully instead of
aborting if the passed-in types don't match. This lets us call
`compareConstant` without checking types first.
Change-Id: Id2acdbdf700e64bcb24825cdad2c0e000992e8cb
Bug: oss-fuzz:28904
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347038
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
'int' is the only integral type that exists in GLSL ES 1.0 (and it's not
really guaranteed to be an integer). This enforces the same restriction
on runtime effects - no unsigned integers, and no short or byte types.
Bug: skia:11093
Change-Id: I938f1e0e125dc8347507f428b46b51c66033c752
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347046
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
More GLSL-isms that weren't used, and weren't really ready for use (most
were defined as "Other", not "Sampler"). If/when we need these, it's
easy to add them back. In the meantime, we should have a simpler system
for reserving keywords that doesn't pollute the type system.
Bug: skia:11115
Change-Id: I436c1e4de6e6b92ff14fc99ed1d47e0c5d1e3aff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347045
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Missed these when I moved the float versions earlier. Non-square
matrices don't exist in our minimum spec (GLSL ES 1.0).
Bug: skia:11093
Change-Id: I09b3ab71199bc70d9b54302c14b93bc3f3dec2d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347042
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
SkPaintPriv methods are just an internal stopgap
Change-Id: Ibe6e37c5871068d8cd67dc0948961444dfd2b62a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347041
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
These are GLSL-isms that weren't really implemented - each one was a
"generic" type that only resolved to a single underlying type. We've
got along just fine without them for years, so update our sample()
declarations to take the actual underlying type. (Note that we had
worked around this by declaring an integer version of sample where
necessary, so we can presumably keep doing that in the future).
Change-Id: I4c46a2fa0c1f19e6278298c8005a2760329e7abf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347040
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Nullable fragment processors still exist, but they're handled
transparently by sample() within C++, so there's no need for .fp files
to ever do these tests manually.
Change-Id: Idf2bc58505207560553066c0126a2a036c5d970b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347039
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Made it private and accessible internally via SkCanvasPriv.
Update SkGpuDevice methods/variables after rename of GrDrawSurfaceContext.
Cq-Include-Trybots: luci.skia.skia.primary:Canary-G3,Canary-Flutter,Canary-Android
Change-Id: I3da64cee1de03c201243ee6c7ccd4b4c44cad8c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346498
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
... and lots and lots of IWYU
Change-Id: Ie5157dcdd2e6d29b95c71b39153278ab48ef4eb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346778
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
These were unused, and I don't think they were correctly defined, given
the inconsistency in naming and construction. If we do end up needing
something like this (a variant type for returns from sample(), we can
re-add them correctly).
Bug: skia:8863
Change-Id: I32a9487ba0e247f67696947babbfd6b64f0a047c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/347037
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>
Change-Id: I2d396907387de4a5f3407d81efb9d2cd80e430d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346265
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Opaque types can no longer be copied via assignment or construction, and
various restrictions originally applied to the "fragmentProcessor" type
have been extended to cover opaque types in general.
Change-Id: I55ab7aefd1e6ef277e56a9408b430e1de5ba12ca
Bug: skia:11027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346264
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 intrinsic was previously lacking a unit test, and wasn't actually
implemented in Metal or SPIR-V. Fortunately it's trivial to add.
Change-Id: I68bbdc58376b579c7f3f0ae5f49323b389c2b8c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346263
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 is slightly simpler than having three separate overloads, and
provides the same results.
Change-Id: Icc7a749fd642f6d6a9e69b769494c566569ea8f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346262
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: chromium:1160579
Change-Id: I3c9c960594b30620897ed0375f8783eda628e1f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346638
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Updated Pair type in SkTHashMap to derive from std::pair to fix C++14
issues with structured bindings.
Original change's description:
> Add support for range-based for loops to SkTHashSet/Map.
>
> This allows loops over SkTHashes to break in the middle, and also
> removes the need to use lambda captures to bring variables inside the
> loop's scope.
>
> Change-Id: Ief55d776b2c57a44b24cfe1c94493a5d514791c8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346496
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I2ac5b2c59e70ed0ec3b42b32e7994d6bcdf56b40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346502
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
I checked that this prevents diffs on my M1 Mac.
Without this we'd see FMAs.
Change-Id: I24fa2e301cde556de556332b472399b97e6a08fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346676
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These effectively compute width/2 or height/2, but switch operations
around so that it's less likely to overflow on finite rects that would
have overflows in width or height.
Bug: skia:1160678
Change-Id: Ic93ca0c1d12598163b3dd48a5e8ba0ac7903301f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344968
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This reverts commit dfc880bd9b.
Reason for revert: chromium:1146701
Original change's description:
> Yank out old reduceOpsTaskSplitting code
>
> The behavior previously triggered by this flag is reimplemented
> in review.skia.org/345168 . The current implementation isn't used
> and can't really be used safely because it may go over the
> GPU memory budget.
>
> Bug: skia:10877
> Change-Id: I2759c688aa60a618ef76dfec0a49ef5e5f0a9afc
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345477
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Auto-Submit: Adlai Holler <adlai@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,adlai@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:10877 chromium:1146701
Change-Id: I90cdd16eaf033b816f2d1830fd0ee72fdc0ec74b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346777
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Previously, a return statement inside a scoped Block would always result
in the return expression being assigned to a temporary variable instead
of replacing the function-call-expression directly. This was done
because there might be variables inside the Block; these would have
fallen out of scope when the expression is migrated to the call site,
resulting in an invalid expression.
We aren't actually examining the return expression so we don't know if
it uses variables from an inner scope at all. (Inspecting the return
expression for variable usage is certainly possible! But it's a fair
amount of code and complexity for a small payoff.)
However, we can very easily get most of the benefit here without paying
for the complexity. In this CL we now look for variable declarations
inside of scoped Blocks. If the code doesn't add any vardecls into
scoped Blocks, there's no risk of scope problems, and we don't need to
use a temp-var to store our return expressions. If any vardecls are
added, we go back to using a temp-var as before.
Change-Id: I4c81400dad2f33db06a1c18eb671ba2140232006
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346499
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
These don't exist in our minimum spec (GLSL ES 1.0)
Bug: skia:11093
Change-Id: Ia2d871199fff2a98dcd517c1eebe46decb0c2dfb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346657
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also renamed $matH to $hmat, to match $hvec convention. Runtime effects
will only support square matrices (like ES2), so this lets us declare
intrinsics like matrixCompMult correctly (and differently) for public
vs. private usage.
Bug: skia:11093
Change-Id: I457d83e4c5e09f8e01e7b8acb116c39ff17e52c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346656
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Ida9d81a8d4e530cb3055f01418a2ad2893ae86db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345422
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 5d00e15625.
Reason for revert: tree breakage on Chromebook standard lib
Original change's description:
> Add support for range-based for loops to SkTHashSet/Map.
>
> This allows loops over SkTHashes to break in the middle, and also
> removes the need to use lambda captures to bring variables inside the
> loop's scope.
>
> Change-Id: Ief55d776b2c57a44b24cfe1c94493a5d514791c8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346496
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=mtklein@google.com,johnstiles@google.com
Change-Id: I165872ac41f66f3b3255cf8970626392e5283412
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346500
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This allows loops over SkTHashes to break in the middle, and also
removes the need to use lambda captures to bring variables inside the
loop's scope.
Change-Id: Ief55d776b2c57a44b24cfe1c94493a5d514791c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346496
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We need to rescan after optimizing away expressions that might exist
in the CFG/definition map, since we are rebuilding them from scratch and
not just stripping off excess parts from them.
Change-Id: I843a2ea3fc38428e7c0bd0e2bf7a7d41101345e3
Bug: oss-fuzz:28794
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344972
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This was added to workaround a bug in D3D (w/SPIRV-cross). It's now
handled correctly in SPIRV-cross itself, so this is unused.
Change-Id: Ic411ee2219cf370bcebff18ae18689d16da90866
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344966
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Android is now using framework-only topLayerBounds()
Cq-Include-Trybots: luci.skia.skia.primary:Canary-Android
Change-Id: I40375ef1de45d3c2160dd913593326950265314e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344974
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Should allow us to test sksl->skvm portably using dump().
Change-Id: If55e8e144f04643c02bd65baa84158ac1bf441b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346336
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This maps to usage better, and makes some code simpler to understand.
Note that there is still a PipelineStage *back-end*, which is specific
to the runtime-effect FP. A kRuntimeEffect_Kind program can be used to
generate a PipelineStage (for the GPU backend), or an skvm program (for
the CPU backend).
Change-Id: Id3f535db93a239726c595225aafe9467f0d19817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344969
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Nulls aren't generally expected here, but this is a debugging utility
and in practice it's more useful to print nulls cleanly than to crash
while trying to dump out a CFG.
Change-Id: I405c1e51875655acaf4f9689b47e3e6a6b8751df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344967
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>
No known clients (CanvasKit, Android) want to ignore the exif
orientation in an animation.
A follow-on CL will deprecate SkAndroidCodec::ExifOrientation, leaving
it up to the client (e.g. SkAnimatedImage, hwui/ImageDecoder) to
handle the orientation. Add SkEncodedOriginSwapsWidthHeight to assist
clients to do so.
Update stoplight_animated_image GM. It previously showed using
SkAnimatedImage without respecting the orientation, which is no longer
supported. The new version replaces the left half of the image with the
right.
Remove assert that is no longer true. Originally, an SkAnimatedImage was
"simple" if it did not have a crop or postProcessor. This is no longer
true if has an exif orientation. Add a test that calls the simple
constructor and verifies it does not crash.
Change-Id: I421fd02700f220fb90458cd03c4431dee7daf399
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344762
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Ops should report HasAABloat::kYes if they use coverage AA approaches.
This was confusing the new GrClipStack into thinking a subpixel path
(that should have been coverage) was non-AA, and in certain transforms
this would then not touch pixel centers and the clip stack would reject
the draw early.
Bug: chromium:1159852
Change-Id: If8ad041af92b915e126ab50e4169c959201c1d95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345717
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 6ea387e7c7.
Reason for revert: valgrind failing
Original change's description:
> Enable GrTessellationPathRenderer by default
>
> Moves GrTessellationPathRenderer to the end of the chain and enables
> it by default.
>
> Also updates nvpr to not draw volatile paths. The tessellator is much
> faster at these.
>
> Bug: skia:10419
> Change-Id: I97ca7d4d1dff65fc9d4040c267f9808c8c33b548
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344377
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com
Change-Id: I3718ae210ebc403959d187160214cf0df895e4f5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10419
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345718
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is part of the bottom-up approach, trying to eliminate
filter-quality from the inside, force us to pass in the sampling
parameters (eventually) from the client/canvas.
Change-Id: I5c17c9912333c7a481e88744569518c06ba4a945
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345576
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Previous code would not align to 4 but not necessarily bytes-per-block.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Debug-All-Vulkan
Change-Id: I06ffc9c38f52c645bafd22d0ac30712efb815e76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345134
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Rather than return a complex ternary expression, we now assign the
complex expression into a variable and then return that variable.
This works around a bug in the Adreno driver where it fails to parse
"(complex-expression).swizzle" and errors out with "Cannot offset into
the vector." Instead, we now store the complex expression into a
variable and return that variable. This changes the final assembled
code into a two-parter, "float2 v = (complex-expression);" followed by
"v.swizzle". This is easier for the Adreno driver to swallow and works
around the error seamlessly.
Example Adreno failure: http://screen/Bkx5zRvaMEbuCTh
Change-Id: Ib58abd404d8a1d7ce664acd38300ef75d0296363
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345133
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The behavior previously triggered by this flag is reimplemented
in review.skia.org/345168 . The current implementation isn't used
and can't really be used safely because it may go over the
GPU memory budget.
Bug: skia:10877
Change-Id: I2759c688aa60a618ef76dfec0a49ef5e5f0a9afc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345477
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
This reverts commit 4129b6b65f.
Reason for revert: WASM breakage: https://task-driver.skia.org/td/UBRwnWYfbc5IwUWqtFMv
Original change's description:
> Revert "Reland "Reland "Reland "Revert "Initial land of SkSL DSL."""""
>
> This reverts commit 346dd53ac0.
>
> Change-Id: I93bb18438cc6c2ad43d058d6c3f95bcc65d0cea9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343916
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: If05145cf9d9c51f4c76fe523f6050a670b5da669
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345169
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Because we use `continue` for flow control handling now, we can escape
from the middle of a switch statement. This wasn't possible when we used
`break`.
This unlocks some pretty stellar optimization opportunities if the
switch value can be determined at compile time; see BlendEnum for an
example.
Change-Id: Id29be92c343c10fd604683a80c5d5bd2bd070cb0
Bug: skia:11097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345419
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Instead, add them to the shared element list (like functions and
variables).
Bug: skia:10905
Change-Id: Ib52b4c5eaca9a3023532c5f7021d0ac263ab64ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345476
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
... on our way to adding the parameter to SkDevice
Bug: skia:7650
Change-Id: Iebde07e747741dfb47888426210bf2f231f295aa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345418
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
None of these earliest testing tools are useful anymore
now that we can do useful work with SkVM and SkVMBlitter.
Change-Id: I8b25ef6ddd101c4ff8617c6742343dedb4764922
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345456
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
When we determine that a function only contains a single return
statement and it is at the top level (i.e. not inside any scopes),
there is no need to create a temporary variable and store the
result expression into a variable. Instead, we can directly replace
the function-call expression with the return-statement's expression.
Unlike my previous solution, this does not require variable
declarations to be rewritten. The no-scopes limitation makes it
slightly less effective in theory, but in practice we still get
almost all of the benefit. The no-scope limitation bites us on
structures like
@if (true) {
return x;
} else {
return y;
}
Which will optimize away the if, but leave the scope:
{
return x;
}
However, this is not a big deal; the biggest wins are single-line
helper functions like `guarded_divide` and `unpremul` which retain
the full benefit.
Change-Id: I7fbb725e65db021b9795c04c816819669815578f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345167
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
If we aren't wrapping the inlined function body in a loop, there's no
need to add a scopeless Block; we've already got one. This doesn't
affect the final output meaningfully--it just suppresses a newline--but
it's one fewer IRNode allocation.
Change-Id: Ib7b0014e908586d8acfcf6c23520873fad31d0b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345163
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Moves GrTessellationPathRenderer to the end of the chain and enables
it by default.
Also updates nvpr to not draw volatile paths. The tessellator is much
faster at these.
Bug: skia:10419
Change-Id: I97ca7d4d1dff65fc9d4040c267f9808c8c33b548
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344377
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
do-while loops aren't compatible with GLSL ES2. For-loops which run
only one time should work exactly the same for our purposes. We expect
such a loop to be unrolled by every driver, so it shouldn't come at any
performance cost.
Change-Id: Ia8de5fcab8128c34da97eaeaf81f91ad1ac36ce4
Bug: skia:11097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345159
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Still TODO:
- Function calls. Can do these by inlining everything (recursively).
- Additional flow control: ES2 for loops can be supported (via
unrolling), and switch() can be implemented. (Was never done in
ByteCode).
- Uniforms and params have been set up to work very generically (caller
just supplies IDs, the generator collates everything into a master
working list). Builtins should work the same way (caller supplies a
map of builtin ID -> skvm::Val[]?), so that we don't need a special
case for device coord.
- Figure out integer type policy. Today, it's a mix of only supporting
signed integers, and just treating all integers as signed, even if
they're not.
- Now that defining intrinsics is *much* simpler, stop defining so many
of them in sksl_public.sksl, and just implement them here.
Change-Id: Id9771444ce54ccf8e6e408c44ebb3780c1170435
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341980
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Fortunately, this had an existing opcode, so it was easy to add to our
intrinsics list, and the rest automatically worked.
Change-Id: Idcd5a2c46d6bf10c05c702faba4280a270c54929
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345398
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We are still missing an implementation for Metal and SPIR-V, but at
least it's correct on GLSL now.
Change-Id: I5b365384eaefacb00faf6af7bda9b690cba00de5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345397
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 shadowRec bounds code didn't handle directional lights, that's now
fixed. Also fixes normalization of the light direction -- it was only
using two components, it should use all three.
Bug: skia:10781
Change-Id: Ia7d39c5187f976627d017ac4abecbe1d1dc62712
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345126
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: Ie9e7b573ef0521fc836ca4ffeef9c14767efd45c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345293
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Mike Reed <reed@google.com>
- that removes the need for onDrawImageNine
- ... that removes a whole lot of plumbing
Bug: skia:11099
Change-Id: I35e2f70320be118aec38a5c783a53c7aa90b3131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345160
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Removed const Device* return
This reverts commit 9e230d8136.
Change-Id: I1151a198e857fb1b539f69fdab917668c35d147d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345285
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Previous approach using magick's -orient tool didn't actually change the
stored JPEG's orientation (they were all tagged Undefined and the data
was top-left).
Regenerate the images using new method where the PNGs are generated with
transformations that the EXIF tag will undo. The script converts to JPEG
using magick and then modifies the TAG using exiftool.
Change-Id: Ic2276711c0f4faa26620c93b24f4f0e58d658ae3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345127
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
In some cases, it's okay if we can't find a path to a CFG block. Two
such cases are:
- The starting block. This is reached implicitly.
- For-loop increment expressions. These are sometimes required due to
GLSL ES2 restrictions, even if they can't actually be reached.
Change-Id: I626c424361a7339d4fb1ab0809f60df3f2f72173
Bug: skia:11097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345162
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit d8b939e0b4.
Reason for revert: fix SkAndroidFrameworkUtils getDevice() reference
Original change's description:
> Revert "Rename getTopDevice/getDevice to topDevice/baseDevice"
>
> This reverts commit 632a23afa4.
>
> Reason for revert: breaking android roll
>
> Original change's description:
> > Rename getTopDevice/getDevice to topDevice/baseDevice
> >
> > This addresses nit from https://skia-review.googlesource.com/c/skia/+/344663
> > and also moves topDevice() to be private, since it's not really meant
> > for arbitrary subclasses to access anyways. Classes outside of SkCanvas
> > that need topDevice/baseDevice are already friended anyways.
> >
> > Bug: skia:10987
> > Change-Id: Ib0b0dffb5b1c35d5a0c78aeb8903e4cf0b8810a9
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344960
> > Reviewed-by: Mike Klein <mtklein@google.com>
> > Reviewed-by: Mike Reed <reed@google.com>
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
>
> TBR=mtklein@google.com,reed@google.com,michaelludwig@google.com
>
> Change-Id: I9b70230f7db5d31cede2ea03e7abc7c154021204
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10987
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345158
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
> Commit-Queue: Derek Sollenberger <djsollen@google.com>
TBR=djsollen@google.com,mtklein@google.com,reed@google.com,michaelludwig@google.com
# Not skipping CQ checks because this is a reland.
Bug: skia:10987
Change-Id: Ibf6397b54655c25ef3d348210de5eb6dab416723
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345165
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Extract the fibonacci progression block size calculation.
Simplify it by using a common array for the fibonacci number.
Add unit tests to check extreme limits.
Change-Id: I270a713881590f7e17be58dc9072ab7a79a7388d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345124
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:11095
Change-Id: Icd69df40675e5ecde5004e04a7dcd78eedf8343c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344765
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Also remove the supporting SkScalerContext::generateGlyphCount. It was
once a possibility that different strikes could have a different number
of glyphs available. This generally does not happen, but if it did this
would now be handled at the typeface level.
Change-Id: I8db7f2726c565447c211a79fe3cdfec619a99a81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345129
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This will be utilized in followup CLs.
Change-Id: I591cff5b8942a5be57660a4bd161517880fbb8a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345157
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 632a23afa4.
Reason for revert: breaking android roll
Original change's description:
> Rename getTopDevice/getDevice to topDevice/baseDevice
>
> This addresses nit from https://skia-review.googlesource.com/c/skia/+/344663
> and also moves topDevice() to be private, since it's not really meant
> for arbitrary subclasses to access anyways. Classes outside of SkCanvas
> that need topDevice/baseDevice are already friended anyways.
>
> Bug: skia:10987
> Change-Id: Ib0b0dffb5b1c35d5a0c78aeb8903e4cf0b8810a9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344960
> Reviewed-by: Mike Klein <mtklein@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=mtklein@google.com,reed@google.com,michaelludwig@google.com
Change-Id: I9b70230f7db5d31cede2ea03e7abc7c154021204
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10987
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345158
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Adds support in GrVkRenderPass and GrVkRenderTarget for making and
managing discardable msaa attachments.
Bug: skia:10979
Change-Id: Ib5005a694f2c54d7b132c3d3767f648b9c9f747f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341379
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This fixes an issue where for loops with no test created invalid code,
and improves tracking and adds asserts to keep us honest in the future.
Change-Id: Ie2212b237948bf245ca4a2918bc6b018540646c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345128
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is a new base class for GrSurfaceDrawContext. It allows any
alpha-type but is restricted to non-blending fills of irects using FPs,
clears,and discards.
Bug: skia:11019
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341680
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Change-Id: I696df3617719fcd8303faa73fb44b32b3fb4f71c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344896
Previously, multiple inliner passes in a row would each apply a
separate name mangling to variable names, so names like "_25_14_3_1_pos"
were not uncommon. This change demangles the name before re-mangling it,
so we would have just "_25_pos" instead.
It's not important, but it makes things easier to read.
Change-Id: I1257222dac2a68e337f431af230ce50730cedc9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345116
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This addresses nit from https://skia-review.googlesource.com/c/skia/+/344663
and also moves topDevice() to be private, since it's not really meant
for arbitrary subclasses to access anyways. Classes outside of SkCanvas
that need topDevice/baseDevice are already friended anyways.
Bug: skia:10987
Change-Id: Ib0b0dffb5b1c35d5a0c78aeb8903e4cf0b8810a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344960
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit e8e4aca955.
Reason for revert: can break ES2 for-loop rules
Original change's description:
> Declare all inlined variables at the topmost scope possible.
>
> By itself, this is uninteresting and even perhaps slightly
> counterproductive (as it separates vardecl from its initializer,
> increasing LOC). However, this enables a followup CL
> (http://review.skia.org/344665) which allows single-return functions to
> be inlined without the creation of a temporary variable at all. This
> applies to the majority of fragment processors in a typical Ganesh
> hierarchy. This change will greatly reduce the number of inliner-created
> temporary copies when compiling a typical tree of FPs.
>
> Change-Id: I03423a13cf35050637dabace4a32973a08a4ed0a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344764
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ica01d6906bcb9cef1f49d22dda714fc9cbfa3885
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345121
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 345d72124d.
Reason for revert: can break ES2 for-loop rules
Original change's description:
> Eliminate inliner temporary variables for functions with a single exit.
>
> When we determine that a function only contains a single return
> statement, there is no need to create a temporary variable and store the
> result expression into a variable. Instead, we can directly replace the
> function-call expression with the return-statement's expression.
>
> This dramatically simplifies the final optimized output from chains of
> very simple inlined functions, which is a very common pattern for trees
> of Skia fragment processors.
>
> Change-Id: I6789064a321daf43db2e1cef4915f25ed74d6131
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344665
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I60845f22159605a06047b030e2686a769121a35a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345120
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 67d2d73d06.
Reason for revert: Triggers errors in some shaders.
Original change's description:
> Fix incorrect 'unreachable code' error in SkSL
>
> This trades one error for another (a potential for incorrect use of
> unassigned variables). False-positives for unassigned variables are
> straightforward to workaround (and produce code that still looks
> reasonable). Working around unreachable code errors is tricky, and
> likely to produce non-idiomatic code. This change also makes the data
> flow analysis of all loop constructs more similar - for loops were
> behaving very differently from while loops.
>
> Note that this effectively a revert of:
> https://skia-review.googlesource.com/c/skia/+/18121/
>
> Change-Id: Ib85d90b22cac8addfb106459c0a5f5616a89c3eb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344957
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I84b93cc7f9309446dcc0e949e90908df31a1ff9c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345119
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Refactoring calls to explicitly build layout group entries, so that
changes to the struct do no break the build.
An example of the types of failures being seen,
https://skia-review.googlesource.com/c/skia/+/344795
Change-Id: Iadd091e399580a743bb9cd5ab65fada84eadaf9d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344959
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: Stephen White <senorblanco@google.com>
Auto-Submit: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@google.com>
When we determine that a function only contains a single return
statement, there is no need to create a temporary variable and store the
result expression into a variable. Instead, we can directly replace the
function-call expression with the return-statement's expression.
This dramatically simplifies the final optimized output from chains of
very simple inlined functions, which is a very common pattern for trees
of Skia fragment processors.
Change-Id: I6789064a321daf43db2e1cef4915f25ed74d6131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344665
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
By itself, this is uninteresting and even perhaps slightly
counterproductive (as it separates vardecl from its initializer,
increasing LOC). However, this enables a followup CL
(http://review.skia.org/344665) which allows single-return functions to
be inlined without the creation of a temporary variable at all. This
applies to the majority of fragment processors in a typical Ganesh
hierarchy. This change will greatly reduce the number of inliner-created
temporary copies when compiling a typical tree of FPs.
Change-Id: I03423a13cf35050637dabace4a32973a08a4ed0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344764
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Adds a new kDirectionLight_ShadowFlag which indicates that lightPosition
is a vector pointing towards the light, and lightRadius is the blur
radius when the occluder is at z == 1. For other values of z it will
scale linearly. The direction is specified in device space.
Bug: skia:10781
Change-Id: I14d530f006e5541ed6e22bfbeb29c7441c21fb8e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321792
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This trades one error for another (a potential for incorrect use of
unassigned variables). False-positives for unassigned variables are
straightforward to workaround (and produce code that still looks
reasonable). Working around unreachable code errors is tricky, and
likely to produce non-idiomatic code. This change also makes the data
flow analysis of all loop constructs more similar - for loops were
behaving very differently from while loops.
Note that this effectively a revert of:
https://skia-review.googlesource.com/c/skia/+/18121/
Change-Id: Ib85d90b22cac8addfb106459c0a5f5616a89c3eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344957
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of ad8b9fa2d7
Original change's description:
> Disable HW tessellation shaders by default
>
> The GLSL backdoor for tessellation shaders is only experimental at
> this point. In order to enable the tessellation path renderer, we need
> to turn off HW tessellation and let it use the indirect draw modes.
>
> Bug: skia:10419
> Change-Id: Ic979978a331c7ad016907cf42b2562b35caab8ab
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344336
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com
Bug: skia:10419
Change-Id: I0bdf34da134a3a34a5c78b35cc0eb86d6e989bb4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344796
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
* Fix visitProxies when using DDL
* Handle when the GrClip is nullptr
* Use the same cropping/hw disable logic for onStencilClip as we do
for onDrawClip.
Bug: skia:10419
Change-Id: I2ebd2b12ac9cc77cece6d3fbf6c16e246bafe544
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344479
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This would require careful handling of the stencil that we have not
yet thought through.
Bug: skia:10419
Change-Id: I78a652309ae3469b64a28d6d96b06384e608695e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344376
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Allowing this in runtime effects lets people break our contracts around
SkShaders on the color side of the paint altering coverage, etc.
Bug: skia:11085
Change-Id: I1ec8e71581c8d50f681cb0ca6ca8416375b3f43f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344760
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The additional scopes were harmless, but didn't really add any value.
Originally they were used to tightly scope inlined variables, but we now
mangle inlined variable names.
Change-Id: I7b35e737598036c0b6d3d9f71cbcd4a53d609ce9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344757
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL is mostly just moving code into shared functions and moving the
renderpass granularity adjustments into GrVkOpsRenderPass. Future
changes for resolve loading will take more use of these new shared
functions.
Bug: skia:10979
Change-Id: I19928b403ac7a71be0a9588bce81db40496d3bb0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344662
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This reverts commit ad8b9fa2d7.
Reason for revert: Benchmark failure
Original change's description:
> Disable HW tessellation shaders by default
>
> The GLSL backdoor for tessellation shaders is only experimental at
> this point. In order to enable the tessellation path renderer, we need
> to turn off HW tessellation and let it use the indirect draw modes.
>
> Bug: skia:10419
> Change-Id: Ic979978a331c7ad016907cf42b2562b35caab8ab
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344336
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,csmartdalton@google.com
Change-Id: Ia74dbcec582168870bd3ccc5e52b8dce0fb2bdfc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10419
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344698
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Bug: skia:10987
Change-Id: Ic1bb363e82cc09e280be5cfe0a46748e11ebb69c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344663
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Key design points...
A) Take care to not commit too much to Half's range or precision.
In particular, don't offer operations whose result might not
be representable in Half. E.g., shy away from Half divide,
not just because we might not be able to do a native divide,
but even more so because division produces results that might
not fit in Half, ±inf or NaN.
B) No native Half loads, stores, uniforms or splats,
instead converting from I32 or F32. This keeps the entire
front-end (user code, Builder, etc.) specified in terms of precise
format and oblivious to the various backends' representations of
Half. Native Half splats would be less trouble than uniforms I
think, and uniforms less trouble than loads and stores, but still
enough a pain that we're better of deferring any of that for now.
(Explicit fp16 uniforms do make sense to me though.)
C) Keep the current F32-based Color and all the effect virtuals
that use it around, introducing parallel Half-based HalfColors
and entry points for those. The key cool idea here is to have
the default entry points for F32/Color and Half/HalfColor call
each other, so that any given effect can implement one, the other,
or both and always be compatble with however it's called.
This is mostly about incremental rollout, but I suspect we'll
have areas that stick to F32 forever. (Think the IEEE 754 32-bit
float specific bit hacks we used for approx_log/approx_exp.)
D) (not done yet) allow implicit Half->F32 conversion, but not
the other way around of course. This makes it easier to lean
on the body of F32 routines we already have, and again mostly
helps enable incremental rollout.
Change-Id: I8bb38efbe476ff89dd2591411e115c2ab3757854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341800
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is a new base class for GrSurfaceDrawContext. It allows any
alpha-type but is restricted to non-blending fills of irects using FPs,
clears,and discards.
Bug: skia:11019
Change-Id: I229ce5f452e66796e2fa5c0e7a6ddccbf23bef5c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341680
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The GLSL backdoor for tessellation shaders is only experimental at
this point. In order to enable the tessellation path renderer, we need
to turn off HW tessellation and let it use the indirect draw modes.
Bug: skia:10419
Change-Id: Ic979978a331c7ad016907cf42b2562b35caab8ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344336
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: chromium:1156804
Change-Id: I95af34415f334bfc3c7ccef48a2110aa499b427c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344557
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
All fragment processors now use explicit returns; sk_OutColor no longer
exists at all.
Change-Id: Ic5cf566a916c1d616edcc56ba84b6780776f8515
Bug: skia:10549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344300
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10419
Change-Id: I63f000e7b3c5623c1e40c3ce6950c8f5565bc11c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343477
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This mostly undoes commit 2777cd37 "Have SkWuffsCodec use two Wuffs
decoders", also known as
https://skia-review.googlesource.com/c/skia/+/250937
Using two Wuffs decoders is more complicated, but was necessary to allow
getFrameCount() to update when part-way through an incremental decode
while more data comes in. Updating that count matches an existing
Chromium test (TestResumePartialDecodeAfterClearFrameBufferCache).
On further discussion (crbug.com/1132828), that behavior is unnecessary
(and that Chromium test will be removed), so having Skia use two Wuffs
decoders is unnecessary complexity.
Removing the now-redundant WhichDecoder enum (an enum with only one
possible value), a mechanical change, will be a follow-up commit.
Bug: chromium:1132828
Bug: skia:8235
Change-Id: I892e4790c7265597be1eee10e6364e76b6402cb5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342836
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
