Unlike DSLPossibleExpression, this was only used in a few rare places,
so it was much easier to remove.
Change-Id: I483dc061691f89d3e808d5bbc23c500bc57b680a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542662
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 193c16380f.
Reason for revert: fixed google3 warning
Original change's description:
> Revert "Eliminate DSLPossibleExpression."
>
> This reverts commit f2d000328f.
>
> Reason for revert: breaking google3 roll
>
> Original change's description:
> > Eliminate DSLPossibleExpression.
> >
> > The Possible(Expression|Statement) classes were added at
> > http://review.skia.org/375069. These classes were responsible for
> > capturing `__builtin_FILE()` and `__builtin_LINE()` when an
> > expression or statement was added to a hand-authored DSL program. This
> > allowed errors to be reported on the C++ file/line where they were
> > encountered. This was a good feature to have, when the plan was to
> > author the majority of SkSL code via DSL.
> >
> > Later, IRNode positions were converted from an integer line number to
> > SkSL Positions at http://review.skia.org/518137. This gave us range
> > tracking, but at a high memory cost (16 bytes per IRNode, versus four
> > bytes when we tracked line numbers only).
> >
> > Positions were reduced to 8 bytes at http://review.skia.org/521005 by
> > removing the filename, which was only used for hand-authored DSL. (The
> > size was pared all the way back to 4 bytes at
> > http://review.skia.org/533699 by packing the data more efficiently.)
> >
> > __builtin_FILE/LINE capturing was removed entirely at
> > http://review.skia.org/528366; the filename was discarded anyway and
> > the line number didn't have a range and wasn't very meaningful without
> > a filename. Also, it didn't matter very much since we no longer intended
> > to hand-craft our programs in DSL.
> >
> > At this stage, DSLPossibleExpression stopped adding value and simply
> > served to move Expressions around.
> >
> > Change-Id: I29ac33e08dcc1804cc0619c1e8856ba28ebcc51d
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542145
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
>
> Change-Id: I33badbdcce8760200246bf50e4932d42721ea952
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543078
> Reviewed-by: Tyler Denniston <tdenniston@google.com>
> Owners-Override: Kevin Lubick <kjlubick@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Tyler Denniston <tdenniston@google.com>
Change-Id: I71f248b2343806f85cad5f0661470c95334bbe22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545236
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
gazelle ended up being more liability than asset for our C++ rules.
It required devs to manually run the command frequently (and was
easy to forget until the CQ failed). The fact that we still had to
edit the source files (e.g. the "srcs" cc_libraries) meant that
the mixture between generated and hand-written caused some
tension (see include/third_party/vulkan for a good example).
The combination of gazelle and our IWYU enforcement added several
bits of churn without any real benefit. The generated rules
also didn't help identify cases where we were not keeping tight
boundaries (e.g. non-gpu code and gpu code).
Identifying third_party deps automatically ended up being trickier
than anticipated (see the deleted //third_party/file_map_for_bazel.json)
Using the "maximum set of dependencies" worked ok, but ended up
increasing build time unnecessarily. For example, compiling
CanvasKit for WebGL always needed to compile Dawn because
SkSLCompiler.cpp sometimes needs to include tint/tint.h.
Follow-up CLs will rebuild the BUILD.bazel rules without gazelle.
Note to Reviewers:
- The only file worth manually reviewing here is bazel/Makefile.
Change-Id: I36d6fc3747487fabaf699690780c95f1f6765770
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543976
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
This reverts commit 35ef74b7a4.
Reason for revert: causing chrome roll failure?
Original change's description:
> Reland "Create updateResourceLabel method."
>
> This is a reland of commit 605f92c7d7
>
> Original change's description:
> > Create updateResourceLabel method.
> >
> > In this CL, the GrSurfaceProxy's and GrDrawOpAtlas's label strings
> > are plumbed so that it can be stored in the label string of
> > GrGpuResource. updateResourceLabel method, which is called from
> > setLabel method, will add labels to Skia OpenGL backend using ANGLE's
> > labeling API.
> >
> > Bug: chromium:1164111
> > Change-Id: I370715d186357e920d260d624d77586cdddd11e8
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541230
> > Reviewed-by: Greg Daniel <egdaniel@google.com>
> > Commit-Queue: Greg Daniel <egdaniel@google.com>
>
> Bug: chromium:1164111
> Change-Id: Ia20a81d22e320813cbc4ad1d41e06e47dc3827bb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544058
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: chromium:1164111
Change-Id: Icacb3a0f2f2ee03afc09e03c2ee59b03d5bdd811
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544241
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Tyler Denniston <tdenniston@google.com>
This is a reland of commit 605f92c7d7
Original change's description:
> Create updateResourceLabel method.
>
> In this CL, the GrSurfaceProxy's and GrDrawOpAtlas's label strings
> are plumbed so that it can be stored in the label string of
> GrGpuResource. updateResourceLabel method, which is called from
> setLabel method, will add labels to Skia OpenGL backend using ANGLE's
> labeling API.
>
> Bug: chromium:1164111
> Change-Id: I370715d186357e920d260d624d77586cdddd11e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541230
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: chromium:1164111
Change-Id: Ia20a81d22e320813cbc4ad1d41e06e47dc3827bb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544058
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This reverts commit 605f92c7d7.
Reason for revert: breaking bots on CQ
Original change's description:
> Create updateResourceLabel method.
>
> In this CL, the GrSurfaceProxy's and GrDrawOpAtlas's label strings
> are plumbed so that it can be stored in the label string of
> GrGpuResource. updateResourceLabel method, which is called from
> setLabel method, will add labels to Skia OpenGL backend using ANGLE's
> labeling API.
>
> Bug: chromium:1164111
> Change-Id: I370715d186357e920d260d624d77586cdddd11e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541230
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>
Bug: chromium:1164111
Change-Id: I1f2d8585fefde6b74d23f2ea14d91ee6d3e1e60d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543486
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
In this CL, the GrSurfaceProxy's and GrDrawOpAtlas's label strings
are plumbed so that it can be stored in the label string of
GrGpuResource. updateResourceLabel method, which is called from
setLabel method, will add labels to Skia OpenGL backend using ANGLE's
labeling API.
Bug: chromium:1164111
Change-Id: I370715d186357e920d260d624d77586cdddd11e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541230
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
In PDF there are two different physical encodings of strings (as a
sequence of bytes). There is the literal string encoding which is
delimited by '(' and ') which stores the bytes as-is except for '(',
')', and the escape character '\' (which can introduce octal encoded
bytes). There is also the hex string encoding delimited by '<' and '>'
and the bytes are encoded in hex pairs (with an implicit '0' at the end
for odd length encodings).
The interpretation of these bytes depends on the logical string type of
the dictionary key. There is a base abstract (well, almost abstract
except for legacy purposes) string type. The subtypes of the string type
are `text string`, `ASCII string`, and `byte string`. The `text string`
is logically further subtyped into `PDFDocEncoded string` and `UTF-16BE
with BOM`. In theory any of these logical string types may have its
encoded bytes written out in either of the two physical string
encodings.
In practice for Skia this means there are two types of string to keep
track of, since `ASCII string` and `byte string` can be treated the
same (in this change they are both treated as `byte string`). If the
type is `text string` then the bytes Skia has are interpreted as UTF-8
and may be converted to `UTF-16BE with BOM` or used directly as
`PDFDocEncoded string` if that is valid. If the type is `byte string`
then the bytes Skia has may not be converted and must be written as-is.
This means that when Skia sets a dictionary key to a string value it
must, at the very least, remember if the key's type was `text string`.
This change replaces all `String` methods with `ByteString` and
`TextString` methods and updates all the callers to the correct one
based on the key being written.
With the string handling corrected, the `/ActualText` string is now
emitted with this new common code as well for better output and to
reduce code duplication. A few no longer used public APIs involving
these strings are removed. The documentation for the URI annotation is
updated to reflect reality.
This change outputs `UTF-16BE with BOM` with the hex string encoding
only and does not attempt to fix the literal string encoding which
always escapes bytes > 0x7F. These changes may be attempted in a
separate change.
Bug: chromium:1323159
Change-Id: I00bdd5c90ad1ff2edfb74a9de41424c4eeac5ccb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543084
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Also cleans up the scattered references remaining in the code base
(including in files I thought I got already...).
Change-Id: I7004354b1e9cea9f9d9f45b791d8ab9ce557ba01
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542647
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Also makes SDFMaskFilter available.
Bug: skia:13118
Change-Id: I27ca3ed85a057267387841be5dad10d8e6c7f7bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543079
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This is cleanup left over from a prior CL's review. We will want to
use the combination builder outside of graphite.
This CL is mainly just moving stuff around except for the addition
of the SkCombinationBuilder.buildCombinations method and CreateKey
more accessible outside of Graphite.
Bug: skia:12701
Change-Id: If2cae6fcff5670e488bc14473b7b1d2f9b1fecd7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543196
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This also adds support for making a Resources unbudgeted state when it
is pulled out of the cache to be used for an SkImage or Surface. It
then puts the resources back as budgeted when returned to the cache.
Bug: skia:12754
Change-Id: I469ace602aa6f5f708b82655e94557ff8ad45a72
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538046
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Adds a saturated_add function that was on SkNx and used in
SkXfermode_opts, but hadn't been ported to skvx yet.
Removes the Sk4px_opts variants and simplifies some of its functions;
many were already defined skvx.
The largest change is that Sk4px does not extend skvx::byte16, since it
used to extend Sk16b. Now it just has a vector as a data type. This
was necessary so that we could define operators that were typed for
Sk4px and Wide w/o conflicting with the free operators that were
defined for the base skvx types.
Change-Id: I8c667ba86f662ccf07ad85aa32e78abfc0a8c7ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542645
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:13118
Change-Id: Id7c92b2e1af9febb3e576c244fcff3aedd27f408
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542646
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This reverts commit f2d000328f.
Reason for revert: breaking google3 roll
Original change's description:
> Eliminate DSLPossibleExpression.
>
> The Possible(Expression|Statement) classes were added at
> http://review.skia.org/375069. These classes were responsible for
> capturing `__builtin_FILE()` and `__builtin_LINE()` when an
> expression or statement was added to a hand-authored DSL program. This
> allowed errors to be reported on the C++ file/line where they were
> encountered. This was a good feature to have, when the plan was to
> author the majority of SkSL code via DSL.
>
> Later, IRNode positions were converted from an integer line number to
> SkSL Positions at http://review.skia.org/518137. This gave us range
> tracking, but at a high memory cost (16 bytes per IRNode, versus four
> bytes when we tracked line numbers only).
>
> Positions were reduced to 8 bytes at http://review.skia.org/521005 by
> removing the filename, which was only used for hand-authored DSL. (The
> size was pared all the way back to 4 bytes at
> http://review.skia.org/533699 by packing the data more efficiently.)
>
> __builtin_FILE/LINE capturing was removed entirely at
> http://review.skia.org/528366; the filename was discarded anyway and
> the line number didn't have a range and wasn't very meaningful without
> a filename. Also, it didn't matter very much since we no longer intended
> to hand-craft our programs in DSL.
>
> At this stage, DSLPossibleExpression stopped adding value and simply
> served to move Expressions around.
>
> Change-Id: I29ac33e08dcc1804cc0619c1e8856ba28ebcc51d
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542145
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I33badbdcce8760200246bf50e4932d42721ea952
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543078
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
The Possible(Expression|Statement) classes were added at
http://review.skia.org/375069. These classes were responsible for
capturing `__builtin_FILE()` and `__builtin_LINE()` when an
expression or statement was added to a hand-authored DSL program. This
allowed errors to be reported on the C++ file/line where they were
encountered. This was a good feature to have, when the plan was to
author the majority of SkSL code via DSL.
Later, IRNode positions were converted from an integer line number to
SkSL Positions at http://review.skia.org/518137. This gave us range
tracking, but at a high memory cost (16 bytes per IRNode, versus four
bytes when we tracked line numbers only).
Positions were reduced to 8 bytes at http://review.skia.org/521005 by
removing the filename, which was only used for hand-authored DSL. (The
size was pared all the way back to 4 bytes at
http://review.skia.org/533699 by packing the data more efficiently.)
__builtin_FILE/LINE capturing was removed entirely at
http://review.skia.org/528366; the filename was discarded anyway and
the line number didn't have a range and wasn't very meaningful without
a filename. Also, it didn't matter very much since we no longer intended
to hand-craft our programs in DSL.
At this stage, DSLPossibleExpression stopped adding value and simply
served to move Expressions around.
Change-Id: I29ac33e08dcc1804cc0619c1e8856ba28ebcc51d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542145
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
These tests would be expected to trigger the CFI sanitizer (skia:13339).
We don't currently support CFI sanitizers, though (skia:13338).
Tests are confirmed to fail in Linux by copying TArray unit tests into
SkSLCompiler and running Chrome unit tests:
http://screen/3cYe9r4MYSXmTTa
Change-Id: If2d9e611bc9902e2ac6e0960a38e876bb1952884
Bug: skia:13338, skia:13339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542639
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:13118
Change-Id: Id4afcfeeb9a5b44a0e2cb24b70c76a81ec5daaea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542300
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Graphite will also need tokens to track draw and upload progress, so
moved the token class to a shared location, and tokens into the Plot
class.
Bug: skia:13118
Change-Id: Ib5748a7f25d79ee0aabca7427511cc9fd6e75117
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541740
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The ShaderType enum seems like it will persist while the ShaderCombo will not.
Bug: skia:12701
Change-Id: Idf9ed89a50bac46b93a81da3e814207c83b3a1ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541723
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Removes specializations for all() on AVX2 and SSE 4.1, which give the
wrong results if the ints didn't have all bits set (inconsistent with
other platforms and non-SIMD). Added a unit test that checks this case.
The mirror specializations for any() on AVX2 and SSE 4.1 are actually
valid, so added those, and added a 2 instruction specialization for
SSE for any() and all(). This is what clang-trunk produces on -O3, but
ToT clang struggles to vectorize it.
Also adds specializations for NEON for any() and all(), since even
clang-trunk was struggling to vectorize it automatically. In
particular, this will help skgpu::graphite::Rect's implementations of
intersect and contains, which use any/all to get a final boolean value.
In the Instruments app, I had see Rect's intersection as a hotspot
on the Mac M1, and this vectorization helps a bit.
Also takes the opportunity to remove fake C++14 constexpr for a
real constexpr.
Change-Id: Ib142e305ae5615056a777424e379b6da82d44f0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542296
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Change-Id: I511f6105537b24953de1533ad7b73d1186afd4fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541060
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Moving DistanceFieldAdjustTable, SDFMaskFilter, and
SDFTControl.
Bug: skia:13118
Change-Id: Id9ace9bc02068437c0f4fd674e5864da83f83228
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542297
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The DSLVar class hierarchy had many constructor variations to support
the idea of creating DSL variables without explicitly assigning a name
to them. Instead, name-mangling would automatically assign them a name
like `_123_var`. This was designed to make it easlier to write a
complete shader in DSL.
We no longer have DSL name-mangling, or intend to create complete
programs in pure DSL. In the absence of name-mangling, these
constructors aren't useful (every variable would be named `var`).
They have been removed.
Change-Id: If533e479cc04c5a6ced9a7e880dcc56063f29374
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542138
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I believe this was only needed during the sprint, before we had
RenderSteps.
Bug: skia:12701
Change-Id: I4945fc69f9c1cd419b8c3143b6638e4769761ff9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541976
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This reverts commit 4b354dfb5b.
Reason for revert: not responsible for chromium:1326848
Original change's description:
> Revert "Remove operator= from DSL expression types."
>
> This reverts commit b143520625.
>
> Reason for revert: might have caused chromium:1326848
>
> Original change's description:
> > Remove operator= from DSL expression types.
> >
> > This means that in raw DSL code, `foo = bar` will need to be written as
> > `foo.assign(bar)`. This is admittedly slightly less convenient. However,
> > in practice, raw DSL code is very rare, and in fact we only had one
> > use-case for operator= across all of our code. This change will allow us
> > to simplify the DSL parser by eliminating the `DSLWrapper` helper class,
> > which was only needed because we overloaded `operator=`.
> >
> > Change-Id: I4fbc39b2c1443fc26a761d424c52e3b9b59169eb
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541062
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Brian Osman <brianosman@google.com>
>
> Change-Id: I7c7e032c465aba004ba78fd69650c8037e019861
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541737
> Auto-Submit: John Stiles <johnstiles@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: I0814f87e360b5574d4a06764bbfa747c49820774
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542136
Commit-Queue: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bug: skia:11209
Change-Id: I89d97bce1934a985c1efed602680dd534702a800
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541072
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This setting was meant to detect errors in hand-authored DSL programs,
where a DSL expression or statement object was instantiated but wasn't
actually added to the program.
The DSL parser always disabled this flag. We don't plan on creating
large hand-crafted DSL programs going forward. Also, if you fail to add
an expression/statement to your program, your program will be missing
part of its code, which is already pretty self-evident as errors go.
Change-Id: I4c7b89d1f05cc65dc84948d98da8d6576c1be244
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541739
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The DSLVarBase `fDeclared` field was used to track whether a variable
was created with `Var`, but never declared with a matching `Declare`, in
a hand-authored DSL program. This was expected to be a common source of
slip-ups in hand-authored DSL, so we tracked it and printed a handy
error message if `Declare` was ever missing. However, this complicated
testing, because in tiny test snippets we _do_ want the ability to
create variables without necessarily declaring them. So, we had a
separate program setting to disable enforcement for test code.
For programs created in DSLParser, tracking this state is unnecessary;
the parser won't ever forget to declare variables.
Since hand-authored DSL programs are no longer expected to be
commonplace, we can remove the state tracking and the associated
ProgramSettings field entirely.
Change-Id: I32b28f8a2ca6591d3da80cd974fff8101f5a4be8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541638
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This feature is always disabled when DSLParser is used. Name mangling
was only useful when compiling hand-authored DSL code that explicitly
declared two variables with the same name in separate scopes. In
practice, as long as we aren't declaring complete programs in DSL form,
we don't need this.
Change-Id: Icad921eb86365b3b114ff1872b1c40c41470a4b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541637
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
* Added support for field access expressions, excluding user-defined
interface blocks.
* Added a test case for vertex stage builtin declarations.
* Added a NOP workaround for sk_PointSize which does not have a
WGSL/WebGPU equivalent.
Bug: skia:13092
Change-Id: I46fd7bb0a9fe55d2e3ed2b3b72f63376c1ed4f31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539082
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit b143520625.
Reason for revert: might have caused chromium:1326848
Original change's description:
> Remove operator= from DSL expression types.
>
> This means that in raw DSL code, `foo = bar` will need to be written as
> `foo.assign(bar)`. This is admittedly slightly less convenient. However,
> in practice, raw DSL code is very rare, and in fact we only had one
> use-case for operator= across all of our code. This change will allow us
> to simplify the DSL parser by eliminating the `DSLWrapper` helper class,
> which was only needed because we overloaded `operator=`.
>
> Change-Id: I4fbc39b2c1443fc26a761d424c52e3b9b59169eb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541062
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7c7e032c465aba004ba78fd69650c8037e019861
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541737
Auto-Submit: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This reverts commit 3b4f862d05.
Reason for revert: might have caused chromium:1326848
Original change's description:
> Remove DSLWrapper helper class.
>
> Without an `operator=` on our expressions and variables, we no longer
> need to wrap all our expressions with a helper.
>
> Change-Id: I8110079f61c9ad01997f7c4b376db223dc4b6e17
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541063
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I7efb3004913f7c85dc551d9740a6b31971de52d2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541736
Auto-Submit: John Stiles <johnstiles@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
- Add support for simple assignment expressions, as well as swizzle,
variable references, and type conversion constructors to support
simple assignment test cases that reference built-ins and local variables.
- Handle a case where the type of a SkSL built-in differs from its WGSL
counterpart and emit a type cast when such a variable gets referenced.
- Add additional test cases for supported WGSL features that could not
be tested without simple assignment support.
Bug: skia:13092
Change-Id: Ib1ff3bcef60e436c9be5c08236c9fe4de02dd005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538420
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
WGSL requires that pipeline stage IO parameters that are scalar or
vector integers be annotated with the "flat" interpolation attribute.
Bug: skia:13092
Change-Id: I2930da6cf468ac8cd9ccaca0004e5849065411dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538419
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This just adds the API. Additional work will be required to actually
create an SkShaderSnippet for the runtime effect.
The SkBlenderID will be used in the combination system to specify the
Blender runtime effect as a combination candidate.
Bug: skia:12701
Change-Id: I524a2fa7a00ece6d9422891bcbc82ce0237679e3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540170
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
1. It is illegal to declare an empty struct in WGSL. The codegen no longer
declares an empty struct for pipeline stage inputs and outputs if the
program does not reference them.
2. WGSL requires that every vertex program declare a @builtin(position) output.
The codegen now emits this pipeline stage output even for an empty
program that doesn't assign to sk_Position. This allows unit tests for
vertex programs to be authored without unnecessary boilerplate.
Bug: skia:13092
Change-Id: Ib023200e2ba13acf96859817249686d7f815cb00
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538418
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
- Introduced the SK_ENABLE_WGSL_VALIDATION macro which is currently only
enabled when skslc gets compiled when using the `skia_compile_sksl_tests`
setting.
- SkSLCompiler::toWGSL now validates its output using Tint's WGSL reader
structures based on conditionally compiled code depending on the
SK_ENABLE_WGSL_VALIDATION flag.
- Fixed `warning: use of deprecated language feature: struct members should be separated with commas"
warnings that were generated for HelloWorld.wgsl.
Bug: skia:13092
Change-Id: Ib894457030004966221faf82f61360e390b95e22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537802
Commit-Queue: Arman Uguray <armansito@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This adds aliases like skvx::float2, float4, etc. to SkVx.h and goes
through existing usages of SkVx to standardize on those aliases, or
refer to the full name directly.
In particular, this lets us clean up the equivalent aliases in
src/gpu/tessellate, src/gpu/graphite/VectorTypes and src/gpu/ganesh/GrVx
Where possible, I switched to using skvx::Foo directly and leveraged
auto to make it less redundant. Headers always used the full type
except for PatchWriter.h and Rect.h because of the number of their
usages. In this case, the alias is scoped to private so it can't leak.
This is prep to migrate older code that is still using SkNx and its
aliases like Sk4f to SkVx as well.
Change-Id: I9dd104e83cf17c2b88995a047cfd2e2b0fe6fac2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541058
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
ProgramSettings now has a fRequiredVersion, based on the #version
directive (if any). There is plenty more to do, and this is an
intermediate state where the "fEnforceES2Restrictions" bool still
exists. Eventually, that will go away, or change into a check between
the required version and the SkCapabilities supplied (also in
ProgramSettings).
As a proof-of-concept, this migrates some use-cases to insert the new
version directive, and unlock ES3 features in some tests and benches.
Bug: skia:11209
Change-Id: I964975931039588d7a302bfedd09246e3d4a79d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540037
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Consolidates some other constants, removes the pow2 and pow4 functions
that just computed `x*x` or `x*x*x*x`. Moves the `mix` function into
PatchWriter since that's the only place it's used.
Change-Id: Idaaba40f1007e1f0331ec8d335e6afbe69ddfbad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540920
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Without an `operator=` on our expressions and variables, we no longer
need to wrap all our expressions with a helper.
Change-Id: I8110079f61c9ad01997f7c4b376db223dc4b6e17
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541063
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This means that in raw DSL code, `foo = bar` will need to be written as
`foo.assign(bar)`. This is admittedly slightly less convenient. However,
in practice, raw DSL code is very rare, and in fact we only had one
use-case for operator= across all of our code. This change will allow us
to simplify the DSL parser by eliminating the `DSLWrapper` helper class,
which was only needed because we overloaded `operator=`.
Change-Id: I4fbc39b2c1443fc26a761d424c52e3b9b59169eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541062
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This is a reland of commit 69fecd6c2d
Original change's description:
> Add SkCapabilities object
>
> This describes the capabilities of a particular Skia rendering context
> (GPU context, or the CPU backend). At the moment, it only contains the
> supported SkSL version (with a new enum added to specify the current
> value as "100" and a new ES3 value as "300".
>
> SkCapabilities can not be retrieved from an SkCanvas - the client must
> have a concrete way of knowing what their destination device that will
> do the actual rendering is (GrCaps or SkSurface).
>
> This CL doesn't make use of the SkCapabilities yet, that's coming in
> follow-up CLs that alter the SkSL compiler and SkRuntimeEffect API.
>
> Bug: skia:11209
> Change-Id: I4e9fd21ff7ffd79f1926c5c2eb34e10b3af4bc9b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537876
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:11209
Change-Id: If76343a8a536ade25f6b3d80e0885c7bc47d2adf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540919
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
DSL runtime effects were unused outside of test code.
Change-Id: I561afc533c7c204183968aff0b9c4389b0587114
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540917
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: I1d8874e1371ce2ddeba119b097d018ad94aa474c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541016
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
