Just verify that the max error on any of the intervals after splitting
is less than the specified tolerance. Looks like we're running into
some FP precision issues with large coord values, so I'm limiting the
max multiplier to 2^15 in this test.
Bug: skia:10419
Change-Id: I39e76dca5f77389833ba3c94930e6b67c2b1bc97
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340116
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
For reference, the relative costs are roughly,
- our hash with CRC32c instructions 1x
- Murmur3 11x
- our hash with CRC32c fallback 23x
So this should be a ~2x speedup for those
older iOS devices not using an arm64e slice.
Bug: skia:11001
Cq-Include-Trybots: luci.skia.skia.primary:Test-iOS-Clang-iPadPro-GPU-PowerVRGT7800-arm64-Debug-All,Test-iOS-Clang-iPhone11-GPU-AppleA13-arm64-Debug-All,Test-iOS-Clang-iPhone6-GPU-PowerVRGX6450-arm64-Debug-All,Test-iOS-Clang-iPhone7-GPU-PowerVRGT7600-arm64-Debug-All,Test-iOS-Clang-iPhone8-GPU-AppleA11-arm64-Debug-All
Change-Id: Ib56195ddc0c522380d263d56e767331d9f635728
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340178
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The proper approach for creating multi-dimensional array types is
complicated, so I added a function in SymbolTable which does it the
right way (addArrayDimensions). I found all the places in SkSL which
created arrays from base types and size arrays, and refactored them to
call addArrayDimensions instead of doing it manually.
I believe that this approach fixes a bunch of minor issues with multi-
dimensional array types; some are visible in the current codegen output,
and others are latent bugs. e.g. in some instances, a Variable's type()
was silently holding flipped array dimensions, but this never led to
a visible bug because we ended up using the VarDeclaration's baseType()
plus sizes() everywhere that the type was used. (In particular, this
caused debugging headaches in http://review.skia.org/340137 where I'd
use a Variable's type and suddenly its array dimensions would be wrong.)
Change-Id: Idd6a86aa5d1dce8918d02a53bcc2f7d7886e3ac5
Bug: skia:11016, skia:10924
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339860
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 35f1b20840.
Reason for revert: ANGLE failures
Original change's description:
> Fix bug in GrClearOp combining and remove some asserts
>
> The buffer combining code path was combining the ops but never
> telling the external system that the second op could be removed.
>
> Bug: skia:10963
> Change-Id: If015d877ffbbb75964aae9ca92ea760d7041372a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339203
> Commit-Queue: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
TBR=robertphillips@google.com,michaelludwig@google.com
Change-Id: Ie188190e7ecf2c39ec067296af20a9794636a226
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10963
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340177
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reintroduces the flipped-array-dimensions bug in skia:10924. It
will be fixed in followup CLs.
Change-Id: I24ec687209b397f5fd0cf44194d0e21fe30dc32c
Bug: skia:10924
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339797
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The buffer combining code path was combining the ops but never
telling the external system that the second op could be removed.
Bug: skia:10963
Change-Id: If015d877ffbbb75964aae9ca92ea760d7041372a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339203
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
The Metal return type from main() diverges from the SkSL source, so we
patch it in the Metal code generator. This CL improves the patching
process in multiple ways:
- A `return` statement from a fragment processor main() is rewritten to:
return *_out;
- A `return` statement from a vertex processor main() is rewritten to:
return (_out->sk_Position.y = -_out->sk_Position.y, *_out);
- We avoid emitting a duplicate `return *_out;` statement if we can
determine that main() already ends in a return statement. This is
harmless either way so it doesn't necessarily catch everything. (e.g.
it doesn't detect an if/else which returns at the end of both blocks.)
Also added a unit test which returns from the middle of a vertex shader,
since we didn't test this anywhere and we need to verify that
sk_Position.y will be negated. (This didn't work properly before.)
Change-Id: I14cf18375894fc712fa6c6466df3888ebaeba7c8
Bug: skia:10903
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, this would generate invalid code such as `[[user(locn-1)]]`.
We now generate a more-useful error at SkSL compilation time.
Change-Id: Ifbe335ec6d4abcbdfe89b892ba51063c94d22b11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339397
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
GLSL only supports arrays of samplers in very limited ways; they aren't
supported at all by SkSL. We now detect arrays of opaque objects and
reject the code.
We have several paths through the IR generator that create and process
array types; the unit test covers global and local variables, and array
on the type versus array on the variable.
Change-Id: I5b45e88e31cf4005723c3bf35561622d65321f7b
Bug: skia:11008
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339317
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: If22eabb68b9293f5bc1d275535135d9760fe1ae5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339578
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
fp16 is a more precise name, given that there are things like bfloat16,
and this may free up the word "half" for the same sort of more nebulous
format as we use it in SkSL.
Change-Id: I55c39f3670f2c300b9306c92a86c4ec7a2e7b5d7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339577
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Rather than taking the x and y values separately (ax, ay, bx, by),
simply take two vec<N*2>'s (a, b), where the x's are in a.lo and the
y's in a.hi.
Bug: skia:10419
Change-Id: I21d659c79247ccb625351c20b93c550d0afffe79
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339458
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
DWrite makes up a bunch of axes that don't exist, so non-variable axes
are ignored. However, the actual index of the non-variable axes was not
being updated. Update it.
Bug: skia:10928
Change-Id: Iedce5d0dd58f447821b403c38f033d214b457e9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338602
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
If font data is used to create an SkTypeface and the font data describes
a variable font and no variation parameters are specified, the
SkTypeface created should have the default values for all axes. This is
particularaly interesting with DirectWrite since it makes this not
straight forward.
Bug: skia:10929
Cq-Include-Trybots: luci.skia.skia.primary:Test-Win10-Clang-NUC5i7RYH-CPU-AVX2-x86_64-Debug-All-NativeFonts
Change-Id: I4620deebf52142bbdffa1a283343b503cd1e6981
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338604
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
I think this is vestigial from some time in the past where RTC was
public.
Also just expose the methods that add ops rather than have so many
friends + testingOnly versions.
Change-Id: I60d9fdff23b2d67039a7b37815da7ff9e73d8999
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339158
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Just filling in a gap in our tests. The output is a little strange as it
exposes a missed opportunity to constant-fold array accesses, but it
seems fine otherwise.
Change-Id: I6df13e0f9a49455015ceb47d7802bb5e1bbdaa1a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339217
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>
Constructors such as `float[2](0, 0)` add a type to the symbol table;
this type needs to be copied into the new symbol table if the
constructor is cloned by the inliner.
Change-Id: Ifa8d2dec87103c6223ce493e2201a904c14c2137
Bug: oss-fuzz:28050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339168
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:10989
Change-Id: I512ccbd3d95178756ae436f7c67ed53d46105ae7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339165
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
SPIR-V previously didn't know what to think when it encountered a Type
with a typeKind of kEnum, and would abort. These are now treated as
32-bit signed integers.
Metal previously emitted the SkSL enum typename, which is meaningless to
Metal since we do not emit the enum itself anywhere. Metal now emits
"int" for an enum-typed variable.
(GLSL already correctly emits "int" for enum types.)
Change-Id: I05975a2a399f9c4a22c00c90be0dccacd99d793b
Bug: skia:11003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338856
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This CL addresses the root cause of the fuzzer issue, by checking for
LayoutIsSupported before getting the MemoryLayout of a type. However,
this array ought to be detected as an error everywhere, as samplers are
opaque types; at present, this code compiles without error in GLSL and
Metal. This is an issue for followup CLs.
GLSL's actual support for arrays of samplers is interesting and probably
too nuanced for us to try to emulate:
https://www.khronos.org/opengl/wiki/Data_Type_(GLSL)#Opaque_arrays
"Under GLSL version 3.30, Sampler arrays (the only opaque type 3.30
provides) can be declared, but they can only be accessed by compile-time
integral Constant Expressions. So you cannot loop over an array of
samplers, no matter what the array initializer, offset and comparison
expressions are.
Under GLSL 4.00 and above, array indices leading to an opaque value can
be accessed by non-compile-time constants, but these index values must
be dynamically uniform. The value of those indices must be the same
value, in the same execution order, regardless of any non-uniform
parameter values, for all shader invocations in the invocation group."
Change-Id: Ib382f5c3b563f996b3c8f1eb6b021b6d31fa9ce7
Bug: oss-fuzz:28107
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339159
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, GLSL and Metal code generators would emit a struct wherever
the type was first used in the code, regardless of where it was
originally defined or what scope the type needs to live in. This CL adds
a ProgramElement for struct definitions, so that structs will now appear
at the top-level as they were originally defined. In the case of Metal,
some special handling is also needed to handle the Globals struct
properly.
Not yet fully supported:
- No special handling for structs declared inside functions yet
- No support for structs in separate scopes with overlapping names
The severity of the remaining issues depends mostly on whether we want
to support structs inside functions in Runtime Effects.
Change-Id: Ia95d4529506cb3fa6da63f5cb548199a93e1c0c5
Bug: skia:10922, skia:10923, skia:10925, skia:10926
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338600
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This test verifies that dead-stripping works on both built-in and user
functions, if their function call is optimized away.
Change-Id: I3125a34640c69de43c383343cd00d97e5a32ac60
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Enums are an SkSL-only concept--when we output code, we emit plain
IntLiterals--so the fix is simply to ignore the Enum program element
when we encounter it. This is what GLSLCodeGen does as well.
Also added a unit test to confirm that enums work normally, and that
enums are subject to optimization and static-comparison checks just as
ints would be.
Change-Id: Ic4f8da7a27983add9eb41b936d46f6638d22bd4b
Bug: skia:11003
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338800
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
There were a surprisingly small number of dedicated SPIR-V tests.
SkSLSPIRVBadOffset was the only test that didn't already exist in the
golden outputs, although it actually contained two tests.
The SPIRVTest.cpp file has been converted to SPIRVTestbed.cpp, which can
be used for local debugging of SPIR-V issues via dm (like GLSLTestbed
and MetalTestbed).
Change-Id: I978d8a7cf5735af7f537113d2b9411ce42cfcf88
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338756
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
^^ is not an operator in Metal. != can be used for the same purpose.
Change-Id: If75b000076ebe0aa81d0ab354a8ae33e6ed52101
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339156
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
- easy: ceil, floor, sqrt
- index is our first arm64 instruction to need a temporary,
but other than that is pretty simple, just N - iota as usual.
With Op::index now supported, `viewer --slide GM_runtime_shader`
frame time drops from ~1ms to ~0.24ms.
I accidentally swapped in a float-subtract for an int-subtract and
everything worked fine. o_O
Change-Id: I44c51506a6a9014b398d6943bb0e3712e4e52445
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338661
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Guard flag has been added to clients
Change-Id: Ib61a48781f5dbd52279c8f4257ba3e22fb2704e0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338596
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Uniforms in practice are always pointers or 32-bit ints or floats, so
these are essentially dead code. The change to SkVMBlitter.cpp is the
only interesting change, and I think it makes more sense now than
before. The program will need float coverage in the end, so might as
well feed it one directly.
Change-Id: I7f1e77731cf10ccc35595012a6df4f9e54a0dad8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338631
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Now that I've been reminded that half-float compute is real and no
longer just a dream, Q14 kind of pales in comparison, and just gets in
my way when working on SkVM.
As usual I've left in assembler support and unit tests for those
instructions. The instructions are all pretty easy to keep working and
tested and don't get in the way, unlike the real "let's do Q14" stuff.
None of this Q14 code was hooked up to anything but unit tests, so no
capability lost here, and no diffs. As always, it'll be easy to restore
should we ever want to by looking at this CL.
Change-Id: Ia42a96652b381267a7c3ec563b5978efcfc717a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338630
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
I'm not using any of these, so nice to move them aside.
Change-Id: Id43c1606c2f9e6bba0d8f6bd7d2f8f5e02d5b762
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338629
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Bug: skia:10997
Change-Id: Ic6da0cbe6dd68009d888bc3174de913852559de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338598
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: Ifc1f0921d983ee09d7bc2632aeca41689f1bf0c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338603
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 allowed unary minus on numbers and vectors (of any type).
Now, we allow them on numbers and vectors of numbers.
Also updated the Boolean arithmetic error test to cover scalars as well
as vectors.
Change-Id: Ie74d1f3bfc1e9353e04c6f8e468fa20e0cbba16f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338396
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
GLSL does not allow most binary operations on bvec types; we can now
detect these and properly flag them as errors.
Note that `determine_binary_type` was also refactored. It originally
started with an enormous omni-switch over every possible Token type,
used to set various bools describing the type of binary expression at
hand. Instead of one big switch, this has been refactored into several
small switches in standalone functions that simply switch on the op and
immediately return true or false. Conceptually this seems like more
work (checking the op multiple times), but these tiny switches actually
boil down to little branchless shift-and-mask functions, so in practice
they should be quite efficient compared to the original omni-switch.
Change-Id: I81b473d98c65da1edd136f35fc8f656261f8930d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338346
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is very unlikely to occur in real-world code, as it's somewhat
nonsense to use the comma operator in this way. However, it's better to
fail cleanly than to assert.
Change-Id: I76481cd8a993cb1a798ee16956400a512efd4c15
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This new version always delivers the same results,
and I think can be simplified like this without
spoiling any of the bulk speed.
Change-Id: I20e42e58418e658278bb5db9472c39722b33160a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338339
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Fiddled with the logic a bit so that when we're in unit test mode, the
output still includes all of the SPIR-V (as well as the validation error
message), so that tracking them down is easier.
Bug: skia:10694
Change-Id: I15e7777af3d268a5952765dbe5d63612cad0ac07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338320
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Fix code generation for Metal and Vulkan with geometric
intrinsics that have scalar versions in GLSL/SkSL, but no
native support in MSL/SPIR-V.
Change-Id: Id4538a00172e0d233ad9d5ed8d33db6436b83208
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338276
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, we assumed that if a vector in `is_constant` was not made of
floats, it must be made of integers. This ignores that boolean vectors
also exist. The original code would abort when `getIVecComponent` was
called on a bool vector.
There is another bug here--arithmetic operators on bool types should be
disallowed entirely. That will be addressed in later CLs.
Change-Id: I78781d839abde9376917fd92f2fe6311a1a58b02
Bug: oss-fuzz:27808
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/338055
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The stroker will need to know the cusps in order to draw circles around
them.
Bug: skia:10419
Change-Id: I05b7e9f4a5ed06bd36450e73edfaf36c4b3f5a6c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337945
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This adds assembler support for a bunch of ARM instructions and uses
them to implement a bunch of SkVM ops. No diffs.
movs() seems strictly more useful than fmovs(), so I've replaced it.
Change-Id: Ied38a44461653598269421b0b56bef4eb19bb1e9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335918
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit e81fb87bb4.
Reason for revert: checking results with less-aggressive inliner
Original change's description:
> Revert "Simplify _blend_set_color_saturation, removing an instruction."
>
> This reverts commit ed289e777c.
>
> Reason for revert: causing strange artifacts, only on Adreno
>
> Original change's description:
> > Simplify _blend_set_color_saturation, removing an instruction.
> >
> > This tightens up our intrinsics slightly; after inlining, it eliminates
> > one scratch variable. (We no longer need to copy `sda` into `hueColor`
> > as hueColor is now unchanged.)
> >
> > Change-Id: Iece5ba2fe11cde54481704a1787114a2c2a66d9b
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336599
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> TBR=brianosman@google.com,johnstiles@google.com
>
> Change-Id: Ica506467b0a4e03d0cbe482034acfa2d9f8d2c16
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337560
> Reviewed-by: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,johnstiles@google.com
Change-Id: Ia93263f3269c057e7eaa69ca2b05e783d18c0199
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337944
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This point is located at fPoints[-1]. We might as well provide it
since it's free, and the stroke iterators for indirect tessellation
will be able to use it.
Bug: skia:10419
Change-Id: If0161a18a9a5a0f3b118a99d7c090d79d424f9db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337637
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Previously, we'd gauge suitability for inlining by counting the nodes in
a function; past a certain limit, the function was considered "too big."
Now, we also incorporate the number of times that function is called.
So if a function is called three times, and its size is 20 nodes, it
would be considered to have an inlining cost of 60 (3 * 20) instead of
20.
This should tamp down the aggressive nature of the inliner in cases like
gaussian convolution or complicated blends, and will hopefully satisfy
Pinpoint.
No change visible in Nanobench (which doesn't test any of these sorts of
patterns, but certainly inlines things): http://screen/AwD5hkgkEfjVx4g
Change-Id: Ie5e32898245ac854adb9ddd52d87001df6a67125
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337676
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
- float4(float2(1, 2), 3, 4) --> float4(1, 2, 3, 4)
- half3(z, half2(fn(x), y*2)) --> half3(z, fn(x), y*2)
Single-argument constructors will be ignored by this optimization; these
might be casts or splats.
This had an unexpected side benefit of simplifying some Metal output,
as we need to output fewer Metal matrix construction helper functions
when matrices use more simple scalars for construction.
Change-Id: I0a161db060c107e35247901619291bf83801cb11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337400
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, every output was labeled ".asm.frag" regardless of the
actual type.
Change-Id: Icf3a56bb04d88cc0443f12c2dfb99c66ee00dff0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337717
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: I1be21b428939d17bbf3a9347a64db56c7cd69eb4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337638
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 ed289e777c.
Reason for revert: causing strange artifacts, only on Adreno
Original change's description:
> Simplify _blend_set_color_saturation, removing an instruction.
>
> This tightens up our intrinsics slightly; after inlining, it eliminates
> one scratch variable. (We no longer need to copy `sda` into `hueColor`
> as hueColor is now unchanged.)
>
> Change-Id: Iece5ba2fe11cde54481704a1787114a2c2a66d9b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336599
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,johnstiles@google.com
Change-Id: Ica506467b0a4e03d0cbe482034acfa2d9f8d2c16
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337560
Reviewed-by: John Stiles <johnstiles@google.com>
These are not actually supported operators in GLSL, Metal or SPIR-V and
we don't emulate them. Their absence was causing SPIR-V to fail the
Operators.sksl test.
Change-Id: Ia6933788392aea48836b7be19e32b9969805f254
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337185
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, the code which calculated Constructor constant values
assumed that a constant-value PrefixExpression would always have an
operand of Constructor. It turns out that another valid case is multiple
PrefixExpressions nested within each other (representing repeated
negation). Updated the code to work regardless of the type of the prefix
operand.
Change-Id: Ic9bf54725ae59330ac817bc4ec7a64def384ab54
Bug: oss-fuzz:27663
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337177
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
pack(x,y,bits) as an alias for x|(y<<bits) only existed originally to
implement it with the SLI arm64 instruction, but I've since realized
that was misguided.
I had thought the assumption on pack ("(x & (y << bits)) == 0"), i.e.
"no overlap between x and the shifted y", was enough to make using SLI
legal, but it's actually not strong enough a requirement.
The SLI docs say "...inserts the result into the corresponding vector
element in the destination SIMD&FP register such that the new zero bits
created by the shift are not inserted but retain their existing value."
The key thing not mentioned there happens with zero bits _not_ created
by the shift, the ones already present at the top of y. They're of
course inserted, overwriting any previous values.
This means SLI (and so pack()) become strictly order dependent in a way
I had never intended. This will work as you'd think,
skvm::I32 px = splat(0);
px = pack(px, r, 0);
px = pack(px, a, 24);
but this version swapping the two calls to pack() will overwrite alpha,
skvm::I32 px = splat(0);
px = pack(px, a, 24);
px = pack(px, r, 0);
I find that error-prone, so I've removed Op::pack and replaced it
with a simple expansion to x|(y<<bits). That of course works in either
order.
This new test can't JIT at head, but if we implement the other missing
instructions (soon, dependent CL) it would start failing when JIT'd.
The interpreter and x86 were both fine, since they're both doing what's
now the only approach to pack(), the simple x|(y<<bits).
I've left assembler support for SLI in case we want to try it again.
Change-Id: Iaf879309d3e1d0a458a688f3a62556e55ab05e23
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337197
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
We now have SPIR-V golden outputs for `blend` and `shared` tests.
This exposes a handful of SPIR-V limitations for us to address.
Change-Id: Ie5278889b8a61432403d06231b17765885bee0ac
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337182
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>
afaict the perf surprises associated with:
https://skia-review.googlesource.com/c/skia/+/334417 (Remove SkBaseDevice::flush)
were bc Ganesh resolves MSAA buffers for SkCanvas::flush but doesn't do so for GrDirectContext::flush.
Where possible this CL switches SkCanvas::flush to SkSurface::flush (which will also resolve MSAA buffers) so that when https://skia-review.googlesource.com/c/skia/+/334417 relands there should not be any performance surprises.
Change-Id: I705ad6219f0f625a88cf3f9e8b2418a3182d298c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335866
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
This allows swizzle removal to apply in more cases; in particular, we
can now optimize away extra swizzles caused by zero/one swizzle-
components quite effectively.
The "trivial expression" code was lifted from the inliner. Some subtle
changes in trivial-expression determination affect the inliner's results
in boring, non-meaningful ways. In particular, multi-argument
constructors containing all-constant values are now considered trivial,
whereas previously only single-argument constructors made the trivial-
ness cut. This allows the inliner to propagate some values that it
wouldn't have before.
Change-Id: I9a009b6803d9ac9595d65538252ba81c2b7166a7
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336156
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
In follow on CLs we need to know what the load op is when we try to use
discardable msaa attachments. For vulkan the load op affects how we
copy the resolve attachment into the msaa attachment, which changes the
render pass we use (adds extra subpass). We need to be able to make a
compatible render pass to compile programs.
Bug: skia:10979
Change-Id: I40c23a18b251af6a2ad3b78a1f6382bdba0b90c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336598
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Previously, any builtin functions would be optimized as a side-effect of
optimizing programs that used them. Now that shared elements aren't
being optimized in that way, we explicitly optimize any shared modules
when they are first created. We don't remove dead elements, but we
we do substitute settings, simplify, and inline.
Bug: skia:10905
Change-Id: I701b5e9f52fb880ef3e6f4c67694d08602f47e95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336440
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The important part is in GrOpFlushState.h were previously
we were taking a mutable pointer to the view, which should
at least be a const pointer and was making us do funky things
in some of the calling code. But I decided to go all the way
and do a const ref instead which is The Way It Should Be (tm).
Change-Id: I399d102e8b5e0a5059168cc450ae66f12ad47e13
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336451
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
The fix submitted at http://review.skia.org/335868 did not support
casts. The fuzzer discovered this shortcoming right away.
Change-Id: I2f5166528cee41367348564d4e664476fd5704ff
Bug: oss-fuzz:27650
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336656
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This tightens up our intrinsics slightly; after inlining, it eliminates
one scratch variable. (We no longer need to copy `sda` into `hueColor`
as hueColor is now unchanged.)
Change-Id: Iece5ba2fe11cde54481704a1787114a2c2a66d9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336599
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>
For non-degenerate gradients, the positions are pinned to [0, 1] and
forced to be monotonically increasing in the constructor of
SkGradientShaderBase. This isn't ever called for degenerate gradients,
so the average_gradient_color helper function needs to make the same
fixes or it may calculate an invalid color (e.g. negative alpha because
the original positions are not sorted).
Bug: chromium:1149216
Change-Id: I95c6e66ebb57722370ef2f5dbba3d8d66727e48b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336437
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
When values from the same argument are used consecutively by the outer
swizzle, they can be merged in the inner swizzle. Merging isn't always
possible, of course, but it will be used where it can be:
`half4(1, colRGB).yzwx` --> `half4(colRGB.xyz, 1)`
`half4(1, colRGB).yxzw` --> `half4(colRGB.x, 1, colRGB.yz)`
Change-Id: Id164b046bc15022ded331c06d722f1ae3605a3bd
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335872
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>
Cubic tangents become unstable if we chop too close to 0 or 1. This
adds an epsilon to simply not chop them if it's too close.
Bug: skia:10419
Change-Id: I99e98c5d433e2cb59c767ee564e015d7ec4f7765
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336280
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This will reorder constructors with swizzles applied, such as
`half4(1, 2, 3, 4).xxyz` --> `half4(1, 1, 2, 3)`
`half4(1, colRGB).yzwx` --> `half4(colRGB.x, colRGB.y, colRGB.z, 1)`
Note that, depending on the swizzle components, some elements of the
constructor may be duplicated and others may be eliminated. The
optimizer makes sure to leave the swizzle alone if it would duplicate
anything non-trivial, or if it would eliminate anything with a side
effect.
Change-Id: I470fda217ae8cf5828406b89a5696ca6aebf608d
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335860
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This was found at https://oss-fuzz.com/testcase-detail/5155684475469824
but the associated oss-fuzz issue ID appears to be misdirected (it's
showing oss-fuzz:24498, an unrelated issue).
PrefixExpressions can return true for `isCompileTimeConstant` but did
not implement `compareConstant`; the fuzzer discovered this. Because
compile-time constants can only be compared if they are of the same
kind, this means that `compareConstant` is actually comparing a pair of
expressions that are both negated. These negations will just cancel
out, so `compareConstant` on a pair of PrefixExpressions can just call
`compareConstant` on the inner operand of each expression.
Change-Id: I7793e25314e6c8a74278b73299d310794baf71f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335870
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 ByteCodeGenerator is needed for SkSL-via-skvm, but almost no one
needs the ByteCode interpreter.
Bug: b/172773885
Change-Id: Ia7b6768dbc00c6c78b971ba50f0b702536bbd5b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336016
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The fuzzer managed to create a test case which temporarily evaluates to
expression `half2(half(0.2)) + 2` as it is optimized. This requires a
bunch of temporary nonsense math as the IR Generator is attempting to
simplify as it goes; various attempts to remove terms from the fuzzer
test-case would cause it to stop reproducing the error.
Constructor::getVecComponent assumed that any constructor with a single
scalar argument would always implement `getConstantFloat` and
`getConstantInt`; however, constructors themselves did not actually
implement these methods. This meant that nesting a scalar constructor
inside a non-scalar constructor would abort when it tried to deduce the
value inside the inner constructor.
This has been fixed by implementing `getConstantFloat` and
`getConstantInt` for Constructors. These methods will assert if the
constructor has more than one argument or is a non-scalar type. This
should allow any number of nested constructors, e.g.
`half4(half(half(half(1))))` should recursively evaluate properly,
should we somehow generate this as an intermediate expression.
Change-Id: Iaee4284cba03974443cd7b5dccfd7909c1a5f3a6
Bug: oss-fuzz:27614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335868
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 optimizer can now turn the expression `half4(1).xyz` into
`half3(1)`, or `half4(1).w` into `1`. This is actually a somewhat common
case when inlining chains of fragment processors, as inputs are often
overridden to `half4(1)` or `half4(0)`. This optimization also applies
to more complex cases, e.g.:
`half2(anyFunc(sqrt(2))).yxyx` --> `half4(anyFunc(sqrt(2)))`
Since the interior of the constructor is always evaluated once in either
case, it does not actually matter what the constructor contains.
Change-Id: I8d5f358502eaa8e35d4968e74fbd6b0ce2ab6365
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335818
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ibe3c3d27a112df8838bc86d6c2482277fdae62af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335821
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This was slightly dangerous, and only used for tests. Fix the tests to
just put a Context on the stack.
Change-Id: Ifc3d600c3498e1dedeee8350f3284163f3195bec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335819
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
... to ensure that, if the topological sort fails, all the nodes still appear in the result.
Change-Id: Ic4acaab2662c872662246b7c1f5471e17c5ba98e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335639
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was slightly complicated by the fact that this syntax indicates an
array with a known size:
float[] x = float[](1, 2, 3, 4);
Of course, the size is 4; it's just never explicitly stated in the
code. (The SkSL parser never actually deduces the size, but it doesn't
apparently have a need to; we don't do much in the way of optimization
for arrays.) However, this prevents us from simply failing whenever we
parse "[]" in non-builtin code; we need to keep scanning and see if the
variable is initialized. We already check this in the
ArrayConstructors.sksl test file.
Change-Id: I5b86958e81bd9bf5edf28a617cecf95c1875583e
Bug: skia:10957
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335240
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, the Type's fOffset was set to -1 during parsing, so any
errors related to the Type would be reported on line 1.
Change-Id: I9834f733bc763c5946b3ff81d8aef4807cdc13d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335584
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 a followup to http://review.skia.org/335196. This detects opaque
types (samplers and textures) at parsing or IR generation time and
reports an error regardless of backend. This check occurs before Metal
or SPIR-V would have a chance to detect the error, so it changes their
output to a slightly more focused error message. The Metal/SPIR-V fix in
the prior CL is still a nice broad catch-all for preventing spurious
ABORTs, though.
Change-Id: I4cce92a8767d72b5d3d7277a8afde8ce5ce86db2
Bug: skia:10956
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335217
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>
Previously, MemoryLayout would ABORT if it encountered any types that
we can't layout in memory (e.g. opaque types like samplers). Instead of
an abort, this case is now detected cleanly and an error is reported
identifying the offending type.
This should unwedge the fuzzer, which appears to be very
enthusiatically generating interface blocks with nonsense types inside.
(Note that code generators which don't actually try to compute a memory
layout--that is, GLSL--will still accept these types. This should still
be caught and reported as an error, since it's still illegal in GLSL,
but that's for a future CL.)
Change-Id: I88a9649bcd8c75dadc8cca679f3c5e94570742bc
Bug: skia:10956, oss-fuzz:27525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335196
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 971b19d0c6.
Reason for revert: See if this CL was causing the Perf-Win10-Clang-ShuttleA-GPU-GTX660-x86_64-Release-All issue
Original change's description:
> Remove SkBaseDevice::flush
>
> Another small step in removing SkCanvas::flush
>
> Change-Id: I6f17edcd1996e1009dad7cc96a97be3b0c4664f4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334417
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>
TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com,adlai@google.com
Change-Id: I0b9116f8ce4eed3a0d49ccf1cc55d8d89675617e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335216
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is a reland of fc4fdc5b25
Original change's description:
> SkAndroidCodec: Support decoding all frames
>
> Bug: b/160984428
> Bug: b/163595585
>
> Add support to SkAndroidCodec for decoding all frames with an
> fSampleSize, so that an entire animation can be decoded to a smaller
> size.
>
> dm/:
> - Test scaled + animated decodes
>
> SkAndroidCodec:
> - Make AndroidOptions inherit from SkCodec::Options. This allows
> SkAndroidCodec to use fFrameIndex. (It also combines the two versions
> of fSubset, which is now const for both.)
> - Respect fFrameIndex, and call SkCodec::handleFrameIndex to decode
> the required frame.
> - Disallow decoding with kRespect + fFrameIndex > 0 if there is a
> non-default orientation. As currently written (except without
> disabling this combination), SkPixmapPriv::Orient would draw the new
> portion of the frame on top of uninitialized pixels, instead of the
> prior frame. This could be fixed by
> - If SkAndroidCodec needs to decode the required frame, it could do so
> without applying the orientation, then decode fFrameIndex, and then
> apply the orientation.
> - If the client provided the required frame, SkAndroidCodec would need
> to un-apply the orientation to get the proper starting state, then
> decode and apply.
> I think it is simpler to force the client to handle the orientation
> externally.
>
> SkCodec:
> - Allow SkAndroidCodec to call its private method handleFrameIndex. This
> method handles decoding a required frame, if necessary. When called by
> SkAndroidCodec, it now uses the SkAndroidCodec to check for/decode the
> required frame, so that it will scale properly.
> - Call rewindIfNeeded inside handleFrameIndex. handleFrameIndex calls a
> virtual method which may set some state (e.g. in SkJpegCodec). Without
> this change, that state would be reset by rewindIfNeeded.
> - Simplify handling a kRestoreBGColor frame. Whether provided or not,
> take the same path to calling zero_rect.
> - Updates to zero_rect:
> - Intersect after scaling, which will also check for empty.
> - Round out instead of in - this ensures we don't under-erase
> - Use kFill_ScaleToFit, which better matches the intent.
>
> Change-Id: Ibe1951980a0dca8f5b7b1f20192432d395681683
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333225
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Derek Sollenberger <djsollen@google.com>
Bug: b/160984428
Bug: b/163595585
Change-Id: I7c1e79e0f92c75b4840eef65c8fc2b8497189e81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334842
Auto-Submit: Leon Scroggins <scroggo@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Metal-specific tests are pretty thin on the ground here, and some of
the remaining tests no longer added value as they were already covered
pretty well by existing tests in Shared. The majority of remaining tests
were specific to Metal's lack of flexible matrix casting (and SkSL's
ability to paper over this with helper functions).
Change-Id: I7b3c445268b95320e7f46ec88d793c315d43ee8a
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334956
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Another small step in removing SkCanvas::flush
Change-Id: I6f17edcd1996e1009dad7cc96a97be3b0c4664f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334417
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This prevents OOMing when given a pathological input, but is large
enough that almost all inputs should continue to compile as-is.
Change-Id: If5c46711b886ee08495bfd09af537e9dc7ea5649
Bug: skia:10945, oss-fuzz:27442
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334838
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
In practice, the inline threshold does a good job of limiting the
blast radius here.
Change-Id: I495184116e733262ea9d84fec30885ea047ca116
Bug: skia:10945
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334597
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This fixes a fuzzer crash in Metal.
Private types aren't meant to be used directly; we can't generate a
valid MemoryLayout for them. We will now detect them during IR
generation and report an error. (Note that unreferenced structs
currently don't have any IR representation at all, so structs have to be
used somewhere in the code to trigger the error.)
Bug: oss-fuzz:27288
Change-Id: I432f0a69fbb54cd33ff5b90a9f3d4757a9370117
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334830
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit fc4fdc5b25.
Reason for revert: Google3 and ASAN failures
Change-Id: I890cd76109c0375391637f879550837d01e650f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334840
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: b/160984428
Bug: b/163595585
Add support to SkAndroidCodec for decoding all frames with an
fSampleSize, so that an entire animation can be decoded to a smaller
size.
dm/:
- Test scaled + animated decodes
SkAndroidCodec:
- Make AndroidOptions inherit from SkCodec::Options. This allows
SkAndroidCodec to use fFrameIndex. (It also combines the two versions
of fSubset, which is now const for both.)
- Respect fFrameIndex, and call SkCodec::handleFrameIndex to decode
the required frame.
- Disallow decoding with kRespect + fFrameIndex > 0 if there is a
non-default orientation. As currently written (except without
disabling this combination), SkPixmapPriv::Orient would draw the new
portion of the frame on top of uninitialized pixels, instead of the
prior frame. This could be fixed by
- If SkAndroidCodec needs to decode the required frame, it could do so
without applying the orientation, then decode fFrameIndex, and then
apply the orientation.
- If the client provided the required frame, SkAndroidCodec would need
to un-apply the orientation to get the proper starting state, then
decode and apply.
I think it is simpler to force the client to handle the orientation
externally.
SkCodec:
- Allow SkAndroidCodec to call its private method handleFrameIndex. This
method handles decoding a required frame, if necessary. When called by
SkAndroidCodec, it now uses the SkAndroidCodec to check for/decode the
required frame, so that it will scale properly.
- Call rewindIfNeeded inside handleFrameIndex. handleFrameIndex calls a
virtual method which may set some state (e.g. in SkJpegCodec). Without
this change, that state would be reset by rewindIfNeeded.
- Simplify handling a kRestoreBGColor frame. Whether provided or not,
take the same path to calling zero_rect.
- Updates to zero_rect:
- Intersect after scaling, which will also check for empty.
- Round out instead of in - this ensures we don't under-erase
- Use kFill_ScaleToFit, which better matches the intent.
Change-Id: Ibe1951980a0dca8f5b7b1f20192432d395681683
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333225
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
A crop is invalid if it is empty/unsorted or does not intersect with
the image. Android (the only known client of the cropping API) will not
pass such a rectangle to the API, but this prevents other clients from
passing an invalid rectangle.
Change-Id: I09e007ecd378c358a1c604aff518035090f1e0a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333224
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
... replacing them w/ calls to GrDirectContextPriv::flushSurface.
Since recording contexts can also possess surface- and renderTarget-
Contexts it is misleading for them to have a flush method.
Change-Id: I10f4fad12d4d5efdd999ba212fda9ce5cdd83130
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334068
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:10914
SkAnimCodecPlayer:
- Properly handle orientation, whether the image is still or not
- Mark const methods as const
- Fix seek() so that if you seek to the duration of frame 0, it will
show frame 1
- Fix the SkImageInfo so if the first frame is opaque, but following
frames are not, those frames can still be decoded
resources:
- Rename "webp-animated.webp" to "stoplight.webp", which better
describes the animation
- Update test files accordingly
- Add "stoplight_h.webp", which is the same animation with an EXIF
that converts it to a horizontal stoplight
AnimCodecPlayer test:
- Test the new image files
- Verify SkAnimCodecPlayer::dimensions behaves as expected
- Remove extra debugging line
- Provide better error messages
AnimCodecPlayerExifGM:
- Add a new GM that shows all frames of the new animation with an EXIF
orientation
- Add a new GM that shows all frames of an animation with an opaque
first frame followed by frames with alpha
Change-Id: I43cf91c16d52aa1901eef8e13e1e644eea6058b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332753
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Sometimes it's helpful to think about subsampling separately from
how the channels are spread across planes.
Bug: skia:10632
Change-Id: Ib03f71195f9706ef6def418b1f2125c29e0cf738
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334102
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
These often became direct references to a varying, which can't be
mutated in GLSL. Fix this by copying them to a local variable in the
FP, and pointing all references at that.
Bug: skia:10918
Change-Id: I705e3c966b1d44fc4dfc3d4b40eb8e46110febdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334043
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
At present, we do not report any error; the values wrap silently.
Change-Id: I8c435cfdd81f6c2e5fd87e9c39c708138bf4ec82
Bug: skia:10932
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333676
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Also adjust the OpsTask terminology to the broader RenderTask term.
Change-Id: I8549e74a3e2f6b2caf765103f31243b776823c16
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332724
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
I've wanted to write a few of these lately, and (just like the shader
tests later in the file), the rigid structure was making things
difficult. It also forced some of these to be overly clever (like
commenting out main). Switch to just providing all of the SkSL for each
test case.
Change-Id: I8aff34224fa6efbdc4d67628744a9cf499eb53b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333796
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This addresses a sanitizer issue discovered in
https://oss-fuzz.com/testcase-detail/4908118777266176 (it has not been
assigned an oss-fuzz bug number yet; coming soon)
This puts an upper bound on struct nesting, again to prevent memory-
layout and other recursive type-handling code from overflowing the
stack. Coincidentally, while researching GLSL behavior around this bug,
I learned that WebGL has a similar limitation but caps nested structs to
4 deep. (I could not find any documented GLSL upper bound.)
Note that both the GLSL and Metal outputs for StructMaxDepth are badly
malformed. (Structs cannot be embedded within another struct in GLSL;
structs SA7 and below are never declared in GLSL; the array list for SA7
is backwards in GLSL; Metal is missing structs SA1 through SA8; Metal
puts the array list on the type instead of the variable name.)
These issues will be addressed in separate CLs.
Change-Id: I0f1059b6faa400cd0647dd7010ec839f73779a36
Bug: skia:10922, skia:10923, skia:10925, skia:10926
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333316
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This addresses a sanitizer issue discovered in
https://oss-fuzz.com/testcase-detail/4908118777266176 (it has not been
assigned an oss-fuzz bug number yet; coming soon)
We need to set some sort of limit here to avoid stack overflow. Eight
array dimensions seems like more than enough for any sort of code that
we might realistically need, but the limit is definitely flexible if we
wanted to increase it. (The fuzzer needed to generate a several-
hundred-dimensional array before encountering a crash.)
Change-Id: I3630ab40e47cc58a2280ba200b485e1958371fdc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333160
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This addresses a sanitizer issue discovered in
https://oss-fuzz.com/testcase-detail/4908118777266176 (it has not been
assigned an oss-fuzz bug number yet; coming soon)
A followup CL will limit array dimensionality to 8. This is an arbitrary
choice which is hopefully larger than any reasonable program will need.
Change-Id: I4cf05f40ec92c1c3444c71c45f759bb30d7da3c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333135
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>
There's no reason to use std::aligned_storage when it's simpler to use
an array and alignas(). This way you don't have to remember whether the
template arguments are size-then-align or align-then-size, you don't
have to remember to use the _t variant or typename ... ::type, and
there's no risk to forgetting the alignment parameter entirely.
It doesn't look like this was deprecated, but I still think this paper
makes good arguments for why we shouldn't use it:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1413r1.pdf
Bug: skia:10921
Change-Id: Ia64a2e43c4cba9b4d64138a7474e353a8eaf01a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333258
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
A GLSL function like:
void fn(int x[1][2][3]) {...}
Will emit SkSL with the array dimensions in reverse order:
void fn(int x[3][2][1]) {...}
Trying to invoke the function will fail because it expects a reverse-
dimensioned array.
Change-Id: I24431aabd2f6111b5493f63f0a85f9c78514d522
Bug: skia:10924
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333317
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Remove done and release distinction. Chrome is not using either as
it tracks texture access using other synchronization mechanisms
(semaphores, flush finish procs). Now there is just fulfill and release
where release is called when the texture can be deleted. Also,
release proc can be null.
Simplify texture idle mechanism as the "flushed" state was only used to
implement the old idea of a release proc. The "finished" idle state is
still used to implement the new release proc. Though, it could also be
removed if GrTexture were to be removed for textures returned by fulfill.
Not directly tied to this bug, but a new YUVA factory will be required
and it's good to clean things up first to avoid adding another
instance of the current complexity.
Bug: skia:10632
Change-Id: I4fe3c0af3f5a591506b1b3c736fd3284a38465a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331836
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This resolves the fuzzer error, as the program will fail compilation
before reaching the SPIR-V translation stage at all.
Change-Id: Ia73af497b1f57314a29878f2d2a29dc80186e630
Bug: oss-fuzz:27300
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333130
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Do these tests run ok once we init the SkSTArray storage first?
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-GCC-GCE-CPU-AVX2-x86-Debug-All-Docker,Test-Debian10-GCC-GCE-CPU-AVX2-x86-Release-All-Docker,Test-Debian10-GCC-GCE-CPU-AVX2-x86_64-Debug-All-Docker,Test-Debian10-GCC-GCE-CPU-AVX2-x86_64-Release-All-Docker
Change-Id: I5009b06ba16edf72692a58a9f2469fd38cefb2a6
Bug: skia:10891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333147
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
`in` vars shouldn't support initializer expressions at all. The fuzzer
noticed that dead-stripping interacts poorly with `in` var initializer
expressions, which makes sense because it's an unsupported and untested
path. In a followup CL, lines 1 and 3 will both become errors.
Change-Id: Ibb64ca319a046b040eea976acb6798a1402451de
Bug: oss-fuzz:27300
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333128
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 4cb5c5e172, and fixes
the Chromium issue by declaring sign(x) in sksl_public.sksl.
Original description:
This makes numerous internal and GLSL types or intrinsics hidden from
public (runtime effect) SkSL. In particular:
- Only core numeric types are visible to all program types. GLSL types
involving images, textures, and sampling are restricted to internal use.
- sk_Caps is no longer visible to runtime effects.
- The set of intrinsics available to runtime effects is now a separate,
curated list in sksl_public.sksl. It exactly matches the GLSL ES 1.00
spec order.
- The blend intrinsics are no longer visible, which also fixes a bug.
These are nice, but we're not going to offer them yet - they involve
enums, which creates complications.
Bug: skia:10680
Bug: skia:10709
Bug: skia:10913
Cq-Include-Trybots: luci.chromium.try:linux-chromeos-rel,linux-rel
Change-Id: I42deeeccd725a9fe18314d091ce253404e3572e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332750
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We already allowed narrowing type conversions, so GLSL type aliases
could be used almost exclusively. This fixes the last spot that a
client would be forced to use half4 rather than vec4.
Bug: skia:10679
Change-Id: Ie9cfc161650b238678861b9b126ce586c229162d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332743
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This is a reland of 87bc83eaa6
Original change's description:
> Add memsets to the GrBlockAllocator unit tests.
>
> These will verify that our blocks are actually set up properly--if not,
> we'll stomp over a sentinel word and/or trip an ASAN poisoned byte,
> causing the test to fail.
>
> Change-Id: I2dcb5b913d00c408f70c71f2660c6ec6017b452c
> Bug: skia:10885
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332260
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10885
Change-Id: Ie1ffcfeffec14a016b50e6464fc23878b4dd2ccb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332716
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This implements constant folding optimizations on int vectors
(== != + - * /) that were previously only supported on float vectors.
Bug: skia:10908
Change-Id: Ibf61ab43eb7ae2ce8e99cce21cc55777359817e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332424
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit bea0dc67f4.
No-Try: true
Change-Id: I7b000de199dc23bd3719a4ee835b2a8d3c93fefd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332747
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We have the ability to copy or move an SkSTArray into an SkTArray, or
vice versa. However, this was not reflected anywhere in our test
coverage, and not all copy/move paths were exercised on every platform.
This led to a blind spot in our tests, meaning that Skia could
successfully build locally but fail on CQ bots due to SkSTArray usage in
various platform-specific code.
Change-Id: Ie77ccc0a4be5c2a47f1eaa026c910c59d38a3e60
Bug: skia:10891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332602
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>
This makes numerous internal and GLSL types or intrinsics hidden from
public (runtime effect) SkSL. In particular:
- Only core numeric types are visible to all program types. GLSL types
involving images, textures, and sampling are restricted to internal use.
- sk_Caps is no longer visible to runtime effects.
- The set of intrinsics available to runtime effects is now a separate,
curated list in sksl_public.sksl. It exactly matches the GLSL ES 1.00
spec order.
- The blend intrinsics are no longer visible, which also fixes a bug.
These are nice, but we're not going to offer them yet - they involve
enums, which creates complications.
Bug: skia:10680
Bug: skia:10709
Bug: skia:10913
Change-Id: I8fa1c94f6e4899f38530bb9cff33d147f6983ab3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332597
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Makes it more sensible.
A bunch of call sites we're already written to expect this. Update
some others.
Change-Id: I77c28045ebf01e6aa9d92d2ebc37287604ec10c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332544
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This test is meant to demonstrate that constant folding for int and
float vectors is not on equal footing. Float vectors currently generate
better-optimized output.
Change-Id: Ib4822c7b594e9bc4eb4fb9cfe6ab46f7f76268d6
Bug: skia:10908
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332423
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL improves on the previous fix for oss-fuzz:26789 by actually
propagating the negation from the PrefixExpression inside the
constructor, which unblocks further optimizations.
Interestingly, this fix also exposes a further missing optimization--we
optimize away comparisons of constant-vectors for floats, but fail to
do the same for ints.
Change-Id: I9d4cb92b10452a74db96ff264322cdc8a8f2a41f
Bug: oss-fuzz:26830, oss-fuzz:26789, skia:10908
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332263
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 87bc83eaa6.
Reason for revert: breaking some platforms on tree
Original change's description:
> Add memsets to the GrBlockAllocator unit tests.
>
> These will verify that our blocks are actually set up properly--if not,
> we'll stomp over a sentinel word and/or trip an ASAN poisoned byte,
> causing the test to fail.
>
> Change-Id: I2dcb5b913d00c408f70c71f2660c6ec6017b452c
> Bug: skia:10885
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332260
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,michaelludwig@google.com,johnstiles@google.com
Change-Id: I93abff1ead8430a233c18e9ff30c76192a090ab9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10885
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332545
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These will verify that our blocks are actually set up properly--if not,
we'll stomp over a sentinel word and/or trip an ASAN poisoned byte,
causing the test to fail.
Change-Id: I2dcb5b913d00c408f70c71f2660c6ec6017b452c
Bug: skia:10885
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332260
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The previous behavior leaked Skia-internal concepts into public SkSL.
Users coming from GLSL will expect that bindable/sampleable objects are
uniform (just like texture2D). This keeps the old support around (and
tested), but updates all of our examples to use 'uniform'.
Bug: skia:10679
Change-Id: I0c98162f5e21dad7014d9778ceb26143d2f6030e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332376
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This CL solves the fuzzer crash. Constant propagation of the negative
sign into the vector will be investigated in a followup CL.
This CL also adds a few cleanups into IRGenerator::constantFold.
Change-Id: If73a4fe2a5777265e7d43cc4f482653a38cb59af
Bug: oss-fuzz:26830, oss-fuzz:26789
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332261
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Ibc1a8d3ebbf62cc55d013f7d9146f6b155d11da2
Bug: oss-fuzz:26830, oss-fuzz:26789
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332377
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 ties the caps to the compiler instance, paving the way for
pre-optimizing the shared code. Most of the time, the compiler is
created and owned the GPU instance, so this is fine. For runtime
effects, we now use the shared (device-agnostic) compiler instance
for the first compile, even on GPU. It's configured with caps that
apply no workarounds. We pass the user's SkSL to the backend as
cleanly as possible, and then apply any workarounds once it's part
of the full program.
Bug: skia:10905
Bug: skia:10868
Change-Id: Ifcf8d7ebda5d43ad8e180f06700a261811da83de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331493
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:9000
This constructor is already declared; provide a definition.
Add a test.
Change-Id: Ic54ccffe0b1adf7c8c411e19a3fc9d825a5c9619
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331799
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Previously, temp variables created by sample() calls were named after
the offset of the sample() call within the code. This was
straightforward but would fail if the sample() call were duplicated via
inlining of helper functions.
FP sample() temp variables are now named using a counter, starting from
zero and counting upwards.
Change-Id: I16f9a3426117677c0df13d15772320def99cc0d6
Bug: skia:10858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331415
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, when a prototype was parsed, this added a function
declaration to the symbol table, but the prototype itself was not
re-emitted during code generation. This meant that the final code might
not be valid, since the absence of prototypes meant that the code might
attempt to invoke a function before its declaration. Now, prototypes are
stored in the ProgramElement list and re-emitted during code generation
for GLSL/Metal/CPP. (SPIR-V doesn't name its functions at all.)
Change-Id: I76446c796000eb0b56f964d82457122182c28b87
Bug: skia:10872
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331136
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
When eliminating a CFG node, we now flag its exit nodes; if our
optimization pass reaches one of those flagged nodes, we stop the
current optimization process in its tracks and initiate a rescan.
We do NOT recursively mark the exits of the exit nodes, so this fix is
reliant on the CFG being ordered in a non-chaotic fashion, but in
practice this seems to be sufficient for the CFGs we generate today.
Change-Id: I892805361c5f4297e02146f37a759dfda83f5488
Bug: oss-fuzz:26942
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331597
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Ief57d9c102b3c7658738920cdf54ccd4d21c5c5e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331656
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: I19a9564ac4d52b709b8fdd757b99222372c626f4
Bug: oss-fuzz:26942
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331598
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 method is only valid in the range 2^(+/-30) due to fp32 overflow.
Adds a comment to the function and updates its test.
TBR=bsalomon@google.com
Change-Id: Ifa2fc0ed4a7f9123f0bebaa02c666c61e06e62a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331481
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This method finds the locations a cubic needs to be chopped at before
it can be passed to the stroke tessellation shader. It's an integral
part of CPU stroke preparation and therefore extremely perf sensitive.
Bug: skia:10419
Change-Id: Ib23c2583b8cfc78814ce52425f7af2c8b2f8b420
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330314
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Adds grvx, Ganesh's addendum to skvx. Here we introduce familiar names
and operations from GPU languages, as well as functions that are
approximate and/or have LSB differences from platform to platform.
The initial implementation has: fast_fma, fast_acos, and
fast_angle_between_vectors. When a function is approximate, its error
range is well documented and tested.
Also establishes GrWangsFormula as the first user of grvx.
Bug: skia:10419
Change-Id: Id0682599cf9c0303eff386095afc3ef9f3a7fa1b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330119
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
- Prototypes for never-declared functions
- Prototype before use
- Prototype after use
- A variety of inputs and outputs on the prototyped functions.
- Calling declared-but-undefined functions
Currently, the prototypes are not actually emitted in the generated GLSL
or Metal output at all. This CL is demonstrates our baseline before
proper prototype support is added.
Change-Id: I6112e0a89ab9bbecefccaca9fba985bb8011fff1
Bug: skia:10872
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331376
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We finally have a reference and derivation of Wang's formula, thanks to
tdenniston@. And it turns out that the formula we had been using for
cubics wasn't quite right. It was overly conservative for certain types
of curves.
This CL fixes the incorrect cubic formulas and adds a citation to the
"Pyramid Algorithms" book. We should now be getting by with fewer linear
segments.
Bug: skia:10419
Change-Id: Ib850c7b4d17b8d9f9abed800cc7cb5f074df6e17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331156
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
This improves the test output for Metal. Previously, the Metal output
was just an error message, since 1D textures were unsupported. Now we
have a valid golden output for the 2D case in Metal. (1D is still
unsupported and is likely to remain unsupported; Skia currently has no
use case for 1D textures.)
Change-Id: I91977712030f08e371cc6bfb2afa578940ca00b7
Bug: skia:10797
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330940
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The triangulating path renderer can generate multiple triangulations of
the same path at different levels of precision. As previously
implemented the more precise triangulations would steal the unique
key from prior triangulations. This new callback will allow us to
replicate this behavior in the thread-safe cache.
Bug: 1108408
Change-Id: I8b445ca1e503b2fd78727a23d9376a2cf77f291c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330562
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Motivated by investigating
https://skia-review.googlesource.com/c/skia/+/330696
This CL does not fix anything, but is meant to better document
the current behavior.
Change-Id: I62b8cbfb39e05404f0f5303f024e1f56fc32b7e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330937
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
This error was caused by an unbalanced symbol table push. This could
occur when an interface block encountered an error while parsing its
var-decls.
Change-Id: I910a980ac92fac7c0786c48b8dc3003ee3e75e5b
Bug: oss-fuzz:26700
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330896
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Also added unit tests for each of SkTArray's various constructors, and
added `SkTArray::value_type` which allows calling code to refer to the
array's value-type. These unit tests exposed some preexisting strict-
aliasing issues in SkSTArray when compiled on GCC 6+ with optimizations
enabled, which are being investigated separately at skia:10891.
Change-Id: Ia0fb18830cfbbdcb1545fe7f7ac51d8e768a3f94
Bug: skia:10891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330279
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
