Commit Graph

577 Commits

Author SHA1 Message Date
Florin Malita
ad324a31e6 [skottie] Remove legacy whitespace guard
No longer used.

Change-Id: Id68a8d4340bc685c938ab22619f2b4067b3f9206
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/528163
Commit-Queue: Florin Malita <fmalita@chromium.org>
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-04-07 15:06:48 +00:00
Florin Malita
de6af36e6e [skottie] Fix white-space/horizontal-alignment interactions
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>
2022-04-01 18:54:14 +00:00
Florin Malita
9b8fcc242f [skottie] Nested animation fixes
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>
2022-03-28 18:17:09 +00:00
Kevin Lubick
08ece0c9a0 [includes] Enforce IWYU on sksl code
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>
2022-03-21 12:43:02 +00:00
Florin Malita
8d29ce3305 [skottie] Remove tracking fix guards
Change-Id: I83becfa4ac05a35cfe47305f1fdc852750135f11
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521359
Commit-Queue: Florin Malita <fmalita@chromium.org>
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-03-16 14:43:11 +00:00
Kevin Lubick
e253cc3e55 [bazel] Use toolchain features to opt files into being IWYU compliant.
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>
2022-03-16 13:09:46 +00:00
Florin Malita
38b9591b5a [skottie] Add support for text stroke line join
New 'lj' text property to match BM values:

  1: miter
  2: round
  3: bevel

Change-Id: I3997c6b8c702a3f80da1cbee6cb950eca775bc77
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517896
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-03-11 15:17:16 +00:00
Florin Malita
1df655a427 Reland "[skottie] Fix text-on-path tracking"
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>
2022-03-11 13:09:17 +00:00
Florin Malita
5fe4b6faeb 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>
2022-03-10 20:33:09 +00:00
Florin Malita
ca973cbea0 [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>
2022-03-10 18:19:16 +00:00
Kevin Lubick
efce17de5d Reland "[includes] Remove link between SkImage.h and SkImageEncoder.h"
This is a reland of commit f60584eb0f

Client changes:
 - https://chromium-review.googlesource.com/c/chromium/src/+/3508565
 - http://cl/433225409
 - http://cl/433450799

Original change's description:
> [includes] Remove link between SkImage.h and SkImageEncoder.h
>
> According to go/chrome-includes [1], this will save about
> 210MB (0.09%) off the Chrome build. http://screen/GVdDaRRneTRuroL
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&included=&limit=1000
>
> Change-Id: If911ec283a9ce2b07c8509768a6a05446573a215
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512416
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>

Bug: 242216
Change-Id: Ic61e4ac2878e7a51f389312a3a434856e2e32be3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518277
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-10 04:47:51 +00:00
Florin Malita
762f8fbcad Reland "[skottie] Max lines text auto-sizing constraint"
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>
2022-03-10 02:22:09 +00:00
Florin Malita
4ea42a38eb Revert "[skottie] Max lines text auto-sizing constraint"
This reverts commit 4d6d9e3f89.

Reason for revert: g3 breakage

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: I8877234d4d267f553b0f07255b5db13179294b90
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518697
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>
2022-03-09 18:24:11 +00:00
Florin Malita
4d6d9e3f89 [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>
2022-03-09 16:39:54 +00:00
Kevin Lubick
a573cbab1a Revert "[includes] Remove link between SkImage.h and SkImageEncoder.h"
This reverts commit f60584eb0f.

Reason for revert: Chrome and G3 issues

Original change's description:
> [includes] Remove link between SkImage.h and SkImageEncoder.h
>
> According to go/chrome-includes [1], this will save about
> 210MB (0.09%) off the Chrome build. http://screen/GVdDaRRneTRuroL
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&included=&limit=1000
>
> Canary-Android-Topic: image-encoder-2
> Change-Id: If911ec283a9ce2b07c8509768a6a05446573a215
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512416
> Reviewed-by: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>

Bug: 242216
Change-Id: Idc906ff54d8baf49989d3ee24e045d7b0ca710df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517676
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Kevin Lubick <kjlubick@google.com>
2022-03-08 13:09:49 +00:00
Kevin Lubick
f60584eb0f [includes] Remove link between SkImage.h and SkImageEncoder.h
According to go/chrome-includes [1], this will save about
210MB (0.09%) off the Chrome build. http://screen/GVdDaRRneTRuroL

[1] https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImage%5C.h%24&included=&limit=1000

Canary-Android-Topic: image-encoder-2
Change-Id: If911ec283a9ce2b07c8509768a6a05446573a215
Bug: 242216
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512416
Reviewed-by: Leon Scroggins <scroggo@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-07 21:32:13 +00:00
Jorge Betancourt
c8e520e722 remove scaled stroke guards
should not be pushed until scuba is updated (cl/431716076)

Change-Id: Ic1b6b76eb0ebd1ab0c9d7875c7b5789af39e21c1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514763
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-03-03 20:41:29 +00:00
Jorge Betancourt
bf9bc6de39 [skottie] scale stroke on text layers after shaping
Change-Id: I657e1ad43dc440fe8b24f690674308ec9b312e9e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514956
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-03-03 16:18:16 +00:00
Jorge Betancourt
4bf57f8958 Revert "[skottie] scale stroke on dynamic text layers"
This reverts commit d5c24c72dc.

Reason for revert: Scuba test blocking g3

Original change's description:
> [skottie] scale stroke on dynamic text layers
>
> initial commit for scaled stroke on text in skottie
>
> Change-Id: Iedce77833e12b2b5d8bc27161392b87a428efe8e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513759
> Reviewed-by: Florin Malita <fmalita@google.com>
> Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>

Change-Id: Ia1dba3f206e5b12168e2e82ba608db943af26c78
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514219
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-03-01 18:12:24 +00:00
Jorge Betancourt
d5c24c72dc [skottie] scale stroke on dynamic text layers
initial commit for scaled stroke on text in skottie

Change-Id: Iedce77833e12b2b5d8bc27161392b87a428efe8e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513759
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-02-28 22:13:14 +00:00
Kevin Lubick
149938e8e3 [canvaskit] Rename some CK-specific defines
The one use of the define in //modules/skottie was in an old
G3 BUILD file which has since been deleted.

Change-Id: I3cbb0dd2bcbff7de433b2d044b3e7a0c34a45240
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/509400
Reviewed-by: Nathaniel Nifong <nifong@google.com>
2022-02-15 22:42:17 +00:00
Brian Osman
cd189e8a1d Remove default/deprecated arguments to makeShader calls
Bug: skia:12643
Change-Id: I37e1718a20283dfb814c85260257d57bac2b7b34
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506211
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2022-02-09 20:41:23 +00:00
Florin Malita
e5d7a8ec89 Revert "[skottie] Visual-only text valign"
This reverts commit dcafc5d2bc.

Reason for revert: too disruptive for existing g3 asssets

Original change's description:
> [skottie] Visual-only text valign
>
> Historically, Skottie started with vertical alignment based on the
> typographic bounding box.  This was meant to account for empty
> leading/trailing lines.
>
> At some point [1], the strategy was changed to also take the visual
> bounding box into account (union of typographic and visual bounds).
>
> It turns out this is still suboptimal: aligning based on font metrics
> yields poor results in practice, and pretty much everyone expects
> visual-only alignment.
>
> This CL is an attempt to fix things:
>
> 1) update kVisualTop/kVisualCenter/kVisualBottom to use visual bounds
>    only (as their name implies)
> 2) introduce kDeprecatedVisualCenter to preserves the old behavior
>    for compatibility, and use it for the legacy sk_vj flags
>
> The latter is done to minimize disruption for clients which have
> adjusted for the current misalignment: luckily they're mostly using the
> old sk_vj flag instead of explicit resize/valign policies, and they can
> continue to do so without change, while new clients can opt into the
> new/improved valign modes.
>
> The change is guarded by a build flag for g3 staging.
>
> [1] https://skia-review.googlesource.com/c/skia/+/224188
>
> Change-Id: I334c1713ce32635e3649711f072a3dcdf6b12244
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501016
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
> Commit-Queue: Florin Malita <fmalita@google.com>

Change-Id: I633dd54cd04727617e845d24a35e5e8cef64f861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506177
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-02-09 14:42:14 +00:00
Florin Malita
dcafc5d2bc [skottie] Visual-only text valign
Historically, Skottie started with vertical alignment based on the
typographic bounding box.  This was meant to account for empty
leading/trailing lines.

At some point [1], the strategy was changed to also take the visual
bounding box into account (union of typographic and visual bounds).

It turns out this is still suboptimal: aligning based on font metrics
yields poor results in practice, and pretty much everyone expects
visual-only alignment.

This CL is an attempt to fix things:

1) update kVisualTop/kVisualCenter/kVisualBottom to use visual bounds
   only (as their name implies)
2) introduce kDeprecatedVisualCenter to preserves the old behavior
   for compatibility, and use it for the legacy sk_vj flags

The latter is done to minimize disruption for clients which have
adjusted for the current misalignment: luckily they're mostly using the
old sk_vj flag instead of explicit resize/valign policies, and they can
continue to do so without change, while new clients can opt into the
new/improved valign modes.

The change is guarded by a build flag for g3 staging.

[1] https://skia-review.googlesource.com/c/skia/+/224188

Change-Id: I334c1713ce32635e3649711f072a3dcdf6b12244
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501016
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-02-07 17:04:17 +00:00
John Stiles
87aa7a9095 Add SK_PRINTF_LIKE to existing variadic print functions.
This shook out a handful of formatting issues:
[SkVMVisualizer]
- We were passing plain text like "width:35%;" through printf.
- One particular opcode type was printing a string as a number.
[Skottie, SortToy]
- Used wrong integer type instead of %zu for size_t

This CL does not update print functions which take printf arguments via
variadic template, as __attribute__((format)) does not support this
style. These could be converted to va_list style, but that's not done in
this CL.

(For some reason, GCC requires the attribute to be set on a prototype
for freestanding functions, so a few of these now have a prototype
immediately followed by a declaration.)

Change-Id: I63a6c2486c785cc38563028fdf8df0662ec04935
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504698
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-02-07 14:55:39 +00:00
John Stiles
4250effcc5 Prepare to reenable unreachable-code warnings.
Adding double-parens around an `if ((false))` squelches the warning.
In other cases, you can squelch the warning by assigning the
always-constant(-on-this-machine) check into a constexpr bool.

Change-Id: I5a344fb45779c5bd2865edb3cffaf839ba9a5d85
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504597
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-02-04 21:45:39 +00:00
Kevin Lubick
e94b52c442 [canvaskit] Add skottie to Bazel build
PS 1 regenerates existing Bazel files
PS 2 adds generated Bazel files to skottie and its dependencies,
as well as incorporating it into CanvasKit.

This changes the version of Bazel we use to 5.0.0 (recently
released).We had been using a pre-release of 6.0 because we
wanted the new features in one of the 5.0 release candidates,
but not the regression that was there (and reverted before the
full 5.0 release). I'd like to stick to the latest stable Bazel
release where possible.

Suggested Review Order:
 - //modules/skottie/BUILD.bazel (this was hand written
   to encapsulate the skottie library). The files in the
   deps are based on skottie.gni.
 - //modules/skresources/BUILD.bazel and //modules/sksg/BUILD.bazel
   which expose all sources
 - //third_party/file_map_for_bazel.json which ignores the
   ffmpeg libraries (we won't actually build the SkVideoDecoder
   stuff because HAVE_VIDEO_DECODER is not set).
 - //modules/canvaskit/BUILD.bazel which makes use of the skottie
   library and includes the interface skottie.js file.
 - .bazelversion which changes the Bazel version used (e.g. by
   Bazelisk).
 - All other changes should be auto-generated or related to
   deleted files.

Change-Id: Ic26f9a9dea5310f2cbd9cda7d701847924a39a22
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/503828
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
2022-02-04 19:18:27 +00:00
Florin Malita
e3ecc34f01 [skottie] Fix text error logging
Don't clear the logger until we actually see some errors.

Change-Id: I275e99a6f2748dff1456a471daabb142fd1a10f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/500996
Auto-Submit: Florin Malita <fmalita@chromium.org>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2022-01-27 16:06:41 +00:00
Florin Malita
1a9e7531e0 [skottie] Fix text opacity animator semantics
The current implementation uses multiplicative composition for opacity
animators (modulate_opacity always scales the new opacity by the old
value).  That means that if one animator drops opacity all the way to
zero, there is no way for subsequent animators to increase opacity.

Instead, AE seems to use the same interpolation as for colors
(prev value/animator value, based on modulation param).

Update to use similar interpolation for opacity properties, and also
to only apply when opacity props are actually specified for a given
animator.

Change-Id: I5a96f9e3722399c8ec661a7843c86dfa60eac5ca
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/499376
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-01-25 17:58:15 +00:00
Brian Osman
a1fd1c189c Mark operator bool() explicit in src, tests, and modules
Change-Id: Ic664ad0134d61dcf939dcf585a81d53e29c6afcc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/496597
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2022-01-19 15:10:45 +00:00
Florin Malita
3e1354a592 [skottie] Fix assert for missing layer type
When the layer type is missing, fType == -1 and we rely on unsigned
(size_t) underflow + check against the known types array size to catch
the condition.

The problem is SkToSizeT() itself asserts the input is a valid size_t,
and even if it didn't clusterfuzz would likely complain about
underflowing.

Refactor to check for negative values explicitly.

Bug: b/200660146
Change-Id: Iae74dca14ac0202ffcdd4449f0d470063916eff5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493116
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-01-10 18:52:39 +00:00
Florin Malita
dd575bc0f1 [skottie] Fix float->int overflow
We do test for and catch int overflows, but after casting -- which is
enough to trip clusterfuzz.

Refactor to catch upfront.

Bug: b/200722666
Bug: b/200663331
Bug: b/200662108
Change-Id: I26da4b1ca21ad0bc34384e957a30211402219841
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/493018
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2022-01-10 15:46:52 +00:00
Jorge Betancourt
68e240d9cd add sharpen effect support to skottie player
Change-Id: I3242897c00b3cbe84c5ec964831df0fd4ae45622
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/481556
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2021-12-14 19:44:26 +00:00
Jorge Betancourt
27db7e6e2a move duplicate code to common SkSLEffectBase class
Change-Id: I9f4100bdb09a2fc0e940c007bf5cca0edec24e7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/480429
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2021-12-09 19:45:35 +00:00
Florin Malita
15949beef2 [skottie] Fix mask feather tiling
We currently default to kClamp, but for mask feathers we want kDecal.

Bug: skia:12676
Change-Id: I7e3e21c310823987819e7374b494de4a48d9cae6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/477297
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
2021-11-29 22:50:06 +00:00
Jorge Betancourt
1bbc6e5cef add plumbing for color filter SkSL effect
Change-Id: I26b28ff4756cda921e4acef32f3da3b43b1fc28f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/472936
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
2021-11-23 16:30:40 +00:00
Jorge Betancourt
3868636fb8 discard SkRuntimeEffect builder in SkSLEffect to use lower level API
Change-Id: I20bb5e6463e79bcdd56fd614d9936fdaf65aefcc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/471776
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-11-17 15:51:38 +00:00
Jorge Betancourt
f2e90d3005 add skottie support for Bulge ADBE effect
This effect does not yet support pinned edges or taper

also sneak in a filename change (CCToner) to keep all effect names standard

Change-Id: I17f2b5463408556775bc12a972358abd4d8d8690
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467319
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-11-04 21:05:22 +00:00
Avery Musbach
a05d3029ac Fix skottie::PropertyHandle export
Bug: chromium:1266877
Change-Id: I6000d0dd8c9214aa37c1956834d6a14db151c929
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467956
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-11-04 19:00:27 +00:00
Florin Malita
ba35f687c3 [skottie] Clean up FractalNoise and SkSLEffect
Pass inval controller and ctm to child nodes during revalidation.

Change-Id: Iee6862af78c77d5164e29b0e41ed6dae9a7f4235
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/466760
Auto-Submit: Florin Malita <fmalita@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-11-02 13:27:15 +00:00
Jorge Betancourt
172c7998e1 plumb experimental SkSL layer effect in native Skottie player
Long term plan is to expose a plugin (standalone or with bodymovin) that allows motion artists to write sksl into a composition.
This is the first step where we test how we'd read in the json data under the hood.

Change-Id: I300d3af5d01e12f5b495970f89fd12b5f464a9a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/464368
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-11-01 18:05:15 +00:00
Jorge Betancourt
aaa70658c2 expose directional blur to skottie
Change-Id: I759e4fff7a6d9cd1aae6ece060d570d05c1af94a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461236
Commit-Queue: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-10-20 19:41:30 +00:00
Florin Malita
b3460f9979 [skottie] Floatify Fractal Noise
Halfs are lacking sufficient precision for this effect.

Change-Id: If3c2cac34d465d74b7ad8228417c42950f48adfd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/461177
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-10-19 19:20:08 +00:00
Florin Malita
decc9ae2fc [skottie] Fractal Noise optimization
Hoist all evolution-related calculations out of the shader, and pass
precomputed noise planes and weight uniforms.

Renders ~20% faster locally (w/ software rasterization).

Change-Id: I771bea8f3425440515d2cb9a8623336d18bcc7b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460336
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-10-18 16:14:54 +00:00
Florin Malita
b6a3aa7eb5 [skottie] Fractal Noise: cycle evolution support
AE allows for optional cycling of evolution, after a certain number of
revolutions.

To support:

  - split off the base/offset component into a separate uniform
    (currently front-loaded into evolution)
  - introduce an additional "cycle" (period) uniform to mod() the noise
    plane calculations


Change-Id: Ib412027114c467934c549cc1438a7d4560aa14bc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/460116
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-10-18 15:11:06 +00:00
Nico Weber
39edd521a2 Fix Wbitwise-instead-of-logical warnings
`a || b` only evaluates b if a is false. `a | b` always evaluates
both a and b. If a and b are of type bool, `||` is usually what you
want, so clang now warns on `|` where both arguments are of type bool.

In Skia, 3 of 3 uses of `|` were intentional as far as I can tell (one
had an explicit comment, the other two didn't). Rewrite them slightly
to make this intent more clear and to suppress the warning.

There was also one use of `&`. That one looks like a (benign) typo for
`&&`, so change it.

Bug: chromium:1255745
Change-Id: I9ac37075311005c0a8fcb8d1379f516510929423
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/459456
Commit-Queue: Nico Weber <thakis@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Auto-Submit: Nico Weber <thakis@chromium.org>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-10-14 14:50:09 +00:00
Florin Malita
ad6f2d3e66 [skottie] Log text layout errors
If text layout fails (e.g. due to unsatisfiable scaling constraints),
log an error to notify clients.

Change-Id: Ic7a028196b3bfb8f45b91c88766f0059145a202f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458722
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-10-14 14:29:11 +00:00
Eric Sum
734d7e2be4 Export some classes callers may use for Skottie.
1) CachingResourceProvider - The immediate user of this is ChromeOS,
but it seems like a generally useful ResourceProvider implementation
that all can use.

2) Animation::Builder - See this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/3171517
It seems that windows builds fail with "undefined reference" linker
errors when trying to use the Animation::Builder directly. My guess
is that this is because the SK_API macro is needed to export it.

Change-Id: Ief39fe6ec03f992a0be73e5be54b0119d2d82930
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/458407
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-10-13 01:35:54 +00:00
Brian Salomon
9fa47cc1c6 Make class members that are static constexpr also be inline.
This is in prep for compiling with -std=c++14 and -Wno-c++17-extensions
when building with clang. Chrome has encountered problems with
third_party headers that are included both in Skia and other Chrome
sources that produce different code based on whether preprocessor macros
indicate a C++14 or C++17 compilation.

In C++17 they are already inline implicitly. When compiling with C++14
we can get linker errors unless they're explicitly inlined or defined
outside the class. With -Wno-c++17-extensions we can explicitly inline
them in the C++14 build because the warning that would be generated
about using a C++17 language extension is suppressed.

We cannot do this in public headers because we support compiling with
C++14 without suppressing the C++17 language extension warnings.

Bug: chromium:1257145
Change-Id: Iaf5f4c62a398f98dd4ca9b7dfb86f2d5cab21d66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457498
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2021-10-11 16:22:59 +00:00
Florin Malita
132d47c90d [skottie] Path support for paragraph text
In addition to single line (point) text, AE also supports path layout
for paragraph text.

At a high level, the paragraph box top is mapped to the path (following
alignment rules), and each glyph is displaced along its path positioning
vector, post orientation.

The main difference compared to point text, is that the distance on path
is based on the fragment position relative to the paragraph left edge.

The paragraph box also plays a role in alignment: left/center/right
aligns with path start/mid/end.

This includes a tangential optimization: instead of validating cached
contour data in each PathInfo::getMatrix() call, we only check once at
a higher level (onSync) -- to avoid performing a shape vector comparison
for each fragment.

Change-Id: I2c31ce3b0a525a3cd2d4525abcf88d5fc943bb6e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457656
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
2021-10-11 15:26:00 +00:00