It was already a no-op.
Bug: skia:13078
Change-Id: I7c7e1a86650ebdbd79bfcc4c1e739a3901345e76
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527836
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This avoids a situation where we didn't know the position of a binary
expression while we were in the middle of evaluating it, which was
leading to problems with upcoming error reporting changes.
Change-Id: Ie41fa82d077de1aa1041cd84c539ba29aaa2f5f0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527500
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This avoids a situation where we didn't know the position of a postfix
expression while we were in the middle of evaluating it, which was
leading to problems with upcoming error reporting changes.
Change-Id: Ie6d4dd8411ffb24ac7ee95d3e908483d0d92ba0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/527499
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This removes some hacks where we were creating nodes with the wrong
positions and later updating them to have the correct positions, along
with a general increase in the rigorousness of position tracking.
Change-Id: I2bc635de6d5f516d5fb6763b00a22a10ccdf135f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526678
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds asserts to DSLParser::expression() to ensure that every
generated expression actually starts at the current offset, and fixes
all of the various spots we weren't successfully doing that.
Change-Id: Ie37dbf9ee8bb47fd03f7a290d2e15da4fbfff938
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525316
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This is a reland of commit 1aedd5dc11
Original change's description:
> Always apply mipmap sharpening on GPU
>
> Bug: skia:13078
>
> Change-Id: If459a96eba09fb10e967bc364435f79b83fdc1ec
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522099
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13078
Change-Id: Ic05b38fc07566f090d609431f2738d64dfdc8a66
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524218
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
We already do this for cached SkSurface snapshot images. However,
sharing the SkPixelRef among any SkImage_Rasters with different
SkMipmaps interacts badly with SkImage texture caching and likely
other image caches.
Bug: skia:11983
Change-Id: I07bb7a93ffb79afb7f8268cd5159ee22f58da1e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526456
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13114
Change-Id: I653ab746927abdd1491e070e2e27252bf056d233
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/526024
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This fixes a bug with the following sequence:
Make SkSurface_Raster, draw to it
snap an image, A, from the surface
make image B by calling withDefaultMipmaps() on image A
Let image A be destroyed.
draw to surface again
snap another image, C, from the surface
Image C and image B would now share the same SkPixelRef, reflecting
the final contents of the SkSurface.
Add a GM that exercises the bug when run through either the pic- or
serialize- vias and DDL.
Bug: skia:13111
Change-Id: Ib079163c84f420baf62fa7960110386303b8fb93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525517
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
SkSL::Operator was previously just a thin wrapper around Token::Kind and
everything was therefore written in terms of Token. Since Token is not
accessible from include/ (and we don't want it to be), this creates an
actual Operator enum and eliminates the dependence on Token. This will
enable followup changes to reference Operator from within the DSL
include files.
Change-Id: I49555c9151618fd15970a303d2284972c78c3f21
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525976
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Change-Id: Icc6d1345c59d6f6529db39bf4b5bd8097f1f6b5f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525837
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This should fix many issues that Chrome is seeing. The problem is the
picture recorder had no idea about Slug, so the canvas super class
would try to use the device to draw. This hooks up the recorder
to capture drawSlug into the picture.
I tested this with a specialized gl sink in DM which I'm struggling
to check in. But, it was functional enough to show that this works.
Bug: chromium:1302036
Bug: chromium:1307279
Bug: chromium:1306329
Bug: chromium:1307446
Change-Id: I6a27bf43702400c80b2044433e7b00347f522763
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/525636
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Bug: skia:12845
Change-Id: If3ac2b6ba2c8e28328ee5805a29fc83353220364
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524756
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
No visible effect yet, but this will enable better error reporting in a
future CL.
Change-Id: I09e1c5d3bb423a7ce42701f15c4bb142b0a9473c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524638
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Most of these are pretty mechanical generated changes.
IWYU noticed one issue with DSLCore.h, which was fixed here.
Change-Id: I5629565ad3c2817daa71907c62f932d93f9d78ab
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524617
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This adds a way to build a wrapping canvas for testing that is allowed
to manipulate the internal state of the canvas. It provides a way
to add friends to SkCanvas without having to change SkCanvas.
Change-Id: I40de8b236ba5acff45b3a8f7e440dcf6fa196fcf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524316
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Herb Derby <herb@google.com>
SkTArray::reset() got rid of any reserved space, so previously we never
actually got to a high-water allocation. Updated the stack to use
SkTDArray just for consistency so we only had to include that instead of
<vector>, too.
Cq-Include-Trybots: luci.skia.skia.primary:Build-Mac-Clang-arm64-Debug-ASAN_Graphite,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-x86_64-Debug-Graphite,Build-Mac-Clang-x86_64-Release-Graphite,Perf-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Release-All-Graphite,Perf-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Test-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Debug-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac12-Clang-MacBookPro16.2-GPU-IntelIrisPlus-x86_64-Debug-All-Graphite
Change-Id: I94c9a38d0e630c7d7f1a3eeeb85076067e066a2f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/524000
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Bug: skia:12974
Change-Id: I15b090e2c3346d71ccf45d5f0d306da3f079821e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523996
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
This is needed for accurate error reporting when we start reporting
ranges rather than line numbers.
Change-Id: If465317e04685e91ab7c408d29e82028b5d59d1a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523425
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:13078
Change-Id: If459a96eba09fb10e967bc364435f79b83fdc1ec
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522099
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Change-Id: Ic88b400f7943804481934ba2c9c54aeb8e04121b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/523656
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Herb Derby <herb@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac12-Clang-MacBookPro16.2-GPU-IntelIrisPlus-x86_64-Debug-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Test-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Debug-All-ASAN_Graphite,Test-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Debug-All-Graphite,Perf-Mac11-Clang-MacMini9.1-GPU-AppleM1-arm64-Release-All-Graphite,Perf-Mac10.15.7-Clang-MacBookPro11.5-GPU-RadeonHD8870M-x86_64-Release-All-Graphite,Build-Mac-Clang-x86_64-Release-Graphite,Build-Mac-Clang-x86_64-Debug-Graphite,Build-Mac-Clang-arm64-Release-iOS_Graphite,Build-Mac-Clang-arm64-Release-Graphite,Build-Mac-Clang-arm64-Debug-iOS_Graphite,Build-Mac-Clang-arm64-Debug-Graphite_NoGpu,Build-Mac-Clang-arm64-Debug-Graphite,Build-Mac-Clang-arm64-Debug-ASAN_Graphite
Change-Id: I2ee0fd8d208bb10e011a5552a60a1a62d7a56ba9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522423
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This is a reland of commit 171450a1a0
Original change's description:
> Add glGetFloatv and glSamplerParameterf to GrGLInterface.
>
>
> Bug: skia:13036
> Change-Id: Ie15e0c2b0ada468b82bced818eec67c32d70126b
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522100
> Auto-Submit: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>
Bug: skia:13036
Change-Id: Id594ea5fd68c76703ff61d8414c203facbf28801
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522359
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
PS1 regenerates the Bazel files. Use it as the base change when
comparing patchsets.
IWYU seems to do a good job of working with MyFile.cpp and
MyFile.h, but if there is just a MyHeader.h, it doesn't always
seem to throw errors if the includes aren't correct. This was
observed with include/sksl/DSL.h This might be due to the fact
that headers are not compiled on their own, so they are never
sent directly to the IWYU binary.
This change sets enforce_iwyu_on_package() on the all sksl
packages and then fixes the includes until all those checks
are happy. There were a few files that needed fixes outside
of the sksl folder. Examples include:
- src/gpu/effects/GrConvexPolyEffect.cpp
- tests/SkSLDSLTest.cpp
To really enforce this, we need to add a CI/CQ job that runs
bazel build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
If that failed, a dev could make the changes described in
the logs and/or run the command locally to see those
prescribed fixes.
I had to add several entries to toolchain/IWYU_mapping.imp
in order to fix some private includes and other atypical
choices. I tried adding a rule there to allow inclusion of
SkTypes.h to make sure defines like SK_SUPPORT_GPU, but
could not get it to work for all cases, so I deferred to
using the IWYU pragma: keep (e.g. SkSLPipelineStageCodeGenerator.h)
Change-Id: I4c3e536d8e69ff7ff2d26fe61a525a6c2e80db06
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/522256
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:13051
Change-Id: I13cdb1b625f65e6d15d3b59ba493897ed88b24c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521005
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
This adds an assert to Position::line() to ensure that we don't run
past the end of the source string. Since the source string can
contain embedded nulls, we also switch the source handling from
const char* to std::string so we can accurately determine its length.
Change-Id: I9df47e98c1a0cbc35222a0ea709d9403762210d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521656
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
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>
Chrome uses a picture to manage some of their drawing in the GPU
process. This CL serializes GrSlugs into an SkPicture, and to
draw from that picture. Pictures with GrSlugs can not be stored;
they must be created and played back directly. In addition, the
SkStrikes used by the slugs must be pinned in the SkStrikeCache.
Bug: chromium:1302036
Change-Id: I19fa9c06c08f64ad066326e90bc6425dd90b3342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/514360
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
This will use the recently added Bazel toolchain feature
to enforce proper includes for all files in //src/svg/...
In the future, I envision a CI/CQ job that will run
bazel build with a few different configurations and the
--feature skia_enforce_iwyu on to make sure we don't
regress.
Change-Id: Ibb9f816ab626415c11bd2b9b74c503297c4b0723
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521036
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
PS1 regenerates the Bazel files.
It is recommended to review this CL with a diff from PS1.
Example output when a file does not pass the test:
tools/sk_app/CommandSet.h should add these lines:
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
namespace sk_app { class Window; }
tools/sk_app/CommandSet.h should remove these lines:
- #include "tools/sk_app/Window.h"
The full include-list for tools/sk_app/CommandSet.h:
#include "include/core/SkString.h"
#include "include/core/SkTypes.h"
#include "include/private/SkTArray.h"
#include "tools/skui/InputState.h"
#include "tools/skui/Key.h"
#include "tools/skui/ModifierKey.h"
#include <functional>
#include <vector>
class SkCanvas;
namespace sk_app { class Window; }
---
This makes use of Bazel's toolchain features
https://bazel.build/docs/cc-toolchain-config-reference#features
to allow us to configure compiler flags when compiling
individual files. This analysis is off by default, and can
be turned on with --features skia_enforce_iwyu. When enabled,
it will only be run for files that have opted in.
Example:
bazelisk build //example:hello_world_gl --config=clang \
--sandbox_base=/dev/shm --features skia_enforce_iwyu
There are two ways to opt files in:
- Add enforce_iwyu = True to a generated_cc_atom rule
- Add enforce_iwyu_on_package() to a BUILD.bazel file
(which enforces IWYU for all rules in that file)
Note that Bazel does not propagate features to dependencies
or dependents, so trying to enable the feature on cc_library
or cc_executable targets will only impact any files listed in
srcs or hdrs, not deps. This may be counter-intuitive when
compared to things like defines.
IWYU supports a mapping file, which we supply to help properly
handle things system headers (//toolchain/IWYU_mapping.imp)
Suggested Review Order:
- toolchain/build_toolchain.bzl to see how we get the IWYU
binaries into the toolchain
- toolchain/BUILD.bazel and toolchain/IWYU_mapping.imp
to see how the mapping file is made available for
all compile steps
- toolchain/clang_toolchain_config.bzl, where we define the
skia_enforce_iwyu feature to turn on any verification at
all and skia_opt_file_into_iwyu to enable the check for
specific files using a define.
- toolchain/clang_trampoline.sh, which is the toolchain is
configured to call instead of clang directly (see line 83
of clang_toolchain_config.bzl). This bash script used to
just forward all arguments directly onto clang. Now it
inspects them and either calls clang directly (if
it does not find the define in the arguments or we are
linking [bazel sometimes links with clang instead of ld])
or calls clang and then include-what-you-use. In all cases,
the trampoline sends the arguments to clang and IWYU
unchanged).
- //tools/sk_app/... to see enforcement enabled (and fixed)
for select files, as an example of that method.
- //experimental/bazel_test/... to see enforcement enabled
for all rules in a BUILD.bazel file.
- all other files.
Change-Id: I60a2ea9d5dc9955b6a8f166bd449de9e2b81a233
Bug: skia:13052
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519776
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Change-Id: I677ad3073725219897bec9f82d88ad3ba3fedb53
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/521002
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Herb Derby <herb@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This CL switches almost all instances of line tracking over to track
Positions instead. This does not yet add full range support - only the
start offsets will be correct currently. Followup CLs will extend the
ranges to fully cover their nodes.
Change-Id: Ie49aee02f35dcb30a3adb8a35f3e4914ba6939d2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/518137
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Bug: skia:12845
Change-Id: Ic036dea6b58682a6463f8c34d915730c4bfe677b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/519617
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
