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>
This adds targets which test our Dawn, GL, and Vulkan backends.
It follows the hierarchical filegroup pattern, as
outlined in https://skia-review.googlesource.com/c/skia/+/543977
Suggested Review order:
- tools/sk_app/BUILD.bazel. For many things in tools, I anticipate
they will depend on //:skia_core and other //tools targets.
sk_app shows this off, as well how to make the target
specific to a given platform and pull in the proper native code.
I'm trying out setting test_only = True, to see if we can
partition Skia's tests and helpers from the actual Skia library.
- other changes to //tools/, especially looking at sk_app's
dependencies.
- //example/BUILD.bazel. This uses the cc_binary_with_flags which
existed previously [1] to make it so people don't have to
specify all the flags for a given binary and can build it as is.
These targets nows how up in //bazel/Makefile
- //include/... and //src/..., where some typos from previous
CLs were fixed and rules expanded.
- Misc changes to .cpp files to remove unnecessary includes
that were assuming the GL backend was being compiled in.
- All other changes
[1] 162dfca340/bazel/cc_binary_with_flags.bzl
Change-Id: Ieacec464d44368cad0da0890c7dc85a6c0b900c9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544317
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@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>
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>
It's time to better organize //third_party/BUILD.bazel
This CL starts by moving the "third party" stuff we closely own.
Change-Id: I3e0be0044b790794e94f34af6202860ce0a7b7aa
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/531999
Reviewed-by: Ben Wagner <bungeman@google.com>
PS1 regenerates BUILD.bazel files
I suggest reviewing the deltas between PS1 and the latest
PS to focus on the interesting bits.
The changes here allow for a Vulkan-only build of HelloWorld
based on sk_app. The toughest change was properly fetching
the VisualID after removing the gl calls that used to
fill that in.
There are a few changes that fix resolution of Dawn
header files, but those won't actually be built until
a follow-on CL.
Change-Id: I54fb58b5dd7ecd4313562aed401759b3eaed53c0
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516999
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Kevin Lubick <kjlubick@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>
This is a reland of be0e255c3e
Original change's description:
> Add OWNERS to include (and include/private) to enforce API review
>
> Change-Id: I3ae83fbdcb4b3b13150df8b6a6461c36cbeaa55e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445641
> Reviewed-by: Ravi Mistry <rmistry@google.com>
> Reviewed-by: Kevin Lubick <kjlubick@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I090456a05128865bc134b9c4df4e5ffcb5d0e1dc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/450479
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Change-Id: I3ae83fbdcb4b3b13150df8b6a6461c36cbeaa55e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/445641
Reviewed-by: Ravi Mistry <rmistry@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This is needed so we can support VK_EXT_image_drm_format_modifier. These
headers are just used internally in Skia so this should have no effect
on anyone using Skia.
Bug: skia:12336
Change-Id: I502f8e7dbbeb8e48ff03fb7ef0e5db3e8bcbb40d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438737
Reviewed-by: Brian Salomon <bsalomon@google.com>
Causing flutter roll issues and we don't need it.
Bug: skia:10105
Change-Id: I2f0ec916984b33045fcd1961417b823f97355725
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281717
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
The generic google notice didn't satisfy the dart script.
Tracked down this LICENSE at the root of the D3D12 Library project:
https://github.com/microsoft/DirectX-Graphics-Samples/blob/master/LICENSE
This matches "the MIT" license mentioned in the d3dx12.h header, hopefully
that's enough for the license script to detect a match, w/o needing to
update the script itself.
Bug: skia:10105
Change-Id: I1d4bf52b888a13f6727503024cb8f3e933078542
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281594
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Supports buffer creation and destruction.
Bug: skia:9935
Change-Id: I6d8c6d67209a8857989a9384818272e7108177ad
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/281577
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:9031
Change-Id: I3b6fb817a63040911fe43378410f3baa7366e90e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/211102
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Add a shim to redirect until clients are updated
Change-Id: Ib43614e5620b1a24ca18187c1646a8ed1a9ee7a4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/211003
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Current strategy: everything from the top
Things to look at first are the manual changes:
- added tools/rewrite_includes.py
- removed -Idirectives from BUILD.gn
- various compile.sh simplifications
- tweak tools/embed_resources.py
- update gn/find_headers.py to write paths from the top
- update gn/gn_to_bp.py SkUserConfig.h layout
so that #include "include/config/SkUserConfig.h" always
gets the header we want.
No-Presubmit: true
Change-Id: I73a4b181654e0e38d229bc456c0d0854bae3363e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/209706
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Hal Canary <halcanary@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
