This also pinpointed one more case where we had an sk_sp<> inside a
static.
Change-Id: I01af66213249334667f107bcb95d53e4ce6f4c52
Bug: skia:13426, skia:13432
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550020
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Note that both `unordered_map` and `sk_sp` had non-trivial
destructors.
Change-Id: I39ffc2193c7935e71aab9b5c7474a64b5f93f753
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550176
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
There is a very old bug (http://crbug.com/510931) that was fixed back
then via a special check when unpacking glyphs. However, it was noted in
another CL that it doesn't seem this should still be possible. The only
case where expectedMaskFormat != maskFormat is when we have a 565 glyph
but a RGBA8 texture due to lack of 565 support. Otherwise they should be
the same. There is no access of the glyph cache that should change the
maskFormat.
Change-Id: I4a6ea11e5095f47f4ce5cc095db8bee084dd8a31
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549846
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Previously, we were storing sk_sp<SkRuntimeEffect> objects in static
variables. This would presumably lead to destructors executing during
atexit time, which is unnecessary and can lead to shutdown bugs. We no
longer attempt to free these objects.
Change-Id: I0b09d7ab3dc67c9b8fbc35c81d72ff57b0592c03
Bug: skia:13426
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549658
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Most snippets have zero, but anything involving a runtime effect will
expect one pointer. At present this value isn't used anywhere.
Change-Id: I796e8c26433fa74512e39be65e7df76fba99f996
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550018
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Most fields in this struct are already accessed directly (including
`fNumChildren`) and this accessor didn't do anything interesting to
justify its existence.
Change-Id: I11e97b384a9272f2f3fc0417849fbb5a59761d8d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550019
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
We didn't have any unit tests confirming that BlockReader can read back
the data out of a block. This seemed like a gap worth filling.
Change-Id: Iefbd5aec199a520ca1ca84b53faef4fd2f2d9ab4
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550017
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Use template guides for deducing the type needed by the CTOR.
This allows SkSpans to be deduced when things like vector,
array, A[], etc. are passed as parameters without using SkMakeSpan.
Change-Id: Ieedcbddc8d40281f819b0c798acd178a12a05891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549568
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
FreeType is designed to handle *alloc calls returning nullptr and can
fail gracefully with an error code. Ideally a sk_realloc_canfail would
exist to back ft_realloc but it does not appear to exist at this time.
Making this change should reduce the noise from OOM crashes in Chromium
which happen to occur in FreeType and shift them to a different call
stack.
Bug: chromium:1334956
Change-Id: I3c6318ede5aa0874e919ae99dc88eb9efcd05554
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549843
Commit-Queue: Ben Wagner <bungeman@google.com>
Auto-Submit: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Adds a new xfermodes2_gray GM, to exercise the various divide-by-zero
cases that show up. Before the fixes, both RP and SkVM were producing
noise in Hue/Saturation/Color/Luminosity. Now they all look right.
Bug: skia:13417
Change-Id: I741461d80ab23b100bbf4d33b9bc023db650bac5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549845
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Label temprory offscreen textures for draws. In this CL, we will
label texture for a part of blur from SkGpuBlurUtils..
Bug: chromium:1164111
Change-Id: Ibfe1c16efa57b6a134a763bc918411108e286704
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549057
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
I saw some strange errors in the logs and locally after
updating to Bazel 5.2.0:
Compiling third_party/externals/freetype/src/base/ftwinfnt.c failed: missing input file 'external/emscripten_npm_linux/node_modules/@caporal/core/CHANGELOG.md', owner: '@emscripten_npm_linux//:node_modules/@caporal/core/CHANGELOG.md'
When I did a bazel clean --expunge locally, it went away.
Looking closely at the Bazel cache folder, some npm dependencies
starting with an @ sign were missing from caches for the 5.0.0
but there in 5.2.0. I could not find an entry in the changelog,
but it's simple enough to clear our caches for now.
Once we update emsdk to a new version, we shouldn't need this
as the dependencies should be downloaded fresh.
Change-Id: Id21d4e4806792d2eee48e1c5e1d6a106230be17a
Bug: skia:12541
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549856
Reviewed-by: Leandro Lovisolo <lovisolo@google.com>
Change-Id: I7a4cd8e2c629ba53e40adc379ea92f86f70d11da
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549857
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Auto-Submit: Jim Van Verth <jvanverth@google.com>
After making http://review.skia.org/549837 I realized we also needed to
check the case of data matching, but snippet IDs mismatching.
Change-Id: Iff9d3ec9921e66c1d65e2c4fe8ca0aa279444d7d
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549841
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously, we didn't seem to have any tests verifying the behavior of
operator== and operator!=.
Change-Id: I1f76b8307a551f687d172782e8a30a01c0718e85
Bug: skia:13428
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549837
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The tests themselves are the same, just split up into their logical
groupings. From the principle go/unit-testing-overview#properties
"Focused. Above all, unit tests are narrow in scope, validating the
correctness of individual pieces of code rather than the correctness of
the system as a whole."
Change-Id: If4219c7ea447155314db885a1e1f0beb3c444b74
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549836
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:13118
Change-Id: Icaf2b0914ce7943f57e1ab23e7778d1dcb811cc4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548781
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Bug: skia:13118
Change-Id: I0d6e0a9f542680456d3de82a32376722c5b9cd1d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549097
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
initializer_list constants only have a scope until the next
sequence point.
Change-Id: I5c52310042fc4773a3ac0e5f9d23ae45bb7b1229
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549574
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Use std::size and std:data to get needed values.
Add a SkMakeSpan for initializer_list.
Change-Id: I43b4b2391323c84b3d6aad22ac4775487c6d5570
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549565
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
No longer call glBufferData with a partial size and track a true
buffer size in the subclass.
Bug: skia:13427
Change-Id: Ibfd04ff8a18d54e1e4cd3fce6863cb48b5495294
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549276
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This is a reland of commit 55f9ee1e35
Original change's description:
> Move RecursiveComparison tests to run on GPU
>
> These all require the GPU to generate NaN values. We don't have a static
> way of checking that in caps. Instead, added a probing function to
> decide if the GPU will generate them, and a flag to indicate which tests
> require that behavior.
>
> Change-Id: I9411969b042684ac583a9eb4e9b1aacf2525cc22
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549099
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: Ie29d32756ceebf872faf1db8d1a1273ad842529b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549562
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Bug: skia:13118
Change-Id: I06d3f899b60ac2cf8d48a46da6097a15c5d40024
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/543923
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
At present, we calculate the coordinates, but we don't yet use them
anywhere, because the runtime effect code isn't being included in the
finished program.
Change-Id: I6eb6b1b43ce78da31884b7a8b1bac2aa9ba5407b
Bug: skia:13405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549101
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Runtime shaders are now distinguished by the hash of their shader text
and number of uniform bytes that they will use. Fortunately, we were
already computing the shader text hash so all we needed to do was
bring it into our data payload.
Change-Id: Ifc0bba6a7f18acd9affca5822e1a54fbff594d88
Bug: skia:13405
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549100
Reviewed-by: Robert Phillips <robertphillips@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was introduced here:
https://skia-review.googlesource.com/c/skia/+/549618
When resizing the buffer pass nullptr as src.
Change-Id: I99b7c9a12379e3285119e62adb5a02c93c2f66d8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549657
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
The previous logic in handling complex bbox intersections in Graphite's
clip stack was fundamentally flawed. It's now replaced with the correct
solution, which is to use the separating axis theorem, instead of just
checking corner points.
The flow through the intersection case has been re-worked to move the
rect-stays-rect dual to the forefront, since that is the most common and
simplest scenario.
Also adds an extra fast check for contains() to avoid more expensive
analysis when the outer bounding boxes already show containment is not
possible.
Found a minor bug in the creation of the clip stack elements where it
was only applying the rect-stays-rect transform to map shapes to device
space when it was *only* rect-stays-rect and not simpler. Importantly,
this meant that translate and positive scale+translates were remaining
un-mapped so comparisons between elements would always then hit the
more expensive "differing coordinate space" codepaths. Previously, the
only cases that would trigger this would be mirrors and 90 degree
rotations.
Change-Id: I328ca1515ff578a617ef18992819c7c9465cb30d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548478
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
This is a reland of commit 02ee07325f
Original change's description:
> Use glInvalidateBufferData when available.
>
> Also move common onUpdateBufferData checks and asserts to base class.
>
> Bug: skia:13427
> Change-Id: Iccae5b654a6ed3eab71ac701a78434aa1116e00e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548482
> Commit-Queue: Brian Salomon <bsalomon@google.com>
> Reviewed-by: Greg Daniel <egdaniel@google.com>
Bug: skia:13427
Change-Id: I4a133bca44b52924453b6e5d2e4e812e80900606
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549618
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
This reverts commit 55f9ee1e35.
Reason for revert: MacMini failures
Original change's description:
> Move RecursiveComparison tests to run on GPU
>
> These all require the GPU to generate NaN values. We don't have a static
> way of checking that in caps. Instead, added a probing function to
> decide if the GPU will generate them, and a flag to indicate which tests
> require that behavior.
>
> Change-Id: I9411969b042684ac583a9eb4e9b1aacf2525cc22
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549099
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
Change-Id: I4e2bb4f0d2908f2fb4a2f2d64953c1366549d397
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549559
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Also move common onUpdateBufferData checks and asserts to base class.
Bug: skia:13427
Change-Id: Iccae5b654a6ed3eab71ac701a78434aa1116e00e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548482
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
These all require the GPU to generate NaN values. We don't have a static
way of checking that in caps. Instead, added a probing function to
decide if the GPU will generate them, and a flag to indicate which tests
require that behavior.
Change-Id: I9411969b042684ac583a9eb4e9b1aacf2525cc22
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549099
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Godbolt before: https://godbolt.org/z/KMG3WE1fq
Godbolt after: https://godbolt.org/z/sPvjn9W97
Our compact lexer tables contain four values--zero plus and three actual
values. Previously, we used a bitfield to squish those each of those
nonzero values into the least amount of space possible--given our
current data set, the first value always fit into 6 bits, and the other
two needed 9 bits. This adds up to 24. Unfortunately though, in practice
the values cost us 32 bits since there's no 3-byte int in C.
I realized that we could do significantly less work if we used the same
number of bits for each of our three values--we can just shift by
`9 * value` and mask it off. This adds up to 27, which is still within
our 32 bit margin. This also plays well with the special-case of zero;
if we make zero live in position 3 (instead of its original home at
position 0), we can shift all the bits off and we'll be left with zero.
This approach will even extend to 10 bits per value without any
problem. If our lexing tables grow past that, we will need to switch to
64-bit ints or come up with a different packing algorithm.
On my desktop machine this was able to win back 1.5%-2.5% in
sksl_large (maybe below the noise floor for typical Skia perf).
Change-Id: Id5c4d6eec749294f2c596d4c81cde6f429af993f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549376
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
This allows mskps produced with Chromium to be displayed in the
debugger. Previously, the debugger would produce invalid json if any
string contained characters which needed to be escaped. The debugger
also treated all strings like NULL terminated strings, but json is
Unicode based and code point U+0000 is a perfectly good code point.
Change-Id: I28150bad666b02be9f1e4af4078a4ca1e65bf000
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/549098
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Since the state per recorded draw is reduced when occlusion culling
isn't part of the API, I switched BruteForceBoundsManager to use two
parallel SkTBlockLists, one for the rects and one for the orders. Since
Rect is over-aligned, this should save on memory.
Bug: skia:13201
Change-Id: Ib9b4b1faf9831f66708475faaf399cc4fadc4f45
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/548777
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
