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>
With a hot Bazel cache, https://task-scheduler.skia.org/job/4XhwJfG1wR38Hp3dF1pI
took 59 seconds and did not have to wait 1-2 minutes
for TaskScheduler to schedule a prerequisite task
and Swarming to deduplicate said task, which is
the best case scenario for "all hot caches".
Change-Id: Ic86599e0b886ecded836641cceb19b623065a91e
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546136
Reviewed-by: Eric Boren <borenet@google.com>
MSVC is a very marginal compiler for us (and we don't call SkPopCount that often) so we've deemed it not worth the extra complexity to handle the __popcnt intrinsic.
Change-Id: I1d838727eab503cd99a72d07e9787280d0948b29
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545896
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
In general, SkSurfaceProps is only needed when rendering text. However,
there are several existing APIs that don't allow SkSurfaceProps to be
passed in by the user.
This change adds new SkSurfaceProps parameters to several public-facing
APIs:
1. SkRasterHandleAllocator::MakeCanvas
-- The props are used by the canvas whenever text is rendered.
2. SkImage::MakeFromPicture and SkImageGenerator::MakeFromPicture
-- The props are used to render any text in the SkPicture object.
Change-Id: Ic48e8a30bb12b3170415c644de1a007b5eefb818
Bug: skia:13369
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545396
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Needed to fix the Dawn and Harfbuzz Bazel rules.
Change-Id: I21f63c970bdc972b97e42ef85d82d7f478bd45e2
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545721
Reviewed-by: Eric Boren <borenet@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>
Currently there is no way to specify the data that is bound to them.
That will come in later changes.
Bug: skia:12720
Change-Id: I3301a825486469396a13a4095b66d9e0b81f183b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543716
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
There are no functional changes to the rules, just a relocation.
Change-Id: I90af5ec792fc54ce2978b0bbb1afd2c932e183b4
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545719
Reviewed-by: Ben Wagner <bungeman@google.com>
Booleans and structs/interface blocks that transitively contain a
boolean member are no longer allowed to be used as a uniform. This is
because SPIR-V and WGSL currently disallow OpTypeBool in host-shareable
storage classes.
Change-Id: I10315c7f261ff10a07636265968a91d9c421da55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542776
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
The cast of newly-allocated memory to T* is the only thing that triggers
a CFI error; we can let CFI instrument the rest of the code normally.
Change-Id: I0e6ab76dbddf031967bee34f5a05986ff58e6714
Bug: skia:13339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545676
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Some unused var and pessimizing move warnings, e.g.
../../experimental/sktext/src/Text.cpp:316:12: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
return std::move(shapedText);
^
../../experimental/sktext/src/Text.cpp:316:12: note: remove std::move call here
return std::move(shapedText);
^~~~~~~~~~
Change-Id: Id5c220b7a850b2013e2bf065a87d4a296bd0369c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542522
Reviewed-by: Julia Lavrova <jlavrova@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Auto-Submit: Florin Malita <fmalita@chromium.org>
I had to copy some config_settings out of //bazel/common_config_settings
because these are now treated as separate entities and cannot
see that file.
For libpng, note that we use a genrule to create the
pnglibconf.h instead of pointing to one somewhere else.
This ended up being easier than other things I tried.
Another approach would be to not depend on the version
in third_party/externals, but to clone it via
new_git_repository [1] and apply a patch that creates
the configuration file.
[1] https://bazel.build/rules/lib/repo/git#new_git_repository
Bug: skia:12541
Change-Id: I9a284775dc0f2bdabb145518d5f0803c74fb99fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545368
Reviewed-by: Ben Wagner <bungeman@google.com>
This reverts commit ef7c45a0fa.
Reason for revert: avoiding perf regression
Original change's description:
> Revert "Disable Control-Flow Integrity in SkTArray when casting buffer to T*."
>
> This reverts commit 0b1384de5a.
>
> Reason for revert: visible impact on performance: http://screen/4N7DNCQNSgtzzBohttp://go/crb/1330618
>
> Original change's description:
> > Disable Control-Flow Integrity in SkTArray when casting buffer to T*.
> >
> > We disable Control-Flow Integrity sanitization (go/cfi) when updating
> > the item-array buffer. CFI flags this code as dangerous because we are
> > casting `buffer` to a T* while the buffer's contents might still be
> > uninitialized memory. When T has a vtable, this is especially risky
> > because we could hypothetically access a virtual method on fItemArray
> > and jump to an unpredictable location in memory. Of course, SkTArray
> > won't actually use fItemArray in this way, and we don't want to
> > construct a T before the user requests one. There's no real risk here,
> > so disable CFI when doing these casts.
> >
> > Change-Id: I5708053339f4a600b12c841fcd38880f9932f7d6
> > Bug: skia:13339
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542643
> > Commit-Queue: Ben Wagner <bungeman@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Ben Wagner <bungeman@google.com>
>
> Bug: skia:13339
> Change-Id: I9f39100fc4a03359fa7712b0a8d8cbe3bc7de625
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545365
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:13339
Change-Id: I7629cb9045e0c7a804785f8c0ad569610e1c67e1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545366
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Factor out the common code for handling SubRuns from Blobs and
Slugs. SubRunContainer will be moved to its own file in the
next CL to reduce the size of TextBlob.cpp.
Change-Id: I185336c5fa7a70841ef7b706fa194aa01649dacc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544248
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Mostly, this adds placeholder BUILD.bazel files corresponding to files
used in public.bzl
The exception is src/ports/BUILD.bazel, which adds an explicit
link command for dl, needed for dlopen etc.
Bug: skia:12541
Change-Id: Id3801a4c718cec37bc2aa3920a8d810f8a80a373
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545359
Reviewed-by: Ben Wagner <bungeman@google.com>
(to fix non-monotonic glyph placement)
With Google3 test failure in Samples
showing that it's not SkParagraph fault but
rather a result of this fix.
Change-Id: I72e025cdaf806bae0e8a8337074f75af5af428b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544336
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
This reverts commit 0b1384de5a.
Reason for revert: visible impact on performance: http://screen/4N7DNCQNSgtzzBohttp://go/crb/1330618
Original change's description:
> Disable Control-Flow Integrity in SkTArray when casting buffer to T*.
>
> We disable Control-Flow Integrity sanitization (go/cfi) when updating
> the item-array buffer. CFI flags this code as dangerous because we are
> casting `buffer` to a T* while the buffer's contents might still be
> uninitialized memory. When T has a vtable, this is especially risky
> because we could hypothetically access a virtual method on fItemArray
> and jump to an unpredictable location in memory. Of course, SkTArray
> won't actually use fItemArray in this way, and we don't want to
> construct a T before the user requests one. There's no real risk here,
> so disable CFI when doing these casts.
>
> Change-Id: I5708053339f4a600b12c841fcd38880f9932f7d6
> Bug: skia:13339
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542643
> Commit-Queue: Ben Wagner <bungeman@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ben Wagner <bungeman@google.com>
Bug: skia:13339
Change-Id: I9f39100fc4a03359fa7712b0a8d8cbe3bc7de625
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545365
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Bug: skia:12720
Change-Id: I7f74b4f88456bf5ddef1a648d2231d28034ed246
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544237
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@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>
DSLParameter no longer has a vtable, and SkTArray no longer triggers CFI
on objects with vtables, so the vector<> workaround is no longer needed.
Change-Id: I035cce23af072623943a2ca751d8c2b3a4ea1750
Bug: skia:13339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545237
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This reverts commit 193c16380f.
Reason for revert: fixed google3 warning
Original change's description:
> Revert "Eliminate DSLPossibleExpression."
>
> This reverts commit f2d000328f.
>
> Reason for revert: breaking google3 roll
>
> Original change's description:
> > Eliminate DSLPossibleExpression.
> >
> > The Possible(Expression|Statement) classes were added at
> > http://review.skia.org/375069. These classes were responsible for
> > capturing `__builtin_FILE()` and `__builtin_LINE()` when an
> > expression or statement was added to a hand-authored DSL program. This
> > allowed errors to be reported on the C++ file/line where they were
> > encountered. This was a good feature to have, when the plan was to
> > author the majority of SkSL code via DSL.
> >
> > Later, IRNode positions were converted from an integer line number to
> > SkSL Positions at http://review.skia.org/518137. This gave us range
> > tracking, but at a high memory cost (16 bytes per IRNode, versus four
> > bytes when we tracked line numbers only).
> >
> > Positions were reduced to 8 bytes at http://review.skia.org/521005 by
> > removing the filename, which was only used for hand-authored DSL. (The
> > size was pared all the way back to 4 bytes at
> > http://review.skia.org/533699 by packing the data more efficiently.)
> >
> > __builtin_FILE/LINE capturing was removed entirely at
> > http://review.skia.org/528366; the filename was discarded anyway and
> > the line number didn't have a range and wasn't very meaningful without
> > a filename. Also, it didn't matter very much since we no longer intended
> > to hand-craft our programs in DSL.
> >
> > At this stage, DSLPossibleExpression stopped adding value and simply
> > served to move Expressions around.
> >
> > Change-Id: I29ac33e08dcc1804cc0619c1e8856ba28ebcc51d
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542145
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> > Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
>
> Change-Id: I33badbdcce8760200246bf50e4932d42721ea952
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543078
> Reviewed-by: Tyler Denniston <tdenniston@google.com>
> Owners-Override: Kevin Lubick <kjlubick@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Tyler Denniston <tdenniston@google.com>
Change-Id: I71f248b2343806f85cad5f0661470c95334bbe22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545236
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This may look like a lot, but //modules/canvaskit/BUILD.bazel
is nearly identical to how it was with gazelle:
162dfca340/modules/canvaskit/BUILD.bazel
I removed the "wasm_gm_tests" targets from it, because they
had bitrotted slightly and fixing them is its own task.
CanvasKit depends on Skottie and Particles, which depend on
the SkParagraph, SkShaper, SkUnicode, and SkResources modules.
I've structured the BUILD.bazel files in the //modules directory
in a similar fashion as the "hierarchical filegroup"
introduced in https://skia-review.googlesource.com/c/skia/+/543977
Suggested Review Order
- //modules/skottie/...
- //modules/skparagraph/...
- all other modules.
- Note that modules/canvaskit/go/gold_test_env/BUILD.bazel is
generated from gazelle, because we like how gazelle handles
golang files and deps.
- All other files in any order.
Change-Id: I0aa9e6f81dba2c00f15cae7b19fe49a2027dcf1d
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544676
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
This adds or fixes rules to build three binaries that previously
had Bazel support.
How we build skslc with Bazel differs from GN in a large way:
GN has a small set of C++ files [1] it compiles in, but with
Bazel that was too hard/clumsy to pipe through (and get the
headers to work well). So, I just had skslc depend on
//:skia_core for simplicity. skslc did need to include something
in //src, so I made a special filegroup for it. For more
complex executables that need more headers from //src, we
should probably make a "src_hdrs" filegroup or something
to expose those. That or a skia_for_tests cc_library that
has all headers as "public".
[1] https://github.com/google/skia/blob/main/gn/sksl.gni#L235
Change-Id: Ie1382e982228059369886f4bfef4947f686b11b5
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544637
Reviewed-by: Ben Wagner <bungeman@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>
We disable Control-Flow Integrity sanitization (go/cfi) when updating
the item-array buffer. CFI flags this code as dangerous because we are
casting `buffer` to a T* while the buffer's contents might still be
uninitialized memory. When T has a vtable, this is especially risky
because we could hypothetically access a virtual method on fItemArray
and jump to an unpredictable location in memory. Of course, SkTArray
won't actually use fItemArray in this way, and we don't want to
construct a T before the user requests one. There's no real risk here,
so disable CFI when doing these casts.
Change-Id: I5708053339f4a600b12c841fcd38880f9932f7d6
Bug: skia:13339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542643
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
