Commit Graph

10495 Commits

Author SHA1 Message Date
John Stiles
63ec9d4045 Cleanup SkSL error test code a bit.
`test_expect_fail` did a lot of fiddly string manipulation work in one
big monolithic function. Now it has helper functions
`get_expected_errors` and `check_expected_errors`. This hopefully makes
the logic a bit easier to understand.

There aren't any functional changes here, just restructuring.

Change-Id: I0961054b475255b6159b4dd05b98b6054b144d71
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522422
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-21 16:17:25 +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
Robert Phillips
42bc49d5d3 [graphite] Better encapsulate SkPipelineData's uniform data
We are going to cache the uniform data and the texture data separately.
Begin by making the uniform data something that could be cached on its own.

Bug: skia:12701
Change-Id: If3ea3a9b6050faf0810549d4076ae44732656a9e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522178
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-18 18:00:38 +00:00
Ethan Nicholas
df40c7eae8 Fixed a past-the-end error location in SPIR-V output
The error was being reported at the position of the var declaration,
rather than the position of the reference. And since the declaration
was in a module, its position was both incorrect (with respect to the
program source) and could be past the end.

Change-Id: I443b9fbbe016c43b93d457abfefd17025e451d8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521522
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-16 21:54:21 +00:00
Herb Derby
4397488ce5 add testing code for serialized Slugs
By setting the compile time flag:
SK_EXPERIMENTAL_SIMULATE_DRAWGLYPHRUNLIST_WITH_SLUG_STRIKE_SERIALIZE
will cause all SkTextBlobs to be rendered by analyzing the Slug
to create strike cache differences and serialize the Blob to a Slug.
Then the serialized strike cache differences are used to populate
the strike cache using a different TypefaceIDs using SkTypeface_Remote
as a proxy for the real SkTypeface. This will create a hard break
between the original glyph data, and the proxied glyph data.
It will then deserialize the Slug doing TypefaceID translation to
the SkTypeface_Remote, and draw the unflattened Slug.

Bug: chromium:1278340

Change-Id: I0f1980dee966b1092a99741793aed9d138451f4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/510228
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2022-03-16 19:42:16 +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
Kevin Lubick
34975a4d58 Reland "[includes] Remove link between SkImageEncoder and SkBitmap"
This is a reland of commit 965d9dc948

Upstream changes
 - http://cl/434609233
 - http://cl/434730162

Original change's description:
> [includes] Remove link between SkImageEncoder and SkBitmap
>
> This cleans up an old TODO and, according to go/chrome-includes
> [1] [2], will save 106 MB (0.05%) of build size.
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/chrome_includes_2022-03-09_114034.html#view=edges&filter=&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageEncoder%5C.h%24&included=&limit=1000
> [2] http://screen/ohtAxmQcnXui47q
>
> Change-Id: Ic53bfa827964dd5d2d2dbce12f1722e57ea5a9bc
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518497
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>

Bug: 242216
Change-Id: I5bad2a8764c54e885c5b6cad6070549f2cbeefcf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520936
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-15 18:26:16 +00:00
Robert Phillips
9565f4bd90 [graphite] Plumb Recorder down into KeyHelpers
The KeyContext is used in the addToKey methods but must appear in the
AddToKey methods bc the latter can call the former.

Bug: skia:12701
Change-Id: I3143afec8337b1e3e12f1c3cc198714009ca6930
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520539
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-15 18:06:05 +00:00
Kevin Lubick
e1880aed8f Revert "[includes] Remove link between SkImageEncoder and SkBitmap"
This reverts commit 965d9dc948.

Reason for revert: Breaking G3

Original change's description:
> [includes] Remove link between SkImageEncoder and SkBitmap
>
> This cleans up an old TODO and, according to go/chrome-includes
> [1] [2], will save 106 MB (0.05%) of build size.
>
> [1] https://commondatastorage.googleapis.com/chromium-browser-clang/chrome_includes_2022-03-09_114034.html#view=edges&filter=&sort=asize&reverse=&includer=%5Ethird_party%2Fskia%2Finclude%2Fcore%2FSkImageEncoder%5C.h%24&included=&limit=1000
> [2] http://screen/ohtAxmQcnXui47q
>
> Change-Id: Ic53bfa827964dd5d2d2dbce12f1722e57ea5a9bc
> Bug: 242216
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518497
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Kevin Lubick <kjlubick@google.com>

Bug: 242216
Change-Id: I71d76582f44317f6ad3a4c0152c5ce10c9bbf783
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520796
Owners-Override: Kevin Lubick <kjlubick@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
2022-03-15 00:04:01 +00:00
Michael Ludwig
fc4fb7a624 Clamp block increment to uint16 max instead of asserting
The block increment parameter, after dividing by address align, has to
fit into 16 bits. SkTBlockList with either large T or a large
"itemsPerBlock" hint can pretty easily exceed this. However, both the
items per block and the block increment are just hints to control
allocation patterns. SkBlockAllocator can have larger blocks than that
based on growth policy, up to its actual allocation size limit.
SkTBlockList also does not need itemsPerBlock in its implementation, so
if the request exceeds what the allocator can do, it's not problematic.

So clamping to the highest storable value is nicer than asserting that
the caller respected the internal limits.

Change-Id: I82b1c20034fd264b65c7eae4d6758caa6b574fb1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520656
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2022-03-14 20:07:24 +00:00
Kevin Lubick
965d9dc948 [includes] Remove link between SkImageEncoder and SkBitmap
This cleans up an old TODO and, according to go/chrome-includes
[1] [2], will save 106 MB (0.05%) of build size.

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

Change-Id: Ic53bfa827964dd5d2d2dbce12f1722e57ea5a9bc
Bug: 242216
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518497
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-14 20:02:23 +00:00
Dan Field
23cb294413 Parse arc flags correctly when there is no optional ws
Today, if the arc command flags are not separated by whitespace, the
parser fails to parse the string. I noticed this when trying to parse a
path similar to the one in the test case when playing around with
PathKit.FromSVGString.

Change-Id: I40967c07dfa03d76d26ac2e060b3ef7ac488d0fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520256
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Dan Field <dnfield@google.com>
2022-03-14 18:42:16 +00:00
Ethan Nicholas
c9e9131f44 Switched SkSL positions from int to Position
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.

Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-14 17:06:17 +00:00
Robert Phillips
4ee8dab839 [graphite] Disallow empty SkUniformData objects
Bug: skia:12701
Change-Id: Iaec7cfbbd595b9382e4362c01d5d6897277d8d7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/520496
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-14 15:55:16 +00:00
Robert Phillips
44263c5d08 [graphite] Add fixed-function blending support
Bug: skia:12701
Change-Id: I1fd8dede3eb216c28408bd613119448704c0e7c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512356
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-14 14:21:46 +00:00
Aditya Kushwah
52771e2bb3 Plumbing the label string through Skia.
This CL defines a const string in GrGPUResource and have the
constructors accept it. The label string is then plumbed through the
system for other components to accept it.

Bug: chromium:1164111
Change-Id: I6cc759f9263dedd4b2cc0c3ca7cf280be5d74174
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/508798
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2022-03-11 03:06:47 +00:00
John Stiles
333159568d Add CurrestTestHarness helper method to WasmGMTests.
Change-Id: Ifcfe69974fabf15f73ac010ab1ae6775868af629
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519618
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-10 21:14:03 +00:00
Arman Uguray
e70f2e0e0f [sksl][test] Add more folding tests for side-effecting expressions
Moved the MatrixFoldingES2.sksl test case for matrix construction with
side-effects into a new PreserveSideEffects.sksl test and added new test
cases for various vector and matrix types and constructors. The new test
is written such that none of its contents should be folded away.

Note: This test does not pass on NVIDIA GPUs when using OpenGL as
discussed in skia:13035. Notably, NONE of the increments are executed on
those GPUs as ALL increment expressions seemingly get subjected to
constant-folding. The test is disabled on NVIDIA GPU bots.

This also means that the remaining MatrixFoldingES2.sksl tests now work
on NVIDIA GPUs when using OpenGL (with the exception of Tegra3 + OpenGL
ES).

Bug: skia:13035, skia:11919
Change-Id: I561bb62fe2b6b814ba80fbc492d3885bbcd6b65b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518278
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-10 18:06:12 +00:00
John Stiles
c5ca065411 Add CurrestTestHarness helper method.
This lets us differentiate SkQP from other testing harnesses (like DM or
Viewer) at runtime.

This allows us to change strictness or deactivate tests when SkQP is
running. Previously we had a macro SK_BUILD_FOR_SKQP for this, but this
did not work on a local skqp binary; it only activated when compiling
for Android.

Change-Id: I7334e04ea1eddda783a5d2f26699edd20828f81a
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518939
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-10 18:05:28 +00:00
John Stiles
07e0a0abd3 Honor SkQP flag when running SkSL tests.
The Android builds of SkQP will conditionally exclude any tests that are
not flagged with `SkQP`.

Note that local builds of the C++ skqp binary do not actually set
SK_BUILD_FOR_SKQP. This flag is set by gn_to_bp. So local skqp binaries
will continue to run these tests. A possible fix for this limitation
has been written in the followup CL: http://review.skia.org/518939

Change-Id: I879650d165c5931693b14102586bae2c9e45dd43
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518936
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
2022-03-10 15:16:23 +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
Kevin Lubick
677a12f38d [includes] Fix up transitive dependencies on SkBitmap
Change-Id: Ie1ba66881743eee01d36f7c7032307fbdca1e671
Bug: 242216
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518496
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-09 18:25:17 +00:00
John Stiles
7f9a95ab66 Restructure SkSLTest to use test flags instead of multiple macros.
At this CL, we should run exactly the same tests as before; the only
difference is that we enable and disable tests via SkSLTestFlags instead
of using different SKSL_TEST_xxxxxx macros for different test types.

There is an inert flag for SkQP; this will be implemented in a followup
CL.

The SKSL_TEST block now extends past 100 characters in some spots
because it was, overall, a lot easier to read and edit tabular data in
a tabular format than with inconsistent formatting and line wrapping.

Change-Id: Ic90f88b10c6e59357275b471e6e86e53e611f101
Bug: skia:13037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518138
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-09 18:01:52 +00:00
Ethan Nicholas
9cd8456afe Renamed SkSL::PositionInfo to SkSL::Position
Change-Id: I1fa160941b18392616708f4e08a4ebbf7398480e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518521
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-09 17:37:38 +00:00
Jim Van Verth
0284676042 [graphite] Fix uniform buffer alignment for MacOS in CommandBufferTest
4 byte alignment was being used, when it should be 256 on Intel Macs.

Bug: skia:13033
Change-Id: If902885f62e019b10c316dcfd7f4a336e0982db7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518522
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2022-03-09 17:29:14 +00:00
Robert Phillips
88f90513a9 [graphite] Make CreateKey work more like ExtractPaintData
ExtractPaintData currently returns:
  std::tuple<SkUniquePaintParamsID, std::unique_ptr<SkPipelineData>>

SkShaderCodeDictionary::findOrCreate is going to change in an upcoming CL and this sets up to better isolate those changes.

Bug: skia:12701
Change-Id: I8d077b3d342e6ea16ac99227eb145d92e8247ea4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517736
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-09 14:07:53 +00:00
Kevin Lubick
cbc954bf57 [includes] Remove include link between SkPath.h and SkPathRef.h
According to go/chrome-includes [1], this will save about
185MB (0.08%) off the Chrome build (some of which is handled
by https://skia-review.googlesource.com/c/skia/+/512776).
http://screen/3foDapQ5BN7srcG

This adds several forward declares and moves several
implementations from SkPath.h to SkPath.cpp.

This also removes privateSetConvexity from SkPathBuilder
because the SkPathConvexity being set to a non-zero default
value would have required moving the enum out of SkPathRef
or adding an include link. This private API was only used
in one location, so I just opted to remove it.

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

Change-Id: Ie1f83ac3eee6eb19d7ded18a41a7532f445c13ee
Bug: 242216
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512357
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2022-03-09 00:55:54 +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
Ben Wagner
6c630f2ce6 Make SkScalerContext enums scoped.
This was motivated by SkScalerContextFlags putting `kNone` into the
global namespace. Instead of adding a suffix, change these internal
enums to be scoped and update the users.

Because SkScalerContextFlags is used like a bitfield,
`SK_MAKE_BITFIELD_OPS` is used to provide the appropriate operators. The
macro implementation is updated to handle scoped enums, remove the very
dangerous, unwanted, and unused templated operators, and add operator~
since that is needed by one existing user.

Also, for some reason this allows the compiler to find an unused field
which is removed.

Change-Id: Id31c80b381bffaf9b623f6a8ec7d86751d3d99f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517616
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
2022-03-08 16:24:47 +00:00
Robert Phillips
335a1d0454 [graphite] Split SkPipelineData out into its own file(s)
SkPipelineData is evolving into a big deal - it deserves it owns files.

Bug: skia:12701
Change-Id: I78b4100f1b90fa10f2b264e6d13fca6f15bba39b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516157
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-08 16:04:35 +00:00
Robert Phillips
0c43db950c [graphite] Make GrBlend shareable between Graphite and Ganesh
Additionally make the enums enum-classes.

We will need this to specify fixed function blend modes in Graphite.

This CL is mainly mechanical (i.e., global replace) but some fix-ups were required for our internal bit twiddling with the enum-classes.

Bug: skia:12701
Change-Id: I67abcf8274d14e691974cf253bf65dcb4697bd9a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515318
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-08 14:51:44 +00:00
John Stiles
5e9388d170 Add support for conditional unit tests.
DEF_CONDITIONAL_TEST and DEF_CONDITIONAL_GPUTEST_FOR_xxxx_CONTEXTS will
allow unit tests to be considered or ignored based on a passed-in
boolean condition. DEF_TEST_DISABLED is now implemented as a conditional
test hard-coded with a "false" condition.

This allows macros (such as SKSL_TEST) to add unit tests based on the
result of a comparison.

Change-Id: Ic5325569009cfce768ba8942907c1d3e7d69ca1c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/517516
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-08 14:35: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
John Stiles
6a10556775 Reject equality operators on opaque types.
ES2 disallows opaque types in expressions (other than passing them to
their associated builtin functions). We now enforce a similar
restriction on SkSL opaque types.

While I was there, I added several other cases to the invalid-shader
test to make sure that they were all caught.

I needed to reorder some code to make sure that ternary expression error
messages didn't change. Ternary expressions now check for opaque types
before checking that the left-side type and right-side type are
compatible. This is because we check for "compatible" ternary
expressions by checking if `leftSide == rightSide` would be accepted.
`shader1 == shader2` used to be considered a valid expression for the
purposes of this test, but not anymore.

Change-Id: I62a0a31feca9dadd428da7d1b48d7693c4b6434d
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516802
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-03-07 15:19:46 +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
John Stiles
b8cee7dab3 Reject operators =, ==, and != on void types.
The fuzzer discovered that we allow == on void types (confusing the SkVM
backend).

Change-Id: Ia9494642faf67f3f86e3a365807be8bd4a7062e4
Bug: skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516796
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-07 14:10:29 +00:00
John Stiles
1c2b4492b8 Only run vector-folding logic on vectors.
Previously, we would take the vector-folding path for all types. This
didn't cause any problem for scalars, but failed for "zero-size" types
like void. It isn't valid to compare zero-size values, but we currently
don't reject such code (see skia:13026), and the fuzzer noticed this.
It's safest to only run the vector-folding code when we actually have
multiple slots that need to be folded into one result.

Change-Id: I0bc88043d9a4aeea38ae24dc1a6d1a7430d3d7b0
Bug: oss-fuzz:45279, skia:13026
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/516676
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2022-03-07 13:58:17 +00:00
Greg Daniel
2e7a5061b0 [graphite] Use ResourceCache for stencil attachments.
Bug: skia:12754
Change-Id: Iec046dedb088f226e5928712222fa786426b2689
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515517
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2022-03-04 15:44:45 +00:00
John Stiles
6ff07e0b02 Add initializer-list construction to SkTHashSet/Map.
This will allow us to easily replace unordered_set/unordered_map in more
places.

Change-Id: Ic0b5245e8cdce2918d7b043bfc370eca95caf889
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515319
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-03 22:40:20 +00:00
John Stiles
7b40626ca9 Fix debugger return-value display with repeated function calls.
The slot-assignment logic has been changed to associate slots with
function calls, instead of function definitions. In our test case, you
can now see that the calls to `get` are now mapped to $15, $17 and $18.
This change also jiggles some existing tests and improves their
register allocation slightly (!).

One minor hitch here is that there's no FunctionCall node associated
with main() (it's never explicitly called). However, our slot map key
can be any IRNode, and we know main() can't be called by anyone else,
so it's harmless to use the function definition as the key in this case.
(This entry could probably stay out the map entirely if it made a
difference, but I don't think it matters.)

Change-Id: I68a6ff24cbd3a2db77f24126502bd3d11e8c0962
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514578
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-03 20:55:25 +00:00
John Stiles
d9f9f1813c Add test demonstrating issue with function calls.
If a function is called multiple times on one line, stepping over that
line does not show all of the function-call results. It only shows the
last result.

e.g. in this example, I have just stepped over the first line which
calls "get" three times. We should see three results, but we only see
one: http://screen/3WfJoZWm77cSexM

In this test you can see that all three calls to `get` are assigned to
the same slot, $15.

Change-Id: Id0c486ef349a1e527001efbcee2ed2b836f56e83
Bug: skia:13011
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514577
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2022-03-03 20:55:25 +00:00
John Stiles
ae8d412b9a Add null check to DebugTracePlayer.
Navigating away from the Debugger in Viewer could lead to a null
dereference.

Change-Id: I2686edd3716e7a375d394a4dc1a296395b5f35b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514579
Reviewed-by: Arman Uguray <armansito@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-03 20:55:25 +00:00
John Stiles
be46542a7f Update SkTHashSet tests to use std::string/string_view.
This leverages our new support for standard strings in SkGoodHash, added
in prior CL http://review.skia.org/515356.

These tests already included checks for SkString, so it was easy to add
a template which tested std::string and std::string_view with the same
checks.

Change-Id: Ia2d9b889e66a50556a25e40493d14a5b692214a8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515358
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2022-03-03 18:09:00 +00:00
John Stiles
847f03ab20 Add std::string/string_view support to SkGoodHash.
This will allow these types to be used as the key in an SkTHashSet/Map
without providing a custom Traits class.

Change-Id: I41bfc1d43f3909d3653b4f12ac39e2a288235052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/515356
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
2022-03-03 18:09:00 +00:00
Robert Phillips
b61366b54f [graphite] Rename SkUniformBlock to SkPipelineData
and:
 UniformCache -> PipelineDataCache
 UniformCacheTest -> PipelineDataCacheTest

Bug: skia:12701
Change-Id: Ia28c223edb8d741d15fa9fa83695ac6b99284d5e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514156
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2022-03-02 17:01:07 +00:00
John Stiles
60c5837632 Fix Housekeeper-PerCommit-CheckGeneratedFiles.
I added a comment and didn't rebuild; this shifted line numbers around,
which is reflected in the debug trace opcodes.

Change-Id: I1b8e00ff65557a03483e8d7ff0c4dbec9852866f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514777
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
2022-03-02 16:42:10 +00:00
John Stiles
e11a14d17f Add workaround for LLVM crash in macOS 12 on Intel or M1.
The `break_loop` test causes LLVM to get confused and crash when
compiled on some GPUs. The crash goes away if we pass a literal 5
instead of a 5 that is computed at runtime. This also results in a
simpler test for SkVM, for better or worse, but we still have
coverage for dynamic loop exits in other tests.

LLVM crash: https://paste.googleplex.com/4718583155261440
Dangerous shader: https://paste.googleplex.com/4776089520963584

Change-Id: Ic6cbd55a36d2de58e5dd3459d4dfd74acdbc9f91
Bug: skia:13005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514538
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2022-03-02 15:20:10 +00:00
Arman Uguray
5f16ed4c58 [sksl] Special-case unary negation on matrix types in MSL/SPIR-V
MSL does not support the unary "-" operator on matrix types. Similarly
the SPIR-V OpFNegate/OpSNegate operations only work on scalar and vector
type.

* An expression such as "-<mat>" is now transformed to "-1.0 * <mat>" when
generating MSL.
* The same expression now generates a component-wise negation in SPIR-V,
matching what glslang outputs for GLSL.
* A unary "+" is now treated as NOP for MSL, matching the SPIR-V backend.
An expression such as "+<expr>" is now evaluated as "<expr>".
* The shared/Negation.sksl has been moved to folding/ as much of its
contents exercise constant-folding of comparison expressions.
* The shared/UnaryPositiveNegative.sksl test has been extended to
exercise scalar and matrix types.

NOTE: The SPIR-V backend changes have caused a minor re-ordering of SSA
IDs generated when writing out a prefix-expression. The affected gold
files have been updated.

Bug: skia:12627, skia:12992
Change-Id: Iec5cdafc591aed7e49b3b52bda42a02661380bab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/513976
Auto-Submit: Arman Uguray <armansito@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Arman Uguray <armansito@google.com>
2022-03-01 20:14:46 +00:00
Ethan Nicholas
f6dc205b14 Improved SkSL array size parsing
We previously had no way to signal a parse error from arraySize,
resulting in a cascade of additional errors downstream. This tightens
up the behavior and allows us to fail more gracefully.

Bug: skia:12416
Change-Id: I83d3d5bc1dc63395edb325297375a6eb52415817
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/512952
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2022-03-01 16:44:49 +00:00