This tightens up our intrinsics slightly; after inlining, it eliminates
one scratch variable. (We no longer need to copy `sda` into `hueColor`
as hueColor is now unchanged.)
Change-Id: Iece5ba2fe11cde54481704a1787114a2c2a66d9b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336599
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This corrects some subtle bugs that can occur with recording canvas or
if a device fails to be created for a new layer, where the stashed
matrix would not be restored properly. Since no new DeviceCM would get
added in those cases, the canvas' total matrix wouldn't get fixed in the
paired onRestore() and it would remain dirty for the remainder of the
canvas's lifetime.
After this change, the underlying SkDevice's bounds are also kept in
sync with the intent of the saveLayer when kNoLayer_Strategy is used.
Previously, the bounds would be applied to the canvas' conservative clip
and quick reject bounds, but the device would remain un-updated. As we
move towards SkNoPixelsDevice taking over the conservative clip bounds,
this ensures bounds remain up to date within a saveLayer/restore pair
even if no layer was allocated.
Change-Id: I5ca389bdd624ea7278106da863a96e9d8f90e2d1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335861
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Instead, add them to the shared program element list of any program that
needs them.
Bug: skia:10905
Change-Id: Ieb470af65eb254154d238554eecffdcbbf268cf1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335867
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
For non-degenerate gradients, the positions are pinned to [0, 1] and
forced to be monotonically increasing in the constructor of
SkGradientShaderBase. This isn't ever called for degenerate gradients,
so the average_gradient_color helper function needs to make the same
fixes or it may calculate an invalid color (e.g. negative alpha because
the original positions are not sorted).
Bug: chromium:1149216
Change-Id: I95c6e66ebb57722370ef2f5dbba3d8d66727e48b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336437
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Despite what the bug says, I believe the only issue here was that
all the flush-time opsTasks weren't guaranteed to be in
'fOnFlushRenderTasks'. Since the users of flush-time opsTasks
use GrRenderTargetContext::addDrawOp, if the opsTask were to be
split, new ops would be added to the correct/new opsTask.
Bug: skia:9357
Change-Id: I90577bcc852419a9e0c31d858f71cda9f6f6b6a3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336435
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Change program iteration so that default iteration (over owned & shared
elements) only permits const access. Add a separate non-const iterator
that only visits owned elements.
Initially, nothing is being placed in the shared list. Follow-up CLs
will move builtin variable declarations, builtin functions, etc.
Bug: skia:10905
Change-Id: I9a5b11170117bad3ff6a43aab780c1189904417c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/330477
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Change-Id: I3f6f23459c5b5572337c3803235f165b139522d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335862
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
When values from the same argument are used consecutively by the outer
swizzle, they can be merged in the inner swizzle. Merging isn't always
possible, of course, but it will be used where it can be:
`half4(1, colRGB).yzwx` --> `half4(colRGB.xyz, 1)`
`half4(1, colRGB).yxzw` --> `half4(colRGB.x, 1, colRGB.yz)`
Change-Id: Id164b046bc15022ded331c06d722f1ae3605a3bd
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335872
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>
This change will facilitate experimenting with direct-to-op
text draws. There should be no performance change because
almost all blobs are a single run.
Change-Id: I07fb3487e0601e00507403d94bc611c5022c1408
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336447
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Cubic tangents become unstable if we chop too close to 0 or 1. This
adds an epsilon to simply not chop them if it's too close.
Bug: skia:10419
Change-Id: I99e98c5d433e2cb59c767ee564e015d7ec4f7765
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336280
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
This reverts commit 5dfa02be69.
Reason for revert: Breaks zooming text on android.
Original change's description:
> tighten up device bounds for glyphs
>
> Change-Id: I61655b9492bdaacc2c1d5c7631b2e67af2b46668
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335280
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>
TBR=herb@google.com,robertphillips@google.com
Bug: chromium:1150411
# Not skipping CQ checks because original CL landed > 1 day ago.
Change-Id: I80e334192bd60995e95ecfc6d5310f5ae7af4d7c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336446
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This will reorder constructors with swizzles applied, such as
`half4(1, 2, 3, 4).xxyz` --> `half4(1, 1, 2, 3)`
`half4(1, colRGB).yzwx` --> `half4(colRGB.x, colRGB.y, colRGB.z, 1)`
Note that, depending on the swizzle components, some elements of the
constructor may be duplicated and others may be eliminated. The
optimizer makes sure to leave the swizzle alone if it would duplicate
anything non-trivial, or if it would eliminate anything with a side
effect.
Change-Id: I470fda217ae8cf5828406b89a5696ca6aebf608d
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335860
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
I've landed some CLs that should make it easier to launch apps on Xcode,
plus it looks like the latest GN has also added some fixes.
Change-Id: Ifaf051020dff7ecad7091c854b8b42fc7f475afa
No-Try: true
Docs-Preview: https://skia.org/?cl=335871
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335871
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Herb Derby <herb@google.com>
This is slightly more efficient than assigning to sk_OutColor; it saves
one assignment in the optimized code.
Change-Id: I225c428a2b0bfdb0da4b60c6a8b488f104868ea7
Bug: skia:10549
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336216
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The macro is slightly awkward right now because it also takes the
code to call into SkConservativeClip. In a later CL this will be hidden
inside SkNoPixelsDevice, and the macro only has to take one argument.
Some of the lines that will go away extend past the 120 limit. I think
it's okay visually given its use in a macro and the short lifetime of
those lines.
Bug: skia:9283
Change-Id: Id2d872e7d098d816e208438c4462251dc1c457d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335859
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
... rather than allocating an extra "result" array.
This CL also changes the sort algorithm to soldier on even after
it has found a loop.
Change-Id: I03fe8da1aade6c9461eb42e1b7d79fae562210f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335824
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Bug: skia:9283
Change-Id: I21f1e5840171929d038781ffd397203ab86927ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335857
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This was found at https://oss-fuzz.com/testcase-detail/5155684475469824
but the associated oss-fuzz issue ID appears to be misdirected (it's
showing oss-fuzz:24498, an unrelated issue).
PrefixExpressions can return true for `isCompileTimeConstant` but did
not implement `compareConstant`; the fuzzer discovered this. Because
compile-time constants can only be compared if they are of the same
kind, this means that `compareConstant` is actually comparing a pair of
expressions that are both negated. These negations will just cancel
out, so `compareConstant` on a pair of PrefixExpressions can just call
`compareConstant` on the inner operand of each expression.
Change-Id: I7793e25314e6c8a74278b73299d310794baf71f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335870
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The ByteCodeGenerator is needed for SkSL-via-skvm, but almost no one
needs the ByteCode interpreter.
Bug: b/172773885
Change-Id: Ia7b6768dbc00c6c78b971ba50f0b702536bbd5b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336016
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Setting this variable sets up the proper compiler flags and
correct minimum version in Xcode.
Change-Id: I8133994332fc9778580745a99a2d5d73a6f88382
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335661
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Change-Id: Ia2862680ac87976ddf2a845b958df055755ce527
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335869
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
The fuzzer managed to create a test case which temporarily evaluates to
expression `half2(half(0.2)) + 2` as it is optimized. This requires a
bunch of temporary nonsense math as the IR Generator is attempting to
simplify as it goes; various attempts to remove terms from the fuzzer
test-case would cause it to stop reproducing the error.
Constructor::getVecComponent assumed that any constructor with a single
scalar argument would always implement `getConstantFloat` and
`getConstantInt`; however, constructors themselves did not actually
implement these methods. This meant that nesting a scalar constructor
inside a non-scalar constructor would abort when it tried to deduce the
value inside the inner constructor.
This has been fixed by implementing `getConstantFloat` and
`getConstantInt` for Constructors. These methods will assert if the
constructor has more than one argument or is a non-scalar type. This
should allow any number of nested constructors, e.g.
`half4(half(half(half(1))))` should recursively evaluate properly,
should we somehow generate this as an intermediate expression.
Change-Id: Iaee4284cba03974443cd7b5dccfd7909c1a5f3a6
Bug: oss-fuzz:27614
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335868
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:9283
Change-Id: I1f808ec9d004f228b17a139dff7a2f44676baf50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335856
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This maintains the status quo around clipping calls with non-finite
paths.
Bug: skia:9283, skia:10970
Change-Id: I99cda50a7117464c12ca7d3409ed6b4892863ec8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335826
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
This better differentiates it from device clip bounds, and the fact that
the stored bounds have been outset by 1 to account for AA, and are only
used for quick reject purposes.
Bug: skia:9283
Change-Id: I47420f23b74e47626596f978c78f01d7d97ae808
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335828
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Reed <reed@google.com>
This is needed so that SkNoPixelsDevice can switch to using
SkConservativeClip and more fully implement the SkDevice API.
Tracking the AA-ness allows SkCanvas/SkDevice::isClipAA() to be more
accurate; previously for recording canvases, it always returns false.
Technically, it will still return false after this CL until
SkNoPixelsDevice is updated to use SkConservativClip's new AA tracking.
Tracking whether or not the clip shape remains an intersection of rects
is similarly meant so that SkDevice's getClipType and
SkCanvas::isClipRect are more accurate. Without this tracking,
SkNoPixelsDevice always claimed the clip was a rectangle. This is not
accurate, even if all it needs to otherwise track is the bounds of the
more complex shape. We got away with it because it doesn't seem to be
critically relied on during any recording steps of clients.
Bug: skia:9283
Change-Id: I3acde969aca9d7eb97a7e1c7ab8374e7230b04fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335825
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
`viewer --slide GM_runtimecolorfilter` improves 10->3ms.
Change-Id: Iab079ddbdc93fdc406372899b26dc7de223855fa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335917
Commit-Queue: Mike Klein <mtklein@google.com>
Commit-Queue: Herb Derby <herb@google.com>
Auto-Submit: Mike Klein <mtklein@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Previously, the worklists were being deleted as soon as they were
closed; by the time skslc executed, they were already gone.
Change-Id: I0d0be87525093a3ff37421cbff553fa481c8e1f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335864
Commit-Queue: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This thing is just extra state tracking we don't need. Also
update a couple out-of-date comment blurbs around there.
Bug: skia:10877
Change-Id: Ibd41afd34d85d52a3d9ebb98e700f5ce9ed3fffb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335276
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
One of the branches of this function creates a temporary
GrSurfaceContext and (recursively) calls readPixels on that object.
In the case that canvas2DFastPath is true, this GrSurfaceContext is
given nullptr as its color space. This value historically mean to
do no color conversion, but now is interpreted as sRGB.
Consequently, if the content being copied is not sRGB, this will
introduce a conversion from sRGB to the content's color space, which
will result in incorrect pixel values being read.
Bug: skia:10965 chromium:1115317
Change-Id: I62b7edb728f0dbefa9a5a9c60905dee0cde6455a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335738
Commit-Queue: Christopher Cameron <ccameron@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This reverts commit 3e1b771ce4.
Reason for revert: Not working on Windows.
Original change's description:
> Replace skslc worklist files with -- delimited command lines.
>
> Command lines with delimiters are a simpler approach; they don't require
> a scratch file to be created and parsed. (I didn't consider this
> approach until after implementing worklists.)
>
> This also fixes a minor issue with result codes when processing multiple
> files at once; in particular, unit tests can ignore compile errors, but
> regular fragment processor compilation should treat compile errors as
> fatal and stop the build.
>
> Change-Id: I3f153e7670d757c6b021bf60a260a2cd3f2090aa
> Bug: skia:10919
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334428
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: skia:10919
Change-Id: I0e4bae8a8e09c61eac4e79453fd38e5e81b29e89
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335858
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
The alloc size calculation is over estimating the size needed.
It should be using DirectSubRun::DevicePosition, which is
4 bytes, instead of the size of the union of the different
sub run types which is 12 bytes.
The DirectSubRun is by for the most common case.
Bug: chromium:1147712
Change-Id: I972b5f4e716448fc72c64204c88fbaa373043392
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335669
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
This code has rotted a bit. The glyph off set is now calculated
by mapping (0,0) through the current matrix, and the initial
matrix, and taking the vector difference. Residual is no longer
needed and all the plumbing can be removed.
Change-Id: I20b56afc6749fd26fe283a7ff22f38951da0e6f5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335823
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Herb Derby <herb@google.com>
The optimizer can now turn the expression `half4(1).xyz` into
`half3(1)`, or `half4(1).w` into `1`. This is actually a somewhat common
case when inlining chains of fragment processors, as inputs are often
overridden to `half4(1)` or `half4(0)`. This optimization also applies
to more complex cases, e.g.:
`half2(anyFunc(sqrt(2))).yxyx` --> `half4(anyFunc(sqrt(2)))`
Since the interior of the constructor is always evaluated once in either
case, it does not actually matter what the constructor contains.
Change-Id: I8d5f358502eaa8e35d4968e74fbd6b0ce2ab6365
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335818
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Change-Id: Ibe3c3d27a112df8838bc86d6c2482277fdae62af
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335821
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
