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: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>
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 adds asserts to DSLParser::expression() to ensure that every
generated expression actually starts at the current offset, and fixes
all of the various spots we weren't successfully doing that.
Change-Id: Ie37dbf9ee8bb47fd03f7a290d2e15da4fbfff938
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525316
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
SkSL::Operator was previously just a thin wrapper around Token::Kind and
everything was therefore written in terms of Token. Since Token is not
accessible from include/ (and we don't want it to be), this creates an
actual Operator enum and eliminates the dependence on Token. This will
enable followup changes to reference Operator from within the DSL
include files.
Change-Id: I49555c9151618fd15970a303d2284972c78c3f21
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525976
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Most of these are pretty mechanical generated changes.
IWYU noticed one issue with DSLCore.h, which was fixed here.
Change-Id: I5629565ad3c2817daa71907c62f932d93f9d78ab
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524617
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@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>
PS 1 regenerates existing Bazel files
PS 2 adds generated Bazel files to skottie and its dependencies,
as well as incorporating it into CanvasKit.
This changes the version of Bazel we use to 5.0.0 (recently
released).We had been using a pre-release of 6.0 because we
wanted the new features in one of the 5.0 release candidates,
but not the regression that was there (and reverted before the
full 5.0 release). I'd like to stick to the latest stable Bazel
release where possible.
Suggested Review Order:
- //modules/skottie/BUILD.bazel (this was hand written
to encapsulate the skottie library). The files in the
deps are based on skottie.gni.
- //modules/skresources/BUILD.bazel and //modules/sksg/BUILD.bazel
which expose all sources
- //third_party/file_map_for_bazel.json which ignores the
ffmpeg libraries (we won't actually build the SkVideoDecoder
stuff because HAVE_VIDEO_DECODER is not set).
- //modules/canvaskit/BUILD.bazel which makes use of the skottie
library and includes the interface skottie.js file.
- .bazelversion which changes the Bazel version used (e.g. by
Bazelisk).
- All other changes should be auto-generated or related to
deleted files.
Change-Id: Ic26f9a9dea5310f2cbd9cda7d701847924a39a22
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503828
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
- Use latest emscripten toolchain (3.1.0)
- Autogenerate the atoms and manually fix some of the file lists.
- Add a known_good_builds target to bazel/Makefile to help
check the things we expect to work with Bazel.
Change-Id: Ia5f51e7b9eb5c108386820ad59180c8f862f5a70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/491438
Reviewed-by: Ben Wagner <bungeman@google.com>
This uses the gazelle extension from
https://skia-review.googlesource.com/c/buildbot/+/473357
Review Tips:
- Ignore any changes to .h or .cpp files. Those have been
pulled out into their own CLs.
- Start with bazel/macros.bzl.
- Read the CL with the generation code, if you haven't already.
- Look at third_party/file_map_for_bazel.json.
- See experimental/bazel_test for an idea of how a cc_binary
would be made.
- Spot check one or two of the BUILD.bazel files.
This CL generates the "atomic" rules for src/, include/ and
modules/skshaper, as a starting point.
`bazel build --config clang //include/...` works
`bazel build --config clang //src/...` starts compiling,
(which verifies that the BUILD.bazel files are all valid),
but runs into errors because not all third_party deps have
been resolved, and there are some files missing from the
toolchain still (e.g. EGL headers).
Change-Id: Ib7e0fb0efdb9f08655f06cbc56e9bb4cf416294b
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/474240
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
These rules can be used to build our GMs on WASM+WebGL and
libskia.a with just the CPU backend (and most other features
turned off).
This can be done with the following commands:
- bazel build //modules/canvaskit:gm-bindings-wasm --gpu_backend=gl_backend --with_gl_standard=webgl_standard
- bazel build :skia-core --config clang
This pivots slightly from http://review.skia.org/463517
by using config_settings [1] instead of platforms for
the optional features that we control. This pivot was
suggested in [2]
We have BUILD.bazel files in many of the subdirectories
that specify filegroups for the appropriate files. In
an effort to make //BUILD.bazel more readable, it is
the responsibility of these subfolders to deal with
conditionally including certain .h or .cpp files.
This is done using select statements and config_settings
or platform constraints as necessary.
For example, src/gpu/BUILD.bazel will different private
filegroups for each of the supported gpu backends [3]
and a more-visible filegroup called "srcs" that has
the right selection of the private files to be used
for compilation.
An effort has been made to avoid using glob() in our
BUILD.bazel files. These file lists were made by using
`ls -1` and some regex to add in quotes. We might want
to make a helper script to assist with that, if necessary.
To specify which options we have, the settings in
//bazel/common_config_settings/BUILD.bazel have been
redesigned. They make use of a macro `string_flag_with_values`
that removes the boilerplate. Patchset 36 shows what the
file looks like w/o the macro.
The top level BUILD.bazel file will still need to use
some logic to handle defines, because local_defines is
a list of strings, not a list of labels [4].
Suggested Review Order:
- WORKSPACE.bazel to see the new dependencies on the
emsdk toolchain and bazel_skylib
- bazel/common_config_settings/* to see the few settings
defined (we have more to define, see BUILD.gn and
//gn/skia.gni for ideas)
- BUILD.bazel to see the "skia-core" cc_library rule.
See also "gms" and "tests"
- modules/canvaskit/BUILD.bazel to see the use of
the emscripten "wasm_cc_binary" rule, which depends
on the "skia-core", "gms", and "tests" rule. Note that
it only builds some of the gms as a proof of concept.
- The other BUILD.bazel files. Some of these are not
platform or feature dependent (e.g. pathops). Others
are (e.g. gpu).
- All other files.
[1] https://docs.bazel.build/versions/4.2.1/skylark/config.html#user-defined-build-settings
[2] https://github.com/emscripten-core/emsdk/pull/920
[3] In this CL, that's just the webgl one.
[4] https://docs.bazel.build/versions/main/be/c-cpp.html#cc_library.local_defines
Change-Id: Ieecf9c106d5e3a6ae97d13d66be06b4b3c207089
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458637
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
