Commit Graph

129 Commits

Author SHA1 Message Date
Kevin Lubick
5e8f45faf1 [includes] Prepare for moving SkColorSpace to forward declare
This updates all our callsites in preparation for removing
the #include "include/core/SkColorSpace.h" from SkImageInfo.h

According to go/chrome-includes [1], this will save ~150MB
(0.07%) from the compilation size. I think SkColorSpace is
a big include because it loads the skcms header, which is
big.

The follow-on CL will remove that link, once clients have
been updated as well.

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

Change-Id: I1b5ff491ac495317b0e5af3a2082b080d43697ae
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525639
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-31 19:50:10 +00:00
Kevin Lubick
7ac7413f08 [bazel] Support Vulkan
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>
2022-03-08 21:21:17 +00:00
Kevin Lubick
2d86f6f5db Reland "[includes] Remove include link between SkPathRef.h and SkRRect.h"
This is a reland of 37b8239bec

Upstack changes:
 - http://ag/17004908
 - https://github.com/flutter/engine/pull/31654

Original change's description:
> [includes] Remove include link between SkPathRef.h and SkRRect.h
>
> According to go/chrome-includes [1], this will save about
> 40MB (0.02%) off the Chrome build. http://screen/4GnPjFaYpwCVHVL
>
> I'm not quite sure why the link is so expensive, but we can
> forward declare it and move the implementation from the .h
> to the .cpp file easily enough.
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fprivate%2FSkPathRef%5C.h%24&sort=includes&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fprivate%2FSkPathRef%5C.h%24&included=&limit=1000
>
> Canary-Chromium-CL: 3485346
> Change-Id: Ie3a5e7a735426f883a6c06ddbd3b8bf148bf1709
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512158
> Reviewed-by: Robert Phillips <robertphillips@google.com>

Bug: 242216
Change-Id: I31288895528251fd59913427b95b9f88a871d14b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512776
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-07 15:15:05 +00:00
Kevin Lubick
32b1e5d861 Revert "[includes] Remove include link between SkPathRef.h and SkRRect.h"
This reverts commit 37b8239bec.

Reason for revert: Need to update clients, including Flutter

Original change's description:
> [includes] Remove include link between SkPathRef.h and SkRRect.h
>
> According to go/chrome-includes [1], this will save about
> 40MB (0.02%) off the Chrome build. http://screen/4GnPjFaYpwCVHVL
>
> I'm not quite sure why the link is so expensive, but we can
> forward declare it and move the implementation from the .h
> to the .cpp file easily enough.
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/include-analysis.html#view=edges&filter=%5Ethird_party%2Fskia%2Finclude%2Fprivate%2FSkPathRef%5C.h%24&sort=includes&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fprivate%2FSkPathRef%5C.h%24&included=&limit=1000
>
> Canary-Chromium-CL: 3485346
> Change-Id: Ie3a5e7a735426f883a6c06ddbd3b8bf148bf1709
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512158
> Reviewed-by: Robert Phillips <robertphillips@google.com>

Bug: 242216
Change-Id: I226c45326b67dff3eb1dc1f96c11e9ef0a12e22b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512639
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-02-24 12:55:26 +00:00
Kevin Lubick
37b8239bec [includes] Remove include link between SkPathRef.h and SkRRect.h
According to go/chrome-includes [1], this will save about
40MB (0.02%) off the Chrome build. http://screen/4GnPjFaYpwCVHVL

I'm not quite sure why the link is so expensive, but we can
forward declare it and move the implementation from the .h
to the .cpp file easily enough.

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

Canary-Chromium-CL: 3485346
Change-Id: Ie3a5e7a735426f883a6c06ddbd3b8bf148bf1709
Bug: 242216
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512158
Reviewed-by: Robert Phillips <robertphillips@google.com>
2022-02-23 22:46:44 +00:00
Brian Osman
0769d45c68 Implement SkTLazy using std::optional
Change-Id: I3d3f175892d23ff5db2c14600c5168273fca8efb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501840
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-02-01 19:51:21 +00:00
John Stiles
f07421bcbd Remove unnecessary ctors now that we have C++17 copy-elision.
This CL backs out C++14 compatibility changes from
http://review.skia.org/457298 :

"2. Lack of C++17 copy elision means classes of objects constructed at
function return need a copy or move constructor even if RVO will mean it
isn't called."

Change-Id: I33a833d33b221e757bf6a6459835a7215b4e6b66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501240
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-01-27 20:26:54 +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
Kevin Lubick
6ba9f702ba [bazel] Try adding cc_binary rules that use generated rules
To make the atomic rules a bit easier to work with, in many
of the folders, this adds in cc_library rules to group
together the sources from that folder (and subfolders
where prudent). We only needs sources because those atoms
should have their headers as deps.

One issue that was pointed out is that there is currently
no way to restrict the inclusion of certain packages,
a la, `gn check`. For example, there is no mechanism from
stopping a dev from adding
#include "modules/canvaskit/WasmCommon.h"
to something in //src/core (except circular dependencies).
We can probably address that using Bazel's visibility
rules as needed:
https://docs.bazel.build/versions/main/visibility.html
https://docs.bazel.build/versions/main/be/functions.html#package_group

It is recommended to look at this CL patchset by patchset.
PS1: Update gazelle command to generate rules in more folders.
PS2: A few changes to make generation work better.
PS3: The result of running make generate in //bazel
PS4: Adding the rules to build sksllex, the simplest binary I
     could find in the Skia repo.
PS5: Adding the rules to build skdiff, a more complex binary.
     I tried a few approaches, but ended up gravitating back
     towards the layout where we have each folder/package
     group up the sources. I imagine at some point, we'll have
     skdiff depend on skia_core or something, which will
     have things like //src/core, //src/codecs, //src/pathops
     all bundled together.
PS7: Added in the groupings of sources, similar to what we had
     earlier. I liked these for readability. These helped fix
     up the //:skia_core build, and by extension, the CanvasKit
     build.

Change-Id: I3faa7c4e821c876b243617aacf0246efa524cbde
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/476219
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
2021-11-30 18:18:53 +00:00
John Stiles
9ec61da31b Fix Clang warning -Wbitwise-instead-of-logical.
warning: use of bitwise '|' with boolean operands [-Wbitwise-instead-of-logical]
    inherit_if_needed(currentNode->fX               , attrs->fX)      |
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  note: cast one or both operands to int to silence this warning

Change-Id: I6d59a5c33826af29560c0e30ddf9f4e16581882c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/463896
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-10-26 21:35:37 +00:00
Brian Salomon
87a0078b89 Support building on clang with -std=c++14 but no C++17 extension warning.
Classes of issues addressed:

1. static constexpr class variables aren't automatically inline. Already handled in a separate CL

2. Lack of C++17 copy elision means classes of objects constructed at function return need a copy or move constructor even if RVO will mean it isn't called.

3. Nested braced init no longer allowed for base classes of subclasses without constructors.

4. template static constexpr var in template class throws error about redundant initialization. Adding inline and removing defn outside of class fixes it.

5. Some places that should have been including std headers now actually need to include them.

6. No auto template parameters.

7. No lambdas in constexpr funcs.


Bug: chromium:1257145
Change-Id: Icb24c6b4ed039287fb4cf27a21a1bb7dc9821728
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/457298
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2021-10-12 16:34:30 +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
Robert Phillips
2af13c135b Reland "Fix compilation w/ "skia_enable_svg = false" (take 2)"
This reverts commit 44b7568c8a.

Reason for revert: Google 3 CL has landed

Original change's description:
> Revert "Fix compilation w/ "skia_enable_svg = false" (take 2)"
>
> This reverts commit 30a6b101f4.
>
> Reason for revert: Maybe blocking G3 roll?
>
> Original change's description:
> > Fix compilation w/ "skia_enable_svg = false" (take 2)
> >
> > Change-Id: I036ae171809af56cc9594704b44705ebd095ec80
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443898
> > Reviewed-by: Florin Malita <fmalita@chromium.org>
> > Commit-Queue: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,fmalita@chromium.org,fmalita@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com
>
> Change-Id: Ibee3819e073b04efdf9736058c1f9b288249620c
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444216
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

# Not skipping CQ checks because this is a reland.

Change-Id: Idf73d864108067ee1c34e88ee4e5236847abd582
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444501
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-09-01 17:54:08 +00:00
Robert Phillips
44b7568c8a Revert "Fix compilation w/ "skia_enable_svg = false" (take 2)"
This reverts commit 30a6b101f4.

Reason for revert: Maybe blocking G3 roll?

Original change's description:
> Fix compilation w/ "skia_enable_svg = false" (take 2)
>
> Change-Id: I036ae171809af56cc9594704b44705ebd095ec80
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443898
> Reviewed-by: Florin Malita <fmalita@chromium.org>
> Commit-Queue: Robert Phillips <robertphillips@google.com>

TBR=robertphillips@google.com,fmalita@chromium.org,fmalita@google.com,skcq-be@skia-corp.google.com.iam.gserviceaccount.com

Change-Id: Ibee3819e073b04efdf9736058c1f9b288249620c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/444216
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-31 22:31:00 +00:00
Robert Phillips
30a6b101f4 Fix compilation w/ "skia_enable_svg = false" (take 2)
Change-Id: I036ae171809af56cc9594704b44705ebd095ec80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443898
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-08-31 18:28:25 +00:00
John Stiles
b65b4da554 Fix cases of variable shadowing in /modules/.
If we manage to fix all the existing cases of variable shadowing, we
could enable -Wshadow.

Change-Id: Id56c227be7c29c56d6e9253aefcd33607bb1129e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/438576
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
2021-08-11 17:14:42 +00:00
Mike Reed
1f261da41e Stop using filterquality
Change-Id: I0fd8ace529cde3b36df72dd4d80bba18c1388b20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/429497
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2021-07-18 15:48:36 +00:00
John Stiles
7bf799956d Reland "Add format-specifier warnings to SkDebugf."
This is a reland of e58831cd95

Original change's description:
> Add format-specifier warnings to SkDebugf.
>
> This CL fixes up many existing format-specifier violations in Skia.
> Note that GCC has a warning for formatting nothing, so existing calls to
> `SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
> These were apparently meant to be used as a place to set a breakpoint.
>
> Some of our clients also use SkDebug with bad format specifiers, so this
> check is currently only enabled when SKIA_IMPLEMENTATION is true.
>
> Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
> Bug: skia:12143
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

Bug: skia:12143
Change-Id: Id3c0c21436ebd13899908d5ed5d44c42a0e23921
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421918
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-06-25 17:57:43 +00:00
Tyler Denniston
283dba5785 Revert "Add format-specifier warnings to SkDebugf."
This reverts commit e58831cd95.

Reason for revert: looks like breaking a few build bots

Original change's description:
> Add format-specifier warnings to SkDebugf.
>
> This CL fixes up many existing format-specifier violations in Skia.
> Note that GCC has a warning for formatting nothing, so existing calls to
> `SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
> These were apparently meant to be used as a place to set a breakpoint.
>
> Some of our clients also use SkDebug with bad format specifiers, so this
> check is currently only enabled when SKIA_IMPLEMENTATION is true.
>
> Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
> Bug: skia:12143
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I07848c1bf8992925c9498e916744d0840355a077
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:12143
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421917
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-06-25 13:33:10 +00:00
John Stiles
e58831cd95 Add format-specifier warnings to SkDebugf.
This CL fixes up many existing format-specifier violations in Skia.
Note that GCC has a warning for formatting nothing, so existing calls to
`SkDebugf("")` have been removed, or replaced with `SkDebugf("%s", "")`.
These were apparently meant to be used as a place to set a breakpoint.

Some of our clients also use SkDebug with bad format specifiers, so this
check is currently only enabled when SKIA_IMPLEMENTATION is true.

Change-Id: I8177a1298a624c6936adc24e0d8f481362a356d0
Bug: skia:12143
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/420902
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-06-25 11:05:59 +00:00
Florin Malita
bbaf630c76 [svg] Document container size semantics
Also add a helper for accessing the root SVG element:
SkSVGDOM::getRoot().

Bug: skia:11144
Change-Id: Icc7af8f0521203de26836e56dee8c503a56997c0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/415746
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
2021-06-04 19:24:37 +00:00
Tyler Denniston
95b2cf912c [svg] Implement feImage
Also cleaned up the IsFilterEffect() predicate a bit.

Bug: skia:10841
Change-Id: I8ef827df42d5d60cf06e74870191de7f27612376
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404218
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-05-05 16:48:03 +00:00
Tyler Denniston
080001cbf1 [svg] Fix nullptr deref for none input paints
This handles the case where a filter effect input is StrokePaint or
FillPaint, but the referencing element has a 'none' paint specified
(explicitly or via parsing error, as in the original bug report).

Bug: skia:11213
Change-Id: Ie710a9fc5ccb3c24014735e01c70a0ba42c7a99d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404696
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-05-05 16:37:53 +00:00
Tyler Denniston
2992fb0d60 [svg] Refactor SkSVGImage to expose image loading
This is in preparation for reusing the image loading + aspect ratio
mapping for <feImage>.

Change-Id: I53dcf904476aae386f672488379201524855a703
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404217
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-05-04 19:46:45 +00:00
Tyler Denniston
86f31f287e [svg] Fix filter effect subregion calculation
Somewhat subtle, but we were improperly handling the override of the
default subregion via the 'width' and 'height' <fe*> attributes.

Simple example: when specifying 'x' but not 'width', previously we
would use the overridden x (subregion.fLeft = boundaries.fLeft;), but
were not updating subregion.fRight based on the new fLeft value.

Bug: skia:10841
Change-Id: I309de4f0cf50a413ea6a0e7605926a3eb6cd94e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/404205
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-05-04 19:45:55 +00:00
Tyler Denniston
80ba617cab [svg] Implement 'display:none'
The full 'display' presentation attribute [1] has a number of settings
that we likely don't want to support. However, 'display:none' has some
practical uses and is easy enough to support. This progresses some
local tests as well as a few W3C tests.

[1] https://www.w3.org/TR/SVG11/painting.html#DisplayProperty

Bug: skia:10842
Change-Id: I60fef959fb45c3cfb229b85b720dfa69fc458bc3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403438
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-05-03 18:11:54 +00:00
Florin Malita
48fb05b81c [svg] Fix text object bounding box resolution
Two issues:

1) we are currently computing object bounding boxes using the
   RenderContext of the client resource, which can override presentation
   attributes

   -> introduce an OBBScope to capture not just the current OBB node but
      also its original RenderContext

2) text fragments must use the root <text> bounding box for OBB-relative
   units [1]

   -> update SkSVGTextFragment::renderText to not override the current
   OBBScope; this ensures we're using the root element scope for the
   whole text subtree


[1] https://www.w3.org/TR/SVG11/text.html#ObjectBoundingBoxUnitsTextObjects

Bug: skia:11936
Change-Id: Ib48ccd3dc22639de42ea150b881a26dda72fa4bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403037
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
2021-04-30 19:09:18 +00:00
Tyler Denniston
cdcd9a76bb [svg] Implement spot light sources
Note Chrome, Firefox and Inkscape (only other viewers I tested) disagree
in smallish ways with the W3C png for filters-light-01. We're somewhere
in the middle, but likely conforming enough to call it correct.

Bug: skia:10841
Change-Id: Ia83b9936d260780148fe326b6d3b2248eef69aac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/403036
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-04-30 18:40:58 +00:00
Florin Malita
3010f3d791 [svg] Refactor object bounding box access
Introduce a helper (SkSVGRenderContext::obbTransform) and refactor all
obb clients to funnel their calls through it.

This will facilitate some follow up obb changes.

Bug: skia:11936
Change-Id: If0d81b0fc9148389c2cb4bff597057a2afa2fb1b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402956
Reviewed-by: Tyler Denniston <tdenniston@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-04-30 15:41:38 +00:00
Tyler Denniston
8cfe718e0c [svg] Implement feDiffuseLighting
https://www.w3.org/TR/SVG11/filters.html#feDiffuseLightingElement

- Add SkSVGFeDiffuseLighting node
- Move distant light source direction computation into a method on
  SkSVGFeDistantLight
- Implement distant and point light sources for feDiffuseLighting

Bug: skia:10841
Change-Id: I74b8b9e04be5d2c5ac9f912d015dce96367040a1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402645
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-04-29 19:59:48 +00:00
Tyler Denniston
f208f81e1d [svg] Implement distant light sources for specular lighting
https://www.w3.org/TR/SVG11/filters.html#feDistantLightElement
(specular lighting only in this CL)

Tests filters-specular-01-f and filters-light-02-f should be passing
now. Note that our filters-light-02 result matches the behavior of
Chrome and Firefox, but is different than the official W3C png. We
currently think the test is somewhat poorly written, so we are
comfortable matching the output of Chrome and Firefox.

Bug: skia:10841
Change-Id: Id9517c8594a3826e5d98685d1e7d1d1b3d3cdf51
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402582
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-04-29 17:17:18 +00:00
Dominik Röttsches
343c15a8f7 Additional trace events for SVG parse and render operations
Helpful for analyzing SVG parsing vs. rendering time.

Bug: skia:11851
Change-Id: I64e7002de1846e9c426d59382611e6b0902ada09
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396776
Commit-Queue: Dominik Röttsches <drott@google.com>
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-04-14 14:25:28 +00:00
Florin Malita
c49c7f591b [svg] Fix handling of <rect> rx/ry
Both rx/ry are optional and clamped to half width/height, respectively:
https://www.w3.org/TR/SVG11/shapes.html#RectElementRXAttribute

Bug: skia:11802
Change-Id: I7b1af7042fdcaa70c0c6b4bfb15d1cb8848e251e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389957
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
2021-03-29 21:13:44 +00:00
Florin Malita
99438408b1 [svg] Add support for funcIRI fallback colors
Per spec [1], funcIRI paint servers can specify a fallback color to be
used when the IRI is invalid.

[1] https://www.w3.org/TR/SVG11/painting.html#SpecifyingPaint

Bug: skia:11802
Change-Id: I9711822f97ed949315d944aff4de2fda97eaa607
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389796
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
2021-03-29 15:45:02 +00:00
Ben Wagner
3d9ab7e7ec Separate general defaults from Skia defaults
In the build there are some defaults which actually apply to every
use of a built-in target type, but there are some (particularaly
warnings) which apply only to targets controlled by Skia. Currently
these unwanted defaults are magically known to exist and removed
wherever they are not wanted. Instead, create 'skia_' prefixed target
templates and apply these defaults to those instead.

Change-Id: I3a2afb53c7205a2e2748d1cfad46319f2e93d3b3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385516
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-03-22 15:30:56 +00:00
Mike Reed
10a5ff2cac Must pass filtering to picture shader
Change-Id: I820867df80daa1594d6202cad5e8e95c060293fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386838
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-03-18 22:18:47 +00:00
Tyler Denniston
c505e435d4 [svg] Refactor <path> to use new parsing
Change-Id: Ib33111e1b00f05d32f8c6add512c38b5a6197af8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367884
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-02-19 19:20:28 +00:00
Ben Wagner
bded42ab51 Split :tests into :test and :tests.
Split the :tests target into a :test target which contains the test
framework and :tests which collects all of the test cases. This allows
for all targets which define tests to depend on :test in order to define
tests, with :tests then depending on all targets which define tests. A
similar split should be considered for gms, samples, and benches.

Change-Id: Ic9f373ec0c1a8ea842fa68327e854db23477caae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371696
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
2021-02-18 17:01:23 +00:00
Tyler Denniston
627c6d2569 [svg] Refactor <pattern> and gradient stop elements to new parsing
Change-Id: I1fb84760f7a657aefeb7e0af3758766daac2d0f9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367882
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-02-08 20:27:17 +00:00
Tyler Denniston
c683482e58 [svg] Refactor <polygon> and <polyline> to use new parsing
Change-Id: I7cc2dcce4a645326dcacadd28cbe1b3ea5f4ae36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367877
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-02-08 20:27:17 +00:00
Tyler Denniston
52d9475b05 [svg] Refactor several simple nodes to new parsing code
Refactored nodes are <circle>, <ellipse>, <rect>, <line>.

Change-Id: I955c6a01d6533e61a0c12f56f97040ae0b5ed879
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366721
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-02-08 20:27:17 +00:00
Tyler Denniston
1f4cd07af8 [svg] Support preserveAspectRatio for images
https://www.w3.org/TR/SVG11/struct.html#ImageElement
https://www.w3.org/TR/SVG11/coords.html#PreserveAspectRatioAttribute

We already had a function to compute the appropriate matrix, and since
we can share the functionality with other elements that establish a new
viewport (including svg, symbol, and a few others), this CL moves the
function to the SVG node base class.

Relevant test for images is struct-image-06.

Bug: skia:10842
Change-Id: I5d6261210d03959e28d0bd7189da7f4ea53abc03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366398
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-02-05 15:59:10 +00:00
Tyler Denniston
8ca4626a4b [svg] Implement <image> element
https://www.w3.org/TR/SVG11/struct.html#ImageElement

Supported reference types are data and non-local URIs. Resource loading
requires a ResourceProvider to have been set on the render context.

Not handled in this CL:
- preserveAspectRatio support
- SVG reference types (i.e. '<image xlink:href="file.svg" ...')

Bug: skia:10842
Change-Id: Ieec7569f60516b01fc847f4160d0733b1e3a1cf5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/364576
Commit-Queue: Tyler Denniston <tdenniston@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-02-02 22:36:05 +00:00
Tyler Denniston
209857c20f [svg] Parse data and non-local IRIs
Not currently used, but will be used for <image> support.

Change-Id: I3bbb79c88890100901a8b85734d3a4d86a618848
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360605
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-02-02 03:46:59 +00:00
Florin Malita
24df67d04e Reland "[svg] Plumb a ResourceProvider"
This is a reland of dd29e20904

Original change's description:
> [svg] Plumb a ResourceProvider
>
> ... for image loading.
>
> Update the SVG tools to pass local/FS resource providers.
>
> Change-Id: I2c0e446047da87f177fde0f23b7ea1f0a856e808
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359996
> Commit-Queue: Florin Malita <fmalita@google.com>
> Reviewed-by: Tyler Denniston <tdenniston@google.com>

Change-Id: I28de60b299fc89f46b090812fc632105143fb6da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360607
Commit-Queue: Florin Malita <fmalita@chromium.org>
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-01-27 21:12:23 +00:00
Tyler Denniston
e71f547302 [svg] Expand SkSVGIRI type
This CL adds a type enum to SkSVGIRI that will eventually be used to
distinguish between local fragments ('#frag-id'), non-local fragments
('/path/to/file.png') and data URIs ('data:image/png;base64,...').

To do so required a bit of refactoring of other types that previously
treated IRIs as basic strings. There are no functional changes in this
CL.

Change-Id: I8bf426f77c0779890bce74d2d243c09617f024c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360601
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-01-27 21:05:41 +00:00
Florin Malita
64400d95d6 Revert "[svg] Plumb a ResourceProvider"
This reverts commit dd29e20904.

Reason for revert: breaking the G3 roll

Original change's description:
> [svg] Plumb a ResourceProvider
>
> ... for image loading.
>
> Update the SVG tools to pass local/FS resource providers.
>
> Change-Id: I2c0e446047da87f177fde0f23b7ea1f0a856e808
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359996
> Commit-Queue: Florin Malita <fmalita@google.com>
> Reviewed-by: Tyler Denniston <tdenniston@google.com>

TBR=fmalita@chromium.org,fmalita@google.com,tdenniston@google.com

Change-Id: If5f9673f9ff40dd5f8a8f9cf8f46a4342f29f31f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/360602
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-01-27 18:53:31 +00:00
Tyler Denniston
8eedcd2dea [svg] Partial implementation of feSpecularLighting
https://www.w3.org/TR/SVG11/filters.html#feSpecularLightingElement

Because a fair amount of functionality will be shared between
feSpecularLighting and feDiffuseLighting, this CL adds an intermediate
SkSVGFeLighting base class for common code.

The only light source type implemented in this CL is fePointLight, which
exercised by the filters-light-03 W3C test.

Bug: skia:10841
Change-Id: Icae26dedb1aae1cd25ba2db7c6468a243ebacbc5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359756
Reviewed-by: Florin Malita <fmalita@chromium.org>
Commit-Queue: Tyler Denniston <tdenniston@google.com>
2021-01-27 16:14:29 +00:00
Florin Malita
dd29e20904 [svg] Plumb a ResourceProvider
... for image loading.

Update the SVG tools to pass local/FS resource providers.

Change-Id: I2c0e446047da87f177fde0f23b7ea1f0a856e808
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359996
Commit-Queue: Florin Malita <fmalita@google.com>
Reviewed-by: Tyler Denniston <tdenniston@google.com>
2021-01-27 14:10:09 +00:00
Florin Malita
131decbf41 [svg] Fix node factories order
TBR=
Change-Id: I622581de30dd290b858fe99ece3ae85fd15a0c2f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/359918
Commit-Queue: Florin Malita <fmalita@chromium.org>
Reviewed-by: Florin Malita <fmalita@google.com>
2021-01-26 23:14:23 +00:00