This is largely a re-implementation of the logic in
src/gpu/ganesh/gradients/GrGradientShader.cpp and
src/shaders/gradients/SkTwoPointConicalGradient.cpp. For simplicity,
most everything is in the shader now, but with an eye towards moving
computations that don't use sk_FragCoord into SkKeyHelpers.cpp.
Bug: skia:13302
Change-Id: I73d2e1f6c6ac0580aa897ffff3cfa87f4ed62b11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539569
Commit-Queue: James Godfrey-Kittle <jamesgk@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Constructor::Convert is a very heavy hammer which does error-checking
and lots of logic to identify the right constructor type to use. It cuts
out a lot of redundant work to call the desired method
`ConstructorCompound::Make` directly instead.
Change-Id: Icf9e513dbce223c2b27ccd0d11dc08eea849ff39
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539821
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Uses the initializer_list constructor to avoid generating a bunch of
insert calls, and then moves everything right into layout().
Change-Id: I09f5538e20ea6f7d3b6117020302a41e18b66295
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539822
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Colors are premultiplied, so the specular component (which is additive)
must be modulated by alpha explicitly.
Change-Id: If5f7b456b0ce518b1806e9b1e754229c3a6d0981
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539559
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Change-Id: Iac6b8a0b74fe23959f5ee814f4a30a5458ccf21e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539936
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We can access shader caps bits directly from SkSL, using sk_Caps. We
don't need to pass in caps flags from fGpu->caps()->shaderCaps().
Also, you might get different caps bits depending on which
backend/device you are using, so it's a little risky to bake it into
a shared cache; SkSL can apply the right caps bits at compile time.
Change-Id: I896b3915f40fbe7f58dfe5ffb51a462b3d2096ea
Bug: skia:13302
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539572
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: James Godfrey-Kittle <jamesgk@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
ATRACE_ANDROID_FRAMEWORK_ALWAYS and TRACE_EVENT0_ALWAYS create a
SkAndroidFrameworkTraceUtilAlways, which formats the input, requiring a
string copy. Mimic the split in SkAndroidFrameworkTraceUtil, which has
two constructors; one for formatting, and one for static strings. This
allows skipping the copy when it's unnecessary.
Make TRACE_EVENT0_ALWAYS call the cheaper constructor, since it never
has formatted input. ATRACE_ANDROID_FRAMEWORK_ALWAYS has few callers;
only one of them forces an unnecessary copy.
Bug: b/224677119
Test: perfetto
Change-Id: I8e699ae1496c94e08e6f7fce3616254b1a627a7f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539896
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
ConstructorCompound::Make() will now turn `float2(myFloat2)` into the
equivalent simpler form `myFloat2` by simply returning the inner
argument as-is. In practice, this codepath is difficult to reach because
`Constructor::Convert` will interpret the above constructor as a cast,
and ConstructorCompoundCast already had a similar optimization in place
(to eliminate same-type casts).
This CL seems large but it's mostly comments. (`is_safe_to_eliminate`
could easily be a single-line return statement if the goal were to
minimize LOC!)
Change-Id: If75d1583e5fe27f7fd903175ebbd9185c67026f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539820
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, each Expression would clone itself at its existing position
and then the position would potentially be overwritten at the end. Now,
rewriting the position happens as part of the clone operation.
This seemed simpler than the previous two-step operation, and also opens
up the possibility of passing the updated position to the clone's
children (but I didn't experiment with that here.)
Change-Id: I07d0c0221fcab460d0db358d059ceecf8a0a7dc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539571
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
`optimize_constructor_swizzle` already does the same transformation, but
in a far more complex way. In a followup CL, I intend to limit
`optimize_constructor_swizzle` to only operating on ConstructorCompound
objects.
Change-Id: Ieb4210a9ea9efb1a021c4b2e15ba55863000ef93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539819
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We added a `clone(Position pos)` overload late; this CL goes back and
uses it in all the places that needed it.
Change-Id: I6924ac9a91ebbd148531dccd25218d6f497ed0d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539568
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In cases where we know the type of constructor being made, it is much
more efficient to create it directly, instead of relying on
Constructor::Convert to deduce the type. This also helps us catch
legitimate bugs that Constructor::Convert will paper over; e.g. Convert
will happily accept mismatched types and just cast them (which was
occurring here, due to calling `scalarTypeForLiteral` in the wrong
spot).
Change-Id: I972046e14cc78708a3f1b8a4a31016d91e60a431
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539508
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
In follow up changes, the PatchAllocator duck type will get a little
more involved with tessellation-specific code, so I figured it'd be good
to buffer it with this new type between GrVertexChunkBuilder (which can
be used more generally if we update ops to do so).
Bug: skia:13012
Change-Id: Ib51d7beceba98cb10678dd04b431e923518ed6d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539037
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
OpenGL docs specifically insist that the sequence (comma) operator
should not be treated as a constant-expression so that attempts to
declare multidimensional arrays with a comma will fail:
http://screen/vJEpAe9yNmbzZTm
(See "12.43 Sequence operator and constant expressions" in the OpenGL
ES3 documentation or read skia:13311 for details.)
In practice, we don't get much benefit from optimizing away unused
comma-expressions; it improves some synthetic tests, but realistically
this will not help Skia in any real-world scenario. The constant folder
no longer attempts this optimization, and comma-expressions are now
rejected in a constant-expression context.
Change-Id: Ic5dea6ff90e36614b548c1ce89a444e81da944ae
Bug: skia:13311
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539565
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, this slide used a bunch of code to render a gray ramp. My
first impression was that the test slide was broken, but this was the
actual intent. We now use the shaders.skia.org zoomy-neurons.
Change-Id: Icd08462e30ab328b533525d74fa6064bccb76e8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539202
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Most shader text displayed in SkSL mode is passed through, exactly as
written--comments and all--and is unaffected by the optimization
setting. However, runtime effects go through the pipeline stage which
can subject them to optimizations. This is unexpected in "SkSL" mode,
where the goal is to display the shader as close to the original input
as possible.
Now, when Viewer displays a runtime effect in "SkSL" mode, it will not
have optimizations applied (other than some basic stuff like constant
folding, which is currently non-negotiable in SkSL).
Change-Id: I9bd3d8f7067d7124b3fbe88e8a422212b645597a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539200
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This includes using SkNoPixelsDevice for the GlyphTrackingDevice
in the SkChromeRemoteGlyphCache because it is the only thing that
uses SkGlyphRunPainter outside of GPU.
Change-Id: Id22e538b2c1aa0d65433878e428c9465fe222d12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539558
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
GLSL/SkSL allow creation of a uniform scale matrix by constructing a
matrix with a single scalar argument. It will be splatted across the
matrix's diagonal.
Change-Id: If3410909f8136f3f4c39f7d7208d5bd6066e4e53
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539047
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Looks like really old drivers struggled with weird no-op blocks.
Change-Id: Ie32754a9c221eb7c20924ee27e5facca7e9701f0
Bug: skia:13309
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539561
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The conceptual meat of this change is that instead of taking a local-to-
device scale factor and the local stroke width, CalcNumRadialSegments
now takes the approximated device-space stroke radius. The main benefit
of this is that hairlines can be defined exactly without needing to
invert the local-to-device matrix or otherwise estimate a local-space
radius that would transform to the 1px hairline. The actual math for
regular strokes remains the same (scale factor * radius), but is moved.
In addition, this CL fixes a bug I had introduced in
https://skia-review.googlesource.com/c/skia/+/502059/
where I had not realized the CPU code operated on stroke *width* and the
GPU code operated on stroke *radius*, so updating them to use the same
equations w/o changing their inputs actually caused us to overestimate
the number of radial segments required. With this change, both the CPU
code and GPU code operate on stroke radius.
Between that bug fix and simplifying the hairline stroke radius math,
I am expecting lots of slight AA changes around stroked paths.
Lastly, I entirely removed the StrokeTolerancesBuffer, which was a class
that buffered up calculations of CalcNumRadialSegments by looking at the
entire list of paths in an op and putting them into a 4-lane SIMD
function instead. This isn't currently available for Graphite and makes
it harder to consolidate the radial segment calculations. Locally I
found no noticeable difference in the patch writing microbenchmarks
(where it would be more significant), and somehow the motionmark SKPs
were actually a touch faster. I'm hopeful that we can drop this
complexity and not take a large performance hit.
Bug: skia:13056, skia:13012
Change-Id: If16103078ebe9e4e60470445c0868deb3904f7de
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537239
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The fuzzer found a subtle bug where our line vs. rect clipping could
produce points outside of the rect. Thanks to floating poing inaccuracy,
the first (horizontal) clip would produce a point just outside the
bounds. Then, we'd re-clip against the vertical segments - but go back
and start from the original points. Again, thanks to floating point,
we'd produce a point just outside the bounds.
The fix here is straightforward: After we've done the first clip
(reducing the magnitude of the values involved, and constraining the X
coordinates), use *that* as the new endpoints for the second clip.
Mathematically this is identical (the tmp values are new upper bounds on
the extents of the final endpoints after the first clip), but the math
is more stable this way.
Bug: chromium:1320467
Change-Id: I9c816110edb2944544243f2ad89665d5bc5c9c53
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539196
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I2a1fedbbaabd57e4546c2a628f6d297e11f4244b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539318
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This CL define a const label string in GrDrawOpAtlas and have the
constructor accept it. This will help us to label atlas.
Bug: chromium:1164111
Change-Id: I6e194b9a59b940b359887736df27c1da130b1d8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539136
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is just a cascade of changes from removing the (unused) 'flushTimeOpsTask' parameter from SurfaceDrawContext::Make.
Change-Id: Ia92dfada8df007761d9de3c16e6a9c8850c3d399
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539066
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This made sense when literals were spread across multiple classes
(FloatLiteral/IntLiteral/BoolLiteral). Nowadays we can just call
Literal::Make and pass the Type that we want our literal to be.
Change-Id: I54ce5fcf7bcadac4be1fe10a358b9c86c234de8f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539397
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We will continue to fold expressions like `10 + 0` or `5 * 1` or
`PI * 1` (assuming PI is a const float) because both sides are known,
but non-constant-expressions like `x + 0` (assuming x is not a constant)
or `foo *= 1` will be left as-is when the optimizer is off.
This improves the accuracy of the Viewer shader tab, because a runtime
effect expression like `color *= scale` will be preserved in the final
output when optimization is off, instead of being replaced with a Nop
when scale equals one.
Change-Id: I218b327cb0cd12654dca446dee8a5baa96f589b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539197
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were already running unit tests twice--once with inlining on, and
once with inlining off, but always with optimization on. Now, the two
passes are instead "inliner + optimizer both on" and "inliner +
optimizer both off". (Testing the full 2x2 matrix of possibilities seems
like overkill.)
Change-Id: I73fb0932877c7b953c1f2e122125e5e7781a8582
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539396
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Fixes a minor issue discovered by http://review.skia.org/539198 .
Change-Id: I63f555cc005df33ce50c412796a8c773a501d271
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539199
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously we didn't have any mechanism for disabling the optimizer when
building golden outputs, so every golden output always had optimizations
applied.
Change-Id: I8f370b06daab6cb50bb4339eab0d39578621413b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539198
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Some APIs have been const-ified.
Change-Id: I08699f219944057bd730d3f7a5f5466cdd829233
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539064
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 167e608bb3.
Reason for revert: breaking Housekeeper-CheckGeneratedFiles
(fetch-clang-format)
Original change's description:
> [infra] Remove old python scripts and urllib2 references
>
> I was searching for urllib2 while resolving issues with
> https://skia-review.googlesource.com/c/skia/+/538636
> when I found several old, apparently unused scripts.
>
> Rather than fix them, let's get rid of them. If they
> are still in use, the conversion to urllib.request is
> pretty easy.
>
> Change-Id: I27d419601e81c93a3d53e280188a379dfab927c4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538936
> Auto-Submit: Kevin Lubick <kjlubick@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
> Commit-Queue: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Ravi Mistry <rmistry@google.com>
Change-Id: I139a3cd070da2e6fee2f8865138cf6a63441a8cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539201
Auto-Submit: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This comment fell out-of-date when the Graphite modules were added.
(I had tweaked it slightly at the time, but had missed some other places
that talked about injecting static code snippets.)
Change-Id: Ibbdf5ce14b8c77393f9da2ee22c82001e3bfd6c3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539045
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Make it easier to figure out READ_FAILURE is reporting a problem.
Bug: chromium:1287356
Change-Id: I3962932aed5c55efc2ded941cf218e5a8e75e972
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539044
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
