This fixes an assertion failure uncovered by the fuzzer.
Bug: oss-fuzz:37469
Change-Id: I626c003cfa8a0bc65851899df3a7695dbe29200b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441311
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: Ie346a2dab7648e28e71ed0b6fd9331895644f1a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441400
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
The last if check in this loop was unnecessary. It was an exact
duplicate of the first check at the top of the loop.
Change-Id: I08adc4f6df8577d168ae965220c0d9ddceb91121
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441240
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
During constant-folding, we baked in an assertion stating that any
const-typed variable reference ought to have an initial value, because
you can't declare a const variable without assigning a value. However,
function parameters are an exception to this rule! They are variable
references and are allowed to be const, but will not have an initial
value. (In this case, `const` just means you can't alter the value.)
In this case, all we needed to do was remove the assertion; we already
treated this case defensively and with the appropriate care.
Change-Id: I61242c6d08c59886c6992898f195771e6334f2b4
Bug: oss-fuzz:37465
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441239
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>
Originally, the cpu blur code could only deal with sigma <= 132. This
is due to limiting the gaussian to using a uint32_t for accumulation.
This change adds a tent filter for sigma > 132 but less < 2183. The
upper limit is not a problem because the GPU code limits sigma to 535
for both GPU and CPU. The output for GPU and CPU should be very
similar now.
Bug = skia:12338
Change-Id: I12299373911f42b914ee329a02ca81bdbd0763bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440545
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Now that Context owns the errors pointer, there is no magic behind its
accessor and no reason not to just use a straight field access.
Change-Id: I3f771f458ffdaf95d6289ba5767535a78126cc0b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441312
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This reverts commit 0f7c10ef56.
Reason for revert: Unhappy rollers
Original change's description:
> Add sRGB 8888 colortype
>
> A color type that linearizes just after loading, and re-encodes to sRGB
> just before storing, mimicking the GPU formats that work the same way.
>
> Notes:
> - No mipmap support
> - No SkPngEncoder support (HashAndEncode's .pngs are ok, though?)
> - Needs better testing
>
> This is a re-creation of reviews.skia.org/392990
>
> Change-Id: I4739c2280211e7176aae98ba0a8476a7fe5efa72
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438219
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com,reed@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ie199535b9b65ec7c7fef3c773452ea06bdbd2d9c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441376
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This was another place where we needed to use
`getConstantSubexpression` to rebuild vectors/matrices; it is a more
robust approach than trying handle each ctor type individually. The
fuzzer found an edge case with double-casting matrices to vectors that
fell through the cracks with the original approach.
In adding additional tests, I also found a case that the constant-folder
seems to ignore, `bool4(x,x,x,x) == bool4(x)`. This does fold for ints
and floats, so this ought to be fixable in a followup, but it's not a
big deal either way; this is very unlikely to occer in real code.
Change-Id: I4d577c87ef7049306685ca95250ecdf93b1dbc06
Bug: oss-fuzz:37464
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441238
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>
GCC 10.2.1 gives the following warning as error in a release build
../../src/core/SkString.cpp:227:22: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
227 | rec->data()[len] = 0;
which is detecting that fBeginningOfData is a char and that this code
cannot be reached with len == 0. Work around this by changing
fBeginningOfData to be an array of one char containing the empty string
by default.
While making this change, also make fBeginningOfData and fRefCnt
private.
Change-Id: Ic254bac465fcd02707a06010e0d7501520f7271d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441136
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This centralizes the ownership of the current ErrorReporter solely in
SkSLContext.
Change-Id: I3fdffb8133e9800ef9b898a62c2d418560079b83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441237
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is a reland of a7f9d8c2b1
Original change's description:
> Metal: Use StoreAndResolve store action when possible.
>
> This is a step towards using memoryless MSAA attachments, and
> discardable resolve. As we're currently doing a Store, Load, and then
> Resolve, this at least saves us the Load. We may fall back to this mode
> if the resolve target is framebufferOnly and hence we can't use it as
> an input attachment.
>
> Bug: skia:12086
> Change-Id: Ib5690400e756a653c4bb3f8a5f79c77cbfc61a78
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441076
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:12086
Change-Id: Icc86bcbd21b4a4eb61695a9d19a3453d823ec39f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441302
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
It was in GrGLSLFPFragmentBuilder and the recursive
step for children was in GrFragmentProcessor::ProgramImpl.
Change name mangling to create suffix on each mangle request.
Remove class GrGLSLProgramBuilder::AutoStageAdvance, combine into
GrGLSLProgramBuilder::reset() and rename to advanceStage(). Presumably
in the past the class did something in its destructor but no more.
Variable name mangling is slightly different than before. Trivially
instead of appending "_Stage0" we append "_S0" for the GP and similar
through the root FPs and XP. Also, previously the first root FP would
would get "_Stage1_c0". Now it gets just "_S1". It's first child, if it
has one, gets "_S1_c0", it's second child "_S1_c1". In other words,
appending "_c<i>" begins with the children of root FPs rather than the
root FPs themselves. All the tracking of stage/substage indices and name
mangling string is now private to GrGLSLProgramBuilder.
Bug: skia:12182
Change-Id: I03ce236b79abc9523fef6557ca575abfa847ed73
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441300
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Improved tests caught a longstanding bug in the compile-time
optimization logic for round/roundEven. These would *always* round to an
even number even when it didn't make sense to do so. (e.g. 3.1 would
round to 4.)
RoundEven isn't available in lower shader models of Direct3D;
SPIRV-Cross throws if it's unavailable. We may need a caps bit for this.
Change-Id: I3cc50238a2116b8d4e2c4059730d8b5cfb2bb056
Bug: skia:12022, skia:12352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441078
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
A color type that linearizes just after loading, and re-encodes to sRGB
just before storing, mimicking the GPU formats that work the same way.
Notes:
- No mipmap support
- No SkPngEncoder support (HashAndEncode's .pngs are ok, though?)
- Needs better testing
This is a re-creation of reviews.skia.org/392990
Change-Id: I4739c2280211e7176aae98ba0a8476a7fe5efa72
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438219
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Includes ~10 months of updates, including some key fixes
for bugs around certain intrinsics.
Change-Id: I6c44be36bf53bdc10bea8c5540f0cb2bcbe9cd92
Bug: skia:12352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441303
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The fuzzer noticed insufficient guards in IndexExpression::Convert when
converting an array size from an IntLiteral to a SKSL_INT. We had code
in IRGenerator which did this properly, so I moved our array-size
conversion logic into SkSLType and had IndexExpression share it.
Also, a variety of tests around similar error conditions were added.
Change-Id: I51529dea25f9029f81ae236511610069d66be29f
Bug: oss-fuzz:37462
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441236
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We now stop processing a var-declaration if its array-size expression is
invalid. Previously, we'd pass a null array-size expression into
convertVar, which would assert (but would fail cleanly afterwards).
Change-Id: I976f3326e32afbc7045a86d73c0dcb28f418a6f4
Bug: oss-fuzz:37457
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441079
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit a7f9d8c2b1.
Reason for revert: Flutter build
Original change's description:
> Metal: Use StoreAndResolve store action when possible.
>
> This is a step towards using memoryless MSAA attachments, and
> discardable resolve. As we're currently doing a Store, Load, and then
> Resolve, this at least saves us the Load. We may fall back to this mode
> if the resolve target is framebufferOnly and hence we can't use it as
> an input attachment.
>
> Bug: skia:12086
> Change-Id: Ib5690400e756a653c4bb3f8a5f79c77cbfc61a78
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441076
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
TBR=egdaniel@google.com,jvanverth@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
Change-Id: Ibde0fb51afd0e7c5f1c8591390091cfb23f6a418
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12086
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441137
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: dawn:22
Change-Id: I0428b7e6a3d678b606a4e51a43a573d5c31172d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440996
Commit-Queue: Stephen White <senorblanco@google.com>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Stephen White <senorblanco@google.com>
Real devices exist which support derivatives, non-square matrices, and
unsigned integers, but do not support all of ES3. On our tree,
ChromeOS-Clang-Sparky360-GPU-IntelUHDGraphics605-x86_64 meets this
criteria.
Change-Id: I6ae438c8cde291ca834f61700463a2df55705cb0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441077
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a step towards using memoryless MSAA attachments, and
discardable resolve. As we're currently doing a Store, Load, and then
Resolve, this at least saves us the Load. We may fall back to this mode
if the resolve target is framebufferOnly and hence we can't use it as
an input attachment.
Bug: skia:12086
Change-Id: Ib5690400e756a653c4bb3f8a5f79c77cbfc61a78
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441076
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This is a reland of 879b2f2e6e
Now includes a test that demonstrates the bug found by Chrome's fuzzers,
and a different (safer) implementation.
Original change's description:
> In SkCanvas destructor, discard (rather than blit) unbalanced layers
>
> Bug: skia:12267
> Change-Id: I6808f62b2385a3466b1a93db905041a6529f58cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/433360
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Florin Malita <fmalita@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Mike Reed <reed@google.com>
Bug: skia:12267
Change-Id: Ide7dc61b054761826faa5bca3eec6be2fc63c83a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440977
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ic11c25f8d2ca844c7b52384805df5454819ba6d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441061
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
These now have proper testing and compile-time optimization support.
Change-Id: I7978161ec126e1c3096b9ca9dfbb2be7d8ea02f5
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440859
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 no-op-arithmetic simplifier was written before we allowed casting a
mat2x2 to a float4, and did not expect a matrix inside a vector ctor.
The expression `float4(myMat2) * float4(anything)` would assert when we
tried to determine if `myMat2` was a constant zero or constant one.
The code has been rewritten to use getConstantSubexpression and now
allows matrices inside.
Change-Id: Id625141256bf89d816c57d2d21f16b0ec252c158
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440858
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, it was assumed that the return type from any evaluated
intrinsic would be the same as the input type. This isn't always true;
in particular, this assumption doesn't hold true for the
`floatBitsToInt` family of intrinsics.
We did have the actual return type all along, but we weren't passing it
through to the evaluation helper functions until now.
Change-Id: Idee2bdfc548abd85461a13cdb7594a173b2d551c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440857
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL just moves the files and renames them. It doesn't move them into the skgpu::v1 namespace.
Bug: skia:11837
Change-Id: Iab322d0dc5b5d1cfd32436785081539dc85c18d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440776
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Acrobat Reader (and no other viewer) appears to have issues applying the
default advances of at least some Type3 fonts. After emitting a glyph
from a Type3 font mark the calculated position after applying the
advance as uncertain, forcing the next emitted glyph to be fully
positioned.
Bug: chromium:1226960
Change-Id: I4008a7ba388e981ba5abbe3aa6fb6cc0c14a0855
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/439281
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This is a reland of a209ba17a1
Original change's description:
> Metal: update to using MTLGPUFamily for caps checks.
>
> MTLGPUFamily provides a cleaner interface, and allows us to detect
> Apple Silicon on Macs. MTLFeatureSet has been deprecated in newer OSes,
> but we fall back to it and programmatically set up the GPU family if
> running on an older OS.
>
> Also removes related code from GrMtlGpu and encapsulates it all in
> GrMtlCaps.
>
> Bug: skia:12086
> Change-Id: Ieb54bf2aca845ea809e86ccc72f34ef2211e1cfb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440517
> Commit-Queue: Jim Van Verth <jvanverth@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:12086
Change-Id: If024803710fb3658420e1b18be3abbf285eef9bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440544
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reusing existing code should be smaller than implementing the same logic
in two different places (and two different ways).
Change-Id: I5ee7768c46d6c28ac163404e9efaf99441bba504
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440717
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 allows us to avoid constructing temporary string copies just to do
a lookup in the intrinsic map.
Change-Id: Id5a5f7640cdfa3ece8a6f08a7a3623c1aeb717b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440716
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 prototype has been added sksl_public, compile-time optimization is
implemented, and test code has been improved.
Change-Id: I536d6bd7fcae437a03744941b008940bf2a3b1c1
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440524
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These were declared as inline functions outside of the GrTest namespace
for some reason, but they are only used in test code and there doesn't
seem to be any reason for putting them in the header. (It doesn't seem
like they would benefit from being inlined.)
Change-Id: I4ba831168a0220d2eb550e08600159ba68220848
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440519
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This is more useful than a boolean true/false result. A zero return
value can be interpreted the same as a false result (non-invertible).
This will be used in a followup to implement compile-time optimization
of determinants.
Change-Id: I8ccb8c97096cdbceec2d1a47c95fe380fc6f3297
Bug: skia:12202
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440523
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Having this enum on GrTessellationPathRenderer forced it to be over-#included and was blocking making GrTessellationPathRenderer.h v1-only.
Bug: skia:11837
Change-Id: I80660ed659946d7aa555057c9f4fd1136b44cca0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/440536
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
