SkTypes.h was found exporting <stddef.h> and <stdint.h>.
Thus if a Skia file includes SkTypes.h, it did not need
to include either of those files to use things like
size_t or uint8_t respectively. [1]
This also resulted in strange IWYU warnings like:
warning: size_t is defined in "include/core/SkTypes.h", which isn't directly #included.
Thus, this CL removes those additional exports, and as a result,
adds many more includes of <cstddef> and <cstdint>.
The second change this CL is to not use the default IWYU
mappings which are hard-coded into IWYU [2]. These defaults
are valid, but make suggestions for the C version of
headers and not the C++ ones (i.e. <stddef.h> over <cstddef>).
To make these recommendations align better with our preferred
style (the C++ ones), we use IWYU with --no_default_mappings
and then have to expand our existing mappings to better deal
with how IWYU would resolve some of these headers.
[1] https://codereview.chromium.org/1207893002/#msg49
[2] 4c0f396159/iwyu_include_picker.cc (L1221-L1234)
Change-Id: Iafebe190f8f3e86f252147b9f538c182bcc34af4
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/555516
Auto-Submit: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I33121076bac3ceca19c81ff9fb47e4b024a14230
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549838
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I964ac2d7d2f1e9d10b0216deb6572b28a26da0fc
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 5270322b46.
Change-Id: If594d04dc657126dce48d69dcc67d1a5e3b0cc8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546856
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 9583759bbd.
Reason for revert: MSAN failure
Original change's description:
> Restored unsized array support to SkSL
>
> This is a prerequisite for compute shaders. As of this CL, there isn't
> yet a way to use unsized arrays, as it is a compute-only feature and
> compute shaders are coming in a followup CL, but this adds the basic
> framework and error tests.
>
> Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Id10b48ef24c0e6219b65b0a201d13fea9632620f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546552
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This is a prerequisite for compute shaders. As of this CL, there isn't
yet a way to use unsized arrays, as it is a compute-only feature and
compute shaders are coming in a followup CL, but this adds the basic
framework and error tests.
Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
DSLType is now a simple wrapper around SkSL::Type*. Previously, its type
was determined through a combination of a Type* and a TypeConstant enum,
and fSkSLType == nullptr was used as a sentinel to mean "use the type
constant instead", which meant that we couldn't have a null DSLType
as it would fall back to the type-constant.
We can now return `DSLType(nullptr)` to indicate an error case in type
handling code, instead of needing to wrap the DSLType in an optional<>.
Change-Id: Iebaab86162b526a7fcb93d253367a7e4881ef6d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545781
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
DSLType stored an `fPosition` which was only used in one place, when
reporting that a function is not allowed to return a particular type.
Those errors now highlight the type and function name together, which is
not really any worse than before. This allows us to shrink DSLType down
to its minimal form, just a pointer to an SkSL::Type and nothing else.
Change-Id: I4b430cb996472da0ae57bc2ab095cd123d2c3f51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546097
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL removes the TypeConstant member from DSLType. The constructor
which takes a TypeConstant now sets the `fSkSLType` field immediately
instead of saving the value and deferring lookup until `skslType()` is
called.
This caused some ripple-effect issues in type setup code which were
fixable by replacing nullptr with Poison.
Change-Id: I8fa73cdf5f0bcd3de143c9a25ea43392d75c7dec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545780
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Organization v3.5, if we are keeping track :)
This splits the "srcs" filegroup into "srcs" and "private_hdrs",
and renames "hdrs" to "public_hdrs".
To assist with the split, I created the macro split_srcs_and_hdrs.
Rather than keep two separate lists of header and source files,
I figured it would be easiest, at least for the common case,
to keep one list of files and then have a for loop split them
apart. I've tried to be consistent with having the list
of files be named with a _FILES suffix - maybe we can use this
as a marker to generate .gni files in the future?
Suggested review order:
- //bazel/macros.bzl. Note this needs a corresponding
G3 change (http://cl/452279799) as well. The exports_files_legacy
change is the better approach to something I manually
handled yesterday when fixing the G3 roll.
- //BUILD.bazel to see the new target skia_internal and
the previous skia_core renamed to skia_public.
- //src/core/BUILD.bazel to see a typical usage of
split_srcs_and_hdrs.
- //include/... to see the change to public_hdrs and
private_hdrs
- //src/... to see many more usages of split_srcs_and_hdrs
- //tools/... to see changes to skia_internal where
appropriate.
- Everything else. Note that //modules/... might also need
to be built with skia_internal instead of skia_public,
but we can fix that up later, if necessary.
Change-Id: Ie1cc969455d97b029b2d77faa222c4a9bad70671
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545716
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Pending errors were used to associate C++ file and line numbers with
C++ DSL code. (We would detect errors in expressions before they had
been associated with a Position, and needed to defer saving the error
until a valid Position had been found.)
We no longer try to assign a position to C++ DSL code, so pending
errors do not do anything useful.
Change-Id: I272a73f64e63269120a6f76f4a0153c11d98fb47
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543018
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
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>
The primary goal of this organization structure is to keep
our top level BUILD.bazel file short, with as little logic
as feasible. The logic required to control which files to
include, which third_party deps are needed, what system libraries
should be linked again, etc, should be in the BUILD.bazel
file best should be as close to the affected files as feasible.
In essence, we use filegroup() rules to bubble up the files
needed to build Skia (all as one big cc_library call) and
cc_library rules to bubble up the other components needed to build.
For example, //src/ports/SkFontHost_FreeType.cpp needs FreeType,
but only if we are compiling Skia with that type of font
support. With the new organization structure in this CL,
//src/ports/BUILD.bazel should have the logic that determines
if the cpp file should be included in the build of Skia and
if it is, that the Skia build should depend on //third_party:freetype2
Another example is //src/gpu/ganesh/BUILD.bazel, which
chooses which of the dawn, gl, vulkan, etc backend sources,
and the associated dependencies to include in the build.
It does not specify what those are, but delegates to the
BUILD.bazel files in the subdirectories housing the
backend-specific code.
The structure guidelines for BUILD.bazel files are as follows:
- Have a filegroup() called "hdrs" (for public headers) or
"srcs" (for private headers and all .cpp files) that is
visible to the parent directory. This should list the
files from the containing directory to include in the
build.
See //include/core/BUILD.bazel and //src/effects/BUILD.bazel
as examples.
- filegroup() rules can list a child directory's "hdrs"
or "srcs" in their "srcs" attributes, but should not contain
select statements pertaining to child directory files.
See //include/gpu/BUILD.bazel and //src/gpu/ganesh/BUILD.bazel
as examples.
- May have a cc_library() called "deps". This can specify
dependencies, cc_opts, and linkopts, but not srcs or hdrs. [1]
See //src/codec/BUILD.bazel as an example. These should
be visible to the parent directory.
- "hdrs", "srcs", and "deps" for the primary Skia build
(currently called "skia_core") should bubble up through
//include/BUILD.bazel and //src/BUILD.bazel, one directory
at a time.
This CL demonstrates a very basic build of Skia with many features
turned off (CPU only, no fonts, no codecs). Follow-on CLs will
add to these rules as more targets are supported. See bazel/Makefile
for the builds that work with just this CL.
Suggested Review Order:
- //BUILD.bazel to see the very small skia_core rule which
delegates all the logic down stack. Note that it has a
dependency on //bazel:defines_from_flags which will set
all the defines listed there when compiling all the
.cpp and .h files in skia_core *and* anything that depends
on skia_core, but *not* //src:deps.
- //include/BUILD.bazel and other BUILD.bazel files in the
subdirectories of that folder. Note that the filegroups in
//include/private/... are called "srcs" to be similar to
how Bazel wants "private headers" to be in the "srcs" of
cc_library, cc_binary, etc. and only public headers are
to be in "hdrs" [2].
- //src/BUILD.bazel and other BUILD.bazel files in the
subdirectories of that folder. //src/gpu/ganesh/...
will be filled in for dawn, vulkan, and GL in the next CL.
- //PRESUBMIT.py, which adds a check that runs buildifier [3]
on modified BUILD.bazel files to make sure they stay
consistently formatted.
- //bazel/... to see the new option I added to make sksl
opt-in or opt-out, so one could build Skia with sksl,
but not with a gpu backend.
- Misc .h and .cpp files, whose includes were removed if
unnecessary or #ifdef'd out to make the minimal build
work without GPU or SkSL includes.
- //bazel/Makefile to see the builds that work with this CL.
[1] Setting srcs or hdrs is error-prone at best, because those
files will be compiled with a different set of defines than
the rest of skia_core, because they wouldn't depend on
//bazel:defines_from_flags.
[2] https://bazel.build/reference/be/c-cpp#cc_library.hdrs
[3] https://github.com/bazelbuild/buildtools/releases
Change-Id: I5e0e3ae01ad42d672506d5aad1239f2512188191
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543977
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Reviewed-by: Ben Wagner <bungeman@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 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 only usage of `virtual` was the `storage()` method. This value is
now stored in a member variable. It is packed next to a bool so it
should be ~zero extra space, versus 8 bytes for a vtable pointer. We
also save cost of a virtual dtor call every time a DSLVar goes away.
Because VariableStorage is not a public type, this required shuffling
some constructors around to live in the cpp instead of the header.
Change-Id: I9fdefc3696d123848fb567029c051b478349cec7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542139
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
It can live in the base class instead of having matching implementations
in each subclass. Also, there didn't seem to be a reason to have a
templated and non-templated version which did the same thing.
Change-Id: I312af9e71561d847580430000fbfae6fa99fe837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542140
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@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 was only used by DSL fragment processors and is unreferenced.
This removal causes the uniform-handle value in DSLVarBase to become
unreferenced as well.
Change-Id: Ifc89a3dda93f08c40341f654981bbb6593d4fb9f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541745
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: 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>
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>
We no longer support compiling in C++14, so we don't need these
workarounds anymore.
Change-Id: Idec5a90e98c242928182f4b32dd47459be8ca47f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541075
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@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>
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>
This reverts commit 69fecd6c2d.
Reason for revert: Why do we even bother separating include from src?
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: I3bc843b0abf154dbaecb209b251f80741757bf70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540858
Commit-Queue: Brian Osman <brianosman@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
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>
G3 prefers license() first.
This was done mechanically with a big find/replace
Change-Id: I8c33c7bc10a6bec42e966cad81c259954e841811
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535898
Reviewed-by: Ben Wagner <bungeman@google.com>
Ran the following commands:
find -name "BUILD.bazel" -exec sed -i -e '1iload("//bazel:macros.bzl", "cc_library", "exports_files_legacy")\nexports_files_legacy()' {} +
buildifier --lint=fix --mode=fix -r .
This had the effect of making sure we can export all of our
files in G3 (until we no longer have legacy targets) and
making all of our cc_libraries shim-able.
bazel/macros.bzl has the human-contributed changes, the rest
were mechanical.
Change-Id: I8e24e30e74b038cfd072cdbe4078bfd1d213dd46
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535359
Reviewed-by: Ben Wagner <bungeman@google.com>
This limits our error reporting to the first 16MB of SkSL code in a
program, and error marks are limited to a run of 255 characters or
less. In practice, these limits do not affect normal code in any way.
This gives us the same tight memory footprint we originally had when
positions were stored as `int32 fLine`.
Change-Id: Idef04344324870a7b92aca154feb5e1a0121d284
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533699
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
In a followup, this will make it easier to save memory.
Change-Id: I33e9838d1b3170362f4e7c58ba5f60a1928a4b55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533698
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Change-Id: I1bf6a41290eebd45e2065fcedf86ef2e20866f54
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534197
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:13169
Change-Id: Icf0b720d3e3a13d490aba8495cf9db83d1d62318
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532762
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13170
Change-Id: I11ef0ea5ac3ae61b24a47805bb3290a37880cfee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/532536
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13173
Change-Id: Ifbcce77605dd781563568293fc501dfa31f143da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528706
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13171
Change-Id: I6dffb98ac2464f930995cf8ea57e422091d20fd2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531743
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
It was decided that being able to report positions from C++ code is not
worth the cost of having all of these captures everywhere.
Change-Id: I94981d9f780bd95df8b56198210c9cdd5f16239c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528366
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Blocks did not previously track their position, creating problems
reporting some errors (which will be improved in followup CLs). Fixing
block positions changed the reporting of do loop errors, requiring do
loop position tracking to be updated as part of this change.
Change-Id: I3bd048a62d912914edf679f42607de1b5eafc2b9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528045
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This addresses (hopefully) all of the remaining suboptimal positions in
SkSL error reporting.
Change-Id: I5bc977b03d51153b841a89fa687e54e3e9cb6ec3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527976
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This avoids a situation where we didn't know the position of a binary
expression while we were in the middle of evaluating it, which was
leading to problems with upcoming error reporting changes.
Change-Id: Ie41fa82d077de1aa1041cd84c539ba29aaa2f5f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527500
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This avoids a situation where we didn't know the position of a postfix
expression while we were in the middle of evaluating it, which was
leading to problems with upcoming error reporting changes.
Change-Id: Ie6d4dd8411ffb24ac7ee95d3e908483d0d92ba0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527499
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
