According to https://en.cppreference.com/w/cpp/container/span/span (2),
it is undefined behavior if [first, first + count) is not a valid
range, so it is standards-conforming to assert on this span.
This caught one case in GlyphRun where we were creating an invalid
span (which was apparently harmless in practice).
Change-Id: Ia0661b3bd13c2c5773492960eea11b1a294072d5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550499
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Will be used in SkMesh buffer update API and likely for unit testing of changes related to Issue 13427.
Bug: skia:13427
Bug: skia:12720
Change-Id: I487ee512d4b8bb61c533c7047bdbffcde9779885
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549570
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
This was originally done with the intent of adding a third field
(number of pointers in the data) but we may be able to get away with
keeping the two-byte Header after all. Having the data in an actual
struct does make refactoring a lot easier, at any rate.
Change-Id: I25a4e46b45f3f9c93eb16bc4d51b6624d7e319ba
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550183
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Bug: skia:12701
Change-Id: I53bbabb2aba44e3bfc215df81dae1ae59d149263
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544319
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
[[maybe_unused]] is built in to C++17.
Change-Id: I5e49d0801878fd9fbca62ab28080aa856e33ca79
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550500
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Spent 10 minutes searching for this macro because I expected it to have
"pack" in the name, like __attribute__((packed)) or #pragma pack.
Adding a little lifeline for future me.
Change-Id: I8c4f8cbd4febdea6bcd5698d267a2673246295e9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550184
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: I5c1040b8236dc792de20495a3fea3c0be6e31c20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549847
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This adds mostly-functional compute shader output to SkSL. This does not
add overall compute shader support to Skia; it still needs to be
integrated into the Skia pipeline before it is possible to actually use
compute shaders in practice.
Change-Id: Ic5c69863704f141d16bb191224a817e44f4a8565
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/539060
Reviewed-by: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I33121076bac3ceca19c81ff9fb47e4b024a14230
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549838
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Previously, we were storing sk_sp<SkRuntimeEffect> objects in static
variables. This would presumably lead to destructors executing during
atexit time, which is unnecessary and can lead to shutdown bugs. We no
longer attempt to free these objects.
Change-Id: I0b09d7ab3dc67c9b8fbc35c81d72ff57b0592c03
Bug: skia:13426
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549658
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Use template guides for deducing the type needed by the CTOR.
This allows SkSpans to be deduced when things like vector,
array, A[], etc. are passed as parameters without using SkMakeSpan.
Change-Id: Ieedcbddc8d40281f819b0c798acd178a12a05891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549568
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Change-Id: I7a4cd8e2c629ba53e40adc379ea92f86f70d11da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549857
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Auto-Submit: Jim Van Verth <jvanverth@google.com>
Bug: skia:13118
Change-Id: Icaf2b0914ce7943f57e1ab23e7778d1dcb811cc4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548781
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: skia:13118
Change-Id: I0d6e0a9f542680456d3de82a32376722c5b9cd1d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549097
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Use std::size and std:data to get needed values.
Add a SkMakeSpan for initializer_list.
Change-Id: I43b4b2391323c84b3d6aad22ac4775487c6d5570
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549565
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:13118
Change-Id: I06d3f899b60ac2cf8d48a46da6097a15c5d40024
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543923
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This adds a new SkShaderType for runtime shaders, but it doesn't do
anything interesting yet; for now, we draw solid magenta as a
placeholder.
Change-Id: If0e4af9000d97fcf1184fbed004a64fff8fa4e63
Bug: skia:13405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548477
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Change-Id: I27eb99450b2b261a4688c62d91a9002dfe7d14df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548484
Auto-Submit: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I0925a52cc3a504c95ec4453e74c4580ce692275c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548896
Reviewed-by: Eric Boren <borenet@google.com>
Reviewed-by: Heather Miller <hcm@google.com>
Auto-Submit: Heather Miller <hcm@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
Change-Id: I964ac2d7d2f1e9d10b0216deb6572b28a26da0fc
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/547256
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Bug: skia:12701
Change-Id: Ibf7b4ac280eb853e3f5d786bd188ae0ac208609b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545367
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 5270322b46.
Change-Id: If594d04dc657126dce48d69dcc67d1a5e3b0cc8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546856
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
SkMesh now takes SkData that contains the uniform values.
Change-Id: I286e7559ba692ce15925a54f59c2e829b6b5448f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546096
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:12701
Change-Id: I88a12b776cf9b14d0c0a94ae777b85d9592696a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/544318
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 9583759bbd.
Reason for revert: MSAN failure
Original change's description:
> Restored unsized array support to SkSL
>
> This is a prerequisite for compute shaders. As of this CL, there isn't
> yet a way to use unsized arrays, as it is a compute-only feature and
> compute shaders are coming in a followup CL, but this adds the basic
> framework and error tests.
>
> Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Id10b48ef24c0e6219b65b0a201d13fea9632620f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546552
Auto-Submit: Brian Osman <brianosman@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bug: skia:12701
Change-Id: I4209a4741a957a4dc60ad6669fc164a2d5993676
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545720
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This is a prerequisite for compute shaders. As of this CL, there isn't
yet a way to use unsized arrays, as it is a compute-only feature and
compute shaders are coming in a followup CL, but this adds the basic
framework and error tests.
Change-Id: I390c0961e324dd474474563bf9a8f6b34c9552a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/538900
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This CL adds the ability to add child combinations to the top-level
combinations added to the builder.
Bug: skia:12701
Change-Id: Ibe182c631bb9811d82e314ba58823b1708058165
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542299
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
DSLType is now a simple wrapper around SkSL::Type*. Previously, its type
was determined through a combination of a Type* and a TypeConstant enum,
and fSkSLType == nullptr was used as a sentinel to mean "use the type
constant instead", which meant that we couldn't have a null DSLType
as it would fall back to the type-constant.
We can now return `DSLType(nullptr)` to indicate an error case in type
handling code, instead of needing to wrap the DSLType in an optional<>.
Change-Id: Iebaab86162b526a7fcb93d253367a7e4881ef6d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545781
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
DSLType stored an `fPosition` which was only used in one place, when
reporting that a function is not allowed to return a particular type.
Those errors now highlight the type and function name together, which is
not really any worse than before. This allows us to shrink DSLType down
to its minimal form, just a pointer to an SkSL::Type and nothing else.
Change-Id: I4b430cb996472da0ae57bc2ab095cd123d2c3f51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/546097
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This CL removes the TypeConstant member from DSLType. The constructor
which takes a TypeConstant now sets the `fSkSLType` field immediately
instead of saving the value and deferring lookup until `skslType()` is
called.
This caused some ripple-effect issues in type setup code which were
fixable by replacing nullptr with Poison.
Change-Id: I8fa73cdf5f0bcd3de143c9a25ea43392d75c7dec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/545780
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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>
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>
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>
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>
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>
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>
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 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>
Change-Id: I7ef9d50eeb49076dbd71fd7070a0f849085c4dc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543978
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
