Previously, any code which emitted a binary expression would always emit
a leading and trailing space. This caused comma expressions to look
goofy: `foo() , bar();` instead of `foo(), bar();`.
Operator::operatorName() now returns the operator token with appropriate
whitespace around it, and tightOperatorName() is a new method which
omits the whitespace. Functions which assemble binary expressions
should now concatenate `x + operatorName() + y` instead of hard-coding
`x + " " + operatorName() + " " + y`. Prefix/postfix expressions should
use `tightOperatorName()` because otherwise negation looks bad (` - 123`
instead of `-123`).
Super low priority, but it was easy to fix.
Change-Id: I3c92832207293a310fb1070b3b5e72455757b0ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497776
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 3530d4d3c3.
Reason for revert: Use after free: context deleted before ops
Original change's description:
> redo AtlasTextOp caching
>
> The fuzzer ash_unittests is passing GrRecordingContext from thread
> to thread. This means that 120 bytes for a AtlasTextOp bytes are
> leaking on the first thread because the ClearCache is never
> called on that thread.
>
> Move the cache to the GrRecordingContext. Use a thread local to
> store a pointer to the GrRecordingContext so that new and delete can
> find the cache.
>
> Add a field to AtlasTextOp to save the recording context so that it can
> populate the thread local just before delete is called.
>
> Bug: chromium:1265033
>
> Change-Id: I9802910428bf091c534c96d4ce729c3b3445c76b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497147
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Herb Derby <herb@google.com>
Bug: chromium:1265033
Change-Id: Ia51ad196cf2966225b177f799d477e1f705187c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498616
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The fuzzer ash_unittests is passing GrRecordingContext from thread
to thread. This means that 120 bytes for a AtlasTextOp bytes are
leaking on the first thread because the ClearCache is never
called on that thread.
Move the cache to the GrRecordingContext. Use a thread local to
store a pointer to the GrRecordingContext so that new and delete can
find the cache.
Add a field to AtlasTextOp to save the recording context so that it can
populate the thread local just before delete is called.
Bug: chromium:1265033
Change-Id: I9802910428bf091c534c96d4ce729c3b3445c76b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497147
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This token is not used anywhere in SkSL.
Change-Id: Ied650f222c349664f67239915be83ffd492c847e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498436
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I3dd8ecb7dc3a5f2781ea86e0526360a1fc3361cb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498277
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We had four separate expression-array cloning implementations spread
throughout the code. It's now unified as a clone() method directly on
ExpressionArray.
Change-Id: I5b38ceeca36bb2ba5d0f41b3298a8740308f7627
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497744
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
If user code created global variables named `__device_FragCoords` or
`__device_Clockwise`, SPIR-V code generation would fail when trying
to synthesize fake variables with those names. Use private names
instead, beginning with `$device_` instead of `__device_`.
Change-Id: I167aaa6680426f9c5f0c7687b6c72500b246423f
Bug: skia:12855, oss-fuzz:43970
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498116
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
These identifiers are reserved for SkSL internal use (and can't be
exposed to GLSL or Metal anyway).
Change-Id: Id554cbf21ed2fb66785e77700ff79424ecdf66db
Bug: skia:12854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498036
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
In practice, we use a leading $ internally to indicate private types,
but we don't use $ characters anywhere else in identifiers, and we don't
want external users to use $ in their identifiers at all. Now our lexing
rules only permit $ as the leading character of an identifier, and
disallow $ in directives entirely.
(This CL will invalidate oss-fuzz:43970 as generated, but the root cause
of this issue is skia:12855, which will still need to be addressed.)
Change-Id: I0eedc97fa4a447506085c9facfa91307a37e3182
Bug: skia:12854
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/498037
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In debug builds, BufferWriters store a Mark that points to the end of
their data array, and all write operations are validated against that.
IndexWriter and UniformWriter require the size or ending mark to be
provided, while VertexWriter does not.
Legacy creation of VertexWriters exist in several places in Ganesh where
it wasn't trivial to have the size of the buffer available when the VW
was created. Most common cases that go through GrMeshDrawTarget or
GrEagerVertexAllocator, or graphite's DrawBufferManager automatically
have this information embedded in the writer.
The major semantic change that comes with this is what
BufferWriter::makeOffset does. Previously you could make as many offset
writers as desired and overwrite each other, but now the intended use
case is enforced: the calling writer can only write up to the offset,
and the returned writer can write from the offset to the end.
Change-Id: I2df0f618cd7e50ff657366aeb4955d729c3a938a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496880
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This reverts commit 0c9b8bae42.
Reason for revert: breaking bots, apparently due to use-after-free?
Could be a real clone bug...!
http://screen/9NenpjnYNoDU58G
Original change's description:
> Added tests for sksl clone() on our test corpus
>
> Change-Id: I9022a6aa53b039e5aec2ceeb0062d536e5e278c9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496601
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Ida8978a38ac24081895cd97a62718b6ec94f57c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497777
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
While working on an unrelated fuzzing task, I noticed in the logs that
~PromiseImageInfo was crashing on a null dereference while the fuzzers
were doing smoke checks. It looks like fuzzer was being detected as
"broken" and presumably is not being run: http://screen/AKPKmyx7mHJ5nir
This CL should resolve the null dereference, but I don't know if this
indicates a deeper problem.
Bug: skia:12851
Change-Id: I4a44891783504372f28ab4b320c01a14df0892a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497436
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Originally, `simplify_vector` took a template parameter for the value
type, and so we needed to distinguish between SKSL_INT and float
vectors when simplifying them. This distinction was eliminated a while
ago (at http://review.skia.org/447836 ) so we can now handle all numeric
vector types the same way.
Change-Id: I55b1f4bf64c766148e8e25a23634e4b13c1c8f1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497699
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This was the last usage of the ptr() function that exposed VW's
underlying pointer to a possibly GPU-mapped buffer. At this point,
VW's can only be written once, never read, and Marks still allow for
typical pointer math you'd want to do when filling a buffer.
(caveat: the creater of the mapped buffer can't then expose the void*
in some other way).
Change-Id: I5401bdd71ec3063141561875d7b8770c624c4c49
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496784
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Our `simplify_vector_equality` function turns out to be equally suitable
for matrices and vectors as well.
Change-Id: Ica6fa3b421357dc40730a76c130a584ab8506954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497613
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
We need this for Graphite enforcing single owner on the cache.
Bug: skia:12754
Change-Id: Ib26167bce5ee5db6ffe60d3c49091ac14e05f32a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496604
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Change-Id: I9d435a7a68ef870ef027ee3c6acb79792773868b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497609
Reviewed-by: Heather Miller <hcm@google.com>
Auto-Submit: Heather Miller <hcm@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
Unfortunately, this won't change any behavior until ExtractPaintData is switched over to using PaintParams::toKey
Bug: skia:12701
Change-Id: I2b00256d34de1b8b99a65e26c08f5f082090a341
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497596
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Presumably, SkVM will also need a way to represent the data passed to its programs
This CL attempts to be as mechanical as possible.
Bug: skia:12701
Change-Id: I7fb2c06d645809bf5422a25b8b537cf6c249fede
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497136
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
We previously had a separate implementation of SkSLStringStream for use
in SKSL_STANDALONE. This is no longer necessary. As part of this change,
we fix an issue with the non-standalone version where calling str() and
then continuing to write to the stream would lead to incorrect results.
Change-Id: If83057470ae7e425dfc33ea7b372990285657f42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497451
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 7e8fdf4706.
Reason for revert: breaking bots
Original change's description:
> [graphite] Add ImageShader and BlendShader SkPaintParamKey support
>
> Unfortunately, this won't change any behavior until ExtractPaintData is switched over to using PaintParams::toKey
>
> Bug: skia:12701
> Change-Id: I4f51dbb43983fe2f01947e26814f581a6d9033cd
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496783
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:12701
Change-Id: If180bd3753c95d920715f37347559cee4fa2da20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497455
Auto-Submit: Robert Phillips <robertphillips@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: I9b22c628147fe3d66e3148351ef765bb8c2ebe50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497141
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Unfortunately, this won't change any behavior until ExtractPaintData is switched over to using PaintParams::toKey
Bug: skia:12701
Change-Id: I4f51dbb43983fe2f01947e26814f581a6d9033cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496783
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Marks can be used to remember the start of a writer, or a later position
(e.g. end) that it can advance to, compare and compute byte differences,
without exposing the underlying pointer to clients.
Change-Id: Ie8823fa1418002f9a5d645678f01c2cbf1a87acc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496780
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The skvm implementation of pow2 uses a clever bit-twiddling trick to
generate a floating-point value that can be cast to integer, then bit-
punned to float, to generate its result. However, the bit trick fails
for large inputs, and the bit-punning step generates a nonsense result.
This is now fixed by using a well-positioned clamp.
Change-Id: I55143a98324f5f518d0875149a0b6ce6d734ded0
Bug: skia:12847
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497283
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Adds makeVertexWriter[AtLeast] and makeIndexWriter[AtLeast] wrappers
around the vertex-space virtuals. Updates call sites of those functions
that immediately passed the returned pointer into a VertexWriter ctor
to call the new function.
This is simplification that will make changing the ctor of VertexWriter
easier (to come in another CL).
Change-Id: I8f4f1a9bed71e368828a42ed798fa6a93030a765
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496776
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Internally, SkSL breaks apart multiple vardecls into individual vardecls
wrapped inside an unscoped Block. Previously, the Debugger did not
properly distinguish a scoped Block from an unscoped one, and the
variables inside unscoped Block would be considered as "out-of-scope" as
soon as the unscoped Block was closed. This would make them vanish from
the Local Variables pane of the debugger at the end of the vardecl
statement.
(Note that this transformation also means that Step stops once for each
individual variable, but that isn't a big deal.)
Change-Id: I2ee2409e8556d77b58d7645658bd497f87c31e39
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497278
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: oss-fuzz:43699
Change-Id: Ie856aed03b513a4ea84a09deda6a05b95b086b07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496876
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Introduce PathOpSubmitter which handles submitting ops for paths.
Use this in all the different flavors of path sub runs.
Change-Id: I5d6ce06e7ffbe921eec4f05181d8fa84b49cf590
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496879
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
On Windows, with depot_tools, git might be a batch file called
git.bat. Add that to the default search list to make it easier
to find a git command.
Change-Id: I825432e0f7fc72c37a1f2394a68e2f3f3e65f958
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/497056
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Dominik Röttsches <drott@google.com>
While working on the debugger, I realized that we had no interface for
getting the current line of any stack frame other than the currently-
executing frame. This can be useful for letting users explore the call
chain on the stack.
(While I was here, I also renamed `tidy` to `tidyState` in order to
match the Typescript implementation.)
Change-Id: Ife8c30d81adc17389a109698e3fba7f93e523e9e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496886
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
The goals here are to:
reduce the use of magic numbers
make casts a bit safer
Bug: skia:12701
Change-Id: I6b1628fff4041bd0998060a757412b6a7260be1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496378
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change-Id: Ib7cffcbdfdb6edf0816151c6dff5961365cabf61
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496882
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
When transforming a for loop with multiple variable initializers, e.g.
for (int x = 0, y = 0; ...), we rewrite it to:
{ int x = 0; int y = 0; for (;...)
But we would keep the symbol table containing x and y attached to the
for loop. This means that while walking the code tree, we would run
into the declarations for x and y before running into the symbol table
containing them. This poses problems for the dehydrator, which would
fail when running into this construct. This fix simply moves the
symbol table to the containing block instead.
Change-Id: Ic806bbda19c954224a726b4a315f3102852e26b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/494717
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This update removes an unused field (the "slot" value was redundant,
since we always emitted arrays in order and there are no gaps). This CL
mostly exists to test out the ramifications of updating the trace
version and make sure things don't break when we do.
Change-Id: I456837db682b0085fc5afbbc0bcb62af3440c6e5
Bug: skia:12755
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496607
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
SkVx.h attempted to not use vector instructions if SKNX_NO_SIMD was set.
However, this was incomplete and could lead to a wasm build with
SKNX_NO_SIMD still trying to use some vector instructions which are
not present if __wasm_simd128__ is defined.
This change requires some additional "include what you use" includes
since some other files were depending on SkVx.h including the vector
instruction headers.
Bug: cl/421848579
Change-Id: I6a878d64b76677a925b94724926c62f3e42ddd4c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496313
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This was added with d2ccc772d "Changes to public.bzl to support building
on iOS with objc_library bazel target." [0]. A recent test showed
nothing that breaks by removing this define, so try removing it now.
[0] https://codereview.chromium.org/1639443002
Change-Id: I9938e6a66b29e18332d9e7c4f60e2f2b2a5355ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496606
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
