We are unable to list proper OWNERS in a way that works with all of
the tooling. Document why, and also leave a recommendation for human
readers to find a knowledgeable reviewer.
Change-Id: Icf7779c55a983aea9b298b6f7a4a639e3090e279
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521517
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Cq-Include-Trybots: luci.skia.skia.primary:Build-Mac-Clang-arm64-Debug-ASAN_Graphite,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-x86_64-Debug-Graphite,Build-Mac-Clang-x86_64-Release-Graphite,Perf-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Release-All-Graphite,Perf-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Debug-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac12-Clang-MacBookPro16.2-GPU-IntelIrisPlus-x86_64-Debug-All-Graphite
Bug: skia:12703
Change-Id: Ie80d495f38614f9b5d0b09741ca0d24560ebe870
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520976
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jim Van Verth <jvanverth@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>
If the paint has a blur mask filter, then we can adjust the sigma
of the filter to use the CTM adjustment case instead of
transforming the path. This allows reusing the mask cache for the
blured part of the drawing.
Tested on TOT Chromium with this CL patched in. Smooth as silk.
Bug: chromium:1304806
Change-Id: I5cda17f1769ef173e6734d22edbd3fef4ed8db6f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521100
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@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>
This is a short term solution. The linked bug has more details about
where we want to land. As is, this doesn't use much of the resource key
system but ideally should be updated to do so if we can handle resource
tracking w/in the CommandBuffer efficiently.
Alternatively we may pursue a direction where that is moot because it's
cached globally or held directly by the RenderStep.
As-is, this is a simple approach and should produce equivalent behavior
from a GPU command perspective in the single Recorder situation we are
in currently.
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,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
Bug: skia:13059
Change-Id: I6a110fafe2b69a8bfbc3d859b02d916c973baf26
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520737
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@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,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,Build-Mac-Clang-arm64-Debug-ASAN_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu
Bug: skia:12703
Change-Id: Iea08a885565811c442e8177b3fa0c668db95fd0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521008
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
DrawList's recording functions that paralleled the defined Renderers are
removed in favor of a single recordDraw() call that takes a Renderer.
Device has to do too much logic around determining draw ordering that
is coupled with the selected Renderer's requirements (e.g. stencil)
that it is easier to have a Renderer selector that Device uses, and
then it can inspect the requirements of that Renderer and modify the
ordering tracking as needed.
Right now, this renderer selection process is just in Device, but I
can imagine it living in its own object that the Gpu exposes so that
all wrappers around DrawContext make consistent decisions. This will
also come in handy if Renderers are created per Gpu instead of static.
Bug: skia:12703
Change-Id: I73b5254b7c4183f4c4e54cf5e2993ab8143bd4bd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521006
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12974
Change-Id: I6ec7736edc871241b0cf78413b0d5f7ff9abd8c5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520736
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The KeyContext is used in the addToKey methods but must appear in the
AddToKey methods bc the latter can call the former.
Bug: skia:12701
Change-Id: I3143afec8337b1e3e12f1c3cc198714009ca6930
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520539
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
In preparation for ag/Ib9bec1e1d72169a18e6ad1ce8f9008a65dbe5a71,
which will define it in SkUserConfigManual first.
Bug: b/224771432
Change-Id: I122895f089ce8949c028bccfcb971bd7d1c6ca72
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521001
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
These logs were always intended to be temporary. Remove them, along with
RENDERENGINE_ABORTF. This is in preparation for turning *all* SK_ABORT
messages into ones that will appear in stack traces (b/224771432).
Revert "Convert RENDERENGINE_ABORTF to LOG_ALWAYS_FATAL"
This reverts commit 98fbe5eb8d.
Revert "Add more debugging info for backend texture failure"
This reverts commit 13f244c95c.
Revert "Add Android Framework specific logging to SkSurface::MakeFromBackendTexture"
This reverts commit 04a9672c0a.
Bug: b/206415266
Bug: b/224771432
Change-Id: I5a0d97b4e7d14e2a4222dc84c9049ebb4f9e7e0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521000
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Leon Scroggins <scroggo@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>
Bug: skia:12974
Change-Id: I70f3ec7901cd32c2f61b23b3f41675fb1db16614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516805
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
The block increment parameter, after dividing by address align, has to
fit into 16 bits. SkTBlockList with either large T or a large
"itemsPerBlock" hint can pretty easily exceed this. However, both the
items per block and the block increment are just hints to control
allocation patterns. SkBlockAllocator can have larger blocks than that
based on growth policy, up to its actual allocation size limit.
SkTBlockList also does not need itemsPerBlock in its implementation, so
if the request exceeds what the allocator can do, it's not problematic.
So clamping to the highest storable value is nicer than asserting that
the caller respected the internal limits.
Change-Id: I82b1c20034fd264b65c7eae4d6758caa6b574fb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520656
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
We no longer intend to run visual GMs from SkQP.
Change-Id: Ib04958a3d445078d65b72e852afc69781873b93c
Bug: skia:13031
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520546
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
Today, if the arc command flags are not separated by whitespace, the
parser fails to parse the string. I noticed this when trying to parse a
path similar to the one in the test case when playing around with
PathKit.FromSVGString.
Change-Id: I40967c07dfa03d76d26ac2e060b3ef7ac488d0fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520256
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Dan Field <dnfield@google.com>
For discrete GPUs the draw buffers need to be unmapped so that
didModifyRange: is called to updated the managed buffer. In addition,
they should be tracked on the CommandBuffer.
Bug: skia:13033
Change-Id: I931b1bfd438bc75652c04734219690506fad5ea6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520537
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Support for GM tests was removed from SkQP at
http://review.skia.org/516336. However, the Java harness still expects
us to supply a non-null GM array, and will crash if we don't.
Change-Id: I1f093254e680bf8d40dcb303cd29ae7b44e09b0a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520538
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: John Stiles <johnstiles@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:12701
Change-Id: I1fd8dede3eb216c28408bd613119448704c0e7c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512356
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This reverts commit 5fe4b6faeb.
Reason for revert: relanding with guards
Original change's description:
> Revert "[skottie] Fix text-on-path tracking"
>
> This reverts commit ca973cbea0.
>
> Reason for revert: g3 image diffs
>
> Original change's description:
> > [skottie] Fix text-on-path tracking
> >
> > Tracking and line spacing computations require knowledge of cumulative
> > values for the whole line => we need two passes:
> >
> > 1) compute cumulative values
> > 2) compute per-fragment position adjustments
> >
> > Currently, #1 is implemented in the main onSync() loop (as we iterate
> > to compute fragment props) and #2 is post-applied via adjustLineProps(),
> > after the main loop.
> >
> > The problem is adjustLineProps() is executed after positioning glyphs on
> > path, and tracking is not taken into account for path positioning
> > (instead it moves glyphs horizontally, unrelated to the path).
> >
> > To fix this, we need tracking adjustments to be applied before
> > positioning on path (which is performed in fragmentMatrix()).
> >
> > - move the cumulative tracking computation to a dedicate lambda
> > (compute_linewide_props)
> > - move the fragment position adjustments to the main onSync() loop
> > (that way they participate in path positioning)
> > - to avoid executing the first pass unnecessarily, add flags to detect
> > the presence of tracking and line spacing animators.
> >
> >
> > Change-Id: Ieef2afb53ffe14177eba0ef41dc5c71149cab070
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518696
> > Reviewed-by: Ben Wagner <bungeman@google.com>
> > Commit-Queue: Florin Malita <fmalita@chromium.org>
> > Commit-Queue: Florin Malita <fmalita@google.com>
>
> Change-Id: Ia99fbb3d7d98eb6a59ff00d796bcc05bc6db63a3
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519597
> Auto-Submit: Florin Malita <fmalita@google.com>
> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Change-Id: Ia39602178099d7fa016997f02e1bdf705b16bb2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519598
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
This CL defines a const string in GrGPUResource and have the
constructors accept it. The label string is then plumbed through the
system for other components to accept it.
Bug: chromium:1164111
Change-Id: I6cc759f9263dedd4b2cc0c3ca7cf280be5d74174
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508798
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Previously, this wrapped an SkTLazy<T>, but in C++17, SkTLazy is just a
thin wrapper on top of std::optional. This CL removes the middle-man.
Change-Id: I78f88d24a7933dfac4b637367a3d1e7ee80b3070
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519622
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This test fails on P400 GPUs (used in our Golo machines) but the wasm
tests do not honor the dm_flags test exclusions. (Another good reason to
implement skia:13034.)
Failing on tree: http://screen/4PxKQrjxaXpL9Q2
Change-Id: I086fb3293b3f4eaad877064470002525a7d6e75f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519621
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Bug: b/212296687
Change-Id: I9f5b5966e7cc497f8c8591463801ef558297f3ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519620
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
This makes the fixed-function blending happier
Bug: skia:12701
Change-Id: I398977a3ce9c25949535f73a83b9eb774d2d1c35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519616
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
