This was originally present to control filter mode, and was never
removed when filtering decisions were placed on the SkShader.
Change-Id: I2b91f02f994c508fd986cd1b3e28fa97f15d8469
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/536496
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In OT-SVG `var` support is required for palette use. Even without
palette support this is needed to get the fallback color.
Change-Id: Iea8c4502b57933115f5cbf44fcc183fef19da984
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/534140
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
G3 prefers license() first.
This was done mechanically with a big find/replace
Change-Id: I8c33c7bc10a6bec42e966cad81c259954e841811
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535898
Reviewed-by: Ben Wagner <bungeman@google.com>
Ran the following commands:
find -name "BUILD.bazel" -exec sed -i -e '1iload("//bazel:macros.bzl", "cc_library", "exports_files_legacy")\nexports_files_legacy()' {} +
buildifier --lint=fix --mode=fix -r .
This had the effect of making sure we can export all of our
files in G3 (until we no longer have legacy targets) and
making all of our cc_libraries shim-able.
bazel/macros.bzl has the human-contributed changes, the rest
were mechanical.
Change-Id: I8e24e30e74b038cfd072cdbe4078bfd1d213dd46
Bug: skia:13211
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/535359
Reviewed-by: Ben Wagner <bungeman@google.com>
* Use SkSTArray instead of stack/deque for fTextStyles
* Use fParagraphStyle for the base style instead of pushing an extra copy
into fTextStyles
* Call endRunIfNeeded only when starting a new styled block
Change-Id: I5f80c052a7589d8da7e6a797b155884efd5449a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533983
Commit-Queue: Jason Simmons <jsimmons@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Reviewed-by: Julia Lavrova <jlavrova@google.com>
An empty viewPort should always send everything to a point. An empty
viewBox should either draw nothing or fill the viewPort with the color
of a single point of no area.
Change-Id: I0d4f0cea3918acc4edc98043f3a7155e4e171bff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533809
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
The Gazelle extension does not verify the hdr/src name (yet).
Bug: skia:12541
Change-Id: Ibd9fd4663d643e864ccc6977bd3c928f722d6c3a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/533804
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
This removes the required dependency on our JSON code. In the Bazel
rules, this dependency is pushed down into sksl instead of required
by the cc_binary rules.
It adds a stub version of SkVMDebugTrace.cpp and removes
SkVMDebugTracePlayer unless the appropriate GN or Bazel flag
is set (skia_enable_sksl_tracing and enable_sksl_tracing,
respectively). There was an existing #define that CanvasKit
used (CK_INCLUDE_SKSL_TRACE) and this was changed to
SKSL_ENABLE_TRACING.
Users of //:skia_core no longer need to specify a JSON dep,
if sksl needs it (e.g. for tracing), then it will specify
the dependency.
This is a reland of https://skia-review.googlesource.com/c/skia/+/528837
Bug: skia:12541
Change-Id: I79612c69fdbefd3db9822a2b66df7552f7c13865
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529278
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit 6bc4bdf645.
Reason for revert: Android roll
Original change's description:
> [sksl] Make sksl tracing optional
>
> This removes the required dependency on our JSON code. In the Bazel
> rules, this dependency is pushed down into sksl instead of required
> by the cc_binary rules.
>
> It adds a stub version of SkVMDebugTrace.cpp and removes
> SkVMDebugTracePlayer unless the appropriate GN or Bazel flag
> is set (skia_enable_sksl_tracing and enable_sksl_tracing,
> respectively). There was an existing #define that CanvasKit
> used (CK_INCLUDE_SKSL_TRACE) and this was changed to
> SKSL_ENABLE_TRACING.
>
> Users of //:skia_core no longer need to specify a JSON dep,
> if sksl needs it (e.g. for tracing), then it will specify
> the dependency.
>
> Change-Id: I2fcd29cde118fc391c269ba2d8f8a40a6f164c99
> Bug: skia:12541
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528837
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>
Bug: skia:12541
Change-Id: Icf75495f19e409d96925ca4dca9e839eca4057ec
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/529129
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
This removes the required dependency on our JSON code. In the Bazel
rules, this dependency is pushed down into sksl instead of required
by the cc_binary rules.
It adds a stub version of SkVMDebugTrace.cpp and removes
SkVMDebugTracePlayer unless the appropriate GN or Bazel flag
is set (skia_enable_sksl_tracing and enable_sksl_tracing,
respectively). There was an existing #define that CanvasKit
used (CK_INCLUDE_SKSL_TRACE) and this was changed to
SKSL_ENABLE_TRACING.
Users of //:skia_core no longer need to specify a JSON dep,
if sksl needs it (e.g. for tracing), then it will specify
the dependency.
Change-Id: I2fcd29cde118fc391c269ba2d8f8a40a6f164c99
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528837
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Change-Id: I238d29ba0250224fa593845ae65192653f58faff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528156
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Change-Id: I337aebf72715339ec0280bab0916968ccbe7f481
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528516
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This also adds a make target to build a release version of
the debugger, which is handy when we want to push a fix
quickly. The make targets will also create the local_build
folder in the infra repo if it does not exist.
Change-Id: I073615a35068c425608454311a51c0c4e0a8152a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527181
Reviewed-by: Brian Osman <brianosman@google.com>
AE alignment semantics:
- leading white-space counts (i.e. "pushes" the line to the right
in left/center alignment mode)
- trailing white-space is ignored (does not push the line to the left
in center/right alignment mode)
Skottie currently always takes white-space into account. This yields
incorrect center/right alignment not just in the presence of explicit
trailing WS, but also due to residual WS from paragraph line breaks.
To fix, detect trailing WS for each committed line, and adjust glyph
positions to compensate.
Change-Id: Id8589cb30f743f21d77961c0eb4fa20a97e5f13c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526457
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Change-Id: Ia12c07752a9d31dfbf846f2cd2bb63d47532d6f1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526677
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: Ben Wagner <bungeman@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
That would allow us not to query glyph bounds
Change-Id: Ibee53446e33fd6a54affc1ded438191186cd21be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526296
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Pardon the duplicate test file, I have not yet finished
moving the CanvasKit tests to be on Bazel. When I do,
the duplicate (font.spec.js) will go away.
Change-Id: I6ad468f3f322280ffa25429fb8732e7266703e91
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526297
Reviewed-by: Ben Wagner <bungeman@google.com>
So we don't have to drag these shifts through the entire process.
This is the first step for simplified layout optimization.
(Need letter spacing because it's common in iOS Flutter)
Change-Id: I28b066e4dd8bcc6e489752dafbda1b73b0444fbe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524223
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
The hb_font will hold a reference to an hb_face created for the typeface
along with other attributes associated with the SkTypeface
(in particular, the variation design position)
See https://github.com/flutter/flutter/issues/100523
Change-Id: I5e211d670996f8f36e0d1027006c7bb67a9b8d2a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524801
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Update our utils to fix a couple of nested animation issues:
1) Use RenderFlag::kSkipTopLevelIsolation to prevent unconditional
nested animation layers. This matches default AE semantics.
2) Use the main animation ResourceProvider when loading nested
animations (otherwise any nested resources are ignored).
Change-Id: Ib489549066c9e42a96e5113fc817278d9ed06d59
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524636
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Most of these are pretty mechanical generated changes.
IWYU noticed one issue with DSLCore.h, which was fixed here.
Change-Id: I5629565ad3c2817daa71907c62f932d93f9d78ab
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524617
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
For entirely RTL text the ellipsis expected to be on the left side.
Bug: skia:13069
Change-Id: I853687e1e741dd57af950f17717fa8553648b477
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523856
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
Bug: skia:12974
Change-Id: I15b090e2c3346d71ccf45d5f0d306da3f079821e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523996
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@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>
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 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 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>
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>
This relands commit 4d6d9e3f89.
Original change's description:
> [skottie] Max lines text auto-sizing constraint
>
> Introduce a new text property ("xl"), to limit the number of lines when
> auto-sizing.
>
> This is a Skottie extension, pending UI/controls in Bodymovin.
>
> Change-Id: Id0f1e633e1b324a97b227d6b187cd540990796a7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518498
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
> Commit-Queue: Florin Malita <fmalita@google.com>
Change-Id: Ib9d28366332866d8b787f89fa1dc13132c02b435
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518699
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
