afaict the perf surprises associated with:
https://skia-review.googlesource.com/c/skia/+/334417 (Remove SkBaseDevice::flush)
were bc Ganesh resolves MSAA buffers for SkCanvas::flush but doesn't do so for GrDirectContext::flush.
Where possible this CL switches SkCanvas::flush to SkSurface::flush (which will also resolve MSAA buffers) so that when https://skia-review.googlesource.com/c/skia/+/334417 relands there should not be any performance surprises.
Change-Id: I705ad6219f0f625a88cf3f9e8b2418a3182d298c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335866
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
As written today we push two sets of the paint color uniforms (4 floats
each set, 8 total): the blitter pushes one unpremul set itself to pass
to shaders that would like to use them, and when there's no shader, we
synthesize an Color4Shader using the paint color that ends up pushing
the other set, this time premul.
When the paint color is opaque there's no difference between these two
sets of uniforms, and when not, well, it's just a few cheap *hoisted*
multiplies to go from that first unpremul set to premul. So no real
pressing need to push those premul values at all.
This CL adds a new SkShaderBase subclass PaintColorShader used only for
that case where we're synthesizing a shader from the paint color, and
just does what I've described, `return premul(paint);`.
I really only did this because I've been poking around in the debugger
looking for some other bug and noticed the duplicate values in the
uniforms array. I expect no real-world effect from this CL except fewer
uniforms to look at in the debugger.
Change-Id: I5e1c17945ed77467c91e351f58fcf900674c4391
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337196
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Change-Id: I4eb9fb50e645f503d71cd6423bfc38076a7c2119
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336777
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>
SkRasterClip::setRect assumes the incoming rect is safe, and has no
knowledge of a root bounds. SkRasterClipStack does know the root bounds
that cannot be changed.
Change-Id: I4e5850aac026d153ee9a8ce2315014896fa1b1d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337076
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Main outcome here is that we only draw once per frame instead
of pinning the CPU. We also cut out a little extra traffic – an
autorelease pool per draw, but trivial. We also fix an overrelease
on NSProcessInfo.arguments, and a missing -init for the UIViewController
etc.
Change-Id: Ibb5ea5832bc65d2ff60351d4e3b76fd3b4ff88ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337116
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Also cleans up dumping of shader code to use GrShaderUtils.
Change-Id: I4d2a13e1a54284d7da0242b74cebb3900921fc35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337077
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Bug: skia:10913
Change-Id: Ie82354b05db141c8ab90b1a615ddfada4f71a98b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335049
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This simplifies the shader down to only 2 states:
- use the explicit SkSamplingOptions it has
- look at the paint's enum
Ultimate goal: remove the paint's enum
Middle goal: remove references/reliance on the enum downstream
in the code. This CL is partway on that.
Possible next step: retool legacy bitmapprocshader to explict work with
FilterOptions (not SkFilterQuality) -- so that eventually the enum
appears *only* when we see the paint, and can immediately turn it
into FilterOptions.
Another next-step: add SkSamplingOptions to drawImage/drawImageRect,
to make it easier to update call-sites to not rely on the paint.
Bug: skia:7650
Change-Id: Ic8e6e96af1dc74c9e1b398e7b66a4368461bf7ae
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/335218
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
We were always pre-loading the fragment and vertex modules, but deferred
loading all others. Those two take up about 300 KB of heap. Now, all
modules are deferred, so compiler instances that don't need them (like
the one used for runtime effects) are much smaller.
Now that we can get better fine-grained numbers, added two more
benchmarks, to track actual baseline usage, plus the usage in the two
most likely configurations.
Change-Id: Idfbcd52c8afee566ac42ab827c80c940f91c4ad7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337176
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
This reverts commit b27ba538ec.
Reason for revert: causes invalid memory accesses due to replaceClip use, and replaceClip() is probably not the right operation to use
to emulate a layer when no layer was the strategy or failed to allocate.
Original change's description:
> Simplify layer bounds syncing and no-device error handling in SkCanvas::internalSaveLayer
>
> 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>
TBR=mtklein@google.com,bsalomon@google.com,reed@google.com,michaelludwig@google.com
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1151195, chromium:1151270, chromium:1151294, chromium:1151320, chromium:1151322
Change-Id: I9db07916ffc450cc6ecc9188d72bb7c35770a974
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/337117
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
The current model is biased towards users who make lots of allocations
so the overall histogram gets more samples for high allocation/memory
use. By switching this to reporting at submit time, it should make
the reports much more even across all users.
Change-Id: I269df9ea5e54439f0cca5e7637b0f39d1eaf903a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336957
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This allows swizzle removal to apply in more cases; in particular, we
can now optimize away extra swizzles caused by zero/one swizzle-
components quite effectively.
The "trivial expression" code was lifted from the inliner. Some subtle
changes in trivial-expression determination affect the inliner's results
in boring, non-meaningful ways. In particular, multi-argument
constructors containing all-constant values are now considered trivial,
whereas previously only single-argument constructors made the trivial-
ness cut. This allows the inliner to propagate some values that it
wouldn't have before.
Change-Id: I9a009b6803d9ac9595d65538252ba81c2b7166a7
Bug: skia:10954
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336156
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
We no longer support MoltenVK, so we can't render using Vulkan on Mac,
but this change allows Macs with the SDK installed to compile Vulkan
code and use SPIRV-Tools.
Change-Id: I5baaf80de259e406495002a5fbfec89dbd1357b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336678
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
Reviewed-by: Jim Van Verth <jvanverth@google.com>
In follow on CLs we need to know what the load op is when we try to use
discardable msaa attachments. For vulkan the load op affects how we
copy the resolve attachment into the msaa attachment, which changes the
render pass we use (adds extra subpass). We need to be able to make a
compatible render pass to compile programs.
Bug: skia:10979
Change-Id: I40c23a18b251af6a2ad3b78a1f6382bdba0b90c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336598
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
There is no longer anything IRNode-centric about SkSLPool; it is not
optimized around specific allocation sizes, and could be used to
allocate anything that is associated with a Program. Update comments and
function names to reflect this.
Change-Id: Ica1a78f7415edb25b93a93d00fe54557883c5eed
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334676
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Previously, any builtin functions would be optimized as a side-effect of
optimizing programs that used them. Now that shared elements aren't
being optimized in that way, we explicitly optimize any shared modules
when they are first created. We don't remove dead elements, but we
we do substitute settings, simplify, and inline.
Bug: skia:10905
Change-Id: I701b5e9f52fb880ef3e6f4c67694d08602f47e95
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336440
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
The important part is in GrOpFlushState.h were previously
we were taking a mutable pointer to the view, which should
at least be a const pointer and was making us do funky things
in some of the calling code. But I decided to go all the way
and do a const ref instead which is The Way It Should Be (tm).
Change-Id: I399d102e8b5e0a5059168cc450ae66f12ad47e13
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336451
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
These cleanups were reverted, as they were part of the CL that added
`--` delimiters to skslc. This CL reinstates the cleanups, but does not
reinstate `--` delimited multiple-command-line support in skslc.
Change-Id: Id70ed87aa239b46d232492fc48791158b35512f3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336677
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
These assertions were all currently reachable simply by compiling the
test code in /tests/sksl/shared/. Presumably many of these will actually
require a better fix going forward.
Change-Id: If59e0bfa1b248a5db9a79def736d437a9e5f7ee4
Bug: skia:10694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336676
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
The fix submitted at http://review.skia.org/335868 did not support
casts. The fuzzer discovered this shortcoming right away.
Change-Id: I2f5166528cee41367348564d4e664476fd5704ff
Bug: oss-fuzz:27650
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/336656
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
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>
