Once triangulated paths are added this will no longer just be storing proxy views.
Bug: 1108408
Change-Id: I82fa47b0b85f738d9a25330c29bc2892c9bfeda4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323999
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This will enable us to use SkTHashMap to store our definition maps.
Change-Id: I6017dfa71e1c5e68a20c97e955bb3d3abf347f0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323891
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This removes VarDeclarationsStatement entirely. VarDeclaration instances
appear directly as statements in Programs. SkSL that declares multiple
variables in a single declaration is transformed to represent that as a
series of VarDeclaration statements.
Similarly, global variable declarations are represented by
GlobalVarDeclaration program elements, one per variable.
Bug: skia:10806
Change-Id: Idd8a2d971a8217733ed57f0dd2249d62f2f0e9c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323102
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also, remove unused #include of SkBitSet.
Change-Id: Ib1b903f78e835a75c8ba88ac35bfa270df7bc0e3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322681
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We'll also be needing this helper for HW-generated blur mask caching
Bug: 1108408
Change-Id: I60d91ae8864239f0cf68830d0a5b4266d27545d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323109
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 57c37ad0e4.
Reason for revert: Breaking the Mali400 bots.
Original change's description:
> Add a 2d cross product intrinsic to sksl
>
> Change-Id: Iebaf4616665547d6ca4900e1247d5b68e0f3512a
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321790
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
TBR=brianosman@google.com,csmartdalton@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I3e1aa251e883e3d2a1170b0fc6cdc84ea06e784a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323556
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
and begin using it for cached SW-generated blur masks.
This is needed to begin mixing and matching HW & SW-generated blur
masks since they have different draw-rects.
It will also be useful if/when we add support for triangulated paths
to the thread-safe cache.
Bug: 1108408
Change-Id: I085ad1127dc2deb98b35d704b06e50b27c72fd1c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322657
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
I wrote code that called SkAutoTArray::data() and discovered that it
was broken, but not generating compile errors because it's part of a
template and never instantiated anywhere else. I fixed the
implementation and added it to our container unit test to prevent later
regression. This revealed another issue, that "containers in
SkTemplates.h [should] all have a consistent api", according to
test_container_apis. However, data() was never added to the non-array
container APIs. So I added data() to the other containers as well.
Change-Id: I52532c91fdab3fc8c4539053ba8420815b7b0ee5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/323276
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Iebaf4616665547d6ca4900e1247d5b68e0f3512a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321790
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Identical to https://skia-review.googlesource.com/c/skia/+/286070.
Bug: skia:10165
Cq-Include-Trybots: skia/skia.primary:Build-Debian10-Clang-arm-Debug-Chromebook_GLES;skia/skia.primary:Test-Mac10.13-Clang-MacBookPro11.5-CPU-AVX2-x86_64-Release-All-TSAN,Build-Debian10-Clang-arm64-Debug-Android_ASAN
Change-Id: I0ec9d5f6875768e665f444e1ada211d3da537678
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322976
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Adds a new helper that creates a GrBackendRenderTarget using
GrGpu and then wraps it in a SkSurface. Uses the SkSurface
release proc to delete the BERT using GrGpu.
Upgrades GrGpu::createTestingOnlyBackendRenderTarget to create MSAA
buffers.
Updates many tests/tool to call sites to use the helper instead of
SkSurface::MakeFromBackendTextureAsRenderTarget.
Adds syncToCpu bool to SkSurface:: and GrContext::flushAndSubmit.
Bug: skia:9832
Change-Id: I73a8f0ce09dc6523729af0814464c6b6657fda06
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/293683
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Added a flag to switch from using the memory pool to
using new and delete for GrOp allocation.
Just add the following to your gn args.
extra_cflags = [
"-DGR_OP_ALLOCATE_USE_NEW",
]
Change-Id: Icea4a6df047cff2cd5e50032f0bd4b714a5740d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322625
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This lets more matrix types pre-convert rects and rrects to device space.
Since the clip geometry isn't itself shaded, we can apply the matrix
without worrying about preserving local vs. device coordinates.
Bug: skia:10730
Change-Id: I61ae3e13eec66f0e5eb83a6504dcb8004620b151
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320222
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is a reland of c1916c34fe
As it turns out, benches are not always given a canvas.
Original change's description:
> Remove use of legacy display globals.
>
> In the ongoing effort to remove the display globals from Skia, allow
> their use only if SK_LEGACY_SURFACE_PROPS is defined. Do not define this
> in a normal Skia build and remove all use from Skia code.
>
> Change-Id: I9ff550f5db246b9024aac687a1bc01321f1be4c8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319343
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: I61a2ac058fafc99653e3304876cf4b97350dac8b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322490
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This takes the "one pass" code path more often, using less memory, as it
does not have to allocate an intermediate width*height pixel buffer.
Wuffs v0.2 did not support SRC_OVER, only SRC, but Wuffs v0.3 does.
The gif-transparent-index.gif test file comes from the
test/data/artificial directory of the github.com/google/wuffs
repository. It was programmatically generated.
The new GifTest.cpp test passes with skia_use_wuffs true or false, with
or without the SkWuffsCodec.cpp change.
Change-Id: I46fb4c849319fbefc39f331416a8b7d3836093ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320116
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This reverts commit c1916c34fe.
Reason for revert: Bots unhappy
Original change's description:
> Remove use of legacy display globals.
>
> In the ongoing effort to remove the display globals from Skia, allow
> their use only if SK_LEGACY_SURFACE_PROPS is defined. Do not define this
> in a normal Skia build and remove all use from Skia code.
>
> Change-Id: I9ff550f5db246b9024aac687a1bc01321f1be4c8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319343
> Reviewed-by: Herb Derby <herb@google.com>
> Commit-Queue: Ben Wagner <bungeman@google.com>
TBR=djsollen@google.com,bungeman@google.com,herb@google.com,reed@google.com
Change-Id: I365d2b1d19241a90130bc1b59663651817966f63
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/322400
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
In the ongoing effort to remove the display globals from Skia, allow
their use only if SK_LEGACY_SURFACE_PROPS is defined. Do not define this
in a normal Skia build and remove all use from Skia code.
Change-Id: I9ff550f5db246b9024aac687a1bc01321f1be4c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319343
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Just a typo fix.
Change-Id: I2fe1f6ae1c99d7f20a4fa5f49eefea514e224652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321977
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This greatly improves the output from a profiler. It makes it much
easier to determine how much time is spent in searching for candidates,
versus actually inlining them.
It also improves the code readability somewhat by breaking a large
monolithic function into several smaller functions.
Change-Id: I1b3ef6ddbe46af60e673f37ded766f8077ed6b03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321376
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We were letting this get further, then asserting.
Bug: skia:10797
Change-Id: Iff6fe43aa32450b5a517c94773031d593f1f62a2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321794
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
As a first step, convert sksl_pipeline.sksl to an IntrinsicMap (rather
than inherited element list). This makes the new code operate on
sk_FragCoord (which was previously being shared by all runtime effect
programs).
The new unit test angered TSAN, and now runs without complaint.
Also finish converting the .fp intrinsics over, so those don't need an
inherited element list either. And while doing that, refactor that
parsing to match all of the others. FP was uniquely implementing
processIncludeFile itself, rather than reusing the pattern of other
pre-include parsing.
The meat of the CL is the subtle changes in Compiler, and the logic in
cloneBuiltinVariables. Note that we need to clone the global variable
declaration element (because one of the goals is to get rid of shared
and inherited program elements), but also the variable itself (and the
new copy needs to live in the program's symbol table).
Bug: skia:10589
Bug: skia:10679
Bug: skia:10680
Change-Id: Ied352f8434dac2b8eacb4e515b014b6af7b57d20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319023
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Part of this change is to move some of this static format information
off of GrCaps since it is not cap dependent in anyway. This allows us
to the need for caps in many places. Also changes the low level format
query to be based off of bytes per block so it can be shared for
compressed and non compressed formats.
This change will also make it easier to add stencil/depth formats in
follow on change since we don't have to fill in a whole caps
FormatInfo block just so we can get the bytesPerPixel which is all
they need.
Bug: skia:10727
Change-Id: I2e6fdabf3ed699b4145ef9e6f0a73078d32a0444
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321463
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Instead of being passed separately.
Check sample count in various onWrap methods in GrMtlGpu.
Bug: skia:9832
Cq-Include-Trybots: luci.skia.skia.primary:Test-iOS-Clang-iPhone8-GPU-AppleA11-arm64-Release-All-Metal,Test-iOS-Clang-iPhone7-GPU-PowerVRGT7600-arm64-Release-All-Metal,Test-iOS-Clang-iPadPro-GPU-PowerVRGT7800-arm64-Release-All-Metal,Test-iOS-Clang-iPhone11-GPU-AppleA13-arm64-Release-All-Metal,Test-iOS-Clang-iPhone6-GPU-PowerVRGX6450-arm64-Release-All-Metal,Test-Mac10.15-Clang-MacBookAir7.2-GPU-IntelHD6000-x86_64-Release-All-Metal
Change-Id: If5813db95b07f5d272e80920486f461cc5a587fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320956
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
We don't need to create a temporary variable for expressions like
`half3(x)`.
Change-Id: Ie0fa6a6dfb3d77d4372f96c676d3081f7e278852
Bug: skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320960
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
For instance, `foo[0].x` is now considered trivial to inline. It
combines two trivial cases: array-indexing by an int literal, and a
swizzle.
Change-Id: Ibb3ca1f324bbee0e9b3556e66644923fc9e0cf45
Bug: skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320768
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 4cf00a814f.
Reason for revert: ANGLE and Mali 400 unhappy.
Original change's description:
> Make GrRRectBlurEffect use the thread-safe uniquely-keyed view cache
>
> This yields a ~8% performance improvement on OOP-R/DDL-like rendering of the simpleblurroundrect GM.
>
> Bug: 1108408
> Change-Id: I1ec9477dffe870e5973f8a334a65b1013a4ca3dd
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311720
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>
TBR=egdaniel@google.com,robertphillips@google.com,adlai@google.com
Change-Id: Ib7135a2244b956d1a5d06c12a2b4c2ce398b9db2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1108408
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320817
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The following types of expression are hoisted directly into the
inlined code:
- Struct field access: `myStruct.myField`
- Swizzles: `myVector.xzy`
- Simple array indexes: `myArray[0]`
This significantly reduces the number of temporary variables generated
by the inliner.
Change-Id: Ifed226ecc87b096ec1e38752c0c38ae32bd31578
Bug: skia:10737, skia:10786
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319919
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This yields a ~8% performance improvement on OOP-R/DDL-like rendering of the simpleblurroundrect GM.
Bug: 1108408
Change-Id: I1ec9477dffe870e5973f8a334a65b1013a4ca3dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311720
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:10785
Change-Id: I01708af63d7e2ffc160022074ea9ff2b3c69eab5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320638
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Pre-req for allowing rendering to an externally created MSSA VkImage.
Bug: skia:9832
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Release-All-Vulkan,Test-Win10-Clang-NUC8i5BEK-GPU-IntelIris655-x86_64-Release-All-Vulkan,Test-Win10-Clang-ShuttleA-GPU-RadeonHD7770-x86_64-Release-All-Vulkan,Test-Android-Clang-GalaxyS9-GPU-MaliG72-arm64-Release-All-Android_Vulkan,Test-Android-Clang-GalaxyS7_G930FD-GPU-MaliT880-arm64-Release-All-Android_Vulkan,Test-Android-Clang-GalaxyS20-GPU-MaliG77-arm64-Release-All-Android_Vulkan,Test-Android-Clang-Pixel4XL-GPU-Adreno640-arm64-Release-All-Android_Vulkan,Test-Android-Clang-P30-GPU-MaliG76-arm64-Release-All-Android_Vulkan
Change-Id: Ibf41944c6946dda7e27bdcd509ecd04976fc9ade
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320262
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is the second CL for isolating the Chrome
remote glyph cache API from the implementation.
See CL/320074 for the first CL, which handles
SkStrikeClient.
Pull out the rest of the tracing functionality.
Change-Id: I4d6aa4bf648a0d2d55cecedbe445a05c27e3a986
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320256
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
The right side of assignments will collapse redundant swizzles, but the
left side does not. Code like this can actually cause the ByteCode-
Generator to assert if it is run (it asserts in `swizzle_is_simple`
during code generation).
Change-Id: I891912fe0b5de2670dfa95f6702a86d5c42bb2ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320296
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This was adapted from a test in SkSLInterpreterOutParams and presents a
challenging double swizzle.
Change-Id: Icb7b3bbb18d4b3cfa0c26acb524c08812ba88096
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319920
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkGlyphRect is a rectangle encoding specialized for
union and intersect. It will be used for calculating
the bounding boxes of glyph runs, and clipping glyphs
for GPU.
Change-Id: Icab826b51dc2254ee4006ada84f7fc09e112a933
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319697
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This allows dead-stripping to properly optimize away unreferenced clones
of intrinsic functions, and allows the inliner to detect intrinsic
functions that are only called once (which can generally always be
inlined without penalty).
Change-Id: I0cf034d880ae5d52f4cc0f93de6e2c7aad34e975
Bug: skia:10776
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320258
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of 435b482638
inlineStatement now takes a `const Expression* resultExpr` instead of
`const Expression& resultExpr` because resultExpr will be null for a
void function.
Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:10756
Change-Id: I35f76c21eccf0ba2ab47e4313e131f7aa26980fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320223
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 435b482638.
Reason for revert: ASAN/UBSAN unhappy.
Original change's description:
> Support out parameters that use a swizzle.
>
> This CL also removes the `VariableExpression` class that was briefly
> added in a prior CL. This class was intended to support cloning an
> expression while changing the refKind of a VariableReference inside of
> the expression, but it added state and complexity. In this CL, rather
> than track this via extra state, the inliner just recurses into the
> expression as needed to find its VariableReference. Since most relevant
> expressions are just a VariableReference anyway, this is inexpensive.
>
> Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
> Bug: skia:10756
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ibdda47607f9e6e7f3a7459915067cf5e20919993
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/320220
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This CL also removes the `VariableExpression` class that was briefly
added in a prior CL. This class was intended to support cloning an
expression while changing the refKind of a VariableReference inside of
the expression, but it added state and complexity. In this CL, rather
than track this via extra state, the inliner just recurses into the
expression as needed to find its VariableReference. Since most relevant
expressions are just a VariableReference anyway, this is inexpensive.
Change-Id: Id4d926b7d7520b5e6ce455446c05a6d59ef62a84
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319917
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Swizzles in combination with out params are currently broken in SkSL.
They are fixed in the followup CL at http://review.skia.org/319917
Change-Id: I22d9436a15631e6ee2acf9fc312a8634cf3b5407
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319918
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
When mapping bounds in the forward direction, return the target rect of
the picture (fCropRect). This matches the behavior of SkImageSource
which can be considered to be semantically similar.
Bug: skia:10744
Change-Id: Ief213a847041ea6b276b2e7493e49db01715eda9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319616
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This tweak makes these conversion functions' names
match the names of the types, e.g. to_F32() makes F32.
Change-Id: I4d71c9bd17d835d09375e3343ee4316082b02889
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319763
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
GLSL does not support assigning to ternaries, and will fail to compile
and/or generate non-functional shaders if we pass in a shader that tries
to assign into a ternary expression.
If SkSL is able to completely eliminate the ternary (e.g. if it boils
down to a simple `true ? x : y` or `false ? x : y`), SkSL can strip out
the ternary entirely and generate valid GLSL. This case is harmless and
so it is still allowed.
Change-Id: I960f119fb9934f998697634e6c4e519cd77d3780
Bug: skia:10767
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319679
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I452e52a87d89cefb5c21a0d9d57e9771f3038d73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319783
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
When reverse mapping bounds through SkDisplacementMapEffect, we need to
consider both the color input and the displacement input because they
could both directly, or indirectly, reference the source.
Bug: chromium:1128962
Change-Id: I03489fd2391f2ea595945961bf7e940a0e200cab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319656
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This was unused and did not work on non-GLSL backends.
Change-Id: I6bd314d43cfefa64871b5c0e964b5ae52e494164
Bug: skia:10757
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319778
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
This will allow the inliner to use IsAssignable.
Change-Id: Ic94f71002779b53d0b3dc97f37fbe4bb98b026d8
Bug: skia:10756
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319414
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL implements and exercises the preference for gpu-generated content.
This CL also switches to drawing a rect (vs. an arrow) since drawing
a concave path on the gpu can be fraught.
Bug: 1108408
Change-Id: Ieec1619b5357ffb31aa74b471ea09c061bd8f74e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319416
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
These tests will also be used for Metal and SPIR-V testing. A small
handful of GLSL-specific stragglers (#version-specific or type-precision
related) will remain in /glsl/.
Change-Id: I7f2b2bd92825c327922c8ce74e438d2daa440dff
Bug: skia:10649
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319408
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
I've been thinking and rethinking and rethinking how best to use 16-bit
values like Q14 fixed-point in SkVM. Here's some ways:
A) don't... just use 32-bit values instead
B) use 16x2-bit pairs to match the narrower 32-bit lane count
C) double-pump 32-bit values to match the wider 16-bit lane count
D) use native 16- and 32-bit values and let the backends sort it out
A) is how things work today, and C) is how SkRasterPipeline's lowp mode
works. Having tried out B) and C) both for a good fair shake, they were
both already awkward to work with after writing just a few functions. I
would not give up on them entirely, but they're no longer my favorites.
D) is subtle and my new favorite. It's easiest to program with SkVM
when the values we're holding represent single values and the backend
handles any parallelism for us. That suggests we add a simple 16-bit
Q14 to the existing 32-bit I32 and F32 types, where they can be actively
converted between as normal, but not freely no-op bit punned. D) says
we people shouldn't have to choose between A-C) up front... each backend
can handle it themselves.
Under strategy D), it's entirely the backend's job to decide how to
represent each value, and how to to vectorize them. We don't need to
know as a user, and the backends can use the program itself to inform
how they vectorize. 16-bit values could live in xmm registers and
32-bit values in ymm, or the 16-bit values could go in the low half of a
ymm, or the even lanes of a ymm, or a full ymm and use two for 32-bit
values, etc. etc. This all is a backend choice, not something we should
have to know about when writing a program using Q14/I32/F32.
My next steps are to get Q14 operations tested and plumbed through the
JIT again, and to build out a blitter and a few effects using Q14 color
channels. Then, independently, we can look at each backend and how to
vectorize them. Some ideas:
1) keep running at current vectorization, with half rate 16-bit ops
2) pump up to 2x wider vectorization unconditionally to favor 16-bit
3) pump up to 2x wider vectorization only when any 16-bit op is used
These choices can be made independently for each backend (JIT, LLVM,
interp), and I wouldn't be surprised to find that we'll want to do them
differently. For instance, the interpreter is already running at 32x
vectorization... might be pumping it higher won't help anything.
Change-Id: Ib8ad2b1bf790e8c4e3acfb4818d4032f7628e8f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319321
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
We don't want to be polluting the global namespace with external values,
especially when the typical/recommended way to use the Compiler is with
a single long-lived instance. Force client code to manage ownership (the
only non-unit-test case was already doing this), and pass external
values to convertProgram, so they can be added to the Program's symbol
table.
Change-Id: If4c1db5e48a62e2cf4333b8d80420f2dfede27ab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319125
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
If the blob is empty, then try to regenerate it. Using
this method caused a slowdown in Skia perf, so we
added an extra check to allow some empty blobs through
for perf performance. The perf problem was caused by
SKPs generate empty blobs because of font mismatches.
Flutter has shown that scaling from very small to
normal size is not correctly handled by the existing
check. This CL favors correctness over optimizing empty
text blob and always regenerates empty blobs.
https://github.com/flutter/flutter/issues/64936
Change-Id: Ib18ecb684b0af5cf6dce274b6dc09a9c61b17c77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319031
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Many calls to `setRefKind` failed to check the return value; if it's
false, an error has occurred and the program is in a bad state.
Specifically, there is an assignment to a variable that's not marked as
"written-to." If we continue processing the program, we're likely to
assert.
Change-Id: I2dd5d1f41aa5ca0d30f8d638f05fe2e838216d78
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Adds various optimizations to GrWangsFormula as well as "pow4" variants
of the formula that are quicker than the standard and/or log2 versions.
Uses the pow4 variants in GrStrokePatchBuilder.
Bug: skia:10419
Change-Id: I8478582df5296b088d25808bcaeb93107ff20797
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318954
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
These cover:
- Properly configured out-params
- Invalid/non-lvalue out-params, which currently cause an SkSL crash
- Interactions between the inliner and variable swizzles
Change-Id: I4874101236084f273e704d8717149b431d813883
Bug: skia:10753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319036
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The code here is resuscitated from the GLSL testing harness at
http://review.skia.org/317771 . Testing and debugging in dm is simpler
than debugging skslc when we encounter compiler issues.
Change-Id: I492dd0bbd2f0ee0b3b6a1b5253da54d72dc1eb3b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319032
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
(Removed one test, SkSLFPSwitchWithMultipleReturnsInside, because it was
redundant with existing tests.)
Change-Id: I1bfc069babdb5eb0cc515f195c3a2e307bb5871a
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319029
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This appears to fix the performance regressions seen on a number of webpage
SKPs for vkmsaa8 and glmsaa targets on devices that can't disable multisampling.
Bug: skia:10730
Change-Id: I2c10a78cfe864f987b7f17eb69eb2e1712ca0676
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317772
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
FP gencode reads back matrix values in its dumpInfo method using rc().
This worked for SkM44; now it will work for SkMatrix as well.
Change-Id: I4efc7018529bd3c84aacc073fad2bfca7e12c517
Bug: skia:10748
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318756
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Cleaned up some nearby code while implementing this fix as well.
Change-Id: Ic084451f0d9fc12169e1720a8889a290249eb5e9
Bug: skia:10750
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318796
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, we copied intrinsic functions in a totally arbitrary order;
it used an unordered_set of pointers, so it could be affected by
switching standard libraries OR by malloc nondeterminism. (Surprisingly,
it was fairly consistent in practice on OS X/Linux.) This CL sorts the
intrinsic functions into a consistent order before copying them.
Change-Id: If90342bb77a9ae237a3ce91be3a9847311a722c4
Bug: skia:10749
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318700
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
I broke this when I switched over to the SkTDynamicHash
Bug: 1108408
Change-Id: I906b41330440d084ba2e97c0162af38a79da0529
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318696
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Code like
bool4 result = val.xy01;
Will now be converted to:
bvec4 result = bvec4(val.xy, bool(0), bool(1));
Previously it tried to do this, but there isn't an implicit conversion
from int to bool, so it was silently failing and adding nulls into the
constructor:
bvec4 result = bvec4(val.xy, $coerceToBool(0), $coerceToBool(1));
This CL also cleans up some related code that I was checking while
trying to understand the nature of the error.
Change-Id: I5b7d96760a03170ff78b46251c4182cc4e89836f
Bug: oss-fuzz:25781
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318636
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This test currently crashes skslc, but will be fixed in the followup CL.
Change-Id: I3683d94a310242e8ca67560296518fd1223b28d0
Bug: oss-fuzz:25781
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318656
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Ideally the inliner would be smart enough to avoid creating a temporary
at all just for a swizzle, but a good first step is to create fewer of
them.
Change-Id: Icd6f86c294237488f7923dc787bb64a5f99bd0ac
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318213
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Early returns can cause the inliner to generate suboptimal code. We
control our built-ins, so let's avoid them where we can.
Patterns like this challenge the inliner:
if (x) return y;
return z;
But this can be replaced by equivalent code that inlines better:
return x ? y : z;
Or, if a ternary can't be used, this also does a better job:
if (x) return y;
else return z;
In several cases, this allows the inliner to avoid generating a
do-while(false) block for control flow.
Change-Id: I921c929122297c40476ff15b4da631fc1581e308
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318211
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I realized that "DefaultSettings" as a name suffix was unclear, because
"Default" is a different settings mode from skslc running with
--nosettings.
In --nosettings mode, skslc uses "standalone" settings.
Change-Id: I1f5d80df0a21cec55948c4ad146169bcb34f4999
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318210
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Several blend functions generate a surprisingly large amount of code,
and appear to have opportunities for further optimization. At any rate,
if we make compiler changes that would affect the output of a blend
function, I think we would want to see the changes reflected in our
golden outputs.
Change-Id: Iff612dcd4bad8824b5e6e97413ffce19e5ea1c0e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318336
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Passing in VK_IMAGE_LAYOUT_UNDEFINED will tell Skia to not update the
layout when doing the state change.
Bug: skia:10742
Change-Id: Ic59b7c95d3a73e29dcd6eec16a2fd138e1a1d95f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318204
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
These are no longer needed now that we've migrated away.
Change-Id: Id308af7d40ffe0d539a3c6fd201220d145080928
Docs-Preview: https://skia.org/?cl=317281
Cq-Include-Trybots: luci.skia.skia.primary:Canary-Android,Canary-G3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317281
Commit-Queue: Adlai Holler <adlai@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Should reduce build time and avoid running GL unit tests redundantly
on Vulkan bots.
Change-Id: I67379e6820dab2f038175a825ee1461086a2da84
Cq-Include-Trybots: luci.skia.skia.primary:Test-Android-Clang-Pixel4XL-GPU-Adreno640-arm64-Release-All-Android_Vulkan
Change-Id: I67379e6820dab2f038175a825ee1461086a2da84
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/318196
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This wasn't rolled back automatically by the revert at
http://review.skia.org/317976, because this unit test did not exist yet
when that CL was submitted.
Change-Id: Ib887e74ddd32c1e576908bbe3d588205e26cdaa5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317978
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 910845fac1.
Reason for revert: IRGenerator inline change reverted
Original change's description:
> Add program-settings flag to disable the inliner.
>
> Change-Id: I6c4e7f6a2aab6710221029022a3a5f3ec323c5e2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317856
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ie38a29495ea8497f9db26d2603df179e696ac5ff
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317977
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 21d7778cb5.
Reason for revert: Pinpoint absolutely hates this change
Original change's description:
> Remove inliner from IR generation stage.
>
> There is no need to inline code during IR generation, as the optimizer
> can now handle this.
>
> Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
> Commit-Queue: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: I62c235415bcdc92a088e2a7f9c3d7dbf7e1bf669
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317976
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This resolves the following TODO block:
TODO(johnstiles): the skslc standalone caps bits do not enable
do-while support, so this test does not actually perform as
described; the `returny` function is not inlined at all. This will
be fixed when customizable caps-bit support is added to the golden
tests.
Change-Id: I3495e4813b9be37264a8fda978453594c1f5fa13
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317859
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Ideally the optimizer should be able to detect and remove this loop.
This CL establishes a baseline.
Change-Id: I6aba0b52fe49552f170fca25d81c29c515044ef5
Bug: skia:10737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317861
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I6c4e7f6a2aab6710221029022a3a5f3ec323c5e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317856
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I72fd3083f75ca5bf74fb2c3b032465864a13aed5
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317771
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
There is no need to inline code during IR generation, as the optimizer
can now handle this.
Change-Id: If272bfb98e945a75ec91fb4aa026e5631ac51b5b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315971
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL:
connects the tests to the live cache
switches the cache over to using an SkTDynamicHash
Bug: 1108408
Change-Id: I1876baf13a8d12a1ec398f49e2b2d51f434d6d0e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317764
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: I01c150d6bfcdd1500033521a87c058c7428c3521
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317769
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I35ff25c4cc394c1a4a964207ece87095a9ba84cf
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317767
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
A stroke that starts and ends at the same point is not the same as a
stroke with an explicit close.
Bug: skia:10419
Change-Id: Ibd8b6cd4ba04b2b9acf3dee6d01ad88ba6ba7071
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317650
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Change-Id: Ic8f4730d035981c32b4ddb48e5e919b0396b6d93
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317578
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
now the builder returns the new image.
Change-Id: Ie56256390b96d3fdbe39f89784276947047df656
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316442
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
The copy_constant function did not expect to see a prefix expression.
Change-Id: Id2beb64b8d65f6b35cd68306452104ba28f56f2b
Bug: skia:10734
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317616
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were allowing these ops on floating point types. The resulting GLSL
would fail to compile. We also allowed >>= and <<= on vectors (but not
any of the other bitwise ops).
The newly added unit tests were failing (eg, not catching errors). New
results are correct.
Bug: skia:10707
Change-Id: I97b769f1ce59261361109a71061b42dc8ef3c74b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317393
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I83a38f2c953a560fea3483e95e31df532b90773e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317456
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We now support building an SkSL golden output twice, once honoring the
custom #pragma settings, and once more ignoring the settings. This
allows us to see the output of the workaround technique, alongside the
"default-settings" output which should not contain a workaround.
To implement this, skslc now supports a flag: --[no]settings.
When it's set, /*#pragma settings*/ comments are honored. When it's not
set, skslc ignores the comments. compile_sksl_tests.py passes this flag
along to skslc.
This approach is not strictly limited to workarounds; the
"TypePrecision" GLSL test was also updated to use this technique.
Change-Id: I79204df047b024533ed6bc1f4c088e0e878d5bb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317246
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This will allow us to write skslc-based golden tests that deviate from
the standalone skslc settings.
This CL provides six options; more will be added as necessary.
- Default (caps)
- UsesPrecisionModifiers (caps)
- Version110 (caps)
- Version450Core (caps)
- ForceHighPrecision (settings flag bit)
- Sharpen (settings flag bit)
Change-Id: I9b779e039b2f0c0ccf43dca226fb4844aff3526b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317237
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
A handful of simplifications were made, but these hew very close to the
original tests and are intended to cover the exact same ground. The
remaining unconverted tests depend on non-default caps bits and will
be updated once caps handling in skslc is fully landed.
Change-Id: I3f3c29bf87f73e501561d7bfcdaabe8acc14b89f
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317390
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:10721
Change-Id: Ib9e51b736bafb6e4493db4f50e9835fbd86c415d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317247
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This fixes the perf bot crashes from https://skia-review.googlesource.com/c/skia/+/317209
and removes a few other instances of kReplace_ClipOp that are equivalent to the
still supported SkClipOp::kIntersect op. Other than the benchmark case, the GPU
clip stack wasn't hitting these cases but they do need to be updated when the deprecated
ops are fully removed.
At this point, the remaining uses of deprecated clip ops are all in unit tests that
specifically test a clip stack (raster, conservative, or skclipstack) that specifically
supports expanded clip ops and the tests are testing those operations. They can remain
until we remove full support.
Bug: skia:10208
Change-Id: I5703bf43fd41b6addf329190a70c5429d5971240
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317380
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This simplifies life when revising the swizzle logic.
Change-Id: I7fc63c0cc845c4741c17c82b3078040264b61ba0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317379
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I suspected this wouldn't work, but turns out it's fine.
Change-Id: I91042d458804f3a6af29389de5e6622a39cf23e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317309
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is by no means perfect or complete but does break up the review.
Bug: 1108408
Change-Id: Ib1826cd40975c7e84b5fdfc16d1ecbec97dab237
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317201
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
SkSL::Swizzle objects never represent constant swizzles - they are
directly converted to IR that represents a small sub-tree, up to the
most complex case of:
Swizzle(Constructor(Swizzle(BaseVector), Literals, ...))
GLSL was the only backend that handled arbitrary complex swizzles
correctly in all cases - this change basically moves that logic into the
IR generator, subsumes the (similar) handling of scalar swizzles that
was there, and simplifies the algorithm so that it's all procedural (not
hard-coded based on bit-patterns).
Bug: skia:10721
Change-Id: Iac96a26da517a7b1bdc9905cc38ad727fb68af12
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317238
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
`fBuiltinFMASupport` is now true on both, and
`fUsesPrecisionModifiers` is now false. Other mismatching flags exist,
but they are non-trivial to synchronize as they are tied to extension
strings.
This will help our skslc-based unit tests generate the same results as
our C++ unit tests did, but should not affect real-world results as
these defaults will all be overwritten in a non-testing scenario.
In practice, the `fUsesPrecisionModifiers` change is responsible for all
of the diffs below. The other flags did not change the results of any of
the currently-ported tests.
Change-Id: Ieb056d852b027fa87c56fd89f971a77a10a8a124
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317204
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
It shifts a negative number left,
which UBSAN reminds us is not allowed in C++.
Cq-Include-Trybots: luci.skia.skia.primary:Test-Debian10-Clang-GCE-CPU-AVX2-x86_64-Debug-All-SkVM_ASAN
Change-Id: I955d53b677f605b3a0f6e2bd31b16e2cc2f64ac5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317260
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These tests cover all the new Q14x2 ops.
As usual, left myself a few TODOs...
While working on this, I uncovered a little subtly about the blend
instructions we use to implement Op::select... unlike the platonic
(cond & true_val) | (~cond & false_val)
the hardware istructions are not bitwise! Each of the fancy blend
instructions like vblendvps or vpblendvb has a fundamental granularity
larger than a bit (4 bytes for vblendvps, 1 byte for vpblendvb). If
you're using a mask with a granularity of say, 2 bytes, you need to be
using something with equally fine granularity --- bitwise is ok,
bytewise is ok, 2-byte-wise is ok, but 4-byte-wise isn't.
Took a quick survey, and the Op::select we're using for x86 and ARM
JITs are both bytewise, so I think they're fine. Would have to think
a bit about LLVM, but these unit tests should at least fire if it were
wrong. The skvx if_then_else() I've been using in the interpreter has
been 4-byte-wise, but I'm refining that down to 1-byte-wise with
https://skia-review.googlesource.com/c/skia/+/317170.
Change-Id: I09cbc8b91cdb9e50088ee4f6ddf202faa1bf2cb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317159
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
If CCPR was enabled on the CLI, it would still be seen for the unit
test. This makes sure CCPR is fully disabled, in addition to the old
setting for disabling other path renderers.
The unit test needs CCPR disabled so that it tests SW mask generation
when CCPR is not available.
Change-Id: I3f728705238121fc179ffd9985f14edad3f8d96e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317236
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Ignores the miter limit if the join type is not kMiter.
Bug: skia:10419
Change-Id: Ib05895cf90c7bb0e25e9e8c3e26c13fef32f2e97
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317163
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Overview doc: https://docs.google.com/document/d/1ddIk74A1rL5Kj5kGcnInOYKVAXs3J2IsSgU5BLit0Ng/edit?usp=sharing
This is the new clip stack that will replace GrClipStackClip. The doc
link in the CL description has a much more detailed overview of what the
strategy of the new clip stack is, but at a very high level:
1. Add a temporary #define that lets SkGpuDevice switch between the old
stack and the new stack. For the new GrClipStack, it extends SkBaseDevice
directly and has to implement all of the device clipping virtuals.
- If you look from patchset 5 and earlier, the define defaults to on
so I can test it on the bots, etc. but the plan will be for it to
default to off when this lands so it's only running on unit tests.
Then in a follow up, I'll turn it on for our bots but keep it off in
chrome and android. If everything looks good, chrome can then be
turned on. There is a more extensive migration plan for android
because of the expanding clip ops, but that is covered at the end of
the overview doc.
2. GrClipStack manages save/restore logic of the stack and extends GrClip,
so the cpp file also includes code to apply a GrAppliedClip. At the moment
the apply strategy is as close to that in GrReducedClip and
GrClipStackClip as I could make it. Down the road, I think we can explore
other analytic coverage options and a clip atlas that replaces the unified
SW mask.
- Once GrClipStack is enabled everywhere, it means GrReducedClip and
GrClipStackClip can be deleted, so I'm not too worried about sharing
code between the two. A lot is already shared through the use of
GrSWMaskHelper and GrStencilMaskHelper.
- SkClipStack and SkClipStackDevice are still used by the PDF and SVG
backends, so they aren't necessarily deletable.
3. The GrClipStack only handles intersect and difference ops. It
represents all geometric clip operations as an element. The stack itself
is controlled by the "save record", which tracks aggregate bounds, valid
elements, and the non-geometric clip shader.
- When a new save record is pushed on the stack, older elements are
inactive. This means they cannot be modified, since they may need to
be activated again when the current save is popped off the stack.
However, they can still affect the clip during application.
- When a new element is pushed on the stack, older elements may be
invalidated. This means they don't need to be considered any more
because they are redundant with the new clip shape (e.g. nested round
rect clips only have to keep the innermost valid).
Bug: skia:10205
Change-Id: I68ccfd414033aa9014b102efaee3ad50a806f793
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308283
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This feels pretty awkward, but it's the most straightforward way I see
to do this without Yet Another Canvas Subclass.
Passes at head, fails without
https://skia-review.googlesource.com/c/skia/+/317110.
Change-Id: I67931b5eb8396621aec1858f2d7c4aaadb1b9132
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317161
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Takes SkYUVAPixmaps. Still implemented in terms of SkYUVAIndex[4]
internally.
Replace all internal use cases except wacky_yuv_formats.
Takes GrRecordingContext rather than GrContext.
SkVideoDecoder updated to take GrRecordingContext.
Bug: skia:10632
Change-Id: I6e9b9b6a4f11333bce6f87c1ebff0acb297f6540
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316837
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
GN lists both the .cpp and the .h as generated outputs, so if they don't
exist, Ninja assumes we need to rebuild the tests every time we compile.
Change-Id: I37b8b3d9e7aef1b0cb8d5c70530c2542a6c0087a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317108
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Our lack of proper caps-bits controls in skslc affects the outcome of
one test: "InlinerWrapsEarlyReturnsWithDoWhileBlock" does not actually
emit the do-while block because the standalone caps bits don't enable
do-while support. This will be fixed in a followup CL that adds caps-bit
support to our tests.
A few tests were renamed for consistency, a few were simplified slightly
and one test was removed because it was simply redundant (there was a
second test that covered the exact same ground as
`ForWithReturnInsideCannotBeInlined`).
Change-Id: I2e3b97cb3aea331b6d806bdb865aa78c35c7a6b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316997
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Add a slew of 16-bit instructions for experiments.
I want to try a fixed-point path through SkVMBlitter, continuing to
represent geometry with F32, but color channels in 16 bits, with several
possible representations:
- unorm8 lowp like SkRasterPipeline (0 -> 0.0, 0x00ff -> 1.0)
- 15-bit SkFixed15 fixed-point (0 -> 0.0, 0x8000 -> 1.0)
- 14-bit signed fixed-point (0 -> 0.0, ±0x4000 -> ±1.0)
I'm leaning towards the 14-bit version for being able to hold a good
range of temporary values in [-2,2), or perhaps even a 13-bit analog for
even a little more safety range. Mostly something new to try.
Most of these instructions are pretty obvious, with notes on a few:
vpavgw is an unsigned (x+y+1)>>1, and is useful for converting
unorm8 up to Q14. There are a couple ways to do this pretty well,
and using vpavgw is the best, and uses the fewest instructions:
A) (x << 6) + ( x >> 2) + (x == 255) // Ok approx.
B) (x << 6) + ((x+1) >> 2) // Better approx.
C) vpavgw(x << 7, x >> 1) // Perfect math!
The best good reverse math I've found is (x >> 6) - (x > 16319).
vpmulhrsw is the key to the whole thing as usual, letting us do
16x16->16-bit multiplies. An SkFixed15 multiply is vpmulhrsw
followed by vpabsw (also added here), and a Q14 multiply is
vpmulhrsw followed by a simple <<1.
I've added both signed and unsigned min and max. Not entirely
sure they'll all be used, but I do have my eye on vpminuw as a
single-instruction clamp to [0,0x4000] ~~> [0.0,1.0], treating
any negative Q14 as very large unsigned.
Change-Id: I0db7f3f943ef6c9a600821444cc5b003fe5f675d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/317119
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This reverts commit eaed918a1d.
Reason for revert: fix for skia:10722 at http://review.skia.org/316977
Original change's description:
> Disable whole-program inliner.
>
> This can be re-enabled once the Metal bug with fp_sample_chaining is
> diagnosed and fixed.
>
> A pure rollback led to a merge conflict; just commenting out the line
> that invokes the inliner serves the same purpose.
>
> Change-Id: I4df556ea8cd44349cbca2ea0ec9995dfffeb8aec
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316937
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,johnstiles@google.com
Change-Id: If490609a9f5f0b0e87baf57638a3da865e173b03
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316996
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I6097f50e7be1fa2f7772f6c454410ecbf3470eea
Bug: skia:10722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316977
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The following conditions lead to the error:
- A pair of nested functions, both of which must be inlined.
- Both inlined functions create a variable with the same name.
- The outer function passes its variable to the inner function.
- The initialization of the inner variable uses the value from the outer
variable.
- The inner function does not mutate the variable, use it as an out-
parameter, or otherwise cause it to receive a temporary copy.
When all these conditions are met, both variable declarations are
inlined as-is without performing any name salting, because it's
seemingly safe to do so. The name overlap issue is not considered in the
safety checks. Inlined variable declarations are not subject to name
salting but they should be; I suspect other adversarial examples could
be crafted as well where unhandled name overlap leads to errors.
Change-Id: Ia754bee8e45c8a5c7548436594bbf04abc7a8396
Bug: skia:10722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316945
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The new test files are intended to be identical to the unit tests in
every meaningful way. (Comments and formatting are not preserved
exactly.) In cases where a unit-test method contained more than one
test, multiple test files were created; in these cases, new names were
invented to match the apparent intent of each invocation.
Followup CLs will continue to migrate additional tests.
Change-Id: I785c6761ba7ee2b25b5ddc0108321734be23b77c
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316678
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I56e19153597e2c4393c5821314b82937828c0d50
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316569
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This can be re-enabled once the Metal bug with fp_sample_chaining is
diagnosed and fixed.
A pure rollback led to a merge conflict; just commenting out the line
that invokes the inliner serves the same purpose.
Change-Id: I4df556ea8cd44349cbca2ea0ec9995dfffeb8aec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316937
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:10679
Change-Id: If464c48b7c31d0d8440d1231d1983829d54ce598
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315281
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
We still occasionally downcast, so this is not airtight,
but it (1) allows us to know where we are downcasting and
(2) lets us move away from GrContext (and hopefully remove
it sooner than later.)
All three canaries are currently broken =( so here we go!
Bug: skia:104662
Change-Id: I84efe132574690b62ea512e194e4f9e318e9c050
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316218
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
separate step.
This ended up uncovering an optimization bug. SkSLNonConstantCase
started failing; it turns out that the optimizer was never being run on
this test, and so we hadn't noticed that it didn't actually work in the
presence of optimization.
Change-Id: Iff1d330be7534113b86f86b00c39f91282903ae3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316568
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Golden SkSL outputs are intended to eventually replace the majority of
our unit tests, since they can automatically update themselves when we
change implementation details of the compiler.
If you change the compiler output without updating the Golden files, the
CheckGeneratedFiles housekeeper will be triggered. Set
`skia_compile_processors` or `skia_compile_sksl_tests` to true in your
GN args to regenerate them.
Almost all of the tests from SkSLFPTests.cpp and SkSLGLSLTests.cpp can
be migrated into separate unit-test .fp/.sksl files in a followup CL.
hcm@ has signed off on removing the copyright boilerplate preamble from
our unit test files.
Change-Id: I9e24a944bbac8f8efd62c92481b022a0b1ecdd0b
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316336
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
In this CL, the IR generator still performs inlining as well; this will
be removed in a followup CL. However, the optimization pass is able to
find more inlining opportunities because it allows itself to exceed
the 50-IRNode limit when a function is only used once. In our test code
this is uncommon, but in SkSL generated from trees of fragment
processors, it is very common; any FP that is only sampled once tends
to qualify.
Change-Id: I8b2f653b2dd05d4de18bb15f3a4c1f034b8252ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315639
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We don't expect raw literals to ever appear here; this is a defensive
maneuver. See http://review.skia.org/316440 for an example case where
raw literals accidentally made it all the way to the inlining pass, due
to a missing `coerce`.
Change-Id: I80198f2f135791931aa53e0d6d92f1edfe4fb3b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315970
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This CL adds a new type GrDstSampleType to say how we will sample the dst.
We add tracking of the GrDstSampleType in the recording of GrOps and
then during execution passing the information along to the GrPipeline.
In general the tracking of GrDstSampleType is a global state of a GrOpsTask
so it is kept separate fro the DstProxyView which is more specific to a
single Op on the GrOpsTask.
Bug: skia:10409
Change-Id: Ie843c31f2e48a887daf96cee99ed159b196cb545
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315645
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Constant propagation wasn't performing type coercions, so we'd end up
propagating a constant into its final location and using the constant's
type rather than the destination type. This allowed things like
$floatLiteral to sneak into final output.
Change-Id: I5b63c70370095e291dea6374e139329b695fa0b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/316440
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Four of the first 12 cases in the new unit test previously failed (the
logic to reduce from vector to scalar type coercion didn't test the
reverse order, so (half * float4) would only test if (half * float) was
possible going right-to-left.
Also, the matrix multiplication logic was missing a check when doing *=,
allowing things like (matrix *= vector) to compile (then fail later in
GL, etc.)
Finally, we were never checking if the op was valid for vectors when
doing the vector/scalar combination. Added test cases to
BinaryTypeMismatch that previously failed, and now work.
Change-Id: I1e2709e3ba4df31f9300672189826151eabe017a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315964
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This will be especially important once more aggressive inlining is
enabled, as the optimizer will need to run dead-function elimination
passes after inlining.
Change-Id: I8ccd5d6a9a041ee2b0a3ec5ef305e5df875b6947
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315738
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Inlining shift=8 is trivial for any compiler.
This eliminates the need to unit test the two functions are the same.
Change-Id: Icd181ff11eab73fba26755a9fbecd57260c38bbf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315887
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Encountered while debugging new GrClipStack.
Basically, with the old checks, if aCorner == bCorner but A was a rect,
we'd end up rejecting it as not a viable intersection (failing the
testCorner == aCorner checks), but wouldn't fall through to the
testCorner == bCorner checks.
Instead of splitting the if-elseif-else into separate ifs, I chose to
add an additional if case that lets the ellipse containment check match
a few more cases when we know the two ellipses share an anchor point.
Change-Id: I6c8bc9a30d19a56f25da7d9be7832ff72dde1765
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315636
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
