Surprisingly, this error is actually caught by our parser, which
interprets the default label in a unique way. From the parser comments:
"Requiring default: to be last (in defiance of C and GLSL) was a
deliberate decision. Other parts of the compiler may rely upon this
assumption."
The comment is true--we don't check for duplicate default switch-case
labels anywhere else in the code, just here in the parser.
We rely on this, so we should have a test for it.
Change-Id: I6df5c565aca4d4b8565b96638dce9504efc39ccc
Bug: skia:11340
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372617
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:11230
Change-Id: I8983d160ff0317514572d13aa2a3f1218e15da8e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371959
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: I898fc64d8df625d22750cfe1145ba4b02acdfbb9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372078
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Also deletes the unnecessary old header file.
Bug: skia:11230
Change-Id: Ie7c3926fe635a37e617e5e1fcac7c05eb576bbf1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368617
Reviewed-by: Robert Phillips <robertphillips@google.com>
Add sources_for_tests to optional targets. If set, a taget_name_tests
source_set will be created which matches the target_name target except
that the sources are the sources_for_tests.
This allows the tests for these targets to be obviously related to the
target they test instead of having to diplicate the logic of the
optional's enabled predicate in the test target. The test target can
just directly depend on the optional test target in the same way it can
depend directly on module tests.
This was motivated by the existence of two such tests and the need to
soon introduce another (for CoreText).
Change-Id: Idc6138044ade063f47a763f14aa7a406e613af26
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371481
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Bug: skia:11230
Change-Id: I9da638b5e1de50aee9a805547f51e013e55747b4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368598
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Split the :tests target into a :test target which contains the test
framework and :tests which collects all of the test cases. This allows
for all targets which define tests to depend on :test in order to define
tests, with :tests then depending on all targets which define tests. A
similar split should be considered for gms, samples, and benches.
Change-Id: Ic9f373ec0c1a8ea842fa68327e854db23477caae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371696
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Also deletes unnecessary header and deprecated types
associated with implementation.
Bug: skia:11230
Change-Id: Ibfd7dcf305febae794edfe777fc64efc0559909b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368597
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:11335
Change-Id: I88c952cbfe2d2c5920e17675da1674928f37b982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371480
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Includes variables with and without initializers. Note that both the
.skvm and .stage output is incorrect right now. (No declarations for
global variables in .stage, and the initializer is dropped in .skvm).
Bug: skia:11295
Change-Id: Icb6d797616be6a1bc7cbdc9db4fefa7e30c65656
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371143
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
None of these are legal in GLSL ES 1.0. Added a new test that previously
compiled without error. Started out with just assignment and equality,
then realized that sequence and ternary should be blocked, too.
Bug: skia:11323
Change-Id: I02691f819565afabeadbb12cab6c07acf40093f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370880
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:11230
Change-Id: Ifa677319a04379dc36ff155590ba85d45790567e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368596
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
The FreeType and CoreText ports would use the default value for an axis
instead of the current value for any unspecified axes. Change this so
that when cloning a typeface any unspecified axes preferentially use the
current axis value if it is known.
Also adds a test that unspecified axes are not changed when cloning.
Change-Id: I751ee5517f1d6b827c6d4ab245e9d681c8df6b42
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370456
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
Change-Id: Id28ed827c4a896805c6d4eead339146fdd49e35f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359560
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:11230
Change-Id: I3ce314916f1fd8dafaf79598de0b6f30eff3b208
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368244
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
GLSL ES2 documentation on out parameters: "Evaluation of an out
parameter results in an l-value that is used to copy out a value when
the function returns."
The inliner does not do any alias checking when inlining an `out` param.
That is, passing the same variable to two separate `out` parameters
would not generate two distinct lvalues in the inlined code; it reuses
the same variable for each out-params in the inlined code.
(Amusingly, our CFG can fully optimize away this test code so it just
returns "red".)
Change-Id: Ib781d2cfdac54f01b6abe159af0c84ff24ff6976
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Cq-Include-Trybots: luci.chromium.try:linux-blink-rel
Change-Id: I4475982f0a5b17bcc4bae4b3f75557969cc70fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369856
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Multi-dimensional arrays aren't legal in GLSL/SkSL, so this should be
caught and flagged as an error. The parser now verifies that a
variable's type isn't an array-type before accepting a `[` token to
open an array on the variable name.
This CL also refactors the IR generator's `convertArraySize` method to
make sure that various checks are made for all callers. Originally this
restructuring was used to verify array multi-dimensionality, but that
didn't detect errors inside struct declarations (which get no error
checking inside the IR generator) so the IR generator updates no longer
need to check the array dimensions.
Bug: skia:11322
Change-Id: Id33f4bdfb544019ddf995a8196c3c09cfe5a4525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369916
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
We now interpret any statement of the form `Type identifier...` as a
var-declaration and report errors as such. Previously, if a var-decl
statement generated an error during parse, we'd report errors as if it
were an expression-statement, which meant that slightly-invalid code
could return out-of-context, misleading errors.
Bug: skia:11287
Change-Id: I2c6cf2984760eb34593c80cb30f8c4e007d42027
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370036
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>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11314
Change-Id: I66476543462ae378a5bfb6cbd902dfa2f5fc45f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369917
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This also deletes the type conversions between the legacy
tile mode and SkTileMode and cleans up the serialization
logic around that just a little bit.
Bug: skia:11230
Change-Id: If404a73e13f42055a0fe7638dcd9f58cf7e9b896
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368243
Reviewed-by: Robert Phillips <robertphillips@google.com>
Implement support for COLRv1 color gradient vector forms. COLR v1 is an
extension to the OpenType COLR tables that introduces support for
gradients, transforms and compositing. [1]
COLRv1 fonts contain an OpenType COLR table with format v1. In a COLR v1
table, glyphs are defined by a directed, acyclic graph of Paint
operations. The task for the Skia implementation is to extract the COLR
v1 information from the font using FreeType's COLRv1 API, then traverse
the glyph graph and translate information from the font into Skia
operations on SkCanvas.
Care needs to be taken for transformations to work correctly. FreeType
sends a top level transform that includes the scaling from font units to
actual glyph size, as well as optional transforms configured on FreeType
using FT_Set_Transform. This is set up as the starting transform at the
beginning of drawing a COLRv1 glyph. At the stage of setting a clip for
a PaintGlyph operation, the glyph outline is retrieved from FreeType
unscaled and untransformed. Since the starting transformation is applied
to the SkCanvas, the unscaled glyph is properly scaled and positioned.
Similarly, computing the initial bounding box for the COLRv1 glyph needs
to consider transformations. COLRv1 glyphs have a base glyph entry in
the glyf table which can use a minimum of two points for defining a
bounding box. This bounding box is an imaged rectangle enclosing those
two points at the identity transform. We need to compute the bounding
box scaled, but at identity transform, then make a rectangle from it,
that we then transform so that we get a large enough area to paint
in. If those two points from the glyf table would be transformed first,
then we would compute the bounding box, the resulting bounding box ends
up too small.
As a test font, add a version of the samples-glyf_colr_1.ttf from [2]
with one glyph added for testing PaintColrGlyph functionality and
compositions.
Add a basic GM test that produces color glyphs untransformed, with
skew and with rotation and combinations of those.
[1] https://github.com/googlefonts/colr-gradients-spec/
[2] https://github.com/googlefonts/color-fonts
Bug: skia:11264
Change-Id: I8357cd0c7ac0039bb2eac18f21fa343bf61871eb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/300558
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Ben Wagner <bungeman@google.com>
Also deletes the old header under the deprecated name.
Bug: skia:11230
Change-Id: Iac42d2b980d3306a544077719ca62903a673210a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368242
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This also adds the relevant Start() / End() calls so that the DSL
is put into the correct state.
Change-Id: I844b0ab5dff06e3f7b0a69458bf4442a5da0f33e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364857
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:11230
Change-Id: I966d9511d6124627308146f1b5f9e18f5d7fb4d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368241
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:11102
Change-Id: I4ed9e44099cd780c5cdede3eb179c0e6a92d3ce5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345219
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Chris Bracken <cbracken@google.com>
Disabled on Adreno 5xx/6xx as the tests do not pass on those GPUs:
http://screen/3Dkgs9syj37cjBV
Change-Id: Ib935d01e8f06dbfe7decd5cc4e52e0688b48be08
Bug: skia:11306, skia:11308
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368805
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: chromium:1174186
Change-Id: I91a88d2d57150dee37f08bc4270d399abfd0d60d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368497
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Bug: skia:11230
Change-Id: Icd927cfdcca71a48dce16bcd6c40489dad92a259
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368238
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This emits SkSL that is more-or-less what the compiler re-ingests when a
runtime effect is used to create a GrFragmentProcessor.
Change-Id: I0926be44fc4493e722a5edc18198e161e4192cde
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367883
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Currently, SkSL is able to constant-propagate `x = x + constant` into
`x = constant` when the starting value of x is known. However, it is not
able to do the same optimization for `x += constant`. This test
demonstrates that once += is encountered, we lose track of x's value and
can no longer propagate its value.
(This is equally true of all the op-assignment operators, += -=
*= /= etc.)
Change-Id: I3523e96baf9a73982cf3b09f0d23b95adacf106b
Bug: skia:11192
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368248
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11226
Change-Id: I5f507a0d3ffe0a2698b10b0535486986c2a8b5b7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367977
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This doesn't change any functionality. It just compartmentalizes a large
chunk of logic that was previously intertwined in the Compiler class.
This logic stands alone and doesn't need anything from the Compiler at
all except the generic "Defined" expression from the Context.
In followup CLs I intend to experiment with changes to the API and
logic, but factoring the code out seemed like a big enough change that
it deserved its own CL.
Change-Id: I3e1a7c62812c6f284167c967086ef4dd828a0b2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367879
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:11226
Change-Id: I7939233559e673b10c9e471137d44282b9612f8e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366317
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This enforces write-only access to the mapped buffers, will enable
chaining of indirect strokes, and gives us the ability to reorder the
fields for Metal.
Bug: chromium:1172543
Bug: skia:11291
Bug: skia:10419
Change-Id: I4449ff85dd0019f6d6d6781ede52bcf26dee8b02
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367416
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
These aren't used any more in favor of lazy proxies.
Bug: skia:11288
Change-Id: I992e1a3dd343e0ebc7f3a4f18c0054453dfebbaf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366896
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Change-Id: I3593e7ab0caac2fd572346038cbc8ff63e6fe970
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366406
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
SkSL.
It would previously catch 1 / 0, but fail to detect x / 0.
Bug: skia:11051
Change-Id: I3adb5942cce03a7ad40a13a8ca5d5a7f2029d6ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366720
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:11226
Change-Id: I58fc7e02dc9ea9a06e855107aad4fbbd7b98d347
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366316
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
