This reverts commit 97c476ecb7.
Reason for revert: tree breakage - Test-Debian10-Clang-GCE-GPU-SwiftShader-x86_64-Debug-All-SwiftShader
Original change's description:
> Disable the MSAA atlas mode for CCPR
>
> We have a long term path rendering plan that uses dynamic MSAA instead.
> This CL is a test to see if we can drop support for CCPR now.
>
> Change-Id: I1bff3ca3143a6b453b65a7932a1805c195922805
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354036
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,csmartdalton@google.com
Change-Id: I9f472807f743b8ddfee92800ee3b62609f2d4717
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354673
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This was disabled a long time ago, when this was (incorrectly) forbidden
by the IR generator, most likely.
Change-Id: I14585d249104b263c152fa59cbeba0c4e9a2e074
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354666
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We have a long term path rendering plan that uses dynamic MSAA instead.
This CL is a test to see if we can drop support for CCPR now.
Change-Id: I1bff3ca3143a6b453b65a7932a1805c195922805
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354036
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
The inliner discovered that when a binary expression is inlined, its
type is not cloned into the destination's SymbolTable. This meant that
when the inlined-from function was later dead-stripped, the type pointer
would become dangling. Did a quick pass over inlineExpression and
inlineStatement and ensured that types are always copied.
Also found that `copy_if_needed` was making a copy of eligible types
each time one was encountered, instead of making one copy and reusing
it. This is fixed as well.
Change-Id: Iee3259ab038dfb04034bf0110af1909ccffec3de
Bug: oss-fuzz:29444
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/354219
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Unlike TriangulateSimpleInnerPolygons, this new method *always*
succeeds. It works by introducing a concept of "breadcrumb" triangles.
The breadcrumb triangles serve as a glue that erases T-junctions
between the outer curves and inner polygon triangulation.
This CL also introduces a test to verify the breadcrumb triangles
actually guarantee to eliminate T-junctions. It works by cancelling
out shared edges in the combined breadcrumb triangles and inner
triangulation, and then verifying that the resulting edge list is
bit-for-bit identical to the outer edges from the original polygon(s).
Bug: skia:10419
Change-Id: Iea8ee2ca3701697246309dceb90e63279aabdfe4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353536
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Unsized arrays are now allowed in exactly one place: On the declaration
of an interface block. This satisfies the one existing use-case, which
is the gl_in (sk_in) declaration for geometry shaders. There is no other
useful scenario, and most of our backends don't support them anyway.
Several spots were using less strict checks when attaching sizes to
arrays, allowing for zero or negative-sized arrays, so those are all
fixed now.
The existing tests that initialize arrays are still a problem, because
Metal doesn't support that (neither does GLES2). Also, ArrayConstructors
has gone from generating an error in the Vulkan backend, to invalid
SPIR-V.
Bug: skia:11013
Bug: skia:11127
Change-Id: Ib08dfe9aeec96bf605661665d6f166419d27e8bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353817
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Ia3ac338bef376aa1649569b9ebd3f7feb23ffd52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353936
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This better matches GLSL's type coercion behavior.
Change-Id: I73fcfd8a9e57fd4cdb1692074d73ebd8fb788ac2
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353712
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I47d02ca63ce64d9cfb3de0888d84b2b8a822f2b5
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353710
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Literals are still flexible; we still allow `1` to coerce to float.
However, we no longer accept code like `int x = sqrt(2);` or
`int x = 0; float y = x;` without an explicit cast.
Change-Id: Ieb294a4877447e2336252f876e8bc489d1e4a59a
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353417
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Icd8982d604881effee31cc1392e2717cb112d06d
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353629
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:10680
Change-Id: Ic77f7355866363ef476a93d8da180cf53207fa6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353707
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also renamed Discard to IllegalStatements, and added testing of while
and do loops.
Change-Id: Ibacf69131267a0436808e2e022ad126704af16ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353706
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>
It's not sound to pass undefined (skvm::NA) values into select(),
but this is working today because the F32a argument is 'fixing' it.
The first time through this snippet updating fReturn value,
int i = 0;
for (skvm::Val& slot : currentFunction().fReturnValue) {
slot = select(returnsHere, f32(val[i]), f32(slot)).id;
i++;
}
the call to f32(slot) creates an F32{builder, NA}. We pass that to
select() and that argument's F32a(F32) constructor, resulting in
F32a{builder, NA, 0.0f}. Then when we need that as an F32, we resolve
it as splat(0.0f) because the F32a's id field is NA.
In short, best to remove F32a. :)
Added some SkASSERTs that would have caught this.
Change-Id: I67324cf20ad39ca555e69b9c407f379d14046043
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353838
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I'm looking to phase out I32a/F32a, and rewriting this expression to
avoid select() makes it easier, making these types unused except in
SkVM.{h,cpp}.
There's no particular reason beyond making that refactor easier to do
this: SkVM can convert select(cond, splat(1), splat(0)) into cond & 1
itself, and once I'm done with removing I32a/F32a, if we prefer select
we should be able to rewrite this back as
dst[i] = skvm::select(i32(src[i]), 1, 0);
Change-Id: I562a112e54fdc2578802db02f6754c64a12798cc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353837
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The "disallowed" tests are largely allowed in the current code, but all
fail properly in the followup CL.
Change-Id: I8e03570165480b60db9701ac1a782e1124ded56b
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353617
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, `writeVectorConstructor` did not consider boolean types at
all when converting scalars to a different type. Now, this code reuses
the existing logic from `castScalarTo(Float|SignedInt|UnsignedInt)`
which supports Booleans. Added `castScalarToBoolean` to cover going in
the opposite direction.
Change-Id: I5479ab181b9b721db7fbff0bdc01718ce8f9f9b9
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353625
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This revealed a gap in our SPIR-V scalar constructor support;
typecasting a number to bool would lead to an ABORT.
Change-Id: Idac6d7ba34adfd214ed3cad8139e22d7170456f0
Bug: skia:11172
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353628
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Idab539a4b39fe5ceab54948c99c0dcd6d19fd345
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352743
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Auto-Submit: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I350a6768ac124362b0d3e0f17e7a026265acf804
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353627
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These need to change because type coercion in SkSL is about to become
more strict in a followup CL; we are disallowing expressions that mix
ints and floats without a cast.
Change-Id: Iff5e2820806b9419afdfcbf25d4a7f96f2eeeccb
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353416
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The test diffs look scary, but the only actual change is a minor
renumbering of IDs. The actual logic is the same.
Change-Id: I5ecc26c8581a4c01834932ff0291deba7d9e4618
Bug: skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353622
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Such loops must be unrollable, so that's what we do.
Bug: skia:11094
Change-Id: I1b34917b6f2d015ae7867415d0120a5df0ffd618
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353619
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:11094
Change-Id: I68a08e79d29579901b74daca3c22f5112fbb3c8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353356
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
These need to change because type coercion in SkSL is about to become
more strict in a followup CL; we are disallowing expressions that mix
ints and floats without a cast.
Change-Id: I0f6c3cba53fb67078f447345338262c153236c51
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353102
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Note that GLSL accepts these sorts of constructors natively, but Metal
and SPIR-V do not. In the generated IR we actually add a cast for
subexpressions where the type does not match. These casts can be seen in
the final output for both GLSL (where they are no-ops) and Metal/SPIR-V
(where they are essential).
This change exposed some missing SPIR-V functionality (vector casts do
not support bool types). This can be fixed up in a followup CL; these
casts were previously disallowed by SkSL entirely, so there won't be any
of them in existing code.
Change-Id: I54ae922e91b38bed032537496428747a081dc774
Bug: skia:11164, skia:11171
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353576
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
... now that bitmap
- converts to image with just bitmap.asImage()
- canvas *always* converts bitmaps to images before they are drawn
Bug: skia:10037
Change-Id: I24292f62e0fd072b3b810d974d0fe5c6d9b9a68d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353582
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
GLSL allows mixed types inside a vector constructor, but SkSL currently
doesn't handle it well; some cases don't compile, and others generate
bad code. This will be fixed in a followup CL.
Change-Id: Ia98b498f320b8fa91595404730f6cdc836615140
Bug: skia:11164
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353577
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I9859081a14b110731f943e09fdd94dc10e0c9dfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353580
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds basic operator overloading (everything but array indexing) and
related tests to the SkSL DSL.
Change-Id: Ic9fdc9a02a5496e2706d18fb435d838b4ee53ad7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353103
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
I started unpacking the mechanics of type coercion, and realized that
the second half of the function was looking up the Symbol for a Type
based on its name (Types are already Symbols), converting that Symbol
back into a Type (we started with a Type anyway), wrapping that Type
in a TypeReference, then calling that TypeReference (which always
calls convertConstructor).
This CL cuts out the middle steps and simply calls convertConstructor
directly. A test was added to confirm that an earlier error encountered
on the CQ is no longer occurring.
Change-Id: I76aae455a301afe4e67ef989d9dfe11f47ed36ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353105
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This will help us narrow down the failing test from
http://review.skia.org/353097
Change-Id: Ida3bbecb09c278913cfb302ba7e7766f1a2cbcb0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353418
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:10632
Change-Id: If4dd7779b0856f6d0b441381bf7f2f51527cdb9d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352497
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This will allow errors to be reported outside of the IRGenerator more
easily (without passing around an ErrorReporter object).
Change-Id: I4bcb59fcd526599fa593fcb3b1de0a5ae64ab901
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352737
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This is the first test that used uniform data, so fix up how uniforms
work in the generic SkSL-to-SkVM function.
Bug: skia:11094
Bug: skia:11096
Change-Id: Ie391c1a6b8b68f0f4f014d7e767d7b5101341fab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352739
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I96b547de4fe4b73096fb26d0ef21a4e7555ca06a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352238
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11127
Change-Id: I3a7fda8bb62f9cf9b6c83441703f537e75461d07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352509
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, these were in SkSL::Context directly. This change doesn't
remove them from the context entirely, but it gives them a dedicated
subclass and firewalls them off from the rest of the context.
Change-Id: I0c344bf7436a11b8494a5fe7542d0a4ef1ece964
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352502
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I20a40af290fd907e1b1a57a2fd09659d62db3af3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352508
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This CL also adds tests for vector*scalar and scalar*vector folding.
We currently do not constant-fold these, but support will be added in a
followup CL.
Change-Id: I68d7374ae15ab2f4d805a095803b645c92fb03d9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352237
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This optimization doesn't perceptibly improve the generated code; it
just replaces a binary expression with an equivalent unary one.
Change-Id: Ib6cd2732a22c26978665c57ee00d7b5e5d0a0aee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352123
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This feature works fully on the skvm backend, and the byte code backend
is going to be removed soon.
Bug: skia:10852
Change-Id: I4711fcea7c85232c0b740f3b3c012f47310a768e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352258
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:10680
Change-Id: I8697bdc157d250f3c390c7f49074318aa8c7bdab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351918
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This makes almost all existing code read more clearly.
Change-Id: I314331d9aa2ecb4664557efc7972e1a820afc250
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352085
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change SkBase64::decode interface completely and better document
how to use. Unfortunately there are users of ::decode (and they are
leaking) so will need to keep the old interface until users can be
updated.
Change-Id: I214b771136d78fef758c5d0d9ec302f956f6e4f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351201
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: I0bbda6a41391fc2a11dc812be5e9c0c0d14c4d75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351921
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The CFG/definition map are no longer valid after replacing an expression
entirely. Swizzle-of-swizzle optimization was another case where the
optimizer would replace an expression wholesale, but failed to set the
needs-rescan flag.
Change-Id: Ida0363d738cd1d3ac2a48c824aa04065a7ca16b7
Bug: oss-fuzz:29085
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351776
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
The code at this point doesn't do anything useful, but establishes some
of the basic types and patterns.
Change-Id: I580a9e75ffa3162879893450fb7d1f0905a10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350697
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I75f907ca673ee67f5d623b032128b97833070a0b
Bug: skia:10931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351504
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ifd7883a4b327aae9fc0a984f08755d6d6f57f72e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351018
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: If6b23d03b02028b51f96e97080cbd7d34cc33b8f
Bug: skia:10931
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351503
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This will flatten out expressions such as `!false` or `!true`. We
already had a similar fix-up at IR generation time which handled simple
cases, but this will catch more complicated ones like `!sk_Caps.xxxxx`
(since caps bits are only flattened out at constant propagation time).
Change-Id: I04282809d9a784266a64dbcafd097f3b0662806c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351497
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 not actually necessary now that constantPropagate can fully
flatten out unary negation into its constant operands. The compilation
results don't change at all.
Change-Id: I7ab55bd3720413609d799dd866e1703973cb2626
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/351202
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>
Currently this doesn't actually handle going over the memory
budget, but it does carve out space to do that later. This algorithm
can't handle everything yet but I want to get it landed for
more iteration as long as it's disabled.
Bug: skia:10877
Change-Id: I37942172345e8cfd6fc2c591a3788a10652377da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345168
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
This fixes SPIR-V code generation when encountering nested constructors
like `float3 v4 = float3(float2(1), 1.0);` as featured in our unit test
VectorConstructors.sksl.
Change-Id: I3a0c4b466b3cb17ba50bd264f899e59c55c768ed
Bug: skia:11141
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350032
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously ExternalValues were flexible, and could be used as raw values
(with the ability to chain access via dot notation), or they could be
callable. The only non-test use-case has been for functions (in
particles) for a long time. With the push towards SkVM, limiting
ourselves to this interface simplifies things: external functions are
basically custom intrinsics (and with the SkVM backend, they'll just get
access to the builder, and be able to do any math, as well as
loads/stores, etc).
By narrowing the feature set, we can rename everything to reflect that,
and it's overall clearer (the SkSL types now mirror FunctionReference
and FunctionCall directly, particularly in how they're handled by the
CFG and inliner).
Change-Id: Ib5dd34158ff85aae6c297408a92ace5485a08190
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350704
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@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>
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>
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>
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>
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>
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>
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>
This actually exposed a latent bug: we don't support bool(1.23) or
bool(1) casts, but these are valid in GLSL:
https://www.khronos.org/opengl/wiki/Data_Type_(GLSL)#Conversion_constructors
"to bool: A value equal to 0 or 0.0 becomes false; anything else is
true."
Change-Id: Ia929a09914ffc96f081d0402d7bb05b5428f8db6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/349977
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@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>
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>
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>
Change-Id: I4798263318c504834f23900dbb3f5d167fd17e65
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348887
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@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>
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 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>
FrExp testing was moved to the intrinsic tests as part of
http://review.skia.org/341977, and the shared versions were removed from
sksl_tests.gni at that time.
Change-Id: Ife7f3622034d97a77b60d5a98c01f71630c161d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348183
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@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>
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>
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>
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>
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>
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>
I found myself updating these for another change and realized they
are very limited tests that are redundant with more thorough tests.
Change-Id: I935da4d1da7ff3825a4042556845a8eb659b6ba8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346856
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Auto-Submit: Brian Salomon <bsalomon@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>
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>
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>
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>
Includes a handful of test cases to exercise the system
Change-Id: I98e73a8bca063f475d2ddb51778e395697392ddb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346637
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We have a handful of tests that demonstrate this behavior indirectly,
but lacked a focused test.
Change-Id: I895cc4e3bebf30721ed649244e42bf170cc6ec06
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346497
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 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>
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>
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>
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>
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>
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>
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>
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 use GrColorTypes wherever possible.
As a consequence, some cases now go through GrSurfaceContext rather
than SkSurface (because it uses GrColorType and allows kUnpremul).
Both changes make it easier to add testing for GL_LUMINANCE8_ALPHA8,
which is required to update Flutter on iOS/GL to use new YUVA
texture image api.
Bug: skia:11019
Bug: skia:10632
Change-Id: I0e8bef4af5aeb7fe58cc11ec7923ebb5029ce18b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344560
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345132
Reviewed-by: Brian Salomon <bsalomon@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 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>
