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>
This reverts commit 67d2d73d06.
Reason for revert: Triggers errors in some shaders.
Original change's description:
> Fix incorrect 'unreachable code' error in SkSL
>
> This trades one error for another (a potential for incorrect use of
> unassigned variables). False-positives for unassigned variables are
> straightforward to workaround (and produce code that still looks
> reasonable). Working around unreachable code errors is tricky, and
> likely to produce non-idiomatic code. This change also makes the data
> flow analysis of all loop constructs more similar - for loops were
> behaving very differently from while loops.
>
> Note that this effectively a revert of:
> https://skia-review.googlesource.com/c/skia/+/18121/
>
> Change-Id: Ib85d90b22cac8addfb106459c0a5f5616a89c3eb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344957
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I84b93cc7f9309446dcc0e949e90908df31a1ff9c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345119
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
When we determine that a function only contains a single return
statement, there is no need to create a temporary variable and store the
result expression into a variable. Instead, we can directly replace the
function-call expression with the return-statement's expression.
This dramatically simplifies the final optimized output from chains of
very simple inlined functions, which is a very common pattern for trees
of Skia fragment processors.
Change-Id: I6789064a321daf43db2e1cef4915f25ed74d6131
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344665
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
By itself, this is uninteresting and even perhaps slightly
counterproductive (as it separates vardecl from its initializer,
increasing LOC). However, this enables a followup CL
(http://review.skia.org/344665) which allows single-return functions to
be inlined without the creation of a temporary variable at all. This
applies to the majority of fragment processors in a typical Ganesh
hierarchy. This change will greatly reduce the number of inliner-created
temporary copies when compiling a typical tree of FPs.
Change-Id: I03423a13cf35050637dabace4a32973a08a4ed0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344764
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Adds a new kDirectionLight_ShadowFlag which indicates that lightPosition
is a vector pointing towards the light, and lightRadius is the blur
radius when the occluder is at z == 1. For other values of z it will
scale linearly. The direction is specified in device space.
Bug: skia:10781
Change-Id: I14d530f006e5541ed6e22bfbeb29c7441c21fb8e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321792
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This trades one error for another (a potential for incorrect use of
unassigned variables). False-positives for unassigned variables are
straightforward to workaround (and produce code that still looks
reasonable). Working around unreachable code errors is tricky, and
likely to produce non-idiomatic code. This change also makes the data
flow analysis of all loop constructs more similar - for loops were
behaving very differently from while loops.
Note that this effectively a revert of:
https://skia-review.googlesource.com/c/skia/+/18121/
Change-Id: Ib85d90b22cac8addfb106459c0a5f5616a89c3eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344957
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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: I941d6272ca460f5b0355d11a90ac9edbc3233a2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344560
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Allowing this in runtime effects lets people break our contracts around
SkShaders on the color side of the paint altering coverage, etc.
Bug: skia:11085
Change-Id: I1ec8e71581c8d50f681cb0ca6ca8416375b3f43f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344760
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The additional scopes were harmless, but didn't really add any value.
Originally they were used to tightly scope inlined variables, but we now
mangle inlined variable names.
Change-Id: I7b35e737598036c0b6d3d9f71cbcd4a53d609ce9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344757
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is a new base class for GrSurfaceDrawContext. It allows any
alpha-type but is restricted to non-blending fills of irects using FPs,
clears,and discards.
Bug: skia:11019
Change-Id: I229ce5f452e66796e2fa5c0e7a6ddccbf23bef5c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341680
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
All fragment processors now use explicit returns; sk_OutColor no longer
exists at all.
Change-Id: Ic5cf566a916c1d616edcc56ba84b6780776f8515
Bug: skia:10549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344300
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This mostly undoes commit 2777cd37 "Have SkWuffsCodec use two Wuffs
decoders", also known as
https://skia-review.googlesource.com/c/skia/+/250937
Using two Wuffs decoders is more complicated, but was necessary to allow
getFrameCount() to update when part-way through an incremental decode
while more data comes in. Updating that count matches an existing
Chromium test (TestResumePartialDecodeAfterClearFrameBufferCache).
On further discussion (crbug.com/1132828), that behavior is unnecessary
(and that Chromium test will be removed), so having Skia use two Wuffs
decoders is unnecessary complexity.
Removing the now-redundant WhichDecoder enum (an enum with only one
possible value), a mechanical change, will be a follow-up commit.
Bug: chromium:1132828
Bug: skia:8235
Change-Id: I892e4790c7265597be1eee10e6364e76b6402cb5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342836
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Change-Id: I41a5aea7b01efe8901498621197b9a5ff0f4fe5f
Bug: skia:10549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344656
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>
No more
friend class ::SkArenaAlloc; // for access to ctor
Change-Id: I76fa3319498a965623e6865b75d1fb507ab845a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344236
Auto-Submit: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Instead we calculate these values on the fly as needed. This is
necessary for hairlines because the tessellator will operate on them
in post-transform space, which requires different tolerances than the
setup code.
Bug: skia:10419
Change-Id: Ia8ffa8858b45949521c085ccbe5712b3842f785f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343499
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
findMSB has one special trick that Metal doesn't naturally have an
equivalent for, specifically in its treatment of negative numbers.
findMSB searches negative numbers for a zero bit, not a one bit!
We emulate this behavior in Metal using select(n, ~n, n<0).
Change-Id: I861c6b8fb3dc5427643cd8c68a39a53f1959bff3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343996
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, coercion between a signed type and an unsigned type was
treated as "no cost" because these types shared the exact same priority.
This meant that we couldn't choose the proper overload with function
calls that only differed in signed-ness, like:
void fn(int4 x);
void fn(uint4 x);
So we would always choose the int4 version since we encountered it
first. Now, we can choose the correct overload; signed types now have
a slightly elevated priority over unsigned types, allowing coercion
costs to work normally.
Also added some comments to `determineFinalTypes` while trying to see
if that needed some improvements as well, but this turned out to be
a red herring--it didn't need any functional changes.
Change-Id: I334debae9ad0e9b290109658d2fde8f6526770a2
Bug: skia:10999
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344017
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: I1a96060b2e52cddb50948a48520aab30bd097bbd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343577
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This makes it much easier to understand what sorts of types we are
creating.
This has some minor repercussions for the SPIR-V code generator, which
actually created temporary Types on the stack occasionally, but these
were simple to fix.
Change-Id: I1ca43cdef0445d2b9789a435221dce50b03d954a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343517
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit b37a693254.
Reason for revert: Breaking Flutter roll
Original change's description:
> Revert "Reland "Reland "Revert "Initial land of SkSL DSL.""""
>
> This reverts commit 6b07e0eb49.
>
> Change-Id: Ic01f31edf55b2d1a7533e0e8ed33b39b4846d937
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343106
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: I3373f186f4d0531bc8ab1e4392c512608389734f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343518
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Essentially GrGpuResources have two counts now. The original fRefCnt has
not changed and is still used for things like knowing if we can reuse
a scratch texture. The new fCommandBufferUsageCnt is used to track
when a resource is in use on a command buffer or gpu in general. We now
delay calling notifyRefCntIsZero until both of the counts are zero.
Bug: skia:11038
Change-Id: I1df62f28e4b98e8c1a5ab2fd33d4aead19788d93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343098
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: If29ee048d359d0ccd7b0ab708f54d40746b92386
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343423
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Id38a3d04fcb1904a4c666d92087b4fe14bd03a27
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343110
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 6b07e0eb49.
Change-Id: Ic01f31edf55b2d1a7533e0e8ed33b39b4846d937
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343106
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I8eae896f5a859b59a1ba0b29dc95d5aa070c12ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343102
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I4a483c455c9a12c92b717a0c2713d32ab44dcd6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343099
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Separating this out from landing SkSL DSL, as it's causing unrelated
test churn as the DSL code gets submitted and reverted.
Change-Id: I0d3ade4ca8d1b0c302ccc494f0192a4f5ae67086
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343109
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I0e289a136678fe863445f739f162a718c341977f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343104
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11072
Change-Id: Ic24e40bfea5bf1d2d14c0f681632228a5ecc7104
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342929
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
This reverts commit 52e5850065.
Reason for revert: Failing on Build-Debian9-Clang-arm-Release-Flutter_Android_Docker: https://logs.chromium.org/logs/skia/5066a8ed31374c11/+/steps/Run_build_script_in_Docker/0/stdout
Original change's description:
> Revert "Reland "Revert "Initial land of SkSL DSL."""
>
> This reverts commit 53f69f1539.
>
> Change-Id: I374b016c8a08d83c99cbab800f30b882244b87f1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342919
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: Ia04ee404478314b3ae034e0a7740ef667364b2f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343100
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The existing code didn't work properly with half types since the $mat
type encompassed both halfNxM and floatNxM. This was fixed by splitting
the half types out of $mat into a separate $matH generic.
Unit tests now compile properly for GLSL, but generate errors in SPIR-V
and generate Metal code which attempts to call a non-existent intrinsic.
Change-Id: I2fff10f0dd7f00534bf6b1d5b13354543694194e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342926
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Just the class/files. variable names and additional comments to follow.
Change-Id: Ic03d07fd5009eaf3d706c2536486a117328963fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342617
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit 53f69f1539.
Change-Id: I374b016c8a08d83c99cbab800f30b882244b87f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342919
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Initial data suggests that the non-rational Wang's formula does not
form an upper bound for the rational case, so additional work is needed
to handle them. Note this new formula is not actually due to Wang,
but is an analogue.
The version added in this CL is for cubic rationals because the current
tessellation code promotes all curve segments to cubic rationals for
ease. If we end up using this approach, it would be likely simpler and
faster to implement the degree-2 version and handle conics separately
during tessellation.
From:
J. Zheng, T. Sederberg. "Estimating Tessellation Parameter Intervals for
Rational Curves and Surfaces." ACM Transactions on Graphics 19(1). 2000.
Bug: skia:10419
Change-Id: Ie02e229c089541ece05c7502217b1ef5d4799b52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337720
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Bug: webp:490
Depending on the encoded image, SkWebpCodec may need to blend the
the output from libwebp with the prior frame. It does so using the
method blend_line, which expects the libwebp output to be unpremul.
Prior to this commit, SkWebpCodec sometimes told libwebp to premultiply,
and then passed that premultiplied data to blend_line, which
premultiplied again.
Use webpInfo's alphaType to decide whether to premultiply. Consolidate
choosing its alphaType into one block. The functional difference is that
if (blendWithPrevFrame), we no longer premul if the dst is kPremul.
Move declaration of webDst to where it's used.
Add a test.
Change-Id: Ic0cfb4d918c2ab434c6787ed5a532c4d1e67fa17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342618
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Change-Id: I7af94d89d349b67b2c070179324fcad7b62e0d1e
Bug: skia:11071
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342758
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I1d5a056e08ba6e67016e45c52518da6074a62c8f
Bug: skia:11071
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342759
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
floatBitsToUint was missing from our intrinsic list entirely, and
u?intBitsToFloat were misspelled.
These intrinsics aren't implemented in SPIR-V or Metal either, but that
will be handled in followup CLs.
Change-Id: Iaf9b9d5a2e46e25d41eef71903fad8bd1c177d4e
Bug: skia:11071
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342757
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Remove more limited tests that exercise the same code.
Reduce the number of test cases to improve run time.
Bug: skia:11019
Change-Id: I68b5b18ebf077e825a260285223b4a31243dcb4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341456
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This reverts commit a3b8ac76e5.
Reason for revert: Need to revert again, red tree.
Original change's description:
> Revert "Revert "Initial land of SkSL DSL.""
>
> This reverts commit dd213e9d46.
>
> Change-Id: I43be020dd1b07dc13862150a9d95493f8c48b3b1
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342622
> Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
No-Presubmit: true
No-Try: true
Change-Id: I8e967ef8ecb7f01dc578d38264e2600b04e9b62d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342917
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I887e700a7bf11bf2d5359c9721798f72f00e53f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit dd213e9d46.
Change-Id: I43be020dd1b07dc13862150a9d95493f8c48b3b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342622
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Adds a vertex shader that maps a variable-length triangle strip to a
stroke and its preceding join. Adds a new op that generates stroke
instances from a path, bins them by log2 triangle strip length (using
SIMD for the calculations), and renders them with indirect draws.
Bug: skia:10419
Change-Id: I6d52df02cffe97d14827c6d66136957f1859f53b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339716
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I674d758c11071582e9fbedcda5596c540bfb5f71
Bug: skia:11054
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342558
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This does not give us 100% coverage of intrinsics yet, but it is a
pretty good start.
Change-Id: I97d49324db1afd9f2975c2eeafbacdead710d4aa
Bug: skia:11054
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
We now insert helper functions which defer the assignment of out-
parameters back into their original variables to the end of the
function call. This allows us to match the semantics listed the GLSL
spec in section 6.1.1:
"All arguments are evaluated at call time, exactly once, in order, from
left to right. [...] Evaluation of an out parameter results in an
l-value that is used to copy out a value when the function returns.
Evaluation of an inout parameter results in both a value and an l-value;
the value is copied to the formal parameter at call time and the lvalue
is used to copy out a value when the function returns."
This technique also allows us to support swizzled out-parameters in
Metal, by reading the swizzle into a temp variable, calling the original
function, and then re-assigning the result back into the original
swizzle expression.
At present, we don't deduplicate these helper functions, so in theory
there could be a fair amount of redundant code generated if a function
with out parameters is called many times in a row. The cost of properly
deduplicating them is probably larger than the benefit in the 99% case.
Change-Id: Iefc922ac9e2b24ef2ff1e9dacb17a735a75ec8ea
Bug: skia:10855, skia:11052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341162
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 6e599511d4.
Reason for revert: Breaking bots: https://logs.chromium.org/logs/skia/5061fbd134144011/+/steps/dm/0/stdout
Original change's description:
> Initial land of SkSL DSL.
>
> This is not 100% complete: it lacks support for several kinds of nodes
> and supports only a bare handful of builtin functions, but it
> demonstrates the core functionality and it should be relatively
> straightforward to fill in the missing pieces.
>
> Change-Id: I3058089338e20eebc3da18ac5571801abcaab564
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331177
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Iee77e5322a0b1efb0f3718ec1f5976a4d4e7323a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342620
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is not 100% complete: it lacks support for several kinds of nodes
and supports only a bare handful of builtin functions, but it
demonstrates the core functionality and it should be relatively
straightforward to fill in the missing pieces.
Change-Id: I3058089338e20eebc3da18ac5571801abcaab564
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/331177
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Iac8096f6c225258b430858bad90199ec00b93b6c
Bug: skia:10998
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342304
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>
Also fixes some additional style mishaps in class method names.
Change-Id: I49e7ac1aa91d84fef5fbc636552f040a2cb58c78
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341466
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: If467390cd3e1d33f0e72db4b75f9889d4562af5a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341722
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Undo pictureshader change from prev version
This reverts commit fde98cbddd.
Change-Id: I52fe7d2753dee91fd08ae26f38e47ecd933d0c94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342378
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Change-Id: I2c958b7aca972b7eec07e10d6c8af95fa53e761a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/342117
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Promoting pictureshader to linear by default
Change-Id: Icd4102c3eee47558ecec845d0581ddbded981ef8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341979
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
Our optimizer ignores index expressions, but has a few simplifications
that it can perform on swizzles. (Added extra code to SwizzleByIndex
which demonstrates this.)
Change-Id: If3c85a0456d98749008d796e422944b602ee6933
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341460
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Short/ushort types are valid as-is and don't need to be coerced to int.
Change-Id: I41d6a537094e0c3f968e47926f541e0f6a3f92b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341459
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: Ib9117dbd1bcd2c3581fba02416d9eabda1dfc6dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341458
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This sort of error would be detected by most backend compilers. This
case was also detected by the bytecode generator. It's easy for us to do
a similar check during SkSL IR generation and report the error sooner.
Also, `convertIndex` had migrated a few hundred lines away from
`convertIndexExpression`, so I moved it back to live next to its parent.
Change-Id: I715d3abf42581782b55ba60df30d0296355667d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341377
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is the portion of:
https://skia-review.googlesource.com/c/skia/+/266916/ (Update two of GrResourceProvider's createTexture entry points)
I still care about.
Converting the boolean to an int and passing that down is dubious at best.
Change-Id: I830cc3bfad36526bfa7884e21c9f376585d27f0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341397
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We will need to emit a helper function to work around this case, as
GLSL supports swizzled out params, but Metal does not. In this CL, we
do not yet synthesize the helper function, but we annotate the code with
a comment indicating affected calls. (Of course, this will be replaced
with a helper function in a followup CL)
Even detecting a swizzle is actually an interesting problem, because
index expressions are sometimes actually swizzles, depending on the type
of the base expression. Also, the index or swizzle might be nested in
several other valid assignable expressions.
Change-Id: I8c74f9a7daec08eff1f32387f8b6b96851c1bd6e
Bug: skia:10855
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341057
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
ForceClose is a pain, and already removed from some
(hopefully all at some point) of our iterators
Follow-on plan: use raw iter, and output code compatible with
SkPath::Make()
Change-Id: Icda4ad2e605eb2fb9842dd35b5e0d4a0939e62eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341237
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This removes the conditional behavior based on #defines, and the private
flag definitions. It removes GMs and updates tests that tested the
feature. Follow-up CLs will go through and simplify the internals of
SkCanvas to take advantage of this support removal.
Bug: skia:10986
Change-Id: Id42c9e7d134dd06507fabf6577e7872942ef9077
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339988
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Several things are accomplished as part of this:
1. The canvas_state_lib build target is brought back so that we can
actually test the canvas state sharing across library boundaries.
2. The canvas state helper functions are updated to work with DLLs
(confirmed tests pass on Windows in cross library mode)
3. The tests now always run, and the cross-state define only changes
what version of the helper functions are used
4. Updated the dlopen code in the test to use the SkOSLibrary ports
instead of calling dlopen/dlclose directly.
5. Fix bugs in SkCanvasStateUtils that were uncovered as part of
always running these tests.
6. Switches the define away from SK_SUPPORT_LEGACY_CLIPTOLAYERFLAG
to SK_TEST_CANVAS_STATE_CROSS_LIBRARY, since these tests are not
strictly speaking testing the unclipped layer feature.
Change-Id: Id4ae41e4bc3fb8227367deac899878ecf9d7b98e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341003
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Pointers require decorating the variable with a * to read back the
value, which the code generator did not properly handle. There was a
special case to add the * but it only supported assignment into the
variable, not reading back. References require no special decoration.
This change fixes compile errors in Functions.sksl with the "bar"
function. (This test marks `x` as an inout but never actually mutates
it.) It also allows us to remove a special-case workaround for `frexp`,
an intrinsic function which uses a reference for its out-parameter.
Additionally, this CL adds a non-inlining copy of "OutParams.sksl" to
the Metal test directory, as most of our tests which use out-parameters
end up inlining all the code, which hides these sorts of bugs.
Change-Id: I31c4db04f6b512b4cd4fe65b3347b82bdbf039cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/341000
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we would emit an invalid [[buffer(-1)]] annotation on the
block, causing the Metal compilation to fail.
Change-Id: I68b2439c05db3163686e84c5dcc9a5c43870ff67
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340761
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
It's not legal to use identifiers like "int" or "sampler" to name your
variables (or enums, or structs, etc.). SkSL will now report this as an
error instead of relying on the driver to catch this.
(Note that in some contexts, it might be legal by the spec to reuse a
name that you introduced yourself, depending on the scope. In practice,
this confuses Apple GLSL, so we shouldn't support it anyway.)
This caught several existing places in our code where we used the name
"sampler." These were never exposed to the driver (they were intrinsics
that we would replace during compilation) so they were harmless before.
Change-Id: Ia6dcfca8c500d02e1eb5f9427bed8727e114dfc2
Bug: skia:11036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340758
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I233398bf34411231d44613d89aed28935fe30a28
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340796
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This will allow us to decide whether we need a stencil test when
drawing strokes.
Bug: skia:10419
Change-Id: Ie6aa0e4c4af6302c3ec28f9515cfae0e5c41a4ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340517
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Previously, our AST structures would include a "sizeCount" for arrays,
which indicated the number of AST nodes associated with array
dimensions. Since GLSL only supports a single array dimension, this
field has been replaced with "isArray," a boolean indicating whether we
have a single AST node for array size. This allowed many array-size
based looping constructs to be replaced with simpler non-looping
equivalents.
This change flushed out a few places where the parser was not actually
enforcing its promised maximum array-dimensionality.
Also found some duplicated code in variable-declaration parsing,
related to parsing array-sizes and initializer expressions. This has
been de-duplicated by using a lambda. (This change was likely why this
CL was not net-negative for LOC, but it's simpler and cleaner.)
Change-Id: I7abed732d3a296edf02c0ec9813fceb5aae4a9a0
Bug: skia:11026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340656
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This finishes up the existing SkVM ops on arm64.
I wish I had a unit test for this, but there's no diffs drawing RGBA F32.
Change-Id: I53725769fa2e7a1701f7360905205356e1ea18cb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340718
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Nothing too tricky here.
Change-Id: I48e51c301e53efc63fc92c378fe45a0e5a2df7e6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340520
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Much like store64, load64 really wants to use ld2.4s but that needs a
way to allocate adjacent registers. So, just like store64, do it
manually, this time with uzp (unzip).
Change-Id: Ie10cc8d2df57390d1c6709bd7485bb5158375078
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340519
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
GLSL only allows one-dimensional arrays. This CL lowers SkSL's array
dimensionality limit from eight to one, and fixes all the tests that
this breaks. The rest of the code still technically supports
arbitrarily-deep array dimensionality; there are many opportunities for
code cleanup and simplification in followup CLs.
Change-Id: I0fc31e4626649ec69d40c5f5597b3924de298df0
Bug: skia:11026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340339
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is illegal in older versions of GLSL and in Metal. We now fail at
SkSL compilation time and properly report the error.
Change-Id: I6ddaeabff5386a1ed6ca3eb8703a6035476ec77a
Bug: skia:11021
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339298
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL fixes cases where array dimensions could be placed on the type
instead of the variable (`float[2] x` instead of `float x[2]`). It also
reports errors in cases where arrays aren't syntactically valid in
Metal, rather than emitting unusable Metal code. (Some of these cases
are actually invalid GLSL as well! But those fixes are coming in
followup CLs.)
Change-Id: I22279127c8a9aa2f22bf5ea3d225e563c2e254f2
Bug: skia:10926, skia:10760, skia:10761
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340137
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Ideally we'd use st2.4s here but that needs its inputs in adjacent
registers, and I don't have a mechanism for that (yet). So instead
interlace manually using zip1/zip2.
Tested by SkVM_64bit.
Change-Id: I7b05fcd1f4398012755fc4f0d4e39743d0c69a94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340518
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@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: I887ebda91673d37139ebc7fc427e80a55b9d9bd1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340101
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 3c161467f0.
Reason for revert: need to keep iOS simulator (Build-Mac-Clang-x64-Release-iOS) in mind
Original change's description:
> restore murmur3 for older iOS devices
>
> 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>
TBR=mtklein@google.com,brianosman@google.com
Change-Id: I0e3b23e63c33910e482031d7475feb624bd6e1f6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11001
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/340396
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: b/160984428
Add more fields to SkCodec::FrameInfo, which describes the properties of
an individual frame in an animated image. This allows a client that
wishes to seek to determine frame dependencies so that they can decode
an arbitrary frame, which in turn will allow SkCodec to remove
SkCodec::FrameInfo::fRequiredFrame. Currently, SkCodec seeks through the
stream to determine frame dependencies, but this is unnecessary work
(and storage) for a client that does not want to seek.
These fields also support the proposed APIs in go/animated-ndk.
Move SkCodecAnimation::Blend from SkCodecAnimationPriv (and delete that
file) into SkCodecAnimation.h. Rename its values to be more clear.
Merge common code for populating SkCodec::FrameInfo.
Add a test for a GIF with offsets outside the range of the image. Note
that libwebp rejects such an image.
Update libgifcodec.
Change-Id: Ie27e0531e7d62eaae153eccb3105bf2121b5aac4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/339857
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Nigel Tao <nigeltao@google.com>
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>
