Some GPUs (Adrenos in particular) perform noticeably better when we
use OpConstantComposite instead of OpCompositeConstruct. This also gives
us some deduplication of redundant ops.
Change-Id: I53b7a3e1cf61e51647a661a08ff4c7b53ee60f10
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528636
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I484067cb1f6025dc9e6770c51c99bfc2c5925652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528365
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This adopts a trick from SkVM to avoid sorting entirely.
Change-Id: I586c8a3613b48241842a7d8eba1c9d68a4717f83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528368
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I238d29ba0250224fa593845ae65192653f58faff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528156
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Blocks did not previously track their position, creating problems
reporting some errors (which will be improved in followup CLs). Fixing
block positions changed the reporting of do loop errors, requiring do
loop position tracking to be updated as part of this change.
Change-Id: I3bd048a62d912914edf679f42607de1b5eafc2b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528045
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Caveat: on my machine, Nanobench doesn't detect any change (pro or con)
on this CL.
I'm working under the assumption that function calls have a non-zero
cost--they may be inlined (bloating code size), or not (incurring the
costs of a function call, register push/popping, etc). This CL avoids
making six calls to $blend_set_color_saturation by using two half3
variables. These half3s are used to swizzle the result--they contain two
zeros and a one, so multiplying them by a scalar will put the result in
the desired component. I've also made some very minor simplifications to
the math that were made possible by reordering.
Change-Id: I0c1ef88d165365376078846324be8bb723548512
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528043
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
A ternary of the form `anything ? value : value` can be reduced to a
comma-expression of the form `anything, value`. This seems like a rare
case in real code, but it's easy enough to detect with our existing
toolbox.
The `anything` test-expression will be eliminated from the expression
if it has no side effects, using our existing constant-folding rules
for the comma expression.
Change-Id: I1285b04cd6a08f1bed614aa1aa6f37ea2447de91
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528439
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, when we vectorized scalars, we would not place them in the
constant buffer, so we could emit simple vectors like (0,0,0) or (1,1,1)
more than once. Now, we use `writeConstructorSplat` to vectorize, which
knows how to write constants.
Change-Id: Ic97c0ce5415fd46ff8c7fb7dac9205844633ef3a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527921
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I76a01cf18583fab619459f258119b780ff978be5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527840
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Today I learned that `mix(a, b, 1)` can reduce precision. Ternaries do
not suffer from this problem.
Change-Id: I58814d00193ccbff53960030d163d31c49234f6c
Bug: skia:9320
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528161
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
The inliner can do a better job with functions that only have a single
return by eliding a temp variable. In this case, it was simple to adapt.
Change-Id: I9a5ee26cf546db1b2647cdf95d4cdba6649ea19b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528160
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkSL errors now identify the specific range of code they are describing,
rather than just the line number.
Change-Id: Ifabb3148476f9b4cd8e532f23e5b38e1cf33a87e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528039
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
While I was fixing up Chrome's uses, I found some failures
there that I did not see in Skia, and tracked them down
to a few other places where we include SkColorSpace
and it is not strictly necessary
- SkCustomMesh.h
- GrColorInfo.h
- GrColorSpaceXform.h
- SkColorSpaceXformSteps.h
For these files (and their .cpp files), I added enforcement
of include-what-you-use, and then fixed the myriad of places
which were depending on these transitive includes.
One change to help Chrome is the manual overloads of
SkImage::MakeFromAdoptedTexture instead of using default
parameters. This makes it so callers of that function
do not need to include SkColorSpace if they were going
to pass nullptr for it anyway.
Bug: skia:13052
Change-Id: I16bf8ed5e258225d887f562f2c189623b1ca9c23
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527056
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This means that the UniformManager w/in the SkPipelineDataGatherer will
now collect all the uniforms into a single block of memory.
Bug: skia:12701
Change-Id: I504a014d37662619191d9b519b4e1add69eac8bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527837
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This removes the use of the dubious void* 'srcs' array.
Bug: skia:12701
Change-Id: I5078d48c119d4e84e83a78b10d0bd4a9a1d8cd8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527777
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 2e6f60f423.
Reason for revert: draws black incorrectly in various iPhone 8 tests
Original change's description:
> Fix color fringes on blend_hue and blend_saturation.
>
> Previously, we checked for division against zero, but didn't do anything
> to prevent division against extraordinarily small values. Now, we only
> saturate if the delta between max and min is greater than 0.00001.
>
> Change-Id: I7d1df3430941c7e1a7f94e597d5449f9259612d6
> Bug: skia:9320
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527498
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: skia:9320
Change-Id: Id83376080eed684577b3592c5e1bee3c80fc3fc9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528038
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
This addresses (hopefully) all of the remaining suboptimal positions in
SkSL error reporting.
Change-Id: I5bc977b03d51153b841a89fa687e54e3e9cb6ec3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527976
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This cleans up a lot of positions produced by DSLParser to make them
actually match the ranges of the elements being parsed.
Change-Id: Ic3a9d62c99c4b5f92b84a597a2ceba386bbcc334
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527501
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is moving to the world where the SkPipelineDataGatherer holds
a UniformManager. In that world the UniformManager will need to
incrementally allocate the uniforms it is collecting.
Bug: skia:12701
Change-Id: I6ed26948d7d0f2979130c4908d0c34c32604cd75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527736
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
With UniformData being stored in a arena and TextureData still being
held by unique pointer, the PipelineDataCache needs to be able to handle
both.
Longer term, TextureData will also be held in an arena and this
split will go away.
Bug: skia:12701
Change-Id: Iab6c6542fbce4f886410f985e6c93ddaa125152e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527504
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We already performed a matching simplification for `0 - x`, so this
seemed like a straightforward improvement.
Performing this simplification causes the expressions in the test code
to match on both sides (e.g. `-one == -one`) which allows them to fold
away.
Change-Id: Idf87a98024dd6831b45d0384285ead2e2e039493
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527656
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We use multiplication by 1 or -1 to branchlessly choose one of `min` and
`max` in the same function.
Change-Id: I44cf747feeae75a9c3e00f36e112e0a429871e86
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527596
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Hard-light is just overlay with the parameters reversed.
Change-Id: I6cf5963b1252cba3a7b71a56f4094a070188f8b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527503
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These functions were functionally almost identical, except:
- Sometimes sda/dsa are flipped
- Sometimes the saturation is not updated
We now have one method (blend_hslc) which can do all four blend
operations. It takes two new parameters ("flip" and "saturate") to
handle these four variations.
This reduces our shader count on some of our most shader-heavy slides
(e.g. aaxfermodes, xfermodeimagefilter) at a pretty reasonable cost.
Change-Id: Ifa8a48399851a9badb5d50038de1e25e60d44ebd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527281
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This will allow expressions like `-x == -x` or `!y == !y` to be detected
as matching expressions (which enables various constant-folding paths).
(Also, migrated the analysis code into a separate cpp.)
Change-Id: I3e317fdaed3762f8fa19e684a5ed557fc9348c7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527617
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Followup CLs will improve their output.
Change-Id: I07059348f68cd6cd3154c31a41f81018b26a44e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527616
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, we checked for division against zero, but didn't do anything
to prevent division against extraordinarily small values. Now, we only
saturate if the delta between max and min is greater than 0.00001.
Change-Id: I7d1df3430941c7e1a7f94e597d5449f9259612d6
Bug: skia:9320
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527498
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Change-Id: Ibff49d1928d7f82d04930c8cfd9d574780732c0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527497
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were not propagating the position into a double-negated expression,
leading to an assertion failure in PrefixExpression.
Change-Id: I1970ff1a06d9631582626c68e151f12f6b3ef278
Bug: oss-fuzz:46381
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527507
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The SkPipelineDataGatherer is going to subsume all the UniformManagers
scattered across the code so it will need the Layout in its ctor and
be plumbed where ever there is a UniformManager (in order to replace it).
Bug: skia:12701
Change-Id: I4f42bf50023e9e66c90f9a14833b976e214e1cc1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526275
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
The unused variable was breaking compilation for some versions of clang
Bug: skia:12701
Change-Id: I8ded5bf3d18136b5679878090186746b88292c3d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527236
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We didn't have any existing tests that exercised this path; it is
separate from most operators since it has no C++ equivalent.
Change-Id: I95b538dad01f8c8b122954fb5f66337371a398a8
Bug: oss-fuzz:46289
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527196
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is currently unused and can be re-obtained from the paint params
key if needed. Currently, it is getting in the way of updating how we
collect uniform data.
Bug: skia:12701
Change-Id: I5057cdfa42252b03b0f9756290735bf4b20dcb6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526272
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
These aren't currently used (we recompute them from the PaintParamsKey)
and are only getting in the way as we change how we accumulate uniforms.
We can always re-add them if the recomputation becomes a hot spot.
Bug: skia:12701
Change-Id: Id91ea0d27543666c75c154fc585d954c5f89aded
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526270
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is a reland of commit 1aedd5dc11
Original change's description:
> Always apply mipmap sharpening on GPU
>
> Bug: skia:13078
>
> Change-Id: If459a96eba09fb10e967bc364435f79b83fdc1ec
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522099
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13078
Change-Id: Ic05b38fc07566f090d609431f2738d64dfdc8a66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524218
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of commit 355f0f9fa2
Original change's description:
> Change GPU LOD bias to be just shy of -.5.
>
> We want to ensure that when a MIP level is 1:1 with device space
> that kNearest picks that level instead of a larger level.
>
> Bug: skia:13078
>
> Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13078
Change-Id: I7fc765a8718d770ebdac68adf9c59ff15d8c8451
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526517
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 355f0f9fa2.
Reason for revert: blocking chrome roll, should be #if defined(...) for the guard
Original change's description:
> Change GPU LOD bias to be just shy of -.5.
>
> We want to ensure that when a MIP level is 1:1 with device space
> that kNearest picks that level instead of a larger level.
>
> Bug: skia:13078
>
> Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13078
Change-Id: I42d6e99509a87f0354f104f2c0177e78cf0d0e21
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526462
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bug: skia:13114
Change-Id: I653ab746927abdd1491e070e2e27252bf056d233
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526024
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We want to ensure that when a MIP level is 1:1 with device space
that kNearest picks that level instead of a larger level.
Bug: skia:13078
Change-Id: I703d08ab394e1d39b31d16946067a2ead415c72a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524224
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This doesn't reuse memory w/in the gatherer but sets us up to do so.
Bug: skia:12701
Change-Id: If5ed962e73609b113581ed2bd681e872fe3214c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526029
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We were not freeing the memory allocated by copyToMask in
one place.
Change-Id: I4404395e8bc2beb43eca5226e947c42b22805ef7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526076
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This doesn't actually change the memory allocation but sets up to do so.
90% of the CL is just renaming SkPipelineData to SkPipelineDataGatherer.
The main interesting changes are those to:
ExtractPaintData
in DrawPass.cpp
and SkPipelineData.h/.cpp
Bug: skia:12701
Change-Id: I3e18f9b3f16166649de1bf1f4399d5521d817eb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524763
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This moves the Build-Debian10-BazelClang-x86_64-Release-IWYU
job from experimental to on when a file in one of the
folders that we enforce IWYU is modified (currently
for svg, sksl, and now, debugger).
Change-Id: Ia6fe1e7b30fc486db3eb081b6a64bc4c250cbf0b
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525796
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
The builtin variable scanner did not check builtin code for the presence
of sk_FragColor, etc. We currently get away with this because none of
the existing builtin code uses a builtin variable.
Now FindAndDeclareBuiltinVariables checks shared program elements too.
Change-Id: Ifb3ee3857ef73b18d9e4f406970f0f67681dd4be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525042
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: skia:12845
Change-Id: If3ac2b6ba2c8e28328ee5805a29fc83353220364
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524756
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Most of these are pretty mechanical generated changes.
IWYU noticed one issue with DSLCore.h, which was fixed here.
Change-Id: I5629565ad3c2817daa71907c62f932d93f9d78ab
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524617
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This closes one of the last gaps in SkSL's constant-folding abilities.
Change-Id: I65c0f2e5fe11a7d47ab2069b2992403fca78b8a7
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524761
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The expression `!x ? y : z` can be optimized to `x ? z : y`, saving a
bit-not. SkVM now supports this optimization.
Change-Id: I06a0d2a716947de1021ba66b054b92e25568c641
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524226
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
SkVM has a `bit_clear` opcode dedicated to the operation `x & ~y`, but
the optimizer was not smart enough to combine a bit-and with a bit-not
and replace it with a bit-clear. Now, it can.
Change-Id: Ida5345c3def0a4bf7afa08bb7f7835e1e2e37677
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524225
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Previously, our ID canonicalization was simply "lower ID numbers before
higher ID numbers" and was done separately at every opcode by taking
the min and max of (x.id, y.id).
Now, this logic is factored out into a helper function
`canonicalizeIdOrder` and has two rules:
- Immediate values go last; that is, "x + 1" instead of "1 + x".
- If both/neither are immediate, lower IDs before higher IDs (as
before)
This change lets us remove a lot of simplification logic. We no longer
need to check for both `x + 0` and `0 + x` when removing no-op
arithmetic; now we can be certain that the immediate will always come
last, so just checking for `x + 0` is sufficient.
Change-Id: I66cc5c23bba414041c0bc556521d3db57fac504d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524222
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:13086
Change-Id: I0fc3243fb3f3974a32726237358522171ae33c41
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523419
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is needed for accurate error reporting when we start reporting
ranges rather than line numbers.
Change-Id: If465317e04685e91ab7c408d29e82028b5d59d1a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523425
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13078
Change-Id: If459a96eba09fb10e967bc364435f79b83fdc1ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522099
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This test fails on Nvidia GPUs on OpenGL due to an issue that only
affects GLSL. Disabling this test to reduce developer noise until we
have a way to re-enable it more selectively in dm.
Bug: skia:13034,skia:13035
Change-Id: I60e0d976774bd474676380583af24865e88471c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523976
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
A few tests were divided into a Runtime Effect-compatible .rts test, and
a Runtime Effect-incompatible .sksl test.
Change-Id: Ib52554892685bdc44fe3622ab314960ee0962b90
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523377
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
In a few cases, this involved splitting a test into two (an ES2-
compatible portion and a ES3+ portion).
Change-Id: Ie6f18f787cf7c10696a2841ff538bbe2b95bf50d
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523187
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
The debugger is now more aware of scopeless blocks and treats them as
a combined statement--in particular, individual inner statements of the
scopeless Block are not counted as stopping points when stepping in the
debugger. Only the Block itself is used as a stopping point. This
improves stepping over multiple var-declarations in one statement.
Change-Id: Ic3ab4715cd0158109d8389ea0650b661d3a8b65e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523185
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
We are going to cache (and uniquify) the UniformBlocks and TextureBlocks
separate from each other.
Bug: skia:12701
Change-Id: I03837c4a38a9bdeb4224a697eab119fca24e8f8c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522916
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We were unintentionally trying to run all GMs in 3 minutes
which sometimes took a bit longer. This uses the batch
mode that we use for unit tests too.
Change-Id: Icf9b415881d57772584a16423bdaff14d862c19d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522936
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
A few tests received minor tweaks to make them Runtime Effect-friendly.
Change-Id: I9b4f66b0974c41d38324dfbb31ac9849338f600a
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523186
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These are wrapped in an unscoped Block. Previously, we didn't assign any
position to the block, so it was implicitly given the position of its
enclosing statement.
Change-Id: Id320eb1db583acd6ae42deba2fbb0b61033c3936
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522922
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
A few tests received minor tweaks to make them Runtime Effect-friendly.
Change-Id: Icbcedb84b7882e42f21425b2d40d7819705c359e
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522918
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This is useful because it will allow these tests to be supported in
Android CTS, where only Runtime Effects are API-accessible.
This CL updates the C++ test harness so that .rts files in the error/
directory are found, and tweaks error tests as necessary to make them
Runtime Effect-compatible. For instance, Runtime Effects enforce the
parameters on main(), which adds extra errors that we don't want. And
some error tests require ES3 (e.g. array constructors) and so those
tests remain as .sksl files.
In this CL, only tests beginning with A are updated. The remaining tests
will be updated in followup CLs.
Change-Id: I70b064df4f0b3ed02d6bc8cc9add7ee844a78691
Bug: skia:13042
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522424
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
`test_expect_fail` did a lot of fiddly string manipulation work in one
big monolithic function. Now it has helper functions
`get_expected_errors` and `check_expected_errors`. This hopefully makes
the logic a bit easier to understand.
There aren't any functional changes here, just restructuring.
Change-Id: I0961054b475255b6159b4dd05b98b6054b144d71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522422
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
PS1 regenerates the Bazel files. Use it as the base change when
comparing patchsets.
IWYU seems to do a good job of working with MyFile.cpp and
MyFile.h, but if there is just a MyHeader.h, it doesn't always
seem to throw errors if the includes aren't correct. This was
observed with include/sksl/DSL.h This might be due to the fact
that headers are not compiled on their own, so they are never
sent directly to the IWYU binary.
This change sets enforce_iwyu_on_package() on the all sksl
packages and then fixes the includes until all those checks
are happy. There were a few files that needed fixes outside
of the sksl folder. Examples include:
- src/gpu/effects/GrConvexPolyEffect.cpp
- tests/SkSLDSLTest.cpp
To really enforce this, we need to add a CI/CQ job that runs
bazel build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
If that failed, a dev could make the changes described in
the logs and/or run the command locally to see those
prescribed fixes.
I had to add several entries to toolchain/IWYU_mapping.imp
in order to fix some private includes and other atypical
choices. I tried adding a rule there to allow inclusion of
SkTypes.h to make sure defines like SK_SUPPORT_GPU, but
could not get it to work for all cases, so I deferred to
using the IWYU pragma: keep (e.g. SkSLPipelineStageCodeGenerator.h)
Change-Id: I4c3e536d8e69ff7ff2d26fe61a525a6c2e80db06
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522256
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
We are going to cache the uniform data and the texture data separately.
Begin by making the uniform data something that could be cached on its own.
Bug: skia:12701
Change-Id: If3ea3a9b6050faf0810549d4076ae44732656a9e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522178
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
The error was being reported at the position of the var declaration,
rather than the position of the reference. And since the declaration
was in a module, its position was both incorrect (with respect to the
program source) and could be past the end.
Change-Id: I443b9fbbe016c43b93d457abfefd17025e451d8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521522
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
By setting the compile time flag:
SK_EXPERIMENTAL_SIMULATE_DRAWGLYPHRUNLIST_WITH_SLUG_STRIKE_SERIALIZE
will cause all SkTextBlobs to be rendered by analyzing the Slug
to create strike cache differences and serialize the Blob to a Slug.
Then the serialized strike cache differences are used to populate
the strike cache using a different TypefaceIDs using SkTypeface_Remote
as a proxy for the real SkTypeface. This will create a hard break
between the original glyph data, and the proxied glyph data.
It will then deserialize the Slug doing TypefaceID translation to
the SkTypeface_Remote, and draw the unflattened Slug.
Bug: chromium:1278340
Change-Id: I0f1980dee966b1092a99741793aed9d138451f4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510228
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
PS1 regenerates the Bazel files.
It is recommended to review this CL with a diff from PS1.
Example output when a file does not pass the test:
tools/sk_app/CommandSet.h should add these lines:
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
namespace sk_app { class Window; }
tools/sk_app/CommandSet.h should remove these lines:
- #include "tools/sk_app/Window.h"
The full include-list for tools/sk_app/CommandSet.h:
#include "include/core/SkString.h"
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
#include <functional>
#include <vector>
class SkCanvas;
namespace sk_app { class Window; }
---
This makes use of Bazel's toolchain features
https://bazel.build/docs/cc-toolchain-config-reference#features
to allow us to configure compiler flags when compiling
individual files. This analysis is off by default, and can
be turned on with --features skia_enforce_iwyu. When enabled,
it will only be run for files that have opted in.
Example:
bazelisk build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
There are two ways to opt files in:
- Add enforce_iwyu = True to a generated_cc_atom rule
- Add enforce_iwyu_on_package() to a BUILD.bazel file
(which enforces IWYU for all rules in that file)
Note that Bazel does not propagate features to dependencies
or dependents, so trying to enable the feature on cc_library
or cc_executable targets will only impact any files listed in
srcs or hdrs, not deps. This may be counter-intuitive when
compared to things like defines.
IWYU supports a mapping file, which we supply to help properly
handle things system headers (//toolchain/IWYU_mapping.imp)
Suggested Review Order:
- toolchain/build_toolchain.bzl to see how we get the IWYU
binaries into the toolchain
- toolchain/BUILD.bazel and toolchain/IWYU_mapping.imp
to see how the mapping file is made available for
all compile steps
- toolchain/clang_toolchain_config.bzl, where we define the
skia_enforce_iwyu feature to turn on any verification at
all and skia_opt_file_into_iwyu to enable the check for
specific files using a define.
- toolchain/clang_trampoline.sh, which is the toolchain is
configured to call instead of clang directly (see line 83
of clang_toolchain_config.bzl). This bash script used to
just forward all arguments directly onto clang. Now it
inspects them and either calls clang directly (if
it does not find the define in the arguments or we are
linking [bazel sometimes links with clang instead of ld])
or calls clang and then include-what-you-use. In all cases,
the trampoline sends the arguments to clang and IWYU
unchanged).
- //tools/sk_app/... to see enforcement enabled (and fixed)
for select files, as an example of that method.
- //experimental/bazel_test/... to see enforcement enabled
for all rules in a BUILD.bazel file.
- all other files.
Change-Id: I60a2ea9d5dc9955b6a8f166bd449de9e2b81a233
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519776
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
The KeyContext is used in the addToKey methods but must appear in the
AddToKey methods bc the latter can call the former.
Bug: skia:12701
Change-Id: I3143afec8337b1e3e12f1c3cc198714009ca6930
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520539
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
The block increment parameter, after dividing by address align, has to
fit into 16 bits. SkTBlockList with either large T or a large
"itemsPerBlock" hint can pretty easily exceed this. However, both the
items per block and the block increment are just hints to control
allocation patterns. SkBlockAllocator can have larger blocks than that
based on growth policy, up to its actual allocation size limit.
SkTBlockList also does not need itemsPerBlock in its implementation, so
if the request exceeds what the allocator can do, it's not problematic.
So clamping to the highest storable value is nicer than asserting that
the caller respected the internal limits.
Change-Id: I82b1c20034fd264b65c7eae4d6758caa6b574fb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520656
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Today, if the arc command flags are not separated by whitespace, the
parser fails to parse the string. I noticed this when trying to parse a
path similar to the one in the test case when playing around with
PathKit.FromSVGString.
Change-Id: I40967c07dfa03d76d26ac2e060b3ef7ac488d0fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520256
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Dan Field <dnfield@google.com>
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.
Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12701
Change-Id: I1fd8dede3eb216c28408bd613119448704c0e7c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512356
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This CL defines a const string in GrGPUResource and have the
constructors accept it. The label string is then plumbed through the
system for other components to accept it.
Bug: chromium:1164111
Change-Id: I6cc759f9263dedd4b2cc0c3ca7cf280be5d74174
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508798
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Moved the MatrixFoldingES2.sksl test case for matrix construction with
side-effects into a new PreserveSideEffects.sksl test and added new test
cases for various vector and matrix types and constructors. The new test
is written such that none of its contents should be folded away.
Note: This test does not pass on NVIDIA GPUs when using OpenGL as
discussed in skia:13035. Notably, NONE of the increments are executed on
those GPUs as ALL increment expressions seemingly get subjected to
constant-folding. The test is disabled on NVIDIA GPU bots.
This also means that the remaining MatrixFoldingES2.sksl tests now work
on NVIDIA GPUs when using OpenGL (with the exception of Tegra3 + OpenGL
ES).
Bug: skia:13035, skia:11919
Change-Id: I561bb62fe2b6b814ba80fbc492d3885bbcd6b65b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518278
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
This lets us differentiate SkQP from other testing harnesses (like DM or
Viewer) at runtime.
This allows us to change strictness or deactivate tests when SkQP is
running. Previously we had a macro SK_BUILD_FOR_SKQP for this, but this
did not work on a local skqp binary; it only activated when compiling
for Android.
Change-Id: I7334e04ea1eddda783a5d2f26699edd20828f81a
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518939
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The Android builds of SkQP will conditionally exclude any tests that are
not flagged with `SkQP`.
Note that local builds of the C++ skqp binary do not actually set
SK_BUILD_FOR_SKQP. This flag is set by gn_to_bp. So local skqp binaries
will continue to run these tests. A possible fix for this limitation
has been written in the followup CL: http://review.skia.org/518939
Change-Id: I879650d165c5931693b14102586bae2c9e45dd43
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518936
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
At this CL, we should run exactly the same tests as before; the only
difference is that we enable and disable tests via SkSLTestFlags instead
of using different SKSL_TEST_xxxxxx macros for different test types.
There is an inert flag for SkQP; this will be implemented in a followup
CL.
The SKSL_TEST block now extends past 100 characters in some spots
because it was, overall, a lot easier to read and edit tabular data in
a tabular format than with inconsistent formatting and line wrapping.
Change-Id: Ic90f88b10c6e59357275b471e6e86e53e611f101
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518138
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I1fa160941b18392616708f4e08a4ebbf7398480e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518521
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
4 byte alignment was being used, when it should be 256 on Intel Macs.
Bug: skia:13033
Change-Id: If902885f62e019b10c316dcfd7f4a336e0982db7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518522
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
ExtractPaintData currently returns:
std::tuple<SkUniquePaintParamsID, std::unique_ptr<SkPipelineData>>
SkShaderCodeDictionary::findOrCreate is going to change in an upcoming CL and this sets up to better isolate those changes.
Bug: skia:12701
Change-Id: I8d077b3d342e6ea16ac99227eb145d92e8247ea4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517736
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
PS1 regenerates BUILD.bazel files
I suggest reviewing the deltas between PS1 and the latest
PS to focus on the interesting bits.
The changes here allow for a Vulkan-only build of HelloWorld
based on sk_app. The toughest change was properly fetching
the VisualID after removing the gl calls that used to
fill that in.
There are a few changes that fix resolution of Dawn
header files, but those won't actually be built until
a follow-on CL.
Change-Id: I54fb58b5dd7ecd4313562aed401759b3eaed53c0
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516999
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This was motivated by SkScalerContextFlags putting `kNone` into the
global namespace. Instead of adding a suffix, change these internal
enums to be scoped and update the users.
Because SkScalerContextFlags is used like a bitfield,
`SK_MAKE_BITFIELD_OPS` is used to provide the appropriate operators. The
macro implementation is updated to handle scoped enums, remove the very
dangerous, unwanted, and unused templated operators, and add operator~
since that is needed by one existing user.
Also, for some reason this allows the compiler to find an unused field
which is removed.
Change-Id: Id31c80b381bffaf9b623f6a8ec7d86751d3d99f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
SkPipelineData is evolving into a big deal - it deserves it owns files.
Bug: skia:12701
Change-Id: I78b4100f1b90fa10f2b264e6d13fca6f15bba39b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516157
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Additionally make the enums enum-classes.
We will need this to specify fixed function blend modes in Graphite.
This CL is mainly mechanical (i.e., global replace) but some fix-ups were required for our internal bit twiddling with the enum-classes.
Bug: skia:12701
Change-Id: I67abcf8274d14e691974cf253bf65dcb4697bd9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515318
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
DEF_CONDITIONAL_TEST and DEF_CONDITIONAL_GPUTEST_FOR_xxxx_CONTEXTS will
allow unit tests to be considered or ignored based on a passed-in
boolean condition. DEF_TEST_DISABLED is now implemented as a conditional
test hard-coded with a "false" condition.
This allows macros (such as SKSL_TEST) to add unit tests based on the
result of a comparison.
Change-Id: Ic5325569009cfce768ba8942907c1d3e7d69ca1c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517516
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
ES2 disallows opaque types in expressions (other than passing them to
their associated builtin functions). We now enforce a similar
restriction on SkSL opaque types.
While I was there, I added several other cases to the invalid-shader
test to make sure that they were all caught.
I needed to reorder some code to make sure that ternary expression error
messages didn't change. Ternary expressions now check for opaque types
before checking that the left-side type and right-side type are
compatible. This is because we check for "compatible" ternary
expressions by checking if `leftSide == rightSide` would be accepted.
`shader1 == shader2` used to be considered a valid expression for the
purposes of this test, but not anymore.
Change-Id: I62a0a31feca9dadd428da7d1b48d7693c4b6434d
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516802
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The fuzzer discovered that we allow == on void types (confusing the SkVM
backend).
Change-Id: Ia9494642faf67f3f86e3a365807be8bd4a7062e4
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we would take the vector-folding path for all types. This
didn't cause any problem for scalars, but failed for "zero-size" types
like void. It isn't valid to compare zero-size values, but we currently
don't reject such code (see skia:13026), and the fuzzer noticed this.
It's safest to only run the vector-folding code when we actually have
multiple slots that need to be folded into one result.
Change-Id: I0bc88043d9a4aeea38ae24dc1a6d1a7430d3d7b0
Bug: oss-fuzz:45279, skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516676
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12754
Change-Id: Iec046dedb088f226e5928712222fa786426b2689
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515517
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This will allow us to easily replace unordered_set/unordered_map in more
places.
Change-Id: Ic0b5245e8cdce2918d7b043bfc370eca95caf889
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515319
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The slot-assignment logic has been changed to associate slots with
function calls, instead of function definitions. In our test case, you
can now see that the calls to `get` are now mapped to $15, $17 and $18.
This change also jiggles some existing tests and improves their
register allocation slightly (!).
One minor hitch here is that there's no FunctionCall node associated
with main() (it's never explicitly called). However, our slot map key
can be any IRNode, and we know main() can't be called by anyone else,
so it's harmless to use the function definition as the key in this case.
(This entry could probably stay out the map entirely if it made a
difference, but I don't think it matters.)
Change-Id: I68a6ff24cbd3a2db77f24126502bd3d11e8c0962
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514578
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
If a function is called multiple times on one line, stepping over that
line does not show all of the function-call results. It only shows the
last result.
e.g. in this example, I have just stepped over the first line which
calls "get" three times. We should see three results, but we only see
one: http://screen/3WfJoZWm77cSexM
In this test you can see that all three calls to `get` are assigned to
the same slot, $15.
Change-Id: Id0c486ef349a1e527001efbcee2ed2b836f56e83
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514577
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Navigating away from the Debugger in Viewer could lead to a null
dereference.
Change-Id: I2686edd3716e7a375d394a4dc1a296395b5f35b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514579
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This leverages our new support for standard strings in SkGoodHash, added
in prior CL http://review.skia.org/515356.
These tests already included checks for SkString, so it was easy to add
a template which tested std::string and std::string_view with the same
checks.
Change-Id: Ia2d9b889e66a50556a25e40493d14a5b692214a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515358
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This will allow these types to be used as the key in an SkTHashSet/Map
without providing a custom Traits class.
Change-Id: I41bfc1d43f3909d3653b4f12ac39e2a288235052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515356
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
I added a comment and didn't rebuild; this shifted line numbers around,
which is reflected in the debug trace opcodes.
Change-Id: I1b8e00ff65557a03483e8d7ff0c4dbec9852866f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
The `break_loop` test causes LLVM to get confused and crash when
compiled on some GPUs. The crash goes away if we pass a literal 5
instead of a 5 that is computed at runtime. This also results in a
simpler test for SkVM, for better or worse, but we still have
coverage for dynamic loop exits in other tests.
LLVM crash: https://paste.googleplex.com/4718583155261440
Dangerous shader: https://paste.googleplex.com/4776089520963584
Change-Id: Ic6cbd55a36d2de58e5dd3459d4dfd74acdbc9f91
Bug: skia:13005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514538
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
MSL does not support the unary "-" operator on matrix types. Similarly
the SPIR-V OpFNegate/OpSNegate operations only work on scalar and vector
type.
* An expression such as "-<mat>" is now transformed to "-1.0 * <mat>" when
generating MSL.
* The same expression now generates a component-wise negation in SPIR-V,
matching what glslang outputs for GLSL.
* A unary "+" is now treated as NOP for MSL, matching the SPIR-V backend.
An expression such as "+<expr>" is now evaluated as "<expr>".
* The shared/Negation.sksl has been moved to folding/ as much of its
contents exercise constant-folding of comparison expressions.
* The shared/UnaryPositiveNegative.sksl test has been extended to
exercise scalar and matrix types.
NOTE: The SPIR-V backend changes have caused a minor re-ordering of SSA
IDs generated when writing out a prefix-expression. The affected gold
files have been updated.
Bug: skia:12627, skia:12992
Change-Id: Iec5cdafc591aed7e49b3b52bda42a02661380bab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513976
Auto-Submit: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
We previously had no way to signal a parse error from arraySize,
resulting in a cascade of additional errors downstream. This tightens
up the behavior and allows us to fail more gracefully.
Bug: skia:12416
Change-Id: I83d3d5bc1dc63395edb325297375a6eb52415817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512952
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
dFdy wasn't working due to the DSL never having been started.
Bug: skia:12985
Change-Id: Ic58018e3dec3e9df15c2e784f8732ee99e793353
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513938
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
These tests were not being run for ES3-only GPU and non-strict-ES2 CPU
tests. CLONE and REHYDRATE variants are now generated for all test
declarations.
Change-Id: I8ce14bd50f175d0bae66d9593b68a42fab701f93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513056
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
On modern hardware, this will give the correct result for `NaN != x`
(true).
Change-Id: I9683f74756da5da5f34ccacec02c1f2449791f26
Bug: skia:12977
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513317
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
The SKSL_TEST_CPU_ES3 has been misinterpreted to mean that the tests
should run on "SkVM and OpenGL" while it was originally meant for tests
that were explicitly disabled on GPU backends.
* This macro is now called "SKSL_TEST_CPU_NO_STRICT_ES2" and the comments
reflect the fact that the tests are meant for CPU-only.
* Removed the infra bot exceptions for the matrix constructor tests as
they are now restricted to run only on CPU.
Bug: skia:12970
Change-Id: I46dcec51ef6998f6a8a7b4610c39560da1e59057
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512578
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
PS1 is the automatic regenerated changes.
PS2-3 adds an #ifdef guard to the addToKey method on shaders.
The SkShaderCodeDictionary class helps generate SkSL and is
only necessary when we are building with SkSL (gpu builds and
cpu builds with SkVM).
Suggested Review order:
- Use Gerrit to diff PS 1 and the last PS
- src/core/BUILD.bazel adds some sources to the "only
necessary if sksl is enabled" bucket
- All the .cpp and .h files to see the #ifdef is added
correctly.
Change-Id: I4d4ce61a4957ef1e0840204acff08ce7e616f9cb
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512157
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This should reduce any possibility of keys being built w/ mixed backend snippets. The decision of which backend the key is being created for is made once, when the builder is created.
Bug: skia:12701
Change-Id: I070330627450501eb93ed98271659e1441aaa935
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511804
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Right now this just checks the creation of the key in debug. In follow up CLs we will pass the data payload to the glue code.
The additional key structure also obviates the need for the block-specific Dump methods.
Bug: skia:12701
Change-Id: Ieddea596de92a4f6b3cf6003f33d7f01d3d28c20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508678
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Switch to using the public API similar to SkTextBlob for
serialize/Deserialize. Add deserializeSlug to SkStrikeClient.
Bug: chromium:1278340
Change-Id: I91b93487859c662e3bfdfba49ba4758f13529cd9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511601
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Use the SkStrikeClient to do the TypefaceID translation if present
when creating a GrGlyphVector.
With this change, Chromium should be able to use the
flatten/MakeFromBuffer API. Currently, this will only
serialize DirectMask (the most common type) drawing.
Other SubRun type will follow soon.
Bug: chromium:1278340
Change-Id: I08a46c7e4e13f7bd899abfdad89c5b3db2548d6a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/511416
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This failure happens when using the SkRemoteGlyphCache system
*and* serializing slugs. The problem is that the origin from the
blob conversion was applied twice causing all the sub-pixel
positions to be off. This often pass because this position often
is {0, 0}.
Change-Id: I0384cfc1de5c32851381390661d1f4a8a6595ab2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510724
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This updates the key builder system so the builder can be used in a tight loop with the memory backing the key being reused.
Bug: skia:12701
Change-Id: I79a72ca26570dcfea9aa45b0fbad8e598688ee98
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510016
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This will leave the PaintParamsKeys more clearly immutable and manage more of the cruft of key creation (e.g., beginBlock/endBlock pairing).
Bug: skia:12701
Change-Id: I1944397465f845e974e950f47cedb90c3f1895b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506881
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit f436cf2343.
Reason for revert: May need to be behind flag or more
suppressions. Breaking linux-rel vulkan_swiftshader_blink_web_tests
css3/filters/effect-blur-hw.html .
Original change's description:
> Preserve base device origin on saveLayer and image filters
>
> SaveLayerOriginTest taken from https://skia-review.googlesource.com/c/skia/+/277977
>
> Bug: skia:12732
> Change-Id: I5ce75355bb16237043c229e1cbc7a106eb636d18
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508919
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12732
Change-Id: I74cc8dc279d22c4fbd313ae3caeb4d0748daf003
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510196
Auto-Submit: Ben Wagner <bungeman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Add fully functional flatten, and MakeFromBuffer functionality
for the DirectMaskSubRunSlug type. The other runs will need to come
later.
Add MakeFromBuffer stubs for all the rest of the subrun types.
This has a single unit test to check basic functionality of
flatten and MakeFromBuffer calls.
Change-Id: I379ded4609a2160170ed6a3670c7c7b6ed2c5b2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509137
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12845
Change-Id: Ia03293c4efdad4c5381a713c9d7d4857b79530c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509398
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
PS1 regenerates the BUILD.bazel files
This allows us to use closure to minify the JS in canvaskit.js
Change-Id: Ib8326d2e3a19cd2168b740b6946f9165a2810133
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509177
Reviewed-by: Ben Wagner <bungeman@google.com>
The name mangling is pretty simple. The uniform padding cheat is that all uniform blocks are now multiples of 16B so can always just be blindly concatenated.
Bug: skia:12701
Change-Id: I738b0669d13b11ad22096caf73af3e0c52fc0585
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506879
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
I wasn't able to find any other test which exercised child color-filters
or child blenders. (SampleWithExplicitCoord evaluates from a shader.)
Change-Id: I58ecee3beca2d3dc11ded5de0eea031e1d7c3e1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507922
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Harden MakeFromBuffer with respect to the array length value.
* Check that the array count is not too large.
Change-Id: I5b398be1690fafce5be5b90ba8aa7af0f12be317
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507637
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
These all stemmed from the same root cause, but are interesting and
distinct enough to include in our error tests.
Bug: oss-fuzz:44555, oss-fuzz:44557, oss-fuzz:44559, oss-fuzz:44561, oss-fuzz:44565
Change-Id: I22c1798809754b4b38c77ffbe369a97c64a2f60e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The fuzzer constructs a long, valid nonsense expression
(x+x+x-x+x-x, etc.) which exceeds parse depth. At that point, the token
stream points to a `+` token. The parser attempts to consume a new
statement but stops in `unaryExpression`; this fails again, due to the
max parse-depth, but doesn't consume a token. The parser continues
trying to parse the statement, but stopping in `unaryExpression`, making
no forward progress in an infinite loop.
I've made a couple of changes as a result.
- Exceeding the max parse depth now sets `fEncounteredFatalError`.
- Encountering a fatal error causes block() to immediately halt. This
actually undoes a few of the arbitrary changes from
http://review.skia.org/506463 but not in a bad way.
- `unaryExpression()` now consumes a token before checking parse-depth.
- `structDeclaration()` had a similar issue where it could potentially
fail without consuming any tokens; this is fixed as well.
- Some unnecessarily-nested logic in ternaryExpression() was flattened
while I tried to ensure that it always consumes a token.
Change-Id: I52c2161965ffbcef1185761ca6897ec1cba5df89
Bug: oss-fuzz:44551
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507436
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Serialize all the glyphID information for the different
sub runs.
Bug: chromium:1278340
Change-Id: I04387ffadcf5cb20fbaca17a02d7ca1faf883806
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507318
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This is a reland of 3225c8cc46
Original change's description:
> Add kR8_unorm_SkColorType
>
> Change-Id: I97b5bc7f90715664f233ca7b7c41c0ecbfc29ac4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505679
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I73fa17625d57e0e58da1b70e2e59ba200383cfe7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506460
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Previously, REPORTER_ASSERT/ERRORF relied on a helper function named
`reporter_string` which papered over zero-argument and one-argument
messages (where one-argument messages are assumed to ignore printf
formatting rules entirely, and just forward the message as-is).
Replacing this helper with a direct call to `SkStringPrintf` allows
the compiler to check format arguments for correctness, but sacrifices
the one-argument special case. In practice the one-argument special
case was very rarely used, so it's not a significant sacrifice,
and this did uncover several real errors in assertion format strings
(including some cases where the wrong number of arguments was passed).
Change-Id: I4378c43b16fd8fdbf4c78d849a9f2f0a254f7abc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506617
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This enables the SkSL error testing logic for runtime effects. The core
logic is identical, only the ProgramKind differs.
(Error creation scripts: http://go/paste/6413797460803584 with some
light post-processing)
Change-Id: I877205b3cc1014b50ccccf6037a2f4034c07543e
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506538
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, when the parser found a bad statement inside a Block, it
would stop processing that Block entirely. This caused our brace
matching to fall out of balance. block() would normally only return once
the Block's closing brace was consumed, but in this case, the closing
brace would still be in the parse stream awaiting consumption even
though block() had returned.
Now, when a bad statement is found inside a Block, we just ignore it and
continue processing. (I tried injecting a poisoned statement as well,
to see if it would affect the test results, but they were identical.)
This seems to generate somewhat better errors.
Change-Id: I8dc781d5602bf99d7610f8280cde8b7c1925cb65
Bug: skia:12868
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506463
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
In the recently introduced FontConfigInterface_MatchStyleNamedInstance
test the highBitsExpectation vector was one bool too short. Because this
was a std::vector<bool> AddressSanitizer because as silent because all
the bits fit in one underlying storage unit. It isn't clear why
MemorySanitizer did not seem to catch this. In any event,
highBitsExpectation[6] was effectively a random causing the test to fail
about half the time.
Restructure these parallel arrays into a Test struct so that the
expectations are obvious and line up. Also make everything constexpr.
Bug: skia:12916
Change-Id: Iaaabb7aef5e041b551a9e2302f954c64509427f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506160
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Bug: skia:12643
Change-Id: I37e1718a20283dfb814c85260257d57bac2b7b34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506211
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
PS 1 is re-generating existing BUILD.bazel files
PS 2 is generating BUILD.bazel files for tests/gms
PS 3+ makes modifications to build all of the gms and tests.
It is recommended to view this CL with just a diff between
PS 2 and the end, due to the large amount of generated changes
in PS 1 and 2.
We make a filegroup for the gms and tests because they need
to be compiled as one large blob in order for the registries
to work. Maybe in the future we will break these up, but at least
for WASM/JS, the overhead of starting a browser for each new
test would likely grind things to a halt, so we just group them
all together for now. It's also the most similar to what we
currently do.
In gm/BUILD.bazel and tests/BUILD.bazel, we add a cc_library
that encapsulates all of the deps of the tests, so we can
easily include that the build. These were discovered via
trial and error, not anything automatic or systematic.
The is_skia_dev_build config_setting is very similar to the
GN equivalent from which it was based.
The list of gms and tests to skip (e.g. which are incompatible
with WASM) was determined by building the wasm bundle:
modules/canvaskit$ make bazel_gms_release
tools/run-wasm-gm-tests$ make run_local_debug
# Don't forget to click the button on the screen after the
# browser loads
This way of invoking the tests will be replace soon with
`bazel test <something>`. As such, I didn't bother fully
documenting the current way.
Suggested review order:
- modules/canvaskit/BUILD.bazel taking note that we always
use profiling-funcs to make the stacktraces human readable.
- gm/BUILD.bazel and tests/BUILD.bazel to see the lists of
gms/tests. Notice the tests are roughly partitioned because
we don't support things like vulkan/PDF in the wasm build
and we will want a way to not build certain tests for
certain configurations
- tools/* noting some of the cc_libraries added to make
dependencies easier to add when needed.
- All other files.
Change-Id: I43059cd93c28af1c4c12b93d6ebd9c46a12d381f
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506256
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
SkVMSlotInfo now contains a "group index" field in each slot. The
group index starts at zero for the first slot of a variable and
increases by 1 for each slot associated with that variable.
For simple types, this group index will always match the component
index. (This is by far the common case, so the groupIndex field is
omitted from the JSON if the indices match.) For more complicated
types--structs, arrays, or nested combinations thereof--it gives us
a simple way to find the start and end slot of a variable. For a given
slot, we can identify the associated variable's initial slot by
subtracting its group index, and we can identify the last slot of any
variable by walking forward and looking for a group index of zero
(indicating the start of a new, different variable).
Change-Id: Iaa71c89ba470a4f9640206dab0774096a7467cce
Bug: skia:12906
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506317
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The `Typeface_glyph_to_char` test assumes that
`ToolUtils::emoji_typeface()` will always return an SkTypeface which
supports all the codepoints in `ToolUtils::emoji_sample_text()`.
However, it is possible in some configurations for there to be no
available emoji font. Detect this and provide a better massage about why
the test failed.
Correct the preprocessor directives in TestFontMgr.cpp so that the
"Emoji" and "Planet" test fonts are consistently defined and used.
Change-Id: Ibc18b03b272d9cb1187bd012a843e550af954565
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506209
Auto-Submit: Ben Wagner <bungeman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Change-Id: I97b5bc7f90715664f233ca7b7c41c0ecbfc29ac4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505679
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Create canonical flattening for SkDescriptor and unflattening
for SkAutoDescriptor.
Eventually Slug serialization and the remote glyphs cache will use
this method for SkDescriptor serialization.
Change-Id: Ia4b6be43058aeca19fbfdcf3c5cdd8d703935775
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505681
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
std::stringstream has a subtle bug in OS X 10.12. Reading in a too-large
floating point value returns INFINITY but does not set failbit. This
caused SkSL to report a different error message than expected
("floating point value is infinite" instead of "floating-point value
is too large: NNNNN"). We now guard against this case in SkSL::stod by
adding an explicit `isfinite` check.
Bug: skia:12928
Change-Id: I9996e64b69512ea5710e6fc3ff00ad1ad83c247b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505939
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This breaks on OS X 10.12: http://screen/7A9bumDr8Z4ihcy
Debugging is difficult via a trybot. This CL can be reverted once the
root cause is discovered and fixed.
Change-Id: Ibbfadc9fbe39eb8d1755e6f382b806d1d648a6fe
Bug: skia:12928
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505803
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
ES2-only GPUs are a lost cause here, but a newer GPU should be able to
construct a matrix from a mix of scalars and vectors in any order.
Change-Id: If9ca5d348cd18b9791c4ee8298f141f6c0edd2c2
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We no longer enforce a particular string form of 3.41e+38.
Change-Id: I33b8a30aa3c7ab54de0c7f4a02181b60cd8f71a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505799
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Previously, a dehydrated SkSL program would include all symbols, even
module builtins. The rehydration API also required a vector of shared
elements, which made the API essentially impossible to use outside of
the very artificial situation in our test cases, where we retained the
original program so we could simply grab the elements from it. We
obviously cannot rely on the original program still being around in
order to successfully rehydrate it.
This CL eliminates the parent symbol tables, referring to module
builtins by name instead. This reduces the size of a small dehydrated
program by roughly 95%. We also encode the shared elements directly
into the output, which both simplifies the API and makes it work in
real-world cases.
Change-Id: I8e5dddf9316fe0886e6b97e7d29638fff8f9f499
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505816
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
There is no need for these headers to be in include/private:
SkPaintParamsKey.h
SkShaderCodeDictionary.h
Added the new header:
src/core/SkBuiltInCodeSnippetID.h
Bug: skia:12701
Change-Id: I413e9a21bc26d5df48765d16dd7390e324006368
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505197
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was (crudely) automated with shell scripts:
http://go/paste/5484300603490304
Change-Id: Ic9e1c93112772d303d1158eb26d995f27b439eba
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505637
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 43539c22a2.
Reason for revert: UB fixed at http://review.skia.org/505678
Original change's description:
> Revert "Verify that tests in errors/ actually generate the expected errors."
>
> This reverts commit 8d646c127a.
>
> Reason for revert: triggering UBSAN
> http://screen/887FeQtZWs2A6oo
>
> Original change's description:
> > Verify that tests in errors/ actually generate the expected errors.
> >
> > Error expectations are embedded in the source with a special *%%*
> > marker, like this:
> >
> > /*%%*
> > expected 'foo', but found 'bar'
> > 'baz' is not a valid identifier
> > *%%*/
> >
> > This unit test compiles every effect in errors/ and verifies that it
> > makes an error. It also verifies that the errors returned include the
> > expectations from the *%%* marker section, in the listed order, if any
> > expectations have been listed. (Error expectations are not meant to be
> > exhaustive; additional errors are allowed.)
> >
> > In this CL, I've manually attached error expectations to the first few
> > error tests. A followup CL will (mechanically) add expectations to every
> > error test, based on their current error reports.
> >
> > Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
> > Bug: skia:12665
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
>
> Bug: skia:12665
> Change-Id: I3bcdbe9fc1abab13656d6462b73f6439967fd96f
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505642
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bug: skia:12665
Change-Id: I49e23869f4ef383a0b076006e319e0a6d7191cad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505643
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
It's undefined behavior to cast a double to an int64 if the double is
out of range. Our SkSL error tests managed to trigger UBSAN on the tree,
pinpointing the issue (which we had already written up a bug for).
Change-Id: Ia06896732223ff310f2c175efcbeb96ba5786fa8
Bug: skia:12863
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505678
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The only use was an executable that only compiled in the pathops tests.
The pathops tests *are* ran in DM and this executable
is not run anywhere.
Change-Id: Ia2d5d7247c25cbad1941b9ee124615c008ea76b7
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505640
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
It appears unused in Skia and Chrome.
Change-Id: I2058374dfda853312087e7e9c50845d2c805b0d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505639
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
It appears unused in Skia and Chrome.
Change-Id: I5d21f6635f96eca58c5efc9eaabe649a7e81db04
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505638
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Namely, SkShaderInfo. This doesn't do anything interesting yet. The ShaderCodeDictionary stores the snippets and then a PaintParamsKey can be traversed to collect the snippets in an SkShaderInfo. Gluing them together will be next-ish.
Bug: skia:12701
Change-Id: Icb4b41716592fc119778ae08f84565da9acaf202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503822
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 8d646c127a.
Reason for revert: triggering UBSAN
http://screen/887FeQtZWs2A6oo
Original change's description:
> Verify that tests in errors/ actually generate the expected errors.
>
> Error expectations are embedded in the source with a special *%%*
> marker, like this:
>
> /*%%*
> expected 'foo', but found 'bar'
> 'baz' is not a valid identifier
> *%%*/
>
> This unit test compiles every effect in errors/ and verifies that it
> makes an error. It also verifies that the errors returned include the
> expectations from the *%%* marker section, in the listed order, if any
> expectations have been listed. (Error expectations are not meant to be
> exhaustive; additional errors are allowed.)
>
> In this CL, I've manually attached error expectations to the first few
> error tests. A followup CL will (mechanically) add expectations to every
> error test, based on their current error reports.
>
> Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
> Bug: skia:12665
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12665
Change-Id: I3bcdbe9fc1abab13656d6462b73f6439967fd96f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505642
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
At some point we'll need to go through the dictionary for user provided SkSL
Bug: skia:12701
Change-Id: I484ae30626dad64f2bce1e0948071380d9f8282e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504596
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Error expectations are embedded in the source with a special *%%*
marker, like this:
/*%%*
expected 'foo', but found 'bar'
'baz' is not a valid identifier
*%%*/
This unit test compiles every effect in errors/ and verifies that it
makes an error. It also verifies that the errors returned include the
expectations from the *%%* marker section, in the listed order, if any
expectations have been listed. (Error expectations are not meant to be
exhaustive; additional errors are allowed.)
In this CL, I've manually attached error expectations to the first few
error tests. A followup CL will (mechanically) add expectations to every
error test, based on their current error reports.
Change-Id: I4add30fef6419c4d3f8d2a221c5aeb53eee35ae7
Bug: skia:12665
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505399
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This code should be easily adaptable to matrix-times-vector as well;
just treat the vector as a 1-row or 1-column matrix. I haven't gotten
around to writing tests for this, though.
Change-Id: If59ae52cd12952b44d3574d54398b2dc66edbcc8
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505221
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These tests only generate an error in the SPIR-V or GLSL backends. We
will soon enforce that everything in errors/ must actually fail to
compile.
Change-Id: Ic54707eb3bfa19287b4ed52335066fc0fbf19ec1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505397
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit d7f7cc8791.
SkFontConfigInterfaceDirect class methods used the FontConfig library
static global "current" FcConfig (implicitly through the use of
nullptr). This was pinned down once per call to each method which used
it (to avoid the "current" FcConfig from being changed out from under it
while running). However, the use of global state as a matter of course
makes it very difficult to reliably test.
Modify SkFontConfigInterface to optionally take an FcConfig on
contruction. If nullptr is provided it is equivelent to the old behavior
so that existing users are unaffected. SkFontConfigInterface takes
ownership of any passed FcConfig and will release it on destruction.
Bug: skia:12916
Change-Id: I20a3cd9405ad40f28b394c713c0514aaa3b08cd0
Revert-Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504776
Revert-Change-Id: I812547bf27371ab716b7a167d7e975f7538d37fb
Revert-Reason: google3 roll failure due to memory leak
Original-Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504477
Origianl-Change-Id: Ie3573403a95c6bf627ce5ff7f2eb5617c9cd162d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505136
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: Ifb387d315fdd083190128f448aadb3e54c4e3369
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505198
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This mirrors a lot of the existing matrix ES2 tests, but using
non-square matrices. This is still important because a lot of subtle
bugs can slip through the cracks when rows == columns.
Change-Id: I626c4c2b176c8280da64513d16f59e76e726cbe7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505218
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of b292c30aa0
This CL also addresses the Wuffs-specific unreachable code.
Original change's description:
> Reenable unreachable-code warnings.
>
> Change-Id: Ie56967a4b823388f0975384e88be23ff2fceecf7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504598
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I655db50134e0de0d0448f22b636ba027513e28f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504757
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This shook out a handful of formatting issues:
[SkVMVisualizer]
- We were passing plain text like "width:35%;" through printf.
- One particular opcode type was printing a string as a number.
[Skottie, SortToy]
- Used wrong integer type instead of %zu for size_t
This CL does not update print functions which take printf arguments via
variadic template, as __attribute__((format)) does not support this
style. These could be converted to va_list style, but that's not done in
this CL.
(For some reason, GCC requires the attribute to be set on a prototype
for freestanding functions, so a few of these now have a prototype
immediately followed by a declaration.)
Change-Id: I63a6c2486c785cc38563028fdf8df0662ec04935
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504698
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 8b618c3e16.
Reason for revert: google3 roll failure due to memory leak
http://screen/4pH356LBiTMTQK7
Original change's description:
> Pin FcConfig for FCIDirect at creation.
>
> SkFontConfigInterfaceDirect class methods used the FontConfig library
> static global "current" FcConfig (implicitly through the use of
> nullptr). This was pinned down once per call to each method which used
> it (to avoid the "current" FcConfig from being changed out from under it
> while running). However, the use of global state as a matter of course
> makes it very difficult to reliably test.
>
> Modify SkFontConfigInterface to optionally take an FcConfig on
> contruction. If nullptr is provided it is equivelent to the user passing
> the result of `FcConfigReference(nullptr)` so that existing users are
> unaffected. SkFontConfigInterface now takes ownership of this FcConfig
> and will release it on destruction.
>
> Change-Id: Ie3573403a95c6bf627ce5ff7f2eb5617c9cd162d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504477
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: I812547bf27371ab716b7a167d7e975f7538d37fb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504776
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Adding double-parens around an `if ((false))` squelches the warning.
In other cases, you can squelch the warning by assigning the
always-constant(-on-this-machine) check into a constexpr bool.
Change-Id: I5a344fb45779c5bd2865edb3cffaf839ba9a5d85
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504597
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
SkFontConfigInterfaceDirect class methods used the FontConfig library
static global "current" FcConfig (implicitly through the use of
nullptr). This was pinned down once per call to each method which used
it (to avoid the "current" FcConfig from being changed out from under it
while running). However, the use of global state as a matter of course
makes it very difficult to reliably test.
Modify SkFontConfigInterface to optionally take an FcConfig on
contruction. If nullptr is provided it is equivelent to the user passing
the result of `FcConfigReference(nullptr)` so that existing users are
unaffected. SkFontConfigInterface now takes ownership of this FcConfig
and will release it on destruction.
Change-Id: Ie3573403a95c6bf627ce5ff7f2eb5617c9cd162d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504477
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change all use of "drawable" to "accepted" in glyph drawing code. To
improve uniformity "rejects" is also changed to "rejected". This is
motivated by the desire to add SkDrawable backed glyph rendering, which
would make the use of "drawable" for both "glyphs which are drawable"
and "glyph which renders with SkDrawable" very confusing.
Change-Id: I6d080bc9ec25f81aa9479757d2cca47ae74f4db6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504236
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This was blocking the Android roll: http://screen/9937JW7waCFer64
Change-Id: I89a88ff5a28e077fb302b5c803d0e4829de35103
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504416
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
GPUs that failed continued to fail when I put in error bars like
`distance(a, b) <= 0.001`, so they're just disabled entirely now.
Presumably their results are very busted.
Change-Id: I0f1b80f661563a20630740f8cfb6ef69f2a47934
Bug: skia:11209, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503817
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This adds a Dehydrator.write(Program) to mirror the Rehydrator's
program() method and simplifies the API.
Change-Id: I1b6d6b722d0ce8e6a292132522f806e43d49ce85
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502704
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Have the AnalysisCanvas (also known as the diff canvas) convert
SkTextBlobs to GrSlugs and record the Strike differences. Make sure
these differences are tracked and recorded.
Bug: chromium:1278340
Change-Id: I2c8d62fa61511abd1e14d4bf595e6db1a0b5e26b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503827
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I8d4816f726196e6f4a5fbb0940b5dd1b2d7db7f5
Bug: skia:11209
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503821
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This is a reland of d921f21fbc
Original change's description:
> Add SkSurface resolve function.
>
> This will insert a resolve msaa call into the stream of commands for
> the SkSurface. This is mostly useful for cases when a client wraps the
> resolve texture but has Skia draw with MSAA, and the client wants to
> make sure Skia resolves to their wrapped texture.
>
> Bug: chromium:1292418
> Change-Id: I6eddae967136716b9215fcd96e7d77a2457efdf2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503340
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: chromium:1292418
Change-Id: I810b5618092c560f5bba900024b3b8c0c88baea9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503717
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is a reland of 995d16fc91
Original change's description:
> For runtime shaders, deduce isOpaque automatically
>
> The forceOpaque parameter is now ignored. Instead, we do a conservative
> analysis of the shader's main function to determine if it always returns
> an opaque color. This is good enough to detect simple cases, including
> things like:
>
> return child.eval(p).rgb1
>
> Bug: skia:12643
> Bug: skia:12896
> Change-Id: I74b331aa12fadb1d0d1bb85f225dc7aa01ba2455
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503346
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12643
Bug: skia:12896
Change-Id: I4ae55cdd3f88c1b5a08bd59913df1c1cd48a4679
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503824
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is a reland of 805acda3f3
It fixes the #if SK_GL which was causing the Android roll
to fail.
This disables unit tests on Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Release-All-TSAN_Vulkan
which were consistently crashing with OOM.
Original change's description:
> [fuzzer] Remove GL from (now-Vulkan) build
>
> The fuzzer runs against the Vulkan version of Swiftshader.
> There are no libGL.so (etc) on the fuzz runtime, so we
> want to avoid linking against those.
>
> The GL code that is #ifdef'd out is still necessary to
> avoid timeouts on TSAN with our NVIDIA jobs.
> https://skia-review.googlesource.com/c/skia/+/502638
>
> Procedure for testing this locally (and iterating):
> 1. In oss-fuzz checkout, run
> `python infra/helper.py shell skia`
> to pull up local interactive version of Docker
> fuzzer build image.
> 2. Run `compile` in fuzzer shell. Stop after
> the swiftshader compiles and is copied into /out
> with Ctrl + C.
> 3. Comment out the swiftshader compilation part [1]
> (no need to re-do this when modifying Skia code).
> `apt-get install nano -y`
> `nano ../build.sh`
> 4. Make change to Skia repo using normal methods.
> 5. Run the following in the Skia repo
> `git diff origin main > foo.patch`
> Copy the patch into the Docker shell using Ctrl+C
> and nano.
> 6. Apply the patch inside the Docker shell
> `git apply foo.patch`
> and re-compile (which should skip right to
> building the fuzzer libs)
> `compile`
> 7. Repeat 4-7 or make small changes directly in
> the Docker shell via nano.
> 8. When compilation and link succeeds, run
> `ldd /out/api_mock_gpu_canvas`
> to verify GL and friends were not dynamically linked.
>
> [1] https://github.com/google/oss-fuzz/pull/7214/files#diff-76f13875e33875cdd372f1f0933206be599cd87952f1bd1eaa57ca928ee9e3e1R49-R53
>
> Change-Id: Idf569820527c1304b0e5a68fd36295be89dfa2a0
> Bug: oss-fuzz:44132
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503016
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: oss-fuzz:44132, skia:12900
Change-Id: Ia2eff9403b0035e7f86098f296d7d9b1bbfd4876
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503716
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Requires tweaking one inliner test to avoid an Intel driver bug (on
ANGLE).
Bug: chromium:709351
Cq-Do-Not-Cancel-Tryjobs: true
Change-Id: I08fac938396d6b90805ba9650c7a520af888bc12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503819
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12701
Change-Id: I33d24fc319acbfbb3a218a8dd916c5a64f15f028
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503342
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
A glyph cache entry is made up of metrics, an image, and a path. If an
entry does not exist then nothing is known of the glyph yet. If there is
an entry it must have at least the metrics. The image and the path may
logically be added lazily as needed. Prior to this change it was not
possible to send both the image and the path of a single glyph. The
added test fails when a given strike is used for both the image and the
path. With this change the test passes, as the image is sent and the
path is sent, and both are merged into the remote glyph. The assertions
around not creating the glyph more than once are replaced with
assertions that no part of the glyph is set more than once.
Change-Id: I2fd4047ee15e0d584b68e1180c6fe15224889310
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503347
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This reverts commit 995d16fc91.
Reason for revert: Breaks one SkSL test on ANGLE
Original change's description:
> For runtime shaders, deduce isOpaque automatically
>
> The forceOpaque parameter is now ignored. Instead, we do a conservative
> analysis of the shader's main function to determine if it always returns
> an opaque color. This is good enough to detect simple cases, including
> things like:
>
> return child.eval(p).rgb1
>
> Bug: skia:12643
> Bug: skia:12896
> Change-Id: I74b331aa12fadb1d0d1bb85f225dc7aa01ba2455
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503346
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12643
Bug: skia:12896
Change-Id: I7d86933f29ddb373222dff7f2a28a413fd777002
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503818
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
The forceOpaque parameter is now ignored. Instead, we do a conservative
analysis of the shader's main function to determine if it always returns
an opaque color. This is good enough to detect simple cases, including
things like:
return child.eval(p).rgb1
Bug: skia:12643
Bug: skia:12896
Change-Id: I74b331aa12fadb1d0d1bb85f225dc7aa01ba2455
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503346
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The previous CLs have removed the last significant differences between
SkSL::String and std::string. This CL removes SkSL::String entirely and
replaces it with std::string throughout the code.
Apologies for the very long CL, but I have done my best to make it as
simple and reviewable as possible. The vast majority of changes are
simple replacement of `SkSL::String` with `std::string`. In the rare
spots where code is moved from one place to another, it is logically
unchanged.
Change-Id: I39563d2db45da229f17f4504dfd63e00bde7a96e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503339
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This reverts commit 805acda3f3.
Reason for revert: Looks to be breaking the Android roll. See https://android-build.googleplex.com/builds/pending/P29733268/aosp_bramble-userdebug/latest/view/logs/build_error.log
Original change's description:
> [fuzzer] Remove GL from (now-Vulkan) build
>
> The fuzzer runs against the Vulkan version of Swiftshader.
> There are no libGL.so (etc) on the fuzz runtime, so we
> want to avoid linking against those.
>
> The GL code that is #ifdef'd out is still necessary to
> avoid timeouts on TSAN with our NVIDIA jobs.
> https://skia-review.googlesource.com/c/skia/+/502638
>
> Procedure for testing this locally (and iterating):
> 1. In oss-fuzz checkout, run
> `python infra/helper.py shell skia`
> to pull up local interactive version of Docker
> fuzzer build image.
> 2. Run `compile` in fuzzer shell. Stop after
> the swiftshader compiles and is copied into /out
> with Ctrl + C.
> 3. Comment out the swiftshader compilation part [1]
> (no need to re-do this when modifying Skia code).
> `apt-get install nano -y`
> `nano ../build.sh`
> 4. Make change to Skia repo using normal methods.
> 5. Run the following in the Skia repo
> `git diff origin main > foo.patch`
> Copy the patch into the Docker shell using Ctrl+C
> and nano.
> 6. Apply the patch inside the Docker shell
> `git apply foo.patch`
> and re-compile (which should skip right to
> building the fuzzer libs)
> `compile`
> 7. Repeat 4-7 or make small changes directly in
> the Docker shell via nano.
> 8. When compilation and link succeeds, run
> `ldd /out/api_mock_gpu_canvas`
> to verify GL and friends were not dynamically linked.
>
> [1] https://github.com/google/oss-fuzz/pull/7214/files#diff-76f13875e33875cdd372f1f0933206be599cd87952f1bd1eaa57ca928ee9e3e1R49-R53
>
> Change-Id: Idf569820527c1304b0e5a68fd36295be89dfa2a0
> Bug: oss-fuzz:44132
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503016
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: oss-fuzz:44132
Change-Id: I3832417c60ff425572717d37dc9609419922b18e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503351
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit d921f21fbc.
Reason for revert: New test breaking GL android bots
Original change's description:
> Add SkSurface resolve function.
>
> This will insert a resolve msaa call into the stream of commands for
> the SkSurface. This is mostly useful for cases when a client wraps the
> resolve texture but has Skia draw with MSAA, and the client wants to
> make sure Skia resolves to their wrapped texture.
>
> Bug: chromium:1292418
> Change-Id: I6eddae967136716b9215fcd96e7d77a2457efdf2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503340
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: chromium:1292418
Change-Id: I86e5f82f0e2a0921906c0caba964929750500965
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503350
Auto-Submit: Greg Daniel <egdaniel@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This will insert a resolve msaa call into the stream of commands for
the SkSurface. This is mostly useful for cases when a client wraps the
resolve texture but has Skia draw with MSAA, and the client wants to
make sure Skia resolves to their wrapped texture.
Bug: chromium:1292418
Change-Id: I6eddae967136716b9215fcd96e7d77a2457efdf2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503340
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
These tests are not guaranteed to pass in ES2 (as ES2 specifically does
not require NaN to be implemented), so they are now run in GPU+ES3 and
on the CPU.
Change-Id: Ica78e15a06b3b00b651c5fabd0decf751853a83f
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501238
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
By being on Recorder, more calls can directly access the ResourceProvider
without needing the Context or Gpu. A different ResourceProvider is
created for each Recorder. Each ResourceProvider stores a ref to the
GlobalCache so that it can access shared resources. Eventually each
ResourceProvider will also have its own ResourceCache for all non shared
Resources.
A big win of this change is that Context can be removed from Recorder.
Bug: skia:12754
Change-Id: Ib6ac71c617de4d6b6b2ac4956580e65d4d7e6f7a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502637
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The fuzzer runs against the Vulkan version of Swiftshader.
There are no libGL.so (etc) on the fuzz runtime, so we
want to avoid linking against those.
The GL code that is #ifdef'd out is still necessary to
avoid timeouts on TSAN with our NVIDIA jobs.
https://skia-review.googlesource.com/c/skia/+/502638
Procedure for testing this locally (and iterating):
1. In oss-fuzz checkout, run
`python infra/helper.py shell skia`
to pull up local interactive version of Docker
fuzzer build image.
2. Run `compile` in fuzzer shell. Stop after
the swiftshader compiles and is copied into /out
with Ctrl + C.
3. Comment out the swiftshader compilation part [1]
(no need to re-do this when modifying Skia code).
`apt-get install nano -y`
`nano ../build.sh`
4. Make change to Skia repo using normal methods.
5. Run the following in the Skia repo
`git diff origin main > foo.patch`
Copy the patch into the Docker shell using Ctrl+C
and nano.
6. Apply the patch inside the Docker shell
`git apply foo.patch`
and re-compile (which should skip right to
building the fuzzer libs)
`compile`
7. Repeat 4-7 or make small changes directly in
the Docker shell via nano.
8. When compilation and link succeeds, run
`ldd /out/api_mock_gpu_canvas`
to verify GL and friends were not dynamically linked.
[1] https://github.com/google/oss-fuzz/pull/7214/files#diff-76f13875e33875cdd372f1f0933206be599cd87952f1bd1eaa57ca928ee9e3e1R49-R53
Change-Id: Idf569820527c1304b0e5a68fd36295be89dfa2a0
Bug: oss-fuzz:44132
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503016
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: skia:12701
Change-Id: Iee63650517a739029f81121cabd45dfcc2e8fa38
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502698
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This class will be used to provide thread safe access to various shared
resources in Graphite. Currently the only thing moved onto here is
the SkShaderCodeDictionary. Eventually it will have things like the
pipeline cache on it as well.
The plan is that users will not access this class directly but instead
via a ResourceProvider (see follow on change).
Bug: skia:12754
Change-Id: I2ae2c4bf7025945de850a618055e59ccd403aaaa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502315
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
We now use std::string_view throughout. SkStringView.h has been moved to
include/private/ and is only used for our C++20/23 compatibility methods
(starts_with/ends_with/contains).
Change-Id: I961842c6778256a03868e7602d48add34f420763
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502306
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Let variable axes override existing or determine style information
with priority over FreeType face flags or information
from the OS/2 table.
Add test case for matching a variable fonts through SkFontMgr_FCI to
ensure the weight determination from variable axes is exercise.
The test does not exercise width or slant.
Add a small (~8.5k) subsetted Noto Sans CJK collection as a test font
(made using [1]), and ensure that matching, reported font style and axis
configuration are correct after matching.
[1] https://github.com/drott/noto-cjk/blob/subsetVFttv/subsetvf.py
Bug: skia:12864, skia:12881
Change-Id: I1fb05d88f68eda308b8864d32d98400c68e46834
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500516
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
Auto-Submit: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Dominik Röttsches <drott@google.com>
An SkUniformData represents the uniforms from a single code snippet
The SkUniformBlock collects the set of SkUniformDatas needed by a whole program
Bug: skia:12701
Change-Id: Ic0596e2fb02ea00bec882af493644def9ebb2468
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497743
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is a reland of 64c971350e
Original change's description:
> Use native std::string_view.
>
> We also used some string_view functionality from C++20/23. These have
> been replaced with free functions with the same name.
>
> Change-Id: I3bf40f99aeb500495f344fd8c6872619267d42be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500897
> Reviewed-by: Herb Derby <herb@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I4ff237381c16179f716ecde1929154fdd4ad3442
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501480
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit ded2548179.
Reason for revert: Windows breakage on tree
Original change's description:
> Added SkSL dehydrate / rehydrate test
>
> Test that we can dehydrate and then rehydrate every program in our test
> corpus without altering them.
>
> Change-Id: I2286fcb691176b3fbbe1d6515279d03867555647
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501436
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I2dcee0b36e7e316c4ed0889141cf38c2d1508653
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502311
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Test that we can dehydrate and then rehydrate every program in our test
corpus without altering them.
Change-Id: I2286fcb691176b3fbbe1d6515279d03867555647
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501436
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is a reland of 490bb34b29
Original change's description:
> Use native std::optional.
>
> Change-Id: I3bcf7a23eb27e98d24840b492930955125d35bd4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501476
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: If14d35ed78905800b43b656f65bb17fc940e7770
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502298
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Our SkVM code generator will always convert `pow(x, y)` into an
`approx_powf` call. However, in many cases, `x` or `y` might boil down
to an immediate value, and for some immediates, we can reduce the
operation into something much simpler:
- `pow(1, y)` is always 1
- `pow(2, y)` can be converted to `approx_pow2(y)`
- `pow(x, 0.5)` can be converted into `sqrt(x)`.
- `pow(x, 1)` can be converted to `x`
- `pow(x, 2)` can be converted to `x * x`
We could go further if there is a need, e.g. `pow(x, 4)` can become
`a = x * x; return a * a`, but these seemed like the best places to
start.
Change-Id: I02b9d4d3f5067581ebad5e53b2d1d7befa308300
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502308
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Ia218fb2249abd479db9d27527b965fd0b8ad3367
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501276
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Test "InlinerHonorsGLSLOutParamSemantics" was failing on Wembley devices
and is now disabled on that GPU.
Also, it turns out that the inliner has ignored functions with out
params for a long time now, but our test names haven't been updated to
account for this. So, did some additional cleanup:
- "InlinerHonorsGLSLOutParamSemantics" (the test in question) has been
moved to shared/ and renamed to "OutParamsAreDistinct."
- Removed test "OutParamsNoInline" as it is functionally the same as
"OutParams".
Change-Id: I1431ed197b9216cb482eee4f5e4eb2579a5303f7
Bug: skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502303
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
sk_SecondaryFragColor corresponds to an ES2-only concept
(gl_SecondaryFragColorEXT) and does not have any SPIR-V equivalent.
Two fixes were needed:
- sk_SecondaryFragColor shouldn't be in SPIR-V code at all. Report it as
an error when it appears.
- We don't stop compilation when this error is reported, so we need to
fix up the assertion that the fuzzer initially discovered.
Specifically, the fuzzer found that the `sk_SecondaryFragColor`
variable never got a SPIR-V ID assigned to it in fVariableMap, so the
compiler would assert when assembling an expression containing that
variable. Now, we make sure to populate fVariableMap with an (unused)
ID in `writeGlobalVar` to avoid this crash.
Change-Id: Ib86919dfc9a325b2b82a7f4b2054b747dad7c32f
Bug: oss-fuzz:44096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501976
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: If0ecc3c1080b8e44f17dbb34ff6cc1d66794ae0b
Bug: skia:11052, skia:11919, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501843
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Removed a special case from `writeFunctionCallArgument` which avoided
using a scratch variable for out params; now we always use the scratch
variable and copy it back to the original variable at the end.
Change-Id: I0e446a3fde6d19554943384210bd911f6f9c8cfa
Bug: skia:11052, skia:11919, skia:12858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501836
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I3bcf7a23eb27e98d24840b492930955125d35bd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501476
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Upcoming dehydration / rehydration changes require $intLiteral and
$floatLiteral to be present in the symbol table (as all other private
types are). It turns out that even with them marked private, having
them in the symbol table allows them to be incorrectly accessed without
error due to a code path that fails to check for private types.
This CL takes care of that and ultimately results in better output from
PrivateTypes.
Change-Id: Ic47b77a770834079f28c3195545a7cabca8e6cb3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501196
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 64c971350e.
Reason for revert: breaks
Housekeeper-PerCommit-CreateDockerImage_Skia_Release
https://status.skia.org/logs/7PAT8xw23VsUyx4oJgZZ/1327ba91-d03a-4e98-b1dd-0dedb401333b
"<string_view> not found"
Original change's description:
> Use native std::string_view.
>
> We also used some string_view functionality from C++20/23. These have
> been replaced with free functions with the same name.
>
> Change-Id: I3bf40f99aeb500495f344fd8c6872619267d42be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500897
> Reviewed-by: Herb Derby <herb@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I3712eddc8ad49ad38d31ca5bf008260e251bdbd7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501396
Auto-Submit: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We also used some string_view functionality from C++20/23. These have
been replaced with free functions with the same name.
Change-Id: I3bf40f99aeb500495f344fd8c6872619267d42be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500897
Reviewed-by: Herb Derby <herb@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This shakes out a few driver issues.
Change-Id: I35041a904fb762a5b933b3e970f2d7a668803dcd
Bug: skia:12858, skia:11919
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501019
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This shook out a few driver issues.
Change-Id: If12f11c4b3b562fec402bddce25edf01a8be83b9
Bug: skia:12858, skia:11919
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500816
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Move to RAII structs wrapping DrawWriter to control the append mode.
This allows each struct to define any extra template parameters/buffers
needed for the appended data, and expose only the appending API that
makes sense.
In follow up CLs, the RAII structs make it easy to support reserving
and returning vertex data, and add new appending modes.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Bug: skia:12703
Change-Id: I5ef1bfdf3b4fa175bcfb25cc61fd0c46a62d46c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498016
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
GLSL ES2 behavior is explicitly undefined if an out-param is never
written to: "If a function does not write to an out parameter, the value
of the actual parameter is undefined when the function returns."
We do see divergence here in practice: SkVM's behavior (the parameter is
left alone) differs from my GPU's behavior (the parameter is zeroed
out).
SkSL will now report an error if an out parameter is never assigned-to.
There is no control flow analysis performed, so we will not report
cases where the out parameter is assigned-to on some paths but not
others. (Technically the return-on-all-paths logic could be adapted
for this, but it would be a fair amount of work.)
Structs are currently exempt from the rule because custom mesh
specifications require an `out` parameter for a Varyings struct, even if
your mesh program doesn't need Varyings.
Bug: skia:12867
Change-Id: Ie828d3ce91c2c67e008ae304fdb163ffa88d744c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500440
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is mostly mechanical. The only interesting bit is that GrSLType was in include/private while SkSLType is in src/core so some #include patterns changed.
Bug: skia:12701
Change-Id: I80bd86ee93796b145f86ded9b4cbf52f24fa59e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497607
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was used for one specialized template in Ganesh, and enables the
preferred appending pattern for Graphite.
Change-Id: I561449004923464efeeabaed5bc00c8fd49e036d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498440
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is a precursor that makes supporting reserving/returning vertices
easier to implement, and adapt to it PatchWriter for the tessellating
path renderers.
It also has the small benefit of being slightly easier to follow, and
the buffer binding remains valid across pipeline changes.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Bug: skia:12703
Change-Id: I84606800937d955d5f4cb98e68cca2ca6c6fb1f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496890
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Previously, when attempting to cast a huge value to an int, SkSL would
report an error, then return the IR for
`ScalarCast(Int, FloatLiteral(huge-value))` . Now, to minimize the blast
radius of the error, we report the error but return `IntLiteral(0)`.
We've already reported an error, so there's no need to preserve the
value, and zero is less likely to produce follow-up errors.
(A similar approach is used here and worked well: https://osscs.corp.google.com/skia/skia/+/main:src/sksl/ir/SkSLConstructorCompoundCast.cpp;l=57-59)
Change-Id: Ie8e8d48380cb963466d1f47d123d64e3301cf87c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499563
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The test input was removed at http://review.skia.org/497742.
Change-Id: I7b30f2f70cd0812b900c9c67b70e742b3d96930a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499574
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The test has been moved to shared/, since it's a valid test, but it is
no longer related to inlining, as the inliner no longer attempts to
inline functions with inouts at all.
Also, one function here (outParameterIgnore) actually invoked undefined
behavior and has been removed. According to the GLSL ES2 docs: "If a
function does not write to an out parameter, the value of the actual
parameter is undefined when the function returns." SkVM leaves the value
unchanged, so SKSL_TEST_CPU would pass, but a GPU might clear it (and in
fact, my GPU does).
Change-Id: I77c77ed1354bc980344ec5c406992bd62015f5e5
Bug: skia:11919
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499752
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Now, constant mat+mat, mat-mat, and mat/mat operations can be optimized
away. mat*mat does not operate componentwise and will need to be
handled differently.
Change-Id: Iabac6e58999eac46c256d7dcdb9b95d05de530bc
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498716
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
GLSL supports adding, subtracting, multiplying, and dividing matrices
with scalars. This works by splatting the scalar across every matrix
component and then performing the op componentwise. Our constant folder
now knows how to fold out these simplifications.
Change-Id: Idb8751ec16135e1b61da0d58cfd0505ab31ac087
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497738
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
In a followup CL, these will be updated to properly fold.
Change-Id: I20d125c0d54cbbcf12f7d096beda1fdf75e51b65
Bug: skia:12819
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498617
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, matrix-scalar operations did not actually fold, so the tests
didn't live in folding/. In a followup CL, these will fold.
Bug: skia:12819
Change-Id: I6fdacf89088920719e7666d6c9b05ddffaf6cb6d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497742
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
SkSL is somehow interpreting a large positive value as a negative one.
Change-Id: I299e0bf389a9fcbfe697741bd33a54df07748753
Bug: skia:12863
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499556
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Some paths through swizzle optimization would replace a swizzles with a
constructor--e.g. `float3(1, 2, 3).y` would be replaced with `float(2)`.
(Constructor::Convert was responsible for replacing this trivial
constructor with the literal `2.0`.)
The optimization code asserted that this replacement would succeed, but
the fuzzer managed to construct a counterexample where the constructor
rejected the value. Specifically, by nesting casts between int3 and
float3, it found a case where Constructor::Convert returned null because
the literal value was out of range for `int` types.
This assertion didn't really add value so removing it was harmless.
Constructor::Convert already reports an error when it fails, and null
returns are handled properly throughout.
Change-Id: I575d441ed90d6b696f6399941c3f6d84698794bc
Bug: oss-fuzz:44045
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499382
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This guards against any potential for conflict with user code.
Change-Id: Iecaf3ead5f8ada50b6dc159a4ad9e7f3e371edc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499296
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The check in SkIsConvexPolygon that determines if a polygon winds around
multiple times turned out to not be correct in all cases. This has been
changed to a much simpler one -- if the sign of the edge vectors change
more than twice, then we know that we've wrapped around more than one
time.
In SkIsSimplePolygon, if both points adjacent to a vertex are on the
right side, we attempt to add both those edges to the active edge list.
However, if they are the same point, then we'll be adding the same edge
twice, which makes the strict ordering of the edge list invalid. Instead
we check for this case earlier on and reject the polygon.
Bug: oss-fuzz:44004
Change-Id: I6ffe11d73fabd6ae2f75cb027db887bcb8c03cb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498916
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This reverts commit a97a6769b5.
Reason for revert: breaking bot Housekeeper-PerCommit-CheckGeneratedFiles -- see http://screen/5FN75fF9tvcQFKR
Based on the diff, I think this just needs to be synced up to latest code and a rebuild should fix it.
Original change's description:
> [skslc] Generate .hlsl test output files
>
> - The build now generates HLSL output when `skia_compile_sksl_tests` is
> enabled.
> - The "blend" and "shared" tests have been enabled for HLSL with the
> exception of 6 tests that exercise intrinsic inverse hyperbolic
> functions, which don't have HLSL equivalents.
>
> Bug: skia:12691, skia:12352
> Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
> Reviewed-by: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: Arman Uguray <armansito@google.com>
> Commit-Queue: Arman Uguray <armansito@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:12691, skia:12352
Change-Id: Iaad607d48edd136eee2b60e48c0643b6e90179e9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499216
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
- The build now generates HLSL output when `skia_compile_sksl_tests` is
enabled.
- The "blend" and "shared" tests have been enabled for HLSL with the
exception of 6 tests that exercise intrinsic inverse hyperbolic
functions, which don't have HLSL equivalents.
Bug: skia:12691, skia:12352
Change-Id: Ia970f878f75ff58e8e3d47249c2dc2f756c165b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/482778
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: Arman Uguray <armansito@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
