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>
Moved the actual field into the base class, changed all of the enums
into non-overlapping enum classes, and finally renamed Type::Kind to
fix the ambiguity there.
Change-Id: I4e6c24d2dbbc7b1d12b7b7bf12e77c651eea7ed9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315318
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
These changes don't seem to add much value in isolation, but they will
smooth our transition to inline analysis during optimization.
- The passed-in FunctionCall no longer needs to be a unique_ptr.
- The fInlinedBody is guaranteed to be a Block now. (This change caused
a slight ripple effect in unit test output; in some cases it creates
an additional newline in the final code. This is harmless.)
- has_early_return is checked earlier, before we've made any mutations
to the FunctionCall. This should work around errors that can occur if
a function is trying to inline itself. (Ideally we wouldn't be trying
to do this at all, but either way, we shouldn't crash.)
Change-Id: I0a565d477f680c0ba061df2686a36e42aa75de95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315599
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>
This is a reland of ed63444587
Original change's description:
> Add idea of DataType to SkYUVAPixmapInfo.
>
> DataType describes the data type of YUVA channels
> independent of how they are grouped into planes.
>
> Adds mapping functions between SkColorType/channel count
> and DataType.
>
> SkYUVAPixmapInfo can be constructed from DataType and will
> choose appropriate SkColorTypes for each plane.
>
> Valid SkYUVAPixmapInfos now have the same DataType for each
> plane (could relax this in the future, esp for alpha plane).
>
> SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
> combinations of SkYUVAInfo::PlanarConfig and
> kYUVAPixmapInfo::DataType supported by a GrContext (based on
> supported texture formats).
>
> SkImageGenerator/SkCodec YUVA query API now takes a
> SupportedDataTypes.
>
> Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
> Bug: skia:10632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:10632
Change-Id: I35b55b7477c11c822fdb3729a9f84acff1eb785d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315284
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Mechanically updated via Xcode "Replace Regular Expression":
typedef (.*) INHERITED;
-->
using INHERITED = $1;
The ClangTidy approach generated an even larger CL which would have
required a significant amount of hand-tweaking to be usable.
Change-Id: I671dc9d9efdf6d60151325c8d4d13fad7e10a15b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314999
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
All tests are now GLSL-based instead of .fp based, for ease of
readability. DEF_TESTs which called test() more than once have been
split up into a separate individual DEF_TEST for each test() call, to
simplify fixups going forward.
Change-Id: I9202e95e9cafff3efc0c32aa3f3ca2eea04719fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314886
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit ed63444587.
Reason for revert: trying to unblock android roller, which incorrectly thinks this has a merge conflict: https://b.corp.google.com/issues/167576324
Original change's description:
> Add idea of DataType to SkYUVAPixmapInfo.
>
> DataType describes the data type of YUVA channels
> independent of how they are grouped into planes.
>
> Adds mapping functions between SkColorType/channel count
> and DataType.
>
> SkYUVAPixmapInfo can be constructed from DataType and will
> choose appropriate SkColorTypes for each plane.
>
> Valid SkYUVAPixmapInfos now have the same DataType for each
> plane (could relax this in the future, esp for alpha plane).
>
> SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
> combinations of SkYUVAInfo::PlanarConfig and
> kYUVAPixmapInfo::DataType supported by a GrContext (based on
> supported texture formats).
>
> SkImageGenerator/SkCodec YUVA query API now takes a
> SupportedDataTypes.
>
> Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
> Bug: skia:10632
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,scroggo@google.com
Change-Id: I72c39539a4766f10cac3ca3cdef6c503a8319ff1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314895
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The bulk of the diff is just reordering existing code; the logical
changes are very small.
Change-Id: I3b918e64f5229da43d43f0922e8b59a007a6ad3e
Bug: skia:10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314882
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
It turns out the parser was accepting empty declarations, turning them
into an empty modifiers declaration. This was causing problems with a
CL that was going to disallow declarations in contexts where they
weren't allowed. Since this was never the intended behavior, we now
disallow it.
Change-Id: Iea56529c76a946e8002ce1e929790aec488fd4f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314879
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The following tests show incorrect results from the inliner:
- SkSLFPInlinedIfBodyMustBeInAScope
- SkSLFPInlinedElseBodyMustBeInAScope
Change-Id: Iafc567f9a97f67f9b734edd348ee25a14939592a
Bug: skia:10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314880
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>
Change-Id: I43479d8543ea4860be45614a65cf8ad4cec307d8
Bug: skia:10687
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314877
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This prevents unnecessary work from being done, and also prevents us
from executing side-effects from the wrong side.
Change-Id: I4dbf3974388807f15e9eadb2abf1b1243d047ce2
Bug: skia:10688
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314797
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>
Some of these tests worked perfectly, but others expose latent inlining
bugs. The following tests show incorrect results from the inliner:
- SkSLFPTernaryExpressionsShouldNotInlineResults
- SkSLFPInlinedWhileBodyMustBeInAScope
- SkSLFPInlinedDoWhileBodyMustBeInAScope
- SkSLFPInlinedForBodyMustBeInAScope
Change-Id: I523e2e3272dea01d5c194a478df6f39ecadf2f5c
Bug: skia:10687, skia:10688
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
DataType describes the data type of YUVA channels
independent of how they are grouped into planes.
Adds mapping functions between SkColorType/channel count
and DataType.
SkYUVAPixmapInfo can be constructed from DataType and will
choose appropriate SkColorTypes for each plane.
Valid SkYUVAPixmapInfos now have the same DataType for each
plane (could relax this in the future, esp for alpha plane).
SkYUVAPixmapInfo::SupportedDataTypes specifies the supported
combinations of SkYUVAInfo::PlanarConfig and
kYUVAPixmapInfo::DataType supported by a GrContext (based on
supported texture formats).
SkImageGenerator/SkCodec YUVA query API now takes a
SupportedDataTypes.
Change-Id: I8791234638e6ba3396d1e7960b7bc210edc6dd57
Bug: skia:10632
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314276
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
When sampler objects exist but we're not using them we bind 0.
Speculatively disable use on ANGLE to see effect on performance.
Change-Id: I5c76bd812760a5ac67a4327f2d02f4f831155029
Bug: skia:10644
Change-Id: I7ef23704b2ca11d92806f196589ec561a3ac9435
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314462
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Ideally, we wouldn't need a counter at all here, and we could just rely
on the symbol table to check for name collisions. Unfortunately, this
fails in practice on a couple of tests, because we don't construct
programs in a strict top-to-bottom order--particularly when inlining.
However, by checking the symbol table before using a name, we can at
least solve cases where the inliner reuses a name that was taken in a
previous pass, which was why salt was originally added. This makes the
generated code much easier to read.
Change-Id: Ib69611c8df457fbd03b31e52158113ad4a8735d5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/314277
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
If only a single statement is necessary, it is returned as-is.
If multiple statements are needed, they are wrapped in a non-scoped
Block statement. If no statements at all are needed (!), null will be
returned.
Change-Id: I6ba373f73d339b8c2e7b8d39dfb28f13655e3d03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313911
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Also, move first-direction into SkPathRef.h so it can be referenced
by name in SkPath (instead of using uint8_t)
No functional change expected.
Change-Id: Ica4a8357a8156fd9a516118f23599a965b0fdd47
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313980
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Subsequent CLs can work on combining convexity and direction
Bug: skia:10670
Change-Id: Ia44769ea88ffd99a56d4c6729a80a2044e790ec2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313837
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Mike Reed <reed@google.com>
This has always been a potential source of a bug. If the same texture is
used twice in a shader with different swizzles we would overwrite the
swizzle for the first use by that of the second use since there is
only one fixed function swizzle per texture. It's not part of the
sampler state.
We set the swizzle when it is a feature, but always to RGBA.
Also, highly speculative that this may improve ANGLE D3D11 ES3
performance compared to ES2.
Bug: skia:10644
Change-Id: I8877afc3043c5ddaafd26ea9f9bd372303328c71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313682
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
In a follow-up, I will explore more clearly tying together convexity
and direction internally -- perhaps a unified enum?
[ unknown, convex_cw, convex_ccw, concave ]
Change-Id: I9fc2a2205f40050f4c24c5bec7fc25c8b6d2461c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313680
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
This also includes all the plumbing of this flag throughout our proxy
and surface system.
Bug: skia:10409
Change-Id: I48d40012049240cfa80e045ea090f68ce2d2ff0d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313676
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Note: The polarity of the staging flag is inverted from usual because
a G3 dependency with no SkUserConfig.h relies on the legacy API.
Once this lands, we will migrate them and others, then remove the
staging API. The inverted staging flag is kind of nice, actually - I may
use that pattern in the future. It means less total CLs and it's just as
easy to flip the bit on or off during debugging.
Bug: skia:104662
Change-Id: I48cba1eeae3e2e6f79918c6d243e0666e68ec71b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310656
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Previously, if you snapped off a shader and then changed uniforms
(without drawing & flushing), we'd trigger the SkData assert about
calling writeable_data when not-uniquely-owned. Now we lazily copy the
SkData when necessary.
Includes unit test that previously failed.
Bug: skia:10667
Change-Id: If8d9dd8106d41e66560d760cb36ed83371791fc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313678
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Previously, early-return detection worked by ensuring that a function
had a maximum of one return statement, positioned at the end of the
function. However, this technique did not support if-else statements:
if (a) { ...; return x; } else { ...; return y; }
This meant that many simple cases were unnecessarily wrapped in a do-
while loop to handle control flow issues that didn't actually occur.
Our early-return detection logic is now more flexible and looks for any
return statements that aren't at an exit point in the control flow,
instead of looking for exactly one return at the end of the code.
Change-Id: Iffe71adf2b9349ce8de42ba8301ccc52abe2882b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313418
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
SkYUVAPixmapInfo is a SkYUVAInfo with per-plane color types and row
bytes. It describes a set of pixmaps that make up a planar image.
Consolidates validity checks of the SkYUVAInfo with the color types
and row bytes. It can provide SkImageInfos for each plane and also
assist with configuring planes to share a common allocation.
SkYUVAPixmaps is a collection of SkPixmaps that are valid for a
SkYUVAInfo. It can either wrap existing SkPixmaps or allocate and
own the memory. It consolidates validity checking of SkPixmaps with
the SkYUVAInfo. Replaces sk_gpu_test::YUVAPixmaps.
Minor tweaks to SkYUVAInfo naming, parameter order consistency, adds a
hasAlpha() method.
Bug: skia:10632
Change-Id: Ib0f48b8448fff22805fd0c04e07887d0b7338b76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312886
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Besides some tests, we always set the renderable flag to kNo for this
function. No need to keep supporting a code path we don't actually use.
Tests that use to pass in kYes here have been converted to calling
createLazyRenderTargetProxy instead.
Change-Id: I91efe6cc51fd7ba04b711509ca26f18eba2af333
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313425
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Existing .fp files that assign their output to sk_OutColor will continue
to work as before. An error will be generated if an .fp file tries to
mix-and-match return statements and sk_OutColor assignment; only one or
the other is allowed within a single .fp file.
Change-Id: I53bc0e8b79b2fdd64a020a7e5b35a905f9b05c94
Bug: skia:10549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313197
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Two cases are added to the tests:
- We wrap early returns in a do-while block.
- We wrap `if (x) { return y; } else { return z; }` in a do-while block.
In a followup CL, the second case will be detected as "not an early
return" because all returns occur at an exit point. This means that the
first unit test will remain the same, and the second test will no longer
wrap its body in a do-while block or add breaks.
Change-Id: Id1ed880c6fb4b3fd2c373e32b7447f74d0901ab8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313417
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 were previously defensively copying the arguments to inline functions
into temporary values in all cases. We now detect some cases where that
is not necessary and use the arguments directly.
Change-Id: I9739643157743fdbcefbac9f36be93ea6ecde6f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312489
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The IR generator was previously unable to see inside sub-blocks when
checking for early return statements, so a function like this:
half fn() {
{
return 0.0;
}
}
Would be wrapped with an unnecessary `do { ...; break; } while (false)`
construct during the inlining process.
Change-Id: Ie804261643b0bab38a08787d0b9d168a03faf070
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313206
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
The inlining output for these statements is optimized in a followup CL.
Change-Id: I3a9a0fdca27e6a0ce80fe80b8869b1be677a8443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313207
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>
Although cool, I don't know if this is worth the increased complexity
for an unused feature.
Change-Id: Id229beab176cebf5b7810fd1e44a5d1e25abcabc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312848
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
There is no more 'inout half4 color'. Effects return their output color.
If an effect wants the input color, it must use the (already existing)
approach of sampling a nullptr input shader.
The change is guarded for Chromium (so we can update their runtime color
filters in skia_renderer.cc).
For the GPU backend, FPs can now override usesExplicitReturn to indicate
that their emitCode will generate a return statement. If that's true,
then writeProcessorFunction doesn't inject the automatic return of the
output color, and emitFragProc will *always* wrap that FP in a helper
function, even as a top-level FP. GrSkSLFP opts in to this behavior, so
that the user-supplied return becomes the actual return in the FP's
emitCode.
Adapting the skvm code to this wasn't too bad: It looks fragile (what
happens if there are multiple returns?), but that's not really possible
today, without varying control flow.
Bug: skia:10613
Change-Id: I205b81fd87dd32bab30b6d6d5fc78853485da036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This reverts commit 6541013b53.
Reason for revert: TSAN issues with GrFence, and crash in GrMtlPipelineStateBuilder::CreatePipelineState.
Original change's description:
> Remove ARC from Metal backend
>
> Change-Id: I5ab28f6eda3b37d1b82c94c7cc6eaa2ce59157da
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311113
> Reviewed-by: Adlai Holler <adlai@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
TBR=jvanverth@google.com,bsalomon@google.com,adlai@google.com,johnstiles@google.com
Change-Id: I031629b483fc46de8bd3751253e5391c2ce87853
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312843
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Tunnels through SkImageGenerator as well.
The new SkCodec interface doesn't assume three 8 bit planes.
New SkYUVASpec more clearly defines chroma subsampling and siting of
the planes.
The intent is to use this for other YUVA APIs as well, in particular
SkImage factories in the future.
In this change we convert to the SkYUVASpec to SkYUVASizeInfo
and SkYUVAIndex[4] representation. But the intent is to use
the SkYUVASpec representation throughout the pipeline once
legacy APIs are removed.
orientation GM is replicated to test a variety of chroma
subsampling configs.
Bug: skia:10632
Change-Id: I3fad35752b87cac16c51b24824331f2ae7d458d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309658
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This is a reland of 26766ad427
Original change's description:
> Add utilities to SkGeometry for quad and cubic rotation angles
>
> Adds methods to measure the rotation angles of quadratics and cubics,
> and to chop curves at locations that divide the rotation angle in half.
>
> Bug: skia:10419
> Change-Id: I840e12034fc66e1a459de875fefda07a27a78335
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308880
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
Bug: skia:10419
Change-Id: I62ea6847a91c054174f829962a901f21910c013b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312438
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Change-Id: Ic09346b6079e6f316c28e03ddb02f12b4af8a38d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312482
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
We will still need to go through clients to remove uses of the main ctors
so that it can be deleted.
Change-Id: I7bdfa00ac56b2404cc7b2f183104ee97b4da1de7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311452
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is a reland of 284f2c4bea
Original change's description:
> SkSL now supports uniform array types
>
> Change-Id: I809e9c424ee92b05f0a87d75d1384c92849e1474
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308498
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I176f7fbdcbf3700e51e2303b06a4e906d22b2300
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312244
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
OpenType and many other font formats have the concept of pre-computed
metrics for the union of all glyph bounding boxes. This allows for fast
though course quick rejecting of bounds, since the glyphs themselves may
potentially be quite a bit larger than the EM. With the introduction of
variable fonts OpenType does not vary these bounds, so the bounds are
only valid for the default non-varied font.
As a result the fTop, fBottom, fXMax, and fXMin reported in
SkFontMetrics may be bogus. Since simply always setting them to empty
zeros may be disruptive, provide a way forward for new users to check if
the bounds are valid.
This is a continuation which implements this in the DirectWrite and
CoreText ports.
Change-Id: I50812e78f31ee06fedf8ab1f005837d673a8f2ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311976
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This reverts commit 1394f71ff0.
Reason for revert: doesn't seem to have been the culprit
Original change's description:
> Revert "Added support for the 'inline' hint on SkSL functions"
>
> This reverts commit 338e57d4b1.
>
> Reason for revert: maybe breaking Chrome roll
>
> Original change's description:
> > Added support for the 'inline' hint on SkSL functions
> >
> > Change-Id: Ib78e0ad9fd1cc15e7afeb2a9ddd6b1249828fbe7
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311603
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Reviewed-by: John Stiles <johnstiles@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: Ifd199db94a78e4cb389576b9ff282f54a582c421
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312078
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: I241f6bf16602bb5e011a2a5879a2df515fa0953b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312097
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 26766ad427.
Reason for revert: Pixel & Nexus 5 failing with
Failures:
../../../../../../skia/tests/GeometryTest.cpp:424 [Geometry]: SkScalarNearlyEqual(leftRotation, expectedChoppedRotation)
../../../../../../skia/tests/GeometryTest.cpp:424 [Geometry]: SkScalarNearlyEqual(leftRotation, expectedChoppedRotation)
Original change's description:
> Add utilities to SkGeometry for quad and cubic rotation angles
>
> Adds methods to measure the rotation angles of quadratics and cubics,
> and to chop curves at locations that divide the rotation angle in half.
>
> Bug: skia:10419
> Change-Id: I840e12034fc66e1a459de875fefda07a27a78335
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308880
> Reviewed-by: Jim Van Verth <jvanverth@google.com>
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
TBR=egdaniel@google.com,jvanverth@google.com,csmartdalton@google.com,reed@google.com
Change-Id: Ic3d5c1ae35b74aa340757e92132bb2c1b8910cc6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10419
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312119
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This reverts commit 338e57d4b1.
Reason for revert: maybe breaking Chrome roll
Original change's description:
> Added support for the 'inline' hint on SkSL functions
>
> Change-Id: Ib78e0ad9fd1cc15e7afeb2a9ddd6b1249828fbe7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311603
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
Change-Id: Ifd199db94a78e4cb389576b9ff282f54a582c421
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/312078
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Other code outside this namespace will want to use this method soon.
Bug: skia:10419
Change-Id: Ib155f224866fd333b8f9a4b78e6c9e51ac0600df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311936
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Adds methods to measure the rotation angles of quadratics and cubics,
and to chop curves at locations that divide the rotation angle in half.
Bug: skia:10419
Change-Id: I840e12034fc66e1a459de875fefda07a27a78335
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308880
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Now that approxOpCount() can recurse into sub pictures,
adding ~0 will overflow its total count.
New tests fail at head, pass now.
Change-Id: Icdb7d8a062739724207c9a6369734510f00f2ffd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311774
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
It's been replaced with a `uniform half4` in tests where it was needed,
which tends to generate similar code overall.
Change-Id: Iacc062efaeae37ebecf78505abe0fc563b69ceb0
Bug: skia:10619
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311099
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The original code assumed that the switch value and case value would
both be IntLiterals, and asserted if this were not true. However, in
sufficiently complicated cases, the switch value can sometimes be a
Constructor containing a scalar.
It's possible that there's a fix to constant propagation which would
flatten out this Constructor to just a scalar, but that has eluded me
for today, and this approach does seem to solve the issue. (Also, it's
surprising that the optimizer does not notice that it can simplify
`half4(1.0, 2.0, 3.0, 4.0).yyyy`.)
Change-Id: Id490b15c724dd174c448665a19242cc80cdce213
Bug: skia:10623
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311636
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I5b1baf2bc3aafa03c1d0cace4f7c09a09cf4a084
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311112
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Allow the user of SkCustomTypefaceBuilder to set the SkFontStyle of the
resulting SkTypeface. This allows users to build font families.
Fix the Font_flatten test to actually work (instead of relying on the
magic behavior of nullptr for SkTypeface), add a test with the custom
typeface, and reduce the number of times the inner loop runs from
302,400 times to 4,032 times so that the test finishes in a reasonable
amount of time.
Bug: skia:10630
Change-Id: I0b5e939552ee4a9a1249eefbb7a7279a59b38e5a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311596
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Xiao Yu <xster@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
While writing test code for skia:10623, I encountered a separate bug in
the SkSL compiler. Specifically, the codegen would assert when trying to
emit a function that returned an int. There were several basic types
missing from `glsltype_string`. I've added them, and created a unit
tests to exercise a subset of them.
Change-Id: Ied6aaacc67f472998cb6673490c4d98ce98886d2
Bug: skia:10631
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311453
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>
Objects in the symbol table are intentionally constant. However, when
converting "ExternalValue" AST nodes into symbols via symbol table
lookup, IRGenerator::convertIdentifier was casting away constness
because ExternalValueReference held a non-const pointer. Fixing this
involved significant ripple-effect additions of "const" throughout the
ExternalValue class and its subclasses.
These changes generally appear to be benign, but one interesting edge
case is `ExternalValue::write`, which intuitively does not seem to make
sense as a const method. However, invoking `write` should not alter the
ExternalValue object itself; rather, it is intended to alter the
*external value* that is being referenced. (In practice, nothing invokes
write() anyway except for one unit test, which continues to pass.)
This issue was discovered while converting casts to `as<T>()` calls.
Change-Id: I8ff6a477e475833d2a99c72f1c79c766b57767ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311276
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These are very easy to mis-read, and they don't make much sense.
Swizzles should refer to the base expression at least once.
Change-Id: I4c2740c7439a11b51b2eb41263c6435e3aefad35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310376
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Just wrapping the backend texture shouldn't invalidate any parameters.
Change-Id: I02859e519b2c8e9612533b2230da753d5fff97b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/261084
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
- reset, findLastOf: these were entirely unused
- find: already existed in C++11's std::string, can just be inherited
- defaulted constructors: in C++11, these can be inherited directly from
std::string via 'using'
Change-Id: I1772ef04e49ab905aaada2ec38c1abeb9a8e26bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/311039
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
To my surprise, this even works with homegrown smart pointers (such as
SkTLazy).
https://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-smartptr-get.html
Find and remove redundant calls to smart pointer’s .get() method.
Examples:
ptr.get()->Foo() ==> ptr->Foo()
*ptr.get() ==> *ptr
*ptr->get() ==> **ptr
if (ptr.get() == nullptr) ... => if (ptr == nullptr) ...
Change-Id: I8ff541e0229656b4d8e875c8053a7e6138302547
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310976
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html
Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.
Checks for calls with implicit comparator and proposed to
explicitly add it:
if (strcmp(...)) // Implicitly compare to zero
if (!strcmp(...)) // Won't warn
if (strcmp(...) != 0) // Won't warn
Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is
< 0 when lower than,
> 0 when greater than,
== 0 when equals.
A common mistake is to compare the result to 1 or -1:
if (strcmp(...) == -1) // Incorrect usage of the returned value.
Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:
if (strcmp(...) < 0.) // Incorrect usage of the returned value.
Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Previously, the inner type was ignored.
Change-Id: I51d251fc38358ef889b5a3f85d5f2d23bd8cf4c5
Bug: skia:10615
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310657
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>
These tests verify that switches and enums only work with constant
integral values. Floats or uniforms should be rejected with an easy-to-
understand error message.
Change-Id: Ib634cb1ca1734a4b66ba53a3476e9ee539a63e3e
Bug: oss-fuzz:24889, skia:10615
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310396
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>
It's possible to construct a case value expression that's a compile time
constant, but fails to produce a value from getConstantInt. MSAN noticed
us using the uninitialized integer. It's now initialized, but also never
used in the failure case: We make getConstantInt return status, and give
better error messages in the two places it's used.
Bug: oss-fuzz:24889
Change-Id: I88e4e5b7bd1caeea1cf53f9b1d6f345dd8a5326f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310296
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Inlined return statements emit two statements--an assignment, then a
break. If scope braces are otherwise missing, the break statement will
end up in the wrong place. i.e.:
Mistranslated:
if (foo) return bar; --> if (foo) out = bar; break;
Translated properly:
if (foo) return bar; --> if (foo) { out = bar; break; }
Change-Id: Id992abf64ad09e6752f64c71cb556f05c868a453
Bug: skia:10607
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310177
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-override.html
Adds override (introduced in C++11) to overridden virtual functions and
removes virtual from those functions as it is not required.
virtual on non base class implementations was used to help indicate to
the user that a function was virtual. C++ compilers did not use the
presence of this to signify an overridden function.
Change-Id: If66d8919358f72a4035190caf8d7569268037a9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310160
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
The new unit test SkASSERT()s a hash collision at head
(but passes in release builds) and does not SkASSERT()
with this patch to SkColorSpace.cpp.
Bug: chromium:1113865
Change-Id: Ibc05af4145a92bbd15c7d5e06ece9d269bd7a242
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310110
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We do not have a good mechanism to break out of a nested for/while loop
or switch statement. Our inlining approach generates incorrect code in
those cases, and there's no apparent fix, since we don't have a
mechanism for subverting control flow like 'goto'. When we detect this
case, disable inlining for that function.
Change-Id: Ic4180b367a3895806b0cc36872155185138826e1
Bug: skia:10606
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310063
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>
Prior to this CL, if we failed to create the DDL Recorder's target
proxy while creating the target surface we could create an invalid
DDL.
The specific repro case involved too big of an target proxy but
many other scenarios could result in the same behavior.
Bug: 1105903
Change-Id: I519a072600c168aa590fbe920f4029d08fe29e6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309777
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:10369
Add SkEncodeImageWithNDK, mirroring the CG and WIC versions, for
encoding with the NDK APIs added to R.
Rename SK_ENABLE_NDK_DECODING to SK_ENABLE_NDK_IMAGES and use it for
both encoding and decoding.
Move code for converting to/from NDK types into a common location.
Update encode_platform.cpp to use NDK encoding APIs when available and
to use both types of webp (lossy and lossless). Add tests specifically
for the new implementation.
Update NdkDecodeTest to use ToolUtils::equal_pixels for comparing
pixels.
Change-Id: Ic62f89af27372ccce90b8e028e01c388a135a68c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308800
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
`onXxxxx` methods in Skia generally appear to be non-public, which I did
not realize until after originally implementing this task.
Followup CLs will update the non-gencode `onDumpInfo` methods to be
private as well.
Change-Id: I807eee132b080f72f6b040e1ca7f687be25dff11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309883
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>
This CL also marks `GrProcessor::dumpInfo` as final. This prevents a
subclass from mistakenly overriding `dumpInfo` instead of `onDumpInfo`.
`onDumpInfo` is responsible for providing the same data as `dumpInfo`,
except that the FP name is automatically prepended.
Change-Id: I2b44c30a01bc65e9d88321cc21651a94e20074c6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309793
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>
and
"Guard gencode dumpInfo() calls with GR_TEST_UTILS, instead of SK_DEBUG"
and
"Remove dumpInfo() entirely when GR_TEST_UTILS is off."
This is a reland of 26900788ef
and b5e8a2533a
and d7b09c4c3a
and unit tests are now fixed.
Original change's description:
> Guard dumpInfo() calls with GR_TEST_UTILS, instead of SK_DEBUG.
>
> (One exception: the `dumpInfo` in GrVkCommandPool is wrapped with
> SK_TRACE_MANAGED_RESOURCES to match its peers in GrVkManagedResource.)
>
> Change-Id: I6cf55fa2bb5687f79a2cc0c2a9c02a629bfd4f8e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309556
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I16e6606082a814b4fa5ef58d1096fc915f5ac704
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309721
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Forgot to add these when I fixed the bug. Verified that these tests
failed without the fix.
Bug: skia:10593
Change-Id: Ia20fad3cd8e5b0f63ca19946b8314eed49bec2bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309716
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
I created this problem with my fix to crbug.com/1108475
Bug: 1112259
Change-Id: Ieb001fa61bd9ed68dcae43e7d511b4581f16ef0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308638
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
These radii are treacherous.
Added a regression test derived from the path found by the fuzzer.
Bug: chromium:1111169
Change-Id: I92fde0c31057ea8adc864336438c0af2869b0374
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309306
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
SkRRect::computeType() can fail to produce an isValid() SkRRect,
with the regression test added here.
We can...
- land something like this that forces a good rect value,
- drop the assert,
- or maybe explore deeper and fix this?
Change-Id: I6d7402a0a4af2141ce45e82d287bfb4806aeb0ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309379
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This is a reland of a1df23c8b7
Original change's description:
> Implement `dumpInfo` for .fp files.
>
> Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
> Bug: skia:8434
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:8434
Change-Id: If485635440b800f8a282c871a1c5f2801608d3c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309660
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit a1df23c8b7.
Reason for revert: Needs c++17 library as written
Original change's description:
> Implement `dumpInfo` for .fp files.
>
> Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
> Bug: skia:8434
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
> 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,johnstiles@google.com
Change-Id: I09b98e83735bc30ec8a2e313e4b76a9eb6a7631a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:8434
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309656
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I40f6c1a02e194f090e67a0e3f2d7d83cd2317efd
Bug: skia:8434
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309139
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These doubles are sometimes used as doubles and sometimes floats,
but it doesn't make sense to use values that can't be a float as one.
Just skip those in the test combinatorics.
Change-Id: Ibfdb699cac80b260258b164f95361110eb85c152
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309483
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
These should work fine through SkVMBlitter,
and I'm not really sure why we didn't add them
to SkRasterPipeline.
Bug: chromium:1113777
Change-Id: Id89f82e6a53bd49d88a9562309acf0ab53ea5ec5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309472
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Profiling showed that repeated calls to `input_texel_color` were
actually a very expensive part of the test. Fortunately, we can expose
the texture's pixel buffer to the unit test method and easily reclaim
that performance.
Change-Id: I2c1cdd57a3e14dc859bdf03d8131137ca81364ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309437
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The fuzzer has a found a pathological case where the maxRadius
needs to be reduce by 17 ulps.
Reduce the larger radius an ulp at a time until the sum fits.
Change-Id: I9cc4528667e7f9e902eff34d447fd4040a09742e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309417
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This is a net reduction in code size because this idea was already
implemented separately in two places:
- dump_fragment_processor_tree (GrProcessorSet)
- describe_fp (ProcessorTest)
This consolidates the implementations. This CL also fixes a handful of
dumpInfo() methods that were not sufficiently descriptive--e.g. the FP
name was missing, or the implementation was just buggy.
Change-Id: If34ac46c97e9ae431c7c64b1247fc619703580b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309324
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit d169e1915c.
Reason for revert: broke chrome via code generator
Original change's description:
> Migrate GrSurfaceContext readPixels to take direct context
>
> After this lands we'll proceed up the stack and add the direct
> context requirement to the public API and SkImage.
>
> Bug: skia:104662
> Change-Id: I4b2d779a7fcd65eec68e631757821ac8e136ddba
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309044
> Commit-Queue: Adlai Holler <adlai@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=robertphillips@google.com,adlai@google.com
Change-Id: I6126f2dca4bc902c903512ac486e22841cc472e5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:104662
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309281
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
Runtime effects previously allowed two kinds of global input variables:
'in' variables could be bool, int, or float. 'uniform' could be float,
vector, or matrix. Uniform variables worked like you'd expect, but 'in'
variables were baked into the program statically. There was a large
amount of machinery to make this work, and it meant that 'in' variables
needed to have values before we could make decisions about program
caching, and before we could catch some errors. It was also essentially
syntactic sugar over the client just inserting the value into their SkSL
as a string. Finally: No one was using the feature.
To simplify the mental model, and make the API much more predictable,
this CL removes 'in' variables entirely. We no longer need to
"specialize" runtime effect programs, which means we can catch more
errors up front (those not detected until optimization). All of the API
that referred to "inputs" (the previous term that unified 'in' and
'uniform') now just refers to "uniforms".
Bug: skia:10593
Change-Id: I971f620d868b259e652b3114f0b497c2620f4b0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309050
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
After this lands we'll proceed up the stack and add the direct
context requirement to the public API and SkImage.
Bug: skia:104662
Change-Id: I4b2d779a7fcd65eec68e631757821ac8e136ddba
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309044
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
This is causing failures on the status dashboard.
Change-Id: I29e8fb5bef72282dbe6fafb5402607ed48aae707
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309136
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Opted the Galaxy S20 out of the test to allow Tryjobs to pass.
Change-Id: I8d4637a23f36edb012c96b7059b184c231c0436d
Bug: skia:10384, skia:10595
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309119
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Previously, MakeChildFP avoided infinite recursion by rejecting any FP
that took inputs. MakeChildFP now generates random inputs up to a
user-supplied tree depth.
The ProcessorOptimizationValidationTest test has been updated to
test up to a tree depth of 3. The ProcessorCloneTest has been left at
a tree depth of 1 due to a bug that only appears on Galaxy S20/Mali G77.
The Mali bug doesn't appear to be related to FP cloning, but probably
deserves further analysis. (It appears that on this device, these
processors hooked together in sequence render a tiny bit differently
each time: DitherEffect -> RectBlurEffect -> ImprovedPerlinNoise. By
visual inspection it looks like the dither varies on each draw.)
Change-Id: Ib8f619eb7a8a9c9254080303504c20065ff35453
Bug: skia:10384, skia:10595
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308556
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, this test would synthesize a random FP, call clone(), render
both, and the compare the results and report differences.
Now, if a difference is discovered, this test will re-synthesize the
originally-created random FP from scratch without using clone().
Instead, we call TestCreate again using the same random seed.
Then we can render and compare that output against the original as well.
This will allow to better differentiate failures that were actually
caused by clone(), versus failures caused by other types of
inconsistency.
If the regenerated version still mismatches, there are a variety of
potential explanations:
- the FP's TestCreate() does not always generate the same FP from a
given seed
- the FP's Make() does not always generate the same FP when given the
same inputs
- the FP itself generates inconsistent pixels (shader UB?)
- the driver has a bug
Change-Id: I71701c0f3d33a08f6ee926313782620487d336bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309076
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-copy-initialization.html
Finds local variable declarations that are initialized using the copy
constructor of a non-trivially-copyable type but it would suffice to
obtain a const reference.
The check is only applied if it is safe to replace the copy by a const
reference. This is the case when the variable is const qualified or when
it is only used as a const, i.e. only const methods or operators are
invoked on it, or it is used as const reference or value argument in
constructors or function calls.
Change-Id: I1261410deccd8ea64e85edec53fbd5360940e587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308759
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Follow-on CLs will push higher up in SkDraw, so that everywhere today
we have to cons-up (with the associated mallocs) a temp SkPath we can
replace it with a stack-based SPath...
- drawRect
- drawOval
- drawRRect
- drawLine(s)
(similar to how this CL already handled quads and triangles)
Bug: skia:10566
Change-Id: I882b4f4c60e80235ca83c86c926e905b269a7afd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307784
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Mike Reed <reed@google.com>
SkArenaAlloc has three fields that are used only for reset. Make a
subclass called SkArenaAllocWithReset which has the three
fields, and has the reset functionality.
An example of a reset() that is used instead of using a better scope
is PathOpsAngleAfter in PathOpsAngleTest.cpp.
Change-Id: Ie1965d128dfb7df9e022f4d18460d3f75f33e1a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307348
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
https://clang.llvm.org/extra/clang-tidy/checks/readability-const-return-type.html
`const` on a non-pointer/reference return type typically doesn't add
value and can have negative side effects. (i.e., returning a
`const std::string` isn't meaningfully different from returning a
`std::string`, but can sometimes inhibit move-related optimizations.)
In Skia's case, the priv() functions are a notable exception where const
return types are intentional and valuable. These calls have been marked
with NOLINT to exclude them from the check.
This check does not affect pointer and reference returns, where
constness is important.
Change-Id: I86cab92332f164e5ab710b4127182eec99831d7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308564
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Discovered via ClangTidy `readability-static-accessed-through-instance`.
Change-Id: I646e3293853e65b5cc2419c8687c035aab4b669a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308659
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Unlike GLSL/SkSL, Metal does not natively support casting an array from
one size to another; we need to synthesize a helper function which will
assemble a new matrix from the values in the old matrix and the
identity.
Previously, our matrix-conversion helpers understood how to glom
together an arbitrary collection of scalars/vectors/matrices into a
matrix containing a matching number of scalars, but it would fail when
given a matrix of unequal size.
Change-Id: I35eb161ed7c17b982b00ecceb7b525cbfb8f3bcb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308190
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 fixes a large number of SkSL namespaces which were labeled as if
they were anonymous, and also a handful of other mislabeled namespaces.
Missing namespace-end comments have been added throughout.
A number of diffs are just indentation-related (adjusting 1- or 3-
space indents to 2-space).
Change-Id: I6c62052a0d3aea4ae12ca07e0c2a8587b2fce4ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308503
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We were only respecting this feature in runtime shaders. Note that use
of any tagged matrices will cause color filter creation to fail, but
color transformation is a totally sensible thing to want in a color
filter.
Change-Id: I482226b287ab794cb341367fce453381cb581966
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308507
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Some of these checks are currently redundant (we don't allow color
filters to have children right now). But the next CL will re-add that
capability, and the unit tests here will ensure we don't re-break things
by allowing child-sampling to violate the color filter invariant.
Change-Id: I54c10d8b1d1e376c13347296765185d42b9f644a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308285
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Android migration landed in Android CL 12234077
Chrome migration is landing in Chrome CL 2335812
Note: makeFromCompressedTexture is not used by Chrome.
Bug: skia:104662
Change-Id: Ibbe6d412cf22e87188926383d10b21f780208e48
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305102
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: Adlai Holler <adlai@google.com>
This function isn't used by Chrome so we migrate directly.
Flutter migration is at https://github.com/flutter/engine/pull/20235
Bug: skia:104662
Change-Id: I9d875acdbd162f50a6d86b3a4cae3f400e4dd38f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305180
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: I9ed98859814e462c63ab29b94f0365ccc57d2e9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307706
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
This reverts commit 07438b0cda.
Bug: skia:10369
Bug: skia:10371
This will allow Skia clients developing for Android 11+ to rely on
Android's NDK APIs for decoding, which will allow them to decode
without including their own decoding libraries (e.g. libjpeg-turbo).
Using these APIs also provides support for static HEIF images.
Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
visually.
Add tests and a grayscale png.
Update some test bots running Android R to specify ndk_api so they will
run the new code.
Change-Id: I4ca07d832dbd6a9d8cff0faea975fd70da00718f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308185
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This reverts commit cfef980939.
Reason for revert: Breaking Google3 roll
Original change's description:
> Add an SkImageGenerator that uses NDK APIs
>
> Bug: skia:10369
> Bug: skia:10371
>
> This will allow Skia clients developing for Android 11+ to rely on
> Android's NDK APIs for decoding, which will allow them to decode
> without including their own decoding libraries (e.g. libjpeg-turbo).
> Using these APIs also provides support for static HEIF images.
>
> Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
> visually.
>
> Add tests and a grayscale png.
>
> Update some test bots running Android R to specify ndk_api so they will
> run the new code.
>
> Change-Id: Ica782339b2414d472ede0b61729a127ce41892a5
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305689
> Reviewed-by: Mike Klein <mtklein@google.com>
> Commit-Queue: Leon Scroggins <scroggo@google.com>
TBR=djsollen@google.com,mtklein@google.com,scroggo@google.com,brianosman@google.com,reed@google.com
Change-Id: Ifed506a76a0ff5903d101c1bf7330d319b8376a6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10369
Bug: skia:10371
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308180
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Bug: skia:10369
Bug: skia:10371
This will allow Skia clients developing for Android 11+ to rely on
Android's NDK APIs for decoding, which will allow them to decode
without including their own decoding libraries (e.g. libjpeg-turbo).
Using these APIs also provides support for static HEIF images.
Run ImageGenSrc in kPlatform_Mode on Android to verify decoding
visually.
Add tests and a grayscale png.
Update some test bots running Android R to specify ndk_api so they will
run the new code.
Change-Id: Ica782339b2414d472ede0b61729a127ce41892a5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305689
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
SkFontDescriptor and its serialization and usage are updated to better
reflect how variations should be serialized. In addition this begins
teasing apart SkFontData since it is now mostly an artifact of how the
FreeType port works than anything else.
This also removes SkTypeface::MakeFromFontData since it is no longer
used and since SkFontData (which it takes as a parameter) was never
public. SkFontMgr::makeFromFontData now only exists to support older
skps and may be removed in the future.
Change-Id: I266bd5e87de85788661cdf5c571592ea1f2ae669
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307344
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
RRect needs 8 radii, 4 corners x XY, so should pass 8 values, not 4.
Change-Id: I997cbf2a61e6ffb32e4a13d5c95d16cb172152be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307789
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I68bb6bb239074d7cf657f56acf6c970771af62f8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
This adds support for things like "foo.xxx1", which is equivalent to
"float4(foo, foo, foo, 1)".
Change-Id: Id52111917e30d7dd8ba2e0633074b64d7ac6c72a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306727
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The majority of existing call sites were automatically updated using
clang-tidy -fix. A small handful required a manual update,
e.g. CppCodeGen.
This check is a bit lenient, and in particular will not flag cases like
`std::unique_ptr<Base>(new Derived())` which is still pretty common
throughout our codebase. This CL does not attempt to replace all the
cases that ClangTidy does not flag.
Change-Id: I5eba48ef880e25d22de80f321a68c389ba769e36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307459
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Need to notify the underlying pathref if we've made oval or rrect
Bug: skia:9000
Change-Id: I57a801f1fb446b99634d7b028249a812a5a978f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/307516
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
We only want SkSL to contain fp variables at global scope, and to sample
directly from references to those global variables. Anything else will
confuse our sample-usage analysis, and often leads to asserts in the CPP
or pipeline-stage generators.
Bug: skia:10514
Change-Id: Ie1ef10821c1b2a946a92d050fea45d95569bc934
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304599
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The repro case for this bug is where we have ~700 non-AA texture ops
with a few coverage-AA texture ops sprinkled throughout. In the old
system this could result in the quad limits being exceeded bc the
AA type for the entire run was believed to be non-AA during the creation
phase only to find that it was actually coverage-AA at execution time.
This problem manifested in both the bulk creation method and the
1-by-1/chaining creation method.
Note: in the original repro case every texture op has its own separate
texture so the problem manifests in the chaining case.
Bug: 1108475
Change-Id: I8f08fa4d5db5dbfe4a28145737895b655a145b08
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306605
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Change-Id: I82f9bffc73dcc4c07050199c755120cc964b7198
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304858
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This speeds up compiler construction, because we no longer have to parse
and process a bunch of SkSL source code during startup.
Change-Id: I6d6bd9b5ce78b1661be691708ab84bf399c6df8b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305717
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This will allow us to enable the ClangTidy check
performance-for-range-copy.
Change-Id: I11f152ffe458f5f353da8715ffd2fd47cf4e71a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306946
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Added vpinsrd to make it easy, and fixed a comment on vpinsrb's tests.
Nothing too tricky, just the naive implementations.
The hardest part was getting all the data to the right places.
No diffs!
Change-Id: Ie4c1f1e429abfa75ca80a93d108061287d5ace80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306872
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This noop refactor ports the lane-immediate idea to the 64-bit loads and
to 128-bit stores, and renames to simple load64, load128, store128.
The store128 part of this change is _slightly_ sneaky in that we pack
the argument pointer index and the lane both into int immz, but there's
plenty of space there: lane needs 1 bit, and the argument pointer index
needs maybe 3 or 4 max.
Change-Id: I3fa01bf31312b8a69c7e287d649470ba15a8ea40
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306810
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
More recontexting for SkImage. Chrome flag in CL 2323135.
Flutter migration landed in https://github.com/flutter/engine/pull/19962
Bug: skia:104662
Change-Id: Id725eb130310639457ba90f378ecdb334dd5f3cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306182
Auto-Submit: Adlai Holler <adlai@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This allows us to JIT functions taking one more argument.
Our x86 JIT was the weak link: LLVM handles arbitrary numbers
of arguments, and our aarch64 JIT handles up to seven already.
I plan to pass one more source varying sometimes, and in extremely
unusual cases it we could need six pointers:
1) uniforms
2) destination
3) 8-bit coverage plane
4) 8-bit multiply plane
5) 8-bit add plane
6) source varying
Those varyings 3-5 are all indexable off the coverage pointer 3) if we
know the dimensions, so I could be convinced that we should only pass
one there, making our maximum number of arguments four. I'll be looking
at that independently, but it doesn't hurt to have our capability to go
up to six either way.
Change-Id: Id7a5b88e382a95bb560633e95c5be273b7ea67d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306241
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
The 3 bits that represent rsp are used as a signaling mechanism to
choose between compact base + disp encoding and full base + disp +
scale*index encoding, one byte longer. If a base is encoded as rsp in
the MOD R/M byte, an SIB byte follows.
r12 shares its 3 bottom bits with rsp, so we need to treat it like rsp
when deciding whether or not we need an SIB byte. (As usual registers
r8-15's distinguishing upper bit is carried by a REX/VEX prefix.)
rsp is also treated as a special signal in the index field of the SIB
byte, meaning essentially scale=0, and as a result it's not possible to
use rsp as an index. It _is_ possible to use r12 as an index, with a
test added here. This worked without any code change. It seems the
index=rsp -> scale=0 signal is triggered by all four bits of the index
registger, including the X upper bit from REX/VEX.
I have found these charts useful:
https://wiki.osdev.org/X86-64_Instruction_Encoding#32.2F64-bit_addressing_2
Looking at those charts I noticed that rbp/r13 are also special cases,
so I'm eyeing them warily and will avoid using them for now.
Change-Id: Id78f826a39c060b03000eae7c50c642ef44d57db
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306237
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Since subsetting may require rasterizing/resolving the generator, we may
also need access to the GrDirectContext. To simplify apis, rely on
makeSubset() for that.
Related: https://skia-review.googlesource.com/c/skia/+/305970
Change-Id: I1980e3c823fb6cf54f197c350942c2f82b03e20f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306136
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: chromium:1101491
Bug: b/161896447
Found using
git grep -wiEIl \ '(he)|(she)|(his)|(hers)|(him)|(her)|(guy)|(guys)'
Change-Id: I6b91853de067fd4c2e84f7ec70275522ce6c8bfc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306186
Commit-Queue: Leon Scroggins <scroggo@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
They have been replaced with equivalent constants (and given the
requisite k prefix).
Change-Id: I70907eec234e0861cc97aac1ca03086faca42f9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306160
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
No longer needed as GrSurface and GrRenderTarget are private.
Change-Id: I2ec653b2d9daa115233bb7eaa1f2b7f880772c0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305730
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: chromium:1101491
Bug: b/161896447
Switch to more inclusive language, like "main", or remove where
simply unnecessary.
Change-Id: I36ef6ec631eb991f54f42b98887333f07c0984c2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306060
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
It's hard to imagine a scenario where it makes sense to recompile a
shader for every possible point or rect.
Change-Id: Ic4ca9a4826c4ae4b604705549170503c8ec580a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/306075
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, floats would be converted to int32 using a C-style cast when
added to a processor key, truncating any fractional component of the
value. This would cause two processors compiled with different floats to
generate the same key.
The float is now sk_bit_cast to a uint32 instead; this will preserve the
uniqueness of all values.
Additionally, the CPP generator will now abort if asked to add an
unsupported type to the processor key. Previously, it would do a C-style
cast to int32 for all unrecognized types and hope for the best.
Change-Id: I270ae8b3036497a9e9a77747255f763d9aad1927
Bug: skia:10486
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305572
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This should be the last batch of tests. All the remaining uses of
GrContext should be resolved when SkImage no longer requires a context.
Change-Id: I47eeb3b74c28f483c20d9bec4daecbdb6d2cb982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305541
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We seed the builder with the imageinfo of an image, but after it is
built we then have to "attach" it to an image (or more than one).
Check that the mip chain is compatible with the parent image we are
attaching to.
Change-Id: Iead6aee54861fdd4910f38f89a0364ed80c341d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305680
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Fixes a bug with name conflicts in the final SkSL.
Bug: skia:10526
Change-Id: Ic238f89dd778c186e775ecbaabfbaed9e426274f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Change-Id: Iaa0829d72d0da1469df2da23102ff0e3572b641b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305556
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 existed because GrTexture used to be public.
Change-Id: I5e507084ae12058a20481b517b9130b41c793d29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305521
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
* Combine find and makeMRU.
* Switch from SkMutex to SkSpinLock
In the gm 'paragraph_$' this reduces acquiring the lock from 1.2% to 0.7%
as measured by instruments and nanobench.
Change-Id: I33e3af31825f175c9de42f001acf68ffe3623a8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305564
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
GrTAllocator implies relatively limited use cases, while GrTBlockLinkedList
helps clarify the underlying data structure (and its associated advantages
and disadvantages). I am not beholden to the name, so am happy to have
a discussion on alternatives like GrTLinkedList or GrTBlockList or
GrTBlockArray.
Change-Id: I5b10801d8593991d5e804c4074a81efb1dd110ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304396
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Only works on raster backend for the now.
Bug: skia:10344
Change-Id: I2c82d5345ae83a2bb2744ab27e3d971c57ea989e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303271
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Did some cleanup to remove repetition that was distracting from the
thing being tested.
Change-Id: Ie385c6ec2d1325a1bd0ba5c2270e7f2ddd1d24b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305076
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
vpermps (added here) makes this very easy,
with an index controlling what 32-bit values go where.
A index of the form {0,2,4,6|?,?,?,?} will put the 4 low 32-bit halves
of 4 64-bit values in lanes 0,1,2,3. We can use that twice to get all 8
low halves, then our new vperm2f128 to put them together. Conveniently
vpermps can also load directly from memory:
vpermps (%rdi), {0,2,4,6|?,?,?,?}, lo
vpermps 32(%rdi), {0,2,4,6|?,?,?,?}, hi
vperm2f128 0x20, lo,hi, dst
We don't care what those top four indices are for load64_lo, so we'll
use them as the indices for load64_hi. That makes the full index
{0,2,4,6|1,3,5,7}, and load64_hi will just vpermf128 the other 128-bits
of lo/hi:
vpermps (%rdi), {?,?,?,?|1,3,5,7}, lo
vpermps 32(%rdi), {?,?,?,?|1,3,5,7}, hi
vperm2f128 0x31, lo,hi, dst
vpermps needs its index in a register, so we use a temporary for that.
Our logical lo can alias dst, and hi can alias that index, so it's just
one extra temporary register in the end.
Change-Id: Ie6a4efbf12ddada45dd09c0f580fa7350cf3019e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305171
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Add some packing instructions to make it possible.
The gist is that we've got
r(x) = {a,b,c,d|e,f,g,h}
r(y) = {i,j,k,l|m,n,o,p}
where r(x) holds each low 32-bit half of a 64-bit value,
and r(y) holds the high halves. We want to write
a,i,b,j,c,k,d,l,e,m,...
So first the vpunpck[lh]dq instructions produce
L = {a,i,b,j|e,m,f,n}
H = {c,k,d,l|g,o,h,p}
which gets us halfway there. The vperm2f128s select the low (0x20) or
high (0x31) 128-bit halves of L/H, so we end up writing to memory
dst+0: a,i,b,j,c,k,d,l
dst+32: e,m,f,n,g,o,h,p
Existing tests cover that store64 works.
Change-Id: Ic00ad9bdb448b79867584c27cf0114a42ed32379
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305156
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Does not add kNearest as a mip map mode yet.
Bug: skia:10344
Change-Id: Ida80cbf19e2b1eed5209d0680837fb45e54b4261
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303481
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change-Id: Ic44e24057b95bb014504f02a736fb4341afc8971
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304856
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: I82276e38ea4a5524127176eb5a34066b6cb06d88
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304799
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This reverts commit 423dd689df.
Reason for revert: had missed corner case when tail block was empty
Original change's description:
> Revert "Support moving blocks from one allocator to another"
>
> This reverts commit 0f064041bf.
>
> Reason for revert: unit test failures on some windows bots
>
> Original change's description:
> > Support moving blocks from one allocator to another
> >
> > Uses this new capability and the previously implemented reserve()
> > feature to implement efficient concatenation of GrTAllocators.
> >
> > Change-Id: I2aff42eaf75e74e3b595d3069b6a271fa7329465
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303665
> > Commit-Queue: Michael Ludwig <michaelludwig@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com
>
> Change-Id: I931f2462ecf6e04d40a671336d0de7d80efd313d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304604
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Michael Ludwig <michaelludwig@google.com>
TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: Ia793c2f0e7ab2f3fd437871fa7fb4f56979f9ceb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304739
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Ic7799b3c5f4294cba9ff72f8c11a2ad285ab189f
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304738
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: Ia92dc2be9a25f334bdbc098564cf2332496677fa
Bug: skia:10217
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304296
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Three TODOs, all basically the same idea: divide-by-zero is not the only
way to produce non-finite results from a division. You can also divide
by very-near-zero, and maybe some other ways.
Added is_finite() to make this clear. is_finite() is almost as cheap as
the comparisons it replaces, so performance shouldn't be affected.
Change-Id: I0a803e9ab4e3286f4e10a13d3aacee370eaaa803
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304669
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
