The Possible(Expression|Statement) classes were added at
http://review.skia.org/375069. These classes were responsible for
capturing `__builtin_FILE()` and `__builtin_LINE()` when an
expression or statement was added to a hand-authored DSL program. This
allowed errors to be reported on the C++ file/line where they were
encountered. This was a good feature to have, when the plan was to
author the majority of SkSL code via DSL.
Later, IRNode positions were converted from an integer line number to
SkSL Positions at http://review.skia.org/518137. This gave us range
tracking, but at a high memory cost (16 bytes per IRNode, versus four
bytes when we tracked line numbers only).
Positions were reduced to 8 bytes at http://review.skia.org/521005 by
removing the filename, which was only used for hand-authored DSL. (The
size was pared all the way back to 4 bytes at
http://review.skia.org/533699 by packing the data more efficiently.)
__builtin_FILE/LINE capturing was removed entirely at
http://review.skia.org/528366; the filename was discarded anyway and
the line number didn't have a range and wasn't very meaningful without
a filename. Also, it didn't matter very much since we no longer intended
to hand-craft our programs in DSL.
At this stage, DSLPossibleExpression stopped adding value and simply
served to move Expressions around.
Change-Id: I29ac33e08dcc1804cc0619c1e8856ba28ebcc51d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542145
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
~DSLStatement and ~DSLExpression previously called
`fFragBuilder->codeAppend`. ~DSLPossibleStatement and
~DSLPossibleExpression would convert themselves to DSLStatements and
DSLExpressions (if otherwise unreleased) in order to leverage this
side-effect. DSL FP support was removed at
http://review.skia.org/540301, and those side-effects were removed.
At that point, these destructors became dead code.
Change-Id: Iec801d66b9ee9ea8c93f7c851966af38c6446037
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542526
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Arman Uguray <armansito@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Previously the function emitted for complex gradients looked like
if (t <= 0) {
ret = color[0]
} else {
if (t <= stop[1]) {
ret = interp(t, color[0], color[1])
} else {
if (t <= stop[2]) {
ret = interp(t, color[1], color[2])
} else {
if (t <= stop[3]) {
ret = interp(t, color[2], color[3])
} else {
if (t <= stop[4]) {
ret = interp(t, color[3], color[4])
} else {
ret = color[4]
}
}
}
}
}
So a gradient with many color stops could end up with extreme nesting of
ifelse. This could cause some viewers to reject the function due to
implementation limits. Even when not rejected the drawing would be slow
due to the linear testing of each range.
Change the function's structure to a binary search with special cases
for the begin and end ranges.
if (t <= 0) {
ret = color[0];
t = 0;
}
if (t > 0 && t <= stop[4]) {
if (t <= stop[2]) {
if (t <= stop[1]) {
ret = interp(t, color[0], color[1]);
} else {
ret = interp(t, color[1], color[2]);
}
} else {
if (t <= stop[3] {
ret = interp(t, color[2], color[3]);
} else {
ret = interp(t, color[3], color[4]);
}
}
t = 0;
}
if (t > 0) {
ret = color[4];
}
This change allows gradients with many stops to work in all known
viewers and greatly speeds up their drawing in viewers which did manage
to render with the old function. The function text is slightly larger
with this change, but minimally. If this becomes a size issue it may be
possible to size optimize gradients with few stops in the future.
Props to Bram De Cooman for suggesting this form for the function, the
initial implementation [0], and testing to make sure this approach would
resolve the issue.
[0] https://skia-review.googlesource.com/c/skia/+/540136
Bug: chromium:1316377
Co-authored-by: Bram De Cooman <bram.decooman@gmail.com>
Change-Id: Ib29cbb17220b9c32105b02222c1e1615ce0196bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541639
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Change-Id: I4ea10bd1354d6f68a6940fd48782bfa00bb3b343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542417
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
These tests would be expected to trigger the CFI sanitizer (skia:13339).
We don't currently support CFI sanitizers, though (skia:13338).
Tests are confirmed to fail in Linux by copying TArray unit tests into
SkSLCompiler and running Chrome unit tests:
http://screen/3cYe9r4MYSXmTTa
Change-Id: If2d9e611bc9902e2ac6e0960a38e876bb1952884
Bug: skia:13338, skia:13339
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542639
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Change-Id: I6db6a31834e55ebcc4af9064eac372281f341533
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542644
Auto-Submit: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:13118
Change-Id: Id4afcfeeb9a5b44a0e2cb24b70c76a81ec5daaea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542300
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Change-Id: I6db702c2d231bfe527b3fd4bf4f99beb6e58bf8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542641
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
An undocumented feature of Preview 11.0 (1033.3) is that `a n mod r eq`
(with a and n both integers, r integer or real) early aborts a function
when false would be put on the stack. Work around this by re-writing
`t 2 mod 1 eq` as `t 2 mod 0 gt`.
Bug: chromium:1316377, chromium:1170345
Change-Id: Ibb6eaaf36f276e9ef2906159622ab44b35289492
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542638
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
Fix for this landed in ANGLE.
Bug: skia:13290
Cq-Include-Trybots: luci.skia.skia.primary:Test-Mac10.15.1-Clang-MacBookAir7.2-GPU-IntelHD6000-x86_64-Debug-All-ANGLE
Change-Id: Id14366c5f9fe33bf1347a9bdaad12ec38b93fa6b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542640
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
In the curve tessellating render step, a path that is only linear
segments is entirely handled by the inner triangles, so the patch
writing never actually emits any instances. This is detected after
the fact by seeing that the per-instance vertex count remains 0, so
we just skip issuing the draw call.
Change-Id: I708797484073f5afcfb5d734467378861091ce8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542616
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Graphite will also need tokens to track draw and upload progress, so
moved the token class to a shared location, and tokens into the Plot
class.
Bug: skia:13118
Change-Id: Ib5748a7f25d79ee0aabca7427511cc9fd6e75117
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541740
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The ShaderType enum seems like it will persist while the ShaderCombo will not.
Bug: skia:12701
Change-Id: Idf9ed89a50bac46b93a81da3e814207c83b3a1ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541723
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
I've created a real bug and assigned to Joe Gregorio for now (please
reassign as you see fit).
Change-Id: I0cc1ef52b254904e545c9c266df484816de60021
Bug: skia:13344
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542636
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Instead of taking the minimum required index/vertex count for an
instance, DynamicInstances::append() now takes a proxy. The proxy can
be used to calculate the index when it's needed at flush time, and can
accumulate its internal state to represent a worst case that
corresponds to the worst case index count as well.
Updates the tessellating RenderSteps to use a single PatchAllocator
definition that just swaps out how tess::LinearTolerances are converted
to a vertex count. This will also work seamlessly with
FixedCountStrokes.
Removes stride() function from PatchAllocator because it's clunky to
try and pass that out of DrawWriter or RenderStep to PatchWriter, and
can be re-computed from fAttribs anyways. Since it's only re-used in
PatchWriter for the deferred patch, it's one sum for every path contour
and will likely not be noticed in perf. If it is, we can have
PatchWriter remember it instead and keep PatchAllocator simple still.
Bug: skia:13056, skia:13012
Change-Id: I52a36da4df23ebb01a98246458b6538744c1815f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/540620
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Removes specializations for all() on AVX2 and SSE 4.1, which give the
wrong results if the ints didn't have all bits set (inconsistent with
other platforms and non-SIMD). Added a unit test that checks this case.
The mirror specializations for any() on AVX2 and SSE 4.1 are actually
valid, so added those, and added a 2 instruction specialization for
SSE for any() and all(). This is what clang-trunk produces on -O3, but
ToT clang struggles to vectorize it.
Also adds specializations for NEON for any() and all(), since even
clang-trunk was struggling to vectorize it automatically. In
particular, this will help skgpu::graphite::Rect's implementations of
intersect and contains, which use any/all to get a final boolean value.
In the Instruments app, I had see Rect's intersection as a hotspot
on the Mac M1, and this vectorization helps a bit.
Also takes the opportunity to remove fake C++14 constexpr for a
real constexpr.
Change-Id: Ib142e305ae5615056a777424e379b6da82d44f0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542296
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Change-Id: I012ea84083a592d7d2303067128857c01cd770dd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541176
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Change-Id: Iad3db284022ad2488119997a2d466a0eee8974b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541136
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Bug: skia:13329
Change-Id: I9992cd5149e50f9a998af712984a82e5d952d58b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541936
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
The only usage of `virtual` was the `storage()` method. This value is
now stored in a member variable. It is packed next to a bool so it
should be ~zero extra space, versus 8 bytes for a vtable pointer. We
also save cost of a virtual dtor call every time a DSLVar goes away.
Because VariableStorage is not a public type, this required shuffling
some constructors around to live in the cpp instead of the header.
Change-Id: I9fdefc3696d123848fb567029c051b478349cec7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542139
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Change categorizeGlyphRunList to be static, and take the
strike cache as a parameter. This will simplify hoisting this
to the SkGlyphRunListBuilder.
Change-Id: I34b7d71aedc0dc67ae256d0121999fca2981a89f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541744
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Change-Id: I511f6105537b24953de1533ad7b73d1186afd4fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541060
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Also fix various compile issues if you try to compile w/o SkSL.
Change-Id: I9a5a176254184a04f4c8af93a8dca958c8c69fee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542142
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
It can live in the base class instead of having matching implementations
in each subclass. Also, there didn't seem to be a reason to have a
templated and non-templated version which did the same thing.
Change-Id: I312af9e71561d847580430000fbfae6fa99fe837
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542140
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Moving DistanceFieldAdjustTable, SDFMaskFilter, and
SDFTControl.
Bug: skia:13118
Change-Id: Id9ace9bc02068437c0f4fd674e5864da83f83228
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542297
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
The DSLVar class hierarchy had many constructor variations to support
the idea of creating DSL variables without explicitly assigning a name
to them. Instead, name-mangling would automatically assign them a name
like `_123_var`. This was designed to make it easlier to write a
complete shader in DSL.
We no longer have DSL name-mangling, or intend to create complete
programs in pure DSL. In the absence of name-mangling, these
constructors aren't useful (every variable would be named `var`).
They have been removed.
Change-Id: If533e479cc04c5a6ced9a7e880dcc56063f29374
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/542138
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
I believe this was only needed during the sprint, before we had
RenderSteps.
Bug: skia:12701
Change-Id: I4945fc69f9c1cd419b8c3143b6638e4769761ff9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541976
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This also makes the PaintCombinations object opaque to the user.
The eventual goal is to have all the bits of the combination allocated
in the builder's arena.
Bug: skia:12701
Change-Id: I9de8ab1fb6a340cee84c2bd437ddd9b0aa3ed6ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541222
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
This was no longer referenced anywhere.
Change-Id: I479c70e528e48c8c1a8945230739ecf0a87c3f9d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/541746
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
