Change-Id: Ic88b400f7943804481934ba2c9c54aeb8e04121b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523656
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Herb Derby <herb@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac12-Clang-MacBookPro16.2-GPU-IntelIrisPlus-x86_64-Debug-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Test-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Debug-All-Graphite,Perf-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Perf-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Release-All-Graphite,Build-Mac-Clang-x86_64-Release-Graphite,Build-Mac-Clang-x86_64-Debug-Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Change-Id: I2ee0fd8d208bb10e011a5552a60a1a62d7a56ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522423
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is a reland of commit 171450a1a0
Original change's description:
> Add glGetFloatv and glSamplerParameterf to GrGLInterface.
>
>
> Bug: skia:13036
> Change-Id: Ie15e0c2b0ada468b82bced818eec67c32d70126b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522100
> Auto-Submit: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13036
Change-Id: Id594ea5fd68c76703ff61d8414c203facbf28801
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522359
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
PS1 regenerates the Bazel files. Use it as the base change when
comparing patchsets.
IWYU seems to do a good job of working with MyFile.cpp and
MyFile.h, but if there is just a MyHeader.h, it doesn't always
seem to throw errors if the includes aren't correct. This was
observed with include/sksl/DSL.h This might be due to the fact
that headers are not compiled on their own, so they are never
sent directly to the IWYU binary.
This change sets enforce_iwyu_on_package() on the all sksl
packages and then fixes the includes until all those checks
are happy. There were a few files that needed fixes outside
of the sksl folder. Examples include:
- src/gpu/effects/GrConvexPolyEffect.cpp
- tests/SkSLDSLTest.cpp
To really enforce this, we need to add a CI/CQ job that runs
bazel build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
If that failed, a dev could make the changes described in
the logs and/or run the command locally to see those
prescribed fixes.
I had to add several entries to toolchain/IWYU_mapping.imp
in order to fix some private includes and other atypical
choices. I tried adding a rule there to allow inclusion of
SkTypes.h to make sure defines like SK_SUPPORT_GPU, but
could not get it to work for all cases, so I deferred to
using the IWYU pragma: keep (e.g. SkSLPipelineStageCodeGenerator.h)
Change-Id: I4c3e536d8e69ff7ff2d26fe61a525a6c2e80db06
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522256
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:13051
Change-Id: I13cdb1b625f65e6d15d3b59ba493897ed88b24c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521005
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds an assert to Position::line() to ensure that we don't run
past the end of the source string. Since the source string can
contain embedded nulls, we also switch the source handling from
const char* to std::string so we can accurately determine its length.
Change-Id: I9df47e98c1a0cbc35222a0ea709d9403762210d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521656
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
By setting the compile time flag:
SK_EXPERIMENTAL_SIMULATE_DRAWGLYPHRUNLIST_WITH_SLUG_STRIKE_SERIALIZE
will cause all SkTextBlobs to be rendered by analyzing the Slug
to create strike cache differences and serialize the Blob to a Slug.
Then the serialized strike cache differences are used to populate
the strike cache using a different TypefaceIDs using SkTypeface_Remote
as a proxy for the real SkTypeface. This will create a hard break
between the original glyph data, and the proxied glyph data.
It will then deserialize the Slug doing TypefaceID translation to
the SkTypeface_Remote, and draw the unflattened Slug.
Bug: chromium:1278340
Change-Id: I0f1980dee966b1092a99741793aed9d138451f4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510228
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Chrome uses a picture to manage some of their drawing in the GPU
process. This CL serializes GrSlugs into an SkPicture, and to
draw from that picture. Pictures with GrSlugs can not be stored;
they must be created and played back directly. In addition, the
SkStrikes used by the slugs must be pinned in the SkStrikeCache.
Bug: chromium:1302036
Change-Id: I19fa9c06c08f64ad066326e90bc6425dd90b3342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514360
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This will use the recently added Bazel toolchain feature
to enforce proper includes for all files in //src/svg/...
In the future, I envision a CI/CQ job that will run
bazel build with a few different configurations and the
--feature skia_enforce_iwyu on to make sure we don't
regress.
Change-Id: Ibb9f816ab626415c11bd2b9b74c503297c4b0723
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521036
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
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>
Change-Id: I677ad3073725219897bec9f82d88ad3ba3fedb53
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521002
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Herb Derby <herb@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.
Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12845
Change-Id: Ic036dea6b58682a6463f8c34d915730c4bfe677b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519617
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: I1fa160941b18392616708f4e08a4ebbf7398480e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518521
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
PS1 regenerates BUILD.bazel files
I suggest reviewing the deltas between PS1 and the latest
PS to focus on the interesting bits.
The changes here allow for a Vulkan-only build of HelloWorld
based on sk_app. The toughest change was properly fetching
the VisualID after removing the gl calls that used to
fill that in.
There are a few changes that fix resolution of Dawn
header files, but those won't actually be built until
a follow-on CL.
Change-Id: I54fb58b5dd7ecd4313562aed401759b3eaed53c0
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516999
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
This was motivated by SkScalerContextFlags putting `kNone` into the
global namespace. Instead of adding a suffix, change these internal
enums to be scoped and update the users.
Because SkScalerContextFlags is used like a bitfield,
`SK_MAKE_BITFIELD_OPS` is used to provide the appropriate operators. The
macro implementation is updated to handle scoped enums, remove the very
dangerous, unwanted, and unused templated operators, and add operator~
since that is needed by one existing user.
Also, for some reason this allows the compiler to find an unused field
which is removed.
Change-Id: Id31c80b381bffaf9b623f6a8ec7d86751d3d99f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
ag/16935648 added a method to detect the color depth of R/G/B channel
for input sources. This change does the following: (1) call this new method and default
SkAndroidCodec to 1010102 for 10-bit images; (2) add color-depth to
SkEncodedInfo.
Change-Id: I3ede4a7a6223da2d4c075403d6ee280db086e0be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512076
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
When deserializing an SkSL shader, it's possible to encounter code that
doesn't compile (particularly from the last year, when syntax tweaks
were being made). By default, we still treat this as an error and fail
to load the SKP. In the SKP debugger, though, we add a lenient mode that
permits these failures. If any of the children were SkShaders, we
instead return that. If all else fails, we just return nullptr, which
will cause the triggering draw to use a solid paint-color, but at least
allow the SKP to be loaded and viewed.
For color-filters and blenders, we allow malformed SkSL, but always
return nullptr.
The bug that prompted this change involved Android's overscroll stretch
shader, where this approach works particularly well (it turns the
stretch into a simple pass-through of the underlying content).
Change-Id: I756c694739d31b11efa1b82c126f34440a7de66a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515543
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
We seem to save about 1K of code size each time an unordered_set/map is
replaced.
Change-Id: I7e04076d470e6c7ad4ae8301ded231ae69168bc8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515361
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This will allow us to easily replace unordered_set/unordered_map in more
places.
Change-Id: Ic0b5245e8cdce2918d7b043bfc370eca95caf889
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515319
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This will allow these types to be used as the key in an SkTHashSet/Map
without providing a custom Traits class.
Change-Id: I41bfc1d43f3909d3653b4f12ac39e2a288235052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515356
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Bug: skia:12845
Change-Id: Ib0b87966b1559b368759ce69c48f0fd27597503a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514836
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Also adds in asView for texture-backed Image as well as stubs for
other Image types.
Bug: skia:12845
Change-Id: I58a90068d4d151ad35f411b08c8484c740253722
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/507336
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: I9ee142f949e644dcd06f359bb6838b5b4f04ec21
Bug: skia:12990
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513736
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
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>
Cq-Include-Trybots: luci.skia.skia.primary:Canary-G3
Change-Id: Iebd4aaec098b6bf2e863d0c93d7fd826d3ec0edc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512724
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
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>
This is a reland of 0596094b81
Original change's description:
> Remove skstd::optional entirely.
>
> Change-Id: Id9862712ea3e769797abd654922879ce6bc4487c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504976
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
Change-Id: Ie5bae44de729aabe50c4e51ad3c7cc476fbc5dc6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512358
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
SkCanvas::fMCStack uses SkCanvas::fMCRecStorage for storage, and will
access it in the destructor. Reorder the fields so that fMCStack is destroyed
before fMCRecStorage.
Change-Id: Ie09b9b52820d2dec4b75552dc5c2349b845a01ef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509536
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
