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>
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>
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>
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>
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>
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>
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>
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>
This node was only used to detect recursion while inlining. We no longer
need to do this, because we disallow recursion in all programs.
The removal of one IRNode per inlined function actually allows for
slightly more aggressive inlining, since we restrict inlining based on
IRNode consumption. This allows the "ExponentialGrowth" tests to inline
a bit more deeply than before.
Change-Id: I894dbb1ca3096bb785b67facb01cc9c630f694c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534780
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we used unscoped blocks for two similar functions:
- Rewrite one statement as two simpler statements:
`int a, b;` -> `int a; int b;`
- Group together multiple statements without braces. e.g. the inliner
uses unscoped Blocks to rearrange statements.
Conceptually, these are different from the debugger's perspective. The
compound statements should be treated as one unit; the grouped
statements should be treated individually (and the enclosing Block
should be ignored). A Block now contains a BlockKind enum to
distinguish between these cases.
Change-Id: Ie14a570bb46992689fb96b8fd3b67f2ca6e5239f
Bug: skia:13189
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528655
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I238d29ba0250224fa593845ae65192653f58faff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528156
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
PS1 regenerates the Bazel files. Use it as the base change when
comparing patchsets.
IWYU seems to do a good job of working with MyFile.cpp and
MyFile.h, but if there is just a MyHeader.h, it doesn't always
seem to throw errors if the includes aren't correct. This was
observed with include/sksl/DSL.h This might be due to the fact
that headers are not compiled on their own, so they are never
sent directly to the IWYU binary.
This change sets enforce_iwyu_on_package() on the all sksl
packages and then fixes the includes until all those checks
are happy. There were a few files that needed fixes outside
of the sksl folder. Examples include:
- src/gpu/effects/GrConvexPolyEffect.cpp
- tests/SkSLDSLTest.cpp
To really enforce this, we need to add a CI/CQ job that runs
bazel build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
If that failed, a dev could make the changes described in
the logs and/or run the command locally to see those
prescribed fixes.
I had to add several entries to toolchain/IWYU_mapping.imp
in order to fix some private includes and other atypical
choices. I tried adding a rule there to allow inclusion of
SkTypes.h to make sure defines like SK_SUPPORT_GPU, but
could not get it to work for all cases, so I deferred to
using the IWYU pragma: keep (e.g. SkSLPipelineStageCodeGenerator.h)
Change-Id: I4c3e536d8e69ff7ff2d26fe61a525a6c2e80db06
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522256
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.
Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I1fa160941b18392616708f4e08a4ebbf7398480e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518521
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
The previous CLs have removed the last significant differences between
SkSL::String and std::string. This CL removes SkSL::String entirely and
replaces it with std::string throughout the code.
Apologies for the very long CL, but I have done my best to make it as
simple and reviewable as possible. The vast majority of changes are
simple replacement of `SkSL::String` with `std::string`. In the rare
spots where code is moved from one place to another, it is logically
unchanged.
Change-Id: I39563d2db45da229f17f4504dfd63e00bde7a96e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503339
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We now use std::string_view throughout. SkStringView.h has been moved to
include/private/ and is only used for our C++20/23 compatibility methods
(starts_with/ends_with/contains).
Change-Id: I961842c6778256a03868e7602d48add34f420763
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/502306
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Added comments to explain the semantics (both what's expected when you
set the uniform, and what you see in the shader). The old name was
confusing, because it sounded like you got an sRGB color in the shader.
This is terse, but I think it's the cleanest syntax - and for embedding
clients, they can use C++ (etc.) API to require that color uniforms are
assigned from color types.
Bug: skia:10479
Change-Id: If00ea754060494aaa83001a5b357687953de8a5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480577
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
I should have realized the fuzzer would find this assert when I added
it. Now the front-end rejects these layout qualifiers on both struct
fields and interface block fields. LayoutInInterfaceBlock.sksl is a
reformatted version of the fuzzer input. LayoutInStruct is hand-crafted
to trigger the same failure on a different code path. Both would
previously assert in the SPIRV generator. Now, neither one gets that
far.
Bug: oss-fuzz:41347
Change-Id: Iff69d8f5482da7b772e9331c4fd2d58e89813c46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476396
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In some contexts, we rely on constant expressions to fold in order for
SkSL to work properly. (e.g. an array size is allowed to be any
constant-expression in GLSL, but the compiler really needs to know the
actual size). Previously, turning off optimization would break several
tests. Now, constant-expression folding always occurs even when the
optimizer is disabled.
Note that disabling the optimizer isn't an end-user option, so this
only affects internal usage.
Change-Id: I106ecb7e5bff3f7a8235cdccf0a7a60b48a97e2e
Bug: skia:12472
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/469520
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
With this change, we no longer have any SkSL tests which are able to
make a Literal integer that overflows its type. Literal::MakeInt now
asserts that its value is within bounds. I look forward to the fuzzer's
inevitable attempts to trigger these assertions.
Change-Id: I7b15e862caaf65984d33f5d72d2c1de816d1d292
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464124
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This completes the long process of moving all of IRGenerator's code and
data into better homes and finally kills the class altogether. There is
a lot of #include churn here due to poor header hygiene in the past;
IRGenerator.h included a ton of stuff and almost everything included
IRGenerator, allowing us to inadvertently be sloppy with our includes.
Change-Id: I70d854e57dec7bd9a311b72f9f72d978d354da98
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This reverts commit eb68973c2f.
Reason for revert: ES2 conformance test checks this
Original change's description:
> Disallow matrix ctors which overflow a column.
>
> The GLSL spec allows matrix constructors containing vectors that would
> split between multiple columns of the matrix. However, in practice, this
> does not actually work well on a lot of GPUs!
>
> - "cast not allowed", "internal error":
> Tegra 3
> Quadro P400
> GTX 660
> GTX 960
> - Compiles, but generates wrong result:
> RadeonR9M470X
> RadeonHD7770
>
> Since this isn't a pattern we expect to see in user code, we now report
> it as an error at compile time. mat2(vec4) is treated as an exceptional
> case and still allowed.
>
> Change-Id: Id6925984a2d1ec948aec4defcc790a197a96cf86
> Bug: skia:12443
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449518
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12443
Change-Id: I5a32744c88b9b830ad657488824c8c7dd0b0a652
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
As DSLWriter evolved, it ended up performing two unrelated duties:
1. Holding per-thread state which is referenced throughout SkSL
(including non-DSL code)
2. DSL-specific utility functions
There's no reason for those to be contained in the same class, and
there's also no reason the per-thread state should be held in a
DSL-specific class. This breaks out an SkSL::ThreadContext class to hold
the state info, leaving the DSL utility functions behind in DSLWriter
(which should perhaps be renamed, but that's a job for a future CL).
Change-Id: Iccd45314bd9b37d4a1d3e27920e32a50c0b07d7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/456736
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I5a57e2db46734ca08825e6aef7a6363bcaada45a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/453759
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit cc91452f0a.
Change-Id: I7eff0ddeebef4ce298893f9b3ba410b09647e9a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/453138
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 58d47fa1ec.
Reason for revert: Tree broken
Original change's description:
> Renamed SkSL "offset" to "line"
>
> https://skia-review.googlesource.com/c/skia/+/451419 changed the meaning
> of "offset" throughout SkSL, so that it was actually tracking line
> numbers rather than offsets (and thus had a misleading name). This
> completes the transition by renaming all of the now-misnamed "offset"
> fields, parameters, and variables to "line'.
>
> Bug: skia:12459
> Change-Id: I394e6441f6ddfaad6d4098352ba9b1bfeaf273be
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450644
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:12459
Change-Id: Idcec3b65cb81d51c8b860c4388578700030b40a9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/452718
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
https://skia-review.googlesource.com/c/skia/+/451419 changed the meaning
of "offset" throughout SkSL, so that it was actually tracking line
numbers rather than offsets (and thus had a misleading name). This
completes the transition by renaming all of the now-misnamed "offset"
fields, parameters, and variables to "line'.
Bug: skia:12459
Change-Id: I394e6441f6ddfaad6d4098352ba9b1bfeaf273be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450644
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The GLSL spec allows matrix constructors containing vectors that would
split between multiple columns of the matrix. However, in practice, this
does not actually work well on a lot of GPUs!
- "cast not allowed", "internal error":
Tegra 3
Quadro P400
GTX 660
GTX 960
- Compiles, but generates wrong result:
RadeonR9M470X
RadeonHD7770
Since this isn't a pattern we expect to see in user code, we now report
it as an error at compile time. mat2(vec4) is treated as an exceptional
case and still allowed.
Change-Id: Id6925984a2d1ec948aec4defcc790a197a96cf86
Bug: skia:12443
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/449518
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12302
Change-Id: I7ff7bae388c5991f2c23c8945355fea55c42095a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447436
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:12302
Change-Id: Ifc107ca2cf13c1daa59521b93fe4ad1d3c215258
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447297
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
I wrote some code which needed Round(); it isn't in ES2 so it wasn't in
the DSL intrinsic set yet.
Change-Id: I304f61b502a68d255d15899181e34fcae2ef16a0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/447017
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This allows us to get rid of a lot of .c_str()'s.
Change-Id: I09102f90d69620614dc5a7a2ebc64bd3e9b1c437
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445816
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Also update RELEASE_NOTES to describe new syntax.
Change-Id: I2666551b98f80b61ae3a48c92a9e306cdc7242b0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444735
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously the magic surrounding sk_RTAdjust was inaccessible to the
DSL, meaning that the DSLParser would not work properly when the
sk_RTAdjust field was present in an interface block. This refactoring
means all interface blocks are processed via the same path.
Change-Id: I99a2fe6875dfcbccc53f7a44f0fb1912cb2722ce
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/442456
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
After further discussion, using intrinsics with signatures similar to
sample keeps us looking like GLSL. However, using "sample" is still
misleading, so this adds explicit "shade", "filter", and "blend"
intrinsics. After migrating clients, the "sample" versions will be
removed.
Bug: skia:12302
Change-Id: Ia03e4b3794fc1fc5ae3c3099a7a350343ec7702e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/441457
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Previously, you would declare child objects (shaders, colorFilters, etc.)
and "sample" them like this:
uniform shader input;
uniform colorFilter filter;
half4 main(float2 coord) {
half4 inColor = sample(input, coord);
return sample(filter, inColor);
}
With the new syntax, those child objects become directly callable,
reflecting the way that Skia assembles all parts of the paint (as functions)
in the overall fragment shader:
uniform shader input;
uniform colorFilter filter;
half4 main(float2 coord) {
half4 inColor = input(coord);
return filter(inColor);
}
Bug: skia:12302
Change-Id: Ia12351964dc5d2300660187933188e738671cd83
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436517
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This eliminates the SkSL ErrorReporter class and funnels everything
through the DSL ErrorHandler. Since the DSL error handler can be
changed, this required a number of updates to ensure that things work
properly in the face of custom error handlers. There is probably more
work to be done in that area, but this at least passes all existing
tests.
Change-Id: Iaee27b79fc4ed426c484ccab257c09d28619ead5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438116
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Previously, there was no way to create a forward declaration for a DSL
function. To avoid introducing new API and make this work in an
intuitive fashion, we now create prototypes for all DSL functions and
remove them when the function is promptly defined.
Change-Id: Ief36164ceb303a3d76a57dc073f2e9b8409bb45f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/436562
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
We were previously using a mix of pass-by-value and pass-by-pointer (to
allow for explicitly null PositionInfo). Being able to pass a null
PositionInfo didn't really add much, since we can just use a nullary-
constructor PositionInfo instead, so these have all been migrated to
by-value.
Change-Id: Ia31e252cac94f64c4b38c29a54e6e7f752e70672
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/437276
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
