Commit Graph

33 Commits

Author SHA1 Message Date
Kevin Lubick
2c65579aad [bazel] Add in hierarchical filegroup Bazel rules.
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>
2022-05-31 14:14:54 +00:00
Kevin Lubick
4511c7b7fb [bazel] Delete gazelle-based BUILD.bazel files
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>
2022-05-31 14:14:54 +00:00
Robert Phillips
dfabbfc0e3 [graphite] Add SkCombinationBuilder.h
This is cleanup left over from a prior CL's review. We will want to
use the combination builder outside of graphite.

This CL is mainly just moving stuff around except for the addition
of the SkCombinationBuilder.buildCombinations method and CreateKey
more accessible outside of Graphite.

Bug: skia:12701
Change-Id: If2cae6fcff5670e488bc14473b7b1d2f9b1fecd7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543196
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-05-23 20:26:17 +00:00
Brian Osman
3cbb13a93b Reland "Add SkCapabilities object"
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>
2022-05-17 13:15:06 +00:00
Brian Osman
88a51fea63 Revert "Add SkCapabilities object"
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>
2022-05-16 16:32:03 +00:00
Brian Osman
69fecd6c2d 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>
2022-05-13 16:31:20 +00:00
Brian Salomon
f7082cb9fa Rename SkCustomMesh to SkMesh
Also rename associated files, types, functions, etc.

Bug: skia:12720
Change-Id: Ibebb4e7071fa19c5a81cd36d567c96a42cd824f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538040
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
2022-05-09 13:36:21 +00:00
Kevin Lubick
c3a448ec61 [bazel] Put licenses() after legacy_exports
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>
2022-05-02 15:04:33 +00:00
Kevin Lubick
fe01e985a7 Reland "Reland "[includes] Remove SkColorSpace include from SkImageInfo""
This is a reland of commit 00de8cc000

New client fixes:
 - http://cl/445903108

Original change's description:
> Reland "[includes] Remove SkColorSpace include from SkImageInfo"
>
> This is a reland of commit 2151fa37fa
>
> New client fixes:
>  - https://crrev.com/c/3616345
>
> Original change's description:
> > [includes] Remove SkColorSpace include from SkImageInfo
> >
> > See also https://skia-review.googlesource.com/c/skia/+/525639
> >
> > This should be landed when clients have been fixed.
> >  - https://github.com/flutter/engine/pull/32532
> >  - https://github.com/flutter/engine/pull/32382
> >  - http://cl/438808657
> >  - http://cl/438803822
> >  - http://cl/438807456
> >  - http://cl/438808026
> >  - http://cl/438821192
> >  - https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17542295
> >
> > Change-Id: I04beeb2e2b6132c40508c98f4f281981dadbb0d4
> > Bug: skia:13052
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526416
> > Commit-Queue: Kevin Lubick <kjlubick@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> Canary-Chromium-CL: 3616345
> Bug: skia:13052
> Change-Id: I670d3d0d0bab2a86559d9a1184cb36ef008196f3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535357
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Bug: skia:13052
Change-Id: Ibb931dde3f0846609df35d2897989eff0499b7f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535896
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2022-05-02 13:33:21 +00:00
Brian Osman
05dd660fee Revert "Reland "[includes] Remove SkColorSpace include from SkImageInfo""
This reverts commit 00de8cc000.

Reason for revert: G3 Breakage?

Original change's description:
> Reland "[includes] Remove SkColorSpace include from SkImageInfo"
>
> This is a reland of commit 2151fa37fa
>
> New client fixes:
>  - https://crrev.com/c/3616345
>
> Original change's description:
> > [includes] Remove SkColorSpace include from SkImageInfo
> >
> > See also https://skia-review.googlesource.com/c/skia/+/525639
> >
> > This should be landed when clients have been fixed.
> >  - https://github.com/flutter/engine/pull/32532
> >  - https://github.com/flutter/engine/pull/32382
> >  - http://cl/438808657
> >  - http://cl/438803822
> >  - http://cl/438807456
> >  - http://cl/438808026
> >  - http://cl/438821192
> >  - https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17542295
> >
> > Change-Id: I04beeb2e2b6132c40508c98f4f281981dadbb0d4
> > Bug: skia:13052
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526416
> > Commit-Queue: Kevin Lubick <kjlubick@google.com>
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
>
> Canary-Chromium-CL: 3616345
> Bug: skia:13052
> Change-Id: I670d3d0d0bab2a86559d9a1184cb36ef008196f3
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535357
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Bug: skia:13052
Change-Id: Ie2d036f0bfcdb481c4227a6d567e38d527c81168
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535202
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-04-29 22:17:12 +00:00
Kevin Lubick
46eaab3959 [bazel] Add shims to help translation into G3
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>
2022-04-29 19:27:54 +00:00
Kevin Lubick
00de8cc000 Reland "[includes] Remove SkColorSpace include from SkImageInfo"
This is a reland of commit 2151fa37fa

New client fixes:
 - https://crrev.com/c/3616345

Original change's description:
> [includes] Remove SkColorSpace include from SkImageInfo
>
> See also https://skia-review.googlesource.com/c/skia/+/525639
>
> This should be landed when clients have been fixed.
>  - https://github.com/flutter/engine/pull/32532
>  - https://github.com/flutter/engine/pull/32382
>  - http://cl/438808657
>  - http://cl/438803822
>  - http://cl/438807456
>  - http://cl/438808026
>  - http://cl/438821192
>  - https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17542295
>
> Change-Id: I04beeb2e2b6132c40508c98f4f281981dadbb0d4
> Bug: skia:13052
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526416
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Canary-Chromium-CL: 3616345
Bug: skia:13052
Change-Id: I670d3d0d0bab2a86559d9a1184cb36ef008196f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535357
Reviewed-by: Brian Salomon <bsalomon@google.com>
2022-04-29 14:56:15 +00:00
Kevin Lubick
cadb3a33e1 Revert "[includes] Remove SkColorSpace include from SkImageInfo"
This reverts commit 2151fa37fa.

Reason for revert: Breaking Chrome

Original change's description:
> [includes] Remove SkColorSpace include from SkImageInfo
>
> See also https://skia-review.googlesource.com/c/skia/+/525639
>
> This should be landed when clients have been fixed.
>  - https://github.com/flutter/engine/pull/32532
>  - https://github.com/flutter/engine/pull/32382
>  - http://cl/438808657
>  - http://cl/438803822
>  - http://cl/438807456
>  - http://cl/438808026
>  - http://cl/438821192
>  - https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17542295
>
> Change-Id: I04beeb2e2b6132c40508c98f4f281981dadbb0d4
> Bug: skia:13052
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526416
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>

Bug: skia:13052
Change-Id: I8a9108a333eebefdfa864c757d8c395a6b59335c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535296
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
2022-04-28 22:56:15 +00:00
Kevin Lubick
2151fa37fa [includes] Remove SkColorSpace include from SkImageInfo
See also https://skia-review.googlesource.com/c/skia/+/525639

This should be landed when clients have been fixed.
 - https://github.com/flutter/engine/pull/32532
 - https://github.com/flutter/engine/pull/32382
 - http://cl/438808657
 - http://cl/438803822
 - http://cl/438807456
 - http://cl/438808026
 - http://cl/438821192
 - https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/17542295

Change-Id: I04beeb2e2b6132c40508c98f4f281981dadbb0d4
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526416
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2022-04-28 20:48:11 +00:00
Ben Wagner
9cbadcd928 Add optional OT-SVG support to FreeType
Bug: skia:12290
Change-Id: I064bee781d3a714e46f102cb48494fbe8f3e46e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516436
Reviewed-by: Florin Malita <fmalita@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
2022-04-25 18:51:01 +00:00
Kevin Lubick
83cee23c98 [bazel] Run buildifier on BUILD.bazel files
buildifier --lint=fix -r .

Change-Id: I6a41858270d20137978f8271c8f6160b51120777
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529751
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-04-14 18:13:43 +00:00
Kevin Lubick
b98328a27b [bazel] Add license to all our BUILD.bazel files
find -name "BUILD.bazel" -exec sed -i -e '1i licenses(["notice"])\n' {} +

Change-Id: Ie48f163b7d8d6ede9ba5f952e87232dd5c9fa8e6
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529808
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-04-13 19:50:29 +00:00
Jim Van Verth
42f710f3fa Reland "[graphite] Move Graphite into Skia base directories."
This is a reland of commit ae5e846047

Original change's description:
> [graphite] Move Graphite into Skia base directories.
>
> Change-Id: Ie0fb74f3766a8b33387c145bd1151344c25808cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528708
> Reviewed-by: Kevin Lubick <kjlubick@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>

Change-Id: Ia575fd49206ad0b665a6a9153317e738bb321446
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529059
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
2022-04-11 18:37:20 +00:00
Jim Van Verth
ab4d4ae9e2 Revert "[graphite] Move Graphite into Skia base directories."
This reverts commit ae5e846047.

Reason for revert: Breaking Google3

Original change's description:
> [graphite] Move Graphite into Skia base directories.
>
> Change-Id: Ie0fb74f3766a8b33387c145bd1151344c25808cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528708
> Reviewed-by: Kevin Lubick <kjlubick@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>

Change-Id: Ia57992a22c42b17216b40fd29452865f957fb802
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528653
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2022-04-09 01:02:26 +00:00
Jim Van Verth
ae5e846047 [graphite] Move Graphite into Skia base directories.
Change-Id: Ie0fb74f3766a8b33387c145bd1151344c25808cb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528708
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
2022-04-08 20:46:40 +00:00
Kevin Lubick
8499e372ce [includes] Remove more includes of SkColorSpace
While I was fixing up Chrome's uses, I found some failures
there that I did not see in Skia, and tracked them down
to a few other places where we include SkColorSpace
and it is not strictly necessary

 - SkCustomMesh.h
 - GrColorInfo.h
 - GrColorSpaceXform.h
 - SkColorSpaceXformSteps.h

For these files (and their .cpp files), I added enforcement
of include-what-you-use, and then fixed the myriad of places
which were depending on these transitive includes.

One change to help Chrome is the manual overloads of
SkImage::MakeFromAdoptedTexture instead of using default
parameters. This makes it so callers of that function
do not need to include SkColorSpace if they were going
to pass nullptr for it anyway.

Bug: skia:13052
Change-Id: I16bf8ed5e258225d887f562f2c189623b1ca9c23
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527056
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-04-06 21:58:24 +00:00
Kevin Lubick
5e8f45faf1 [includes] Prepare for moving SkColorSpace to forward declare
This updates all our callsites in preparation for removing
the #include "include/core/SkColorSpace.h" from SkImageInfo.h

According to go/chrome-includes [1], this will save ~150MB
(0.07%) from the compilation size. I think SkColorSpace is
a big include because it loads the skcms header, which is
big.

The follow-on CL will remove that link, once clients have
been updated as well.

[1] https://commondatastorage.googleapis.com/chromium-browser-clang/chrome_includes_2022-03-31_124042.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageInfo%5C.h%24&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageInfo%5C.h%24&included=&limit=1000

Change-Id: I1b5ff491ac495317b0e5af3a2082b080d43697ae
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525639
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-31 19:50:10 +00:00
Kevin Lubick
08ece0c9a0 [includes] Enforce IWYU on sksl code
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>
2022-03-21 12:43:02 +00:00
Kevin Lubick
e253cc3e55 [bazel] Use toolchain features to opt files into being IWYU compliant.
PS1 regenerates the Bazel files.

It is recommended to review this CL with a diff from PS1.

Example output when a file does not pass the test:
    tools/sk_app/CommandSet.h should add these lines:
    #include "include/core/SkTypes.h"
    #include "include/private/SkTArray.h"
    #include "tools/skui/InputState.h"
    #include "tools/skui/Key.h"
    #include "tools/skui/ModifierKey.h"
    namespace sk_app { class Window; }

    tools/sk_app/CommandSet.h should remove these lines:
    - #include "tools/sk_app/Window.h"

    The full include-list for tools/sk_app/CommandSet.h:
    #include "include/core/SkString.h"
    #include "include/core/SkTypes.h"
    #include "include/private/SkTArray.h"
    #include "tools/skui/InputState.h"
    #include "tools/skui/Key.h"
    #include "tools/skui/ModifierKey.h"
    #include <functional>
    #include <vector>
    class SkCanvas;
    namespace sk_app { class Window; }
    ---

This makes use of Bazel's toolchain features
https://bazel.build/docs/cc-toolchain-config-reference#features
to allow us to configure compiler flags when compiling
individual files. This analysis is off by default, and can
be turned on with --features skia_enforce_iwyu. When enabled,
it will only be run for files that have opted in.
Example:
    bazelisk build //example:hello_world_gl --config=clang \
       --sandbox_base=/dev/shm --features skia_enforce_iwyu

There are two ways to opt files in:
 - Add enforce_iwyu = True to a generated_cc_atom rule
 - Add enforce_iwyu_on_package() to a BUILD.bazel file
   (which enforces IWYU for all rules in that file)

Note that Bazel does not propagate features to dependencies
or dependents, so trying to enable the feature on cc_library
or cc_executable targets will only impact any files listed in
srcs or hdrs, not deps. This may be counter-intuitive when
compared to things like defines.

IWYU supports a mapping file, which we supply to help properly
handle things system headers (//toolchain/IWYU_mapping.imp)

Suggested Review Order:
 - toolchain/build_toolchain.bzl to see how we get the IWYU
   binaries into the toolchain
 - toolchain/BUILD.bazel and toolchain/IWYU_mapping.imp
   to see how the mapping file is made available for
   all compile steps
 - toolchain/clang_toolchain_config.bzl, where we define the
   skia_enforce_iwyu feature to turn on any verification at
   all and skia_opt_file_into_iwyu to enable the check for
   specific files using a define.
 - toolchain/clang_trampoline.sh, which is the toolchain is
   configured to call instead of clang directly (see line 83
   of clang_toolchain_config.bzl). This bash script used to
   just forward all arguments directly onto clang. Now it
   inspects them and either calls clang directly (if
   it does not find the define in the arguments or we are
   linking [bazel sometimes links with clang instead of ld])
   or calls clang and then include-what-you-use. In all cases,
   the trampoline sends the arguments to clang and IWYU
   unchanged).
 - //tools/sk_app/... to see enforcement enabled (and fixed)
   for select files, as an example of that method.
 - //experimental/bazel_test/... to see enforcement enabled
   for all rules in a BUILD.bazel file.
 - all other files.

Change-Id: I60a2ea9d5dc9955b6a8f166bd449de9e2b81a233
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519776
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-16 13:09:46 +00:00
Kevin Lubick
7ac7413f08 [bazel] Support Vulkan
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>
2022-03-08 21:21:17 +00:00
Kevin Lubick
acab911351 [bazel] Make use of test_on_env to spin up server for gms
In order to extract the PNG files produced by our CanvasKit gms,
we need our JS tests to POST them to a server which can write to
disk. The easiest way to do this is to use the test_on_env
rule defined in the Skia Infra repo for exactly this purpose.

This required https://skia-review.googlesource.com/c/buildbot/+/510717
to be able to configure the binary correctly and
https://skia-review.googlesource.com/c/buildbot/+/511862, for nicer
debugging so the skia-infra dep was updated via the following commands:
$ go get go.skia.org/infra@d8a552a29e
$ go mod download
$ make -C infra/bots train
$ make -C bazel gazelle_update_repo
This caused many automated changes to infra/bots/tasks.json

The flow is:
1. User types bazelisk test :hello_world_test_with_env
2. The test_on_env rule starts gold_test_env and waits
   for the file defined in $ENV_READY_FILE to be created.
3. gold_test_env starts a web server on a random port. It
   writes this port number to $ENV_DIR/port. Then, it
   creates $ENV_READY_FILE to signal ready.
4. test_on_env sees the ready file and then starts the
   karma_test rule. (Reminder: this is a bash script
   which starts karma using the Bazel-bundled chromium).
5. The karma_test rule runs the karma.bazel.js file (which
   has been injected with some JS code to fill in Bazel
   paths and settings) using Bazel-bundled node. This reads
   in the port file and sets up a Karma proxy to redirect
   /gold_rpc/report to http://localhost:PORT/report
6. The JS tests run via Karma (and do assertions via Jasmine).
   Some tests, the gms, make POST requests to the proxy.
7. gold_test_env gets these POST requests writes the images
   to a special Bazel folder on disk as defined by
   $TEST_UNDECLARED_OUTPUTS_DIR.
8. test_on_env identifies that the tests finish (because the
   karma_test script returns 0). It sends SIGINT to gold_test_env.
9. gold_test_env stops the webserver. The special Bazel folder
   will zip up anything inside it and make it available for
   future rules (e.g. a rule that will upload to Gold via goldctl).

Suggested Review Order:
 - bazel/karma_test.bzl to see the test_on_env rule bundled into
   the karma_test macro. I chose to put it there because it might
   be confusing to have to define both a karma_test and test_on_env
   rule in the same package but not be able to call one because it
   will fail to talk to the server.
 - gold_test_env.go to see how the appropriate files are written
   to signal the environment is ready and the handlers are set up.
 - karma.bazel.js to see how we make our own proxy given the
   port from the env binary. The fact that we could not create
   our own proxy with the existing karma_test rule was why the
   chain ending in https://skia-review.googlesource.com/c/skia/+/508797
   had to be abandoned.
 - tests/*.js to see how the environment is probed via /healthz
   and then used to make POST requests with data.
 - Everything else.

Change-Id: I32a90def41796ca94cf187d640cfff8e262f85f6
BUG: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510737
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
2022-02-28 14:05:54 +00:00
Kevin Lubick
b902658a1a [bazel] Fix Bazel build by guarding addToKey
PS1 is the automatic regenerated changes.
PS2-3 adds an #ifdef guard to the addToKey method on shaders.
The SkShaderCodeDictionary class helps generate SkSL and is
only necessary when we are building with SkSL (gpu builds and
cpu builds with SkVM).

Suggested Review order:
 - Use Gerrit to diff PS 1 and the last PS
 - src/core/BUILD.bazel adds some sources to the "only
   necessary if sksl is enabled" bucket
 - All the .cpp and .h files to see the #ifdef is added
   correctly.

Change-Id: I4d4ce61a4957ef1e0840204acff08ce7e616f9cb
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512157
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-02-24 18:10:40 +00:00
Kevin Lubick
e94b52c442 [canvaskit] Add skottie to Bazel build
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>
2022-02-04 19:18:27 +00:00
Kevin Lubick
7a14f783bd [bazel] Sketching out HelloWorld sk_app using GL
bazel run //example:hello_world --config=clang
causes a window to open and draws a circle and a square.
Text to follow in a future CL.

To make this work, I had to get rid of musl and use glibc.
All the shared libraries (.so files) that were pre-built
and available for download (e.g. from https://packages.debian.org/bullseye/amd64/libgl1/download)
were compiled against glibc. When I tried to run a
program statically linked with musl and dynamically linked
against things using glibc, I got a segmentation fault
on things like calloc().

Initial attempts to use glibc had failed because it was thought
that the libc.so.6 file could only be referred to by absolute
path (and thus Bazel would not be happy about it). As it turns out,
that was simply a misconfiguration of the builtin_sysroot
parameter to cc_common.create_cc_toolchain_config_info
(see //toolchain/clang_toolchain_config.bzl). By setting that
to `external/clang_linux_amd64` and not
`external/clang_linux_amd64/usr`, the libc binary which had
been extracted to `external/clang_linux_amd64/lib/x86_64-linux-gnu`
was perfectly reachable from
`external/clang_linux_amd64/usr/usr/lib/x86_64-linux-gnu/libc.so`

To bring in the shared libraries to link against (e.g. X11, GL)
I made build_toolchain.bzl easier to modify in that we simply need
to add a debian download url and sha256 hash to a list (rather than
having to plumb this through via arguments).

Recommended Review Order:
 - example/BUILD.bazel (not sure if we always want to set bare
   link arguments like that or if we want to use "features" to
   pass those along to the toolchain).
 - tools/sk_app/BUILD.bazel to see initial cc_library for
   wrapping sk_app code.
 - toolchain/build_toolchain.bzl to see removal of musl and
   new list of debs.
 - toolchain/clang_toolchain_config.bzl (where use of the
   no-canonical-prefixes was key to compilation success).
   Notice also that we statically linked libc++ (I did not
   have any shared libraries for it locally, so I guessed
   a typical developer might not either).
 - Rest of toolchain/ for trivial renames.
 - bazel/Makefile to see extra docs on those targets and
   a new target that compiles all the exes so far for a
   quick way to test the build.
 - third_party/BUILD.bazel and src/gpu/BUILD.bazel which have
   non-generated changes. (all other BUILD.bazel files do).
 - go.mod, which needed to update the infra repo version in
   order to pick up http://review.skia.org/491736).

Change-Id: I8687bd227353040eca2dffa9465798d8bd395027
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/492117
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-01-11 13:06:19 +00:00
Kevin Lubick
96e4053be7 Update Bazel files
- 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>
2022-01-06 18:10:57 +00:00
Kevin Lubick
9488e0237d [infra] Experiment generating BUILD.bazel files
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>
2021-11-23 18:53:16 +00:00
Kevin Lubick
1f8c31b101 [infra] Add initial Bazel rules and files
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>
2021-11-09 12:32:25 +00:00
Kevin Lubick
97284f255b [infra] Add initial platforms and constraint values
This makes use of Bazel's pre-defined platforms
https://github.com/bazelbuild/platforms
and some of our own defined values (see
//bazel/common_config_settings/BUILD.bazel) to customize
the build rules.

I verified this by building bazel_test locally for
linux x64 as well as using the third_party deps for
a WASM build (using build files not seen in this CL).

Suggested Review Order:
 - https://docs.bazel.build/versions/main/platforms.html if not
   already familiar with Bazel Platforms
 - third_party/BUILD.bazel to see that 1) all globs have
   been removed and 2) select() targets various
   platform constants or groups of constants to control
   sources, headers, and local_defines.
 - common_config_settings/ to see the groups of constraints
   created, as well as new constraint_settings defined
   (skdebug_impl)
 - supported_combinations/ to see how we can define supported
   sets of the constraint values (aka Bazel platforms).
   I imagine expanding this more, so we might have platforms
   named "linux_x64_emptyfontmgr_vulkan" or such.
 - //BUILD.bazel and bazel_test.cpp to see use of SkDebugf.
 - Everything else.

Change-Id: I49e4abdbcf7b76f0674efdbb1f53dc8823d110ee
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463517
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
2021-10-26 18:27:13 +00:00