Commit Graph

9348 Commits

Author SHA1 Message Date
Derek Sollenberger
396cd1dfab Revert "Lift atlas clip FP creation out of GrClip::apply"
This reverts commit 92b35673c5.

Reason for revert: breaking the android roll (see ag/14076257)

Original change's description:
> Lift atlas clip FP creation out of GrClip::apply
>
> Atlas clips always had a potential point of failure: If the SDC's
> opsTask ever got closed between GrClip::apply and
> GrOpsTask::addDrawOp, their mask would have gotten sent to the wrong
> opsTask. It didn't _look_ like this could happen with the current
> code, but it could have also been inadvertently changed quite easily.
>
> This CL adds a "pathsForClipAtlas" array for GrClip::apply to fill out
> instead of creating FPs. The SDC then generates the actual clip atlas
> FPs once it knows exactly which opsTask they will belong in.
>
> Bug: chromium:928984
> Change-Id: I507ab13b2b5e8c3c3c1916d97611297dbbd8a522
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389926
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>

TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com,adlai@google.com

Change-Id: I9597d822a9f31b7070aee691ddf3a52f4f424a24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:928984
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392339
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>
2021-04-02 15:44:47 +00:00
Chris Dalton
92b35673c5 Lift atlas clip FP creation out of GrClip::apply
Atlas clips always had a potential point of failure: If the SDC's
opsTask ever got closed between GrClip::apply and
GrOpsTask::addDrawOp, their mask would have gotten sent to the wrong
opsTask. It didn't _look_ like this could happen with the current
code, but it could have also been inadvertently changed quite easily.

This CL adds a "pathsForClipAtlas" array for GrClip::apply to fill out
instead of creating FPs. The SDC then generates the actual clip atlas
FPs once it knows exactly which opsTask they will belong in.

Bug: chromium:928984
Change-Id: I507ab13b2b5e8c3c3c1916d97611297dbbd8a522
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389926
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2021-04-01 20:21:18 +00:00
Brian Osman
d18967c67d Add integer uniforms to runtime effects
Bug: skia:11803
Change-Id: I925f14be282b96355721986de6049090b35adf3d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391856
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-04-01 17:20:03 +00:00
Brian Salomon
d8db5884de ComparePixels helper takes GrCPixmaps
Change-Id: I56d902b652663df7893626d19bc88d18f980c02c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391301
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2021-03-31 20:38:22 +00:00
Chris Dalton
8ed7a8d1c6 Extract a GrVertexChunkArray class with a builder
This will be used by the new stroke tessellator. All the other
tessellators should start chopping and chunking too. That will allow us
to quit cropping paths if we are afraid they might need more segments
than are supported.

Bug: chromium:1172543
Change-Id: I30f0ebb581f56cac099d8c05e0e181c4657c3db8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390096
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-31 17:27:52 +00:00
John Stiles
22a54543a8 Add support for constant-folded array inequality checks.
This isn't something I expect to occur often. However, we have always
reported that array-ctors with constant arguments are compile-time
constants, and `compareConstant` is supposed to be implemented for all
compile-time constants.

Change-Id: I0bbfe2a3f78c29c14f69c3b6faca71067a0e45c8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391116
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-31 15:50:02 +00:00
John Stiles
8037c9e2ed Add test for folding of == and != for arrays.
We don't directly support this today at all. In practice, though, simple
constant arrays are detected as equal in the constant-folding pass
because they hit the `x == x` self-equality check (using
`IsSameExpressionTree`).

This does not work for our inequality tests, though, so those do not
fold.

Change-Id: I6730a9a2d1da9ac613ee58889d651f3ff65b1d2d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/391057
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>
2021-03-31 15:32:03 +00:00
Brian Salomon
5392c94fe8 GrConvertPixels takes pixmaps
Add GrCPixmap, a GrPixmap but with const void* instead of void*. Share
impl via template base class GrPixmapBase.

Change-Id: I7dfdf24a73c1bc8557ff7b90f93a9399da2f3f75
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/350022
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
2021-03-31 00:23:17 +00:00
Chris Dalton
9b5b7db793 Move GrStrokeTessellator into its own header file
Simple refactor to make things reusable for the next non-hardware stroke
tessellator.

Change-Id: I0898b54a616e60f0475ac74cbd6f518e8696e0e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/390078
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-30 18:00:16 +00:00
Brian Salomon
1d2a26d0df Add more comprehensive test for GPU write pixels.
Similar to existing SurfaceContextReadPixels but for writes. Tries all
combinations of src/dst color type and alpha type for write pixels.
Always reads back pixels for verification using the ImageInfo of the
tested surface context.

Bug: skia:8862
Bug: skia:11130
Bug: skia:11786

Reviewed-on: https://skia-review.googlesource.com/c/skia/+/348886
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>
Change-Id: I22b38a99c8b4e5b506cd0b94ab9cac3d347f6b2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388236
2021-03-30 15:50:36 +00:00
John Stiles
fabed8bb79 Fix fuzzer-discovered error with variable declarations.
As soon as a single VarDeclaration is successfully created, its Variable
is added to the current symbol table. However, if a variable-declaration
line declared several variables in a row, we would stop if ANY of the
declarations contained an error and discard the entire statement, but
would continue processing the rest of the program. This left us in a
position where some Variables existed in the SymbolTable with valid,
reachable names, but their corresponding VarDeclaration statement had
been thrown away as erroneous. Since Variables point back to
VarDeclarations for their initialValues, this gave us a stale pointer.
Any future reference to that variable name which could trigger an
access to its initialValue would read from this dead pointer.

This CL fixes the conversion of VarDeclarations so that we no longer
throw away any VarDeclarations associated with a successfully-parsed
Variable.

Change-Id: If8ec3c160933e48a0e1f36414234b3a849d8978c
Bug: oss-fuzz:32587
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389636
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>
2021-03-29 14:35:09 +00:00
John Stiles
2396fb8be5 Convert division-by-literal in SPIR-V to multiplication.
This is particularly relevant for "vector / scalar" divides, as these
end up generating an extra OpFDiv to take the reciprocal of the scalar.
This meant we were generating code to calculate "literal 1 / literal N"
at runtime whenever dividing vector V against scalar N. This fix moves
the reciprocal calculation to compile-time. And of course, it's a valid
strength reduction in all cases.

Change-Id: I0986057e516120fe872e70135f1ac365155d5377
Bug: skia:11792
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389156
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-26 02:30:54 +00:00
John Stiles
d94bfdd7d8 Fix codegen for integer vector-scalar arithmetic in SPIR-V.
Due to missing type-checks, we would emit `OpVectorTimesScalar` when
multiplying an ivec with an int, or an `OpFDiv` when dividing an ivec
against an int.

Change-Id: Idc214dbe0ec208cb44f28b22e585584ac2ab7dae
Bug: skia:11267, skia:11788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388742
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-26 02:30:04 +00:00
John Stiles
2febb5b423 Add SkSL test for vector-scalar math.
We had constant-folding tests for vector-scalar arithmetic, but didn't
have an equivalent SkSL unit test for vector-scalar arithmetic that
actually needs to be computed at runtime.

This exposes two SPIR-V bugs: one was previously known, but the other
is a new discovery.

Change-Id: I28737128f20b445797c6c29872335d05f94cc95c
Bug: skia:11267, skia:11788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388739
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-25 23:30:34 +00:00
John Stiles
0c9d888748 Run SkSL inliner tests as part of dm.
Previously, these tests were never actually executed, only read during
code review. They are now properly tested for correctness whenever dm
is run. Non-ES2 compliant statements (do/while/switch) are unfortunately
excluded here, as they are not compatible with Runtime Effects yet.

Change-Id: I965c782baad6f8dd3961a400ae791fb2c1f844d3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389296
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-25 22:58:54 +00:00
John Stiles
9e13fe86b1 Rewrite matrix * vector multiplication on ARM GPUs.
If sk_Caps.rewriteMatrixVectorMultiply is false, we generate the same
code as before.

If sk_Caps.rewriteMatrixVectorMultiply is true, and optimization is
enabled, medium-precision multiplication of `matrix * vector` is
rewritten in BinaryExpression as the sum of
(m[0]*v[0] + m[1]*v[1] + ... + m[N]*v[N]).

Change-Id: I9796158fd4203b5cd2278c0d0e9a2e8cda82b136
Bug: skia:11769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388460
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-24 19:21:44 +00:00
John Stiles
85749c0b22 Add caps bit for RewriteMatrixVectorMultiply.
On ARM GPUs, the Vulkan driver does not honor relaxed precision when
evaluating SpvOpMatrixTimesVector. This leads to reduced performance
(compared to GLSL, where mediump matrices and floats do evaluate all
intermediate values at mediump).

This caps bit will be enabled on ARM GPUs and, in a followup CL, will
be used to toggle a workaround where `m*v` is rewritten as the sum of
(m[0]*v[0] + m[1]*v[1] + ... + m[N]*v[N]).

Change-Id: I310fa73639b6498552c9672e76860f2eded15d0a
Bug: skia:11769
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388459
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-23 22:40:02 +00:00
Ethan Nicholas
7f01588f15 Revert "Revert "Added more RelaxedPrecision decorations""
This reverts commit 7bba1f55e8.

Change-Id: I707a3c215f37376086e22eaa43916afeed6da4c7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388456
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-23 19:34:42 +00:00
Brian Osman
d1f3b97a64 Remove sk_SampleMask support from SkSL
Change-Id: I22837a4921238749664217e595d24d196503534d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388096
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-23 14:37:28 +00:00
Robert Phillips
f10535f6fa Fuse GrGLSLPrimitiveProcessor and GrGLSLGeometryProcessor ...
into a new GrGLSLGeometryProcessor. Since NVPR is no more this
distinction (between GLSL- Primitive and Geometry -Processor)
probably isn't needed/useful.

Bug: skia:11760
Change-Id: I75621725bd2b0ef3dbac2ea6449bd571551babab
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388036
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-23 14:30:18 +00:00
John Stiles
9d26af9a82 Fix testing of Runtime Effects with inlining enabled and disabled.
Previously, the act of painting a Runtime Effect was causing its helper
functions to get inlined, even if inlining was disabled during the
initial SkSL generation. This meant that the "NoInline" path was not
actually very effective.

Change-Id: If8e3933be61df4a49d2e11d916d7fff22876315e
Bug: skia:11362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/388099
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-23 14:23:28 +00:00
Chris Dalton
4ac0cbe769 Remove the scary raw pointer from GrCCClipProcessor into GrCCClipPath
This also allows us to rip out extra work ccpr was doing to maintain
the lifetimes of these objects.

Bug: chromium:928984
Change-Id: I094784235e23f00fb2ed10756a5eaa22a6028c2e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387356
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-22 21:45:17 +00:00
John Stiles
1b843bdcb7 Fix up outdated tests.
Removed error tests which no longer test anything:
- UseWithoutInitialize...: we no longer track variable use-before-init.
- InlineDivideByZero: we no longer constant-fold the result of an
  inlined function call into its parent statement.
- Unreachable: we no longer report unreachable statements.

And fixed some minor test-case issues:
- StaticSwitchConditionalBreak: we still check this, but the function
  was being dead-stripped before this check could run. Renamed to main.
- OssFuzzXxxxx: these cases no longer report errors, but they are still
  valuable as regression tests; moved to `shared/`.

Change-Id: Iade3cff821dc998cacfd02f62d3ac4625e48904c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387820
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 21:11:47 +00:00
Robert Phillips
787fd9d80a Fuse GrPrimitiveProcessor & GrGeometryProcessor into a new GrGeometryProcessor
With the removal of NVPR we no longer need this distinction.

Bug: skia:11760
Change-Id: I225a4feb764395fb72aca3ffc8b6d05396bf0b1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386890
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2021-03-22 19:44:40 +00:00
Robert Phillips
22970175f7 Remove sample locations usage from GL backend
Bug: skia:8921

Change-Id: If9e65a78f9e1fddb5c5ad5afa5612294935122e5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386837
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-03-22 18:00:06 +00:00
John Stiles
f9e8551edf Mangle function names in SPIR-V.
SPIR-V doesn't seem to mind overlapping function names, since they're
not load-bearing in any way, but this keeps us consistent with the other
code generators.

Change-Id: Ifdb4cb17795da88eabc0db841af746fb76caf423
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387757
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-22 17:32:46 +00:00
John Stiles
e1068349fc Mangle function names in Metal.
Change-Id: Ib9a1a51f8a29b2d03792a3abd93b3a9a321b5464
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387756
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-22 17:23:21 +00:00
John Stiles
ece1d794b9 Mangle function names in GLSL.
This will be implemented in Metal and SPIR-V in followup CLs.

Change-Id: I397b4db40b15dd54cf1d8a17f414c3fe184b48d2
Bug: skia:10851
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387638
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-22 17:18:26 +00:00
John Stiles
e67bd1371e Migrate VarDeclaration conversion out of IRGenerator.
Change-Id: If5fb4f99d327bb429f60e8d6c526720dd02b0928
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386800
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-22 14:25:26 +00:00
Mike Reed
7db854c779 Hide getFilterQuality
Skia does not call set or get filter-quality any more
(except for legacy picture deserialization)

Change-Id: I504caf407ca68392481b771040e5d3280bf7da7f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387439
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2021-03-21 02:23:27 +00:00
Mike Reed
12a75588e7 GrFPArgs no longer needs sampling
Change-Id: I20fc4b4c3d6997f3ebc1d6d0049447b348f8a110
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387416
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2021-03-20 17:33:56 +00:00
Mike Reed
7cee3efb6b FilterQuality should no longer be needed.
Start to clean up.

Change-Id: I6dea9344d8a19010d6e22ee8d0cd2b795910a82c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386843
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2021-03-20 14:30:06 +00:00
Chris Dalton
bf9c63b760 Don't fail clip atlas instantiation when the access is out of bounds
This means the draw is entirely clipped out, so we just don't even
create the FP to begin with.

Change-Id: I6d8a2a2e18be07c8a1408437c4bcc3d9349b77a2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/387057
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-20 01:35:04 +00:00
Adlai Holler
7df8d22c40 Do register allocation in GrResourceAllocator (take 2)
This lets us plan out the allocation of resources without
actually committing to the resulting plan. In the future,
the user will be able to do the register allocation, then
query the estimated memory cost, and either commit to
that allocation or try a different order of operations.

The difference between this and the original 286097 are that we sorted
fFinishedIntvls by increasing start instead of increasing end and we
use the GrUniqueKey.hash instead of the default crc hash.

Bug: skia:10877
Change-Id: Idc405e2b4532c4cd0ae4127210ba3b42de27bd46
Cq-Include-Trybots: luci.skia.skia.primary:Canary-Chromium,Test-Debian10-Clang-GCE-GPU-SwiftShader-x86_64-Debug-All-SwiftShader_MSAN
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386888
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
2021-03-19 20:05:52 +00:00
John Stiles
b66645830d Only run the inliner one time.
This saves a significant amount of CPU time and, now that the inliner
can handle nested expressions, still inlines almost everything.

Change-Id: I8f198630fa9627bc433ef8fb72f6bcf94595cdaa
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386917
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-19 17:02:21 +00:00
John Stiles
708faba16b Allow multiple expressions on the same statement to be inlined.
This will allow the inliner to successfully do more work in a single
pass.

Change-Id: I26e8831737c10bdf9a35eebd94ea8b74f6487077
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386916
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-19 15:14:24 +00:00
John Stiles
049f0dfaba Reduce unnecessary scratch variables in Inliner.
Arguments without side-effects that aren't read from more than once can
be moved directly into the inlined function, and don't need a scratch
variable. This can allow functions like `guarded_divide` to inline
completely in more cases.

Change-Id: I0bfce35635cf9779f4af1bc0790da966ccfe4230
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386678
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-19 14:10:29 +00:00
Robert Phillips
31798c2796 Remove NVPR
Bug: skia:11760
Change-Id: Ie0fc1aaa3120b37b1d452fdc9a8b5cb91b6ffe1e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386559
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-03-19 12:26:38 +00:00
John Stiles
7bba1f55e8 Revert "Added more RelaxedPrecision decorations"
This reverts commit ab52d95634.

Reason for revert: didn't intend for this to submit, oops

Original change's description:
> Added more RelaxedPrecision decorations
>
> Change-Id: I3814e7144f22c8f838082df6ed1f41119efb2ec2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385157
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I8190069d8a428e541782cb2b47ec280f4c7e5686
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386956
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-19 03:01:50 +00:00
Ethan Nicholas
ab52d95634 Added more RelaxedPrecision decorations
Change-Id: I3814e7144f22c8f838082df6ed1f41119efb2ec2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385157
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-19 03:01:07 +00:00
Mike Reed
10a5ff2cac Must pass filtering to picture shader
Change-Id: I820867df80daa1594d6202cad5e8e95c060293fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386838
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Florin Malita <fmalita@chromium.org>
2021-03-18 22:18:47 +00:00
Adlai Holler
9f358825f9 Revert "Do register allocation in GrResourceAllocator"
This reverts commit c6f78ff55d.

Reason for revert: Broke Chrome roll and MSAN

Original change's description:
> Do register allocation in GrResourceAllocator
>
> This lets us plan out the allocation of resources without
> actually committing to the resulting plan. In the future,
> the user will be able to do the register allocation, then
> query the estimated memory cost, and either commit to
> that allocation or try a different order of operations.
>
> Bug: skia:10877
> Change-Id: I34f92b01986dc2a0dd72e85d42283fc438c5fc82
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386097
> Commit-Queue: Adlai Holler <adlai@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>

TBR=robertphillips@google.com,adlai@google.com

Change-Id: I7492c12b8188ed22c3cd80fd4068da402d8d3543
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10877
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386856
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
2021-03-18 20:41:18 +00:00
Adlai Holler
c6f78ff55d Do register allocation in GrResourceAllocator
This lets us plan out the allocation of resources without
actually committing to the resulting plan. In the future,
the user will be able to do the register allocation, then
query the estimated memory cost, and either commit to
that allocation or try a different order of operations.

Bug: skia:10877
Change-Id: I34f92b01986dc2a0dd72e85d42283fc438c5fc82
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386097
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-18 17:45:46 +00:00
Ethan Nicholas
8f352ce20c Revert "Revert "Refactored SPIR-V RelaxedPrecision handling""
This reverts commit a9c187e5cc.

Change-Id: Icbfb8abdfc67fc2e6428d97a6cdede2726fb56e4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385596
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-18 15:01:15 +00:00
John Stiles
3150839b59 Disable CommaSideEffects test on GPU.
This test causes the Adreno 330 driver to crash, and does not pass on
Quadro P400 in wasm. The CPU test confirms that we can get it right,
even if not all drivers do.

Change-Id: I5ffb72ac647a49dab7130ab2c6e94f587ded6cf9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/386216
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>
2021-03-17 21:56:00 +00:00
Leon Scroggins
d5e94d90e8 Move skbug5883.gif to a better test
Bug: skia:11754

This image is invalid - SkCodec draws *something* but it's not
particularly meaningful. Remove it from our CodecSrc tests and add it to
BadImage tests so that we still verify we don't crash (etc) but we no
longer expect to be able to draw it using the platform generator.

Change-Id: I4781d645896d9f01afbd70fb0c5acfd262dd3169
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385880
Commit-Queue: Leon Scroggins <scroggo@google.com>
Auto-Submit: Leon Scroggins <scroggo@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
2021-03-17 19:44:10 +00:00
John Stiles
bff24abab8 Disallow inlining a function with out-parameters.
It is difficult to do this both efficiently and correctly while honoring
GLSL semantics (which require the lvalues to be kept distinct, even when
they point to the same variable). We could make it work by making copies
of every out parameter in each direction (going in for inouts, and
coming out for outs and inouts).

However, this could be self-defeating if it makes it harder for the
driver to track variable lifetimes. Simply opting out of inlining these
functions entirely seems like the best tradeoff; let the driver optimize
them if it can, and we can enjoy reduced complexity in the SkSL inliner.

Change-Id: I62f7b4550cc181cfe789e4f2ff4e408ba1baf9cb
Bug: skia:11326
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370257
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-17 18:04:00 +00:00
Ethan Nicholas
e0707b7075 No longer passing the results of OpAccessChain to function calls
It turns out it is not legal to pass the results of OpAccessChain as a
function argument, for... reasons. This CL switches us over to passing
the argument via a temp variable instead.

Bug: skia:11748
Change-Id: Ib5e86c1d000655ebd7bb62ceea6a27b823808645
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385936
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-17 16:37:00 +00:00
John Stiles
dc20847579 Disallow inlining functions containing early returns.
This allows us to remove 100 LOC from the inliner and is very unlikely
to affect any existing benchmark. We don't have any evidence to support
the idea that a one-iteration `for` loop with `continue`-based exits
will be any faster than a standard function call on any existing GPU.
Our fragment processors are generally written to avoid early returns,
in large part to avoid hitting this path.

This drastically impacts BlendEnum.sksl (which can no longer flatten out
a switch over every blend function in SkSL) but is otherwise a wash.

See: http://go/optimization-in-sksl-inliner suggestion 4(a)

Change-Id: I1f9c27bcd7a8de46cc4e8d0b9768d75957cf1c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385377
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-17 16:29:00 +00:00
John Stiles
2810beed23 Improve do-while test in CanExitWithoutReturningValue.
This put the coverage for do-while loops on par with for loops.

Change-Id: I53e0d733edd02a6a139792a8d74c68116453e5ff
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385500
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-17 14:50:56 +00:00
Ethan Nicholas
961d944648 SkSL DSL now uses node pooling
Change-Id: I6404cea5267b5da5a5948f0d6246688fef1fe4c4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383758
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-16 21:18:55 +00:00
John Stiles
2654187396 Eliminate unused local variables during SkSL optimization.
This can eliminate const variables which have been completely folded
away, unnecessary synthetic variables created during codegen/inlining,
or code that simply didn't need to exist at all.

Change-Id: I37a65e455e6527a6a6c2f4dde918f48d84dc2638
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383496
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-16 17:22:53 +00:00
John Stiles
f10eff363b Fix misdetection of dead global variables.
A global variable should be considered "dead" if it's never written and
never read. The previous code checked if it was never written OR never
read, which is not the same.

This would generate GLSL/Metal that didn't compile. In SPIR-V, it would
SkASSERT, then crash, during codegen. The fuzzer was able to detect the
SPIR-V issue, but it was wrong in all three cases.

Change-Id: Id59a2499eb5baa3839b93826bfbc24191bfd490b
Bug: oss-fuzz:32005
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385280
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>
2021-03-16 16:02:03 +00:00
Michael Ludwig
bf58add00e Revert "Only include header once in combined MSL shader."
This reverts commit e7a8f85e4f.

Reason for revert: must revert dependent CL

Original change's description:
> Only include header once in combined MSL shader.
>
> Bug: skia:11389
> Change-Id: I3e24dcaa2cfeddc7efd7985f9f42a59bfc8175f2
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385137
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Jim Van Verth <jvanverth@google.com>

TBR=jvanverth@google.com,johnstiles@google.com

Change-Id: I7a886b6c57a666e54e65365e41dcb57bd9ab4ba6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11389
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385237
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
2021-03-16 14:44:25 +00:00
Brian Osman
8c595fed7c Change sksl tests to avoid SPIR-V validation errors
'in' variables without locations aren't allowed. Use uniforms instead.

Bug: skia:11738
Change-Id: Ic066106deb7409cff154b4be7cfb3e03a7025c7d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385000
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-16 13:39:53 +00:00
John Stiles
132cfdd49d Revert "Inline functions of the form 'return (expr)' only."
This reverts commit 92748af1a5.

Reason for revert: SkSLCommaSideEffects_GPU crashing on Android

Original change's description:
> Inline functions of the form 'return (expr)' only.
>
> This drastically reduces the number of functions which we allow to be
> inlined. If this change does not hurt our performance, it will allow us
> to trivially remove hundreds of LOC. All current data leads us to
> believe that it may affect the Mali 400 but is highly unlikely to change
> results on any other device in the tree.
>
> More info: http://go/optimization-in-sksl-inliner
>
> Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
> Auto-Submit: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I6a670dacaa58fe3386ff50375ac6d1cac4fd7f2c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385161
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-15 22:08:48 +00:00
Jim Van Verth
e7a8f85e4f Only include header once in combined MSL shader.
Bug: skia:11389
Change-Id: I3e24dcaa2cfeddc7efd7985f9f42a59bfc8175f2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385137
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Jim Van Verth <jvanverth@google.com>
2021-03-15 21:42:06 +00:00
Ethan Nicholas
a9c187e5cc Revert "Refactored SPIR-V RelaxedPrecision handling"
This reverts commit 9e476b798f.

Reason for revert: Angry Vulkan bots

Original change's description:
> Refactored SPIR-V RelaxedPrecision handling
>
> The RelaxedPrecision decoration is now handled by nextId(), to make it
> easier to see all spots where a RelaxedPrecision decoration might be
> necessary. The goal of this initial refactor is not to actually fix the
> issues with RelaxedPrecision decorations, but rather to lay the
> groundwork for doing so in followup CLs.
>
> The initial intent of this change was to not affect the SPIR-V at all,
> saving modifications for followups, but there ended up being three kinds
> of changes to the output:
>
> 1. Doing things at nextId() time rather than later means some
> decorations move to an earlier spot in the output. This results in
> diffs, but should not cause any behavioral changes.
> 2. We were incorrectly tagging bools as RelaxedPrecision in some
> situations. By funneling things through fewer code paths, the refactor
> would have caused this to happen in even more situations, and the code
> responsible for the bug was being rewritten in this CL anyway, so it
> seemed worth just fixing the issue as part of this change.
> 3. Funneling things through fewer code paths ended up adding
> (correct) RelaxedPrecision modifiers to binary operations that had
> previously been missing them. It seemed better to just let it happen
> than to try to maintain bug-for-bug compatibility with the previous
> approach.
>
> Change-Id: Ia9654d6b5754e2c797e02226660cb618c9189b36
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384318
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>

TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: I9ada728e5fd5798bc1179640560c2e6045b7efd1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/385158
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-15 21:24:47 +00:00
Ethan Nicholas
9e476b798f Refactored SPIR-V RelaxedPrecision handling
The RelaxedPrecision decoration is now handled by nextId(), to make it
easier to see all spots where a RelaxedPrecision decoration might be
necessary. The goal of this initial refactor is not to actually fix the
issues with RelaxedPrecision decorations, but rather to lay the
groundwork for doing so in followup CLs.

The initial intent of this change was to not affect the SPIR-V at all,
saving modifications for followups, but there ended up being three kinds
of changes to the output:

1. Doing things at nextId() time rather than later means some
decorations move to an earlier spot in the output. This results in
diffs, but should not cause any behavioral changes.
2. We were incorrectly tagging bools as RelaxedPrecision in some
situations. By funneling things through fewer code paths, the refactor
would have caused this to happen in even more situations, and the code
responsible for the bug was being rewritten in this CL anyway, so it
seemed worth just fixing the issue as part of this change.
3. Funneling things through fewer code paths ended up adding
(correct) RelaxedPrecision modifiers to binary operations that had
previously been missing them. It seemed better to just let it happen
than to try to maintain bug-for-bug compatibility with the previous
approach.

Change-Id: Ia9654d6b5754e2c797e02226660cb618c9189b36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384318
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-15 20:45:26 +00:00
John Stiles
92748af1a5 Inline functions of the form 'return (expr)' only.
This drastically reduces the number of functions which we allow to be
inlined. If this change does not hurt our performance, it will allow us
to trivially remove hundreds of LOC. All current data leads us to
believe that it may affect the Mali 400 but is highly unlikely to change
results on any other device in the tree.

More info: http://go/optimization-in-sksl-inliner

Change-Id: Ia6b706742ce5407453e0e697b6c1f9201084c0e8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384858
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-15 19:46:46 +00:00
John Stiles
65abd2f556 Disable OutParams test on GPU.
This test fails on SPIR-V when the inliner is turned off:

error: SPIR-V validation error: Pointer operand 250[%250] must be a memory object declaration
  %252 = OpFunctionCall %void %out_half %250

Change-Id: Ibfa9cef371af2eea766a4218ec8a581289ee100e
Bug: skia:11748
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384999
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-15 19:46:16 +00:00
Brian Osman
fc4b9919d3 Force global initializers to be constant expressions
Prevents us from accepting code that can't be correctly transformed to
GLSL, like:

  uniform float x;
  float y = x;

(Previously, writing code like that in a runtime effect would
effectively produce the exact same code all the way through to GLSL, and
the driver would fail to compile it).

Bug: skia:11336
Change-Id: Iaa797587c4a4a7289ed59ce2736cf0bf0fc5bca3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384698
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-15 16:54:05 +00:00
Herb Derby
310dc4e0b9 remove unused code SkGlyphIDSet
Change-Id: I66340bbe5c4fcf8f9bcdac3436602c5edf689761
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384756
Reviewed-by: Ben Wagner <bungeman@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2021-03-15 15:31:15 +00:00
Robert Phillips
d074b6293c Switch GrTextureFreedMessages over to using DirectContextIDs
Bug: skia:11728
Change-Id: I514f917577a4166c2834f72fc8c64ab85b259938
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382879
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2021-03-15 15:23:35 +00:00
Greg Daniel
e895ab2fc8 Have GrVkTexture not derive from GrVkImage.
A side effect of this change is that I've tried to pass GrVkAttachments
around GrVkGpu instead of GrVkTextures where I could to start the
transition within the backend code.

Bug: skia:10727
Change-Id: Ibc9553cdbd7f6ae845c56aad3f25f58e4c478e46
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379577
Reviewed-by: Jim Van Verth <jvanverth@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-03-15 15:18:55 +00:00
Brian Osman
4f3f64c110 Preserve 'const' on globals and function parameters in runtime effects
Bug: skia:11716
Change-Id: Ic09071544b5b5216b01fbc9b478b6269dd96202f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382280
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-13 13:32:27 +00:00
Brian Osman
ddcb4d94f9 In pipeline stage generator, never emit declarations for opaque types
This only affects fragmentProcessors (children) - and the backend SkSL
we're emitting should not contain those. We've just been silently
ignoring those declarations when converting to GLSL, MSL, etc.

Change-Id: I241f2f4fe4614b49ebccc9c2976fd408e94656d0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384316
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-13 01:46:06 +00:00
Ethan Nicholas
2f4652f309 Revert "Fixed a number of spots where we should have been using RelaxedPrecision"
This reverts commit a04692f69e.

Reason for revert: Angry Vulkan bots.

Original change's description:
> Fixed a number of spots where we should have been using RelaxedPrecision
>
> Our SPIR-V output was missing many RelaxedPrecision decorations, which
> was presumably impacting performance.
>
> Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

TBR=egdaniel@google.com,brianosman@google.com,ethannicholas@google.com,johnstiles@google.com

Change-Id: If4fe945cb363c9b61b5a4abfde649a437689d2eb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384217
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-12 18:48:57 +00:00
John Stiles
e8b5a73b56 Remove extraneous line-breaks in generated GLSL/Metal code.
I ran into an issue in an upcoming CL which generated a particularly
ugly switch statement:

    switch (x) {
        default:
             discard;}

So I cleaned this up, and while resolving this issue, managed to improve
a bunch of existing codegen as well. The formatting change has been
split out to a separate CL since it impacts so many golden outputs.

Change-Id: I7a6be29903c47560dcc7f6acd3ef15fd0c5c3c50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384179
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-12 18:39:57 +00:00
Ethan Nicholas
a04692f69e Fixed a number of spots where we should have been using RelaxedPrecision
Our SPIR-V output was missing many RelaxedPrecision decorations, which
was presumably impacting performance.

Change-Id: Iee32d4a42f37af167fe0e45f3db94c2142129695
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/384178
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-03-12 18:00:56 +00:00
Chris Dalton
03730e6ca6 Delete path caching and path rendering from ccpr
All that's left is a clip atlas renderer.

Bug: chromium:1158093
Change-Id: I8b509904a752a202ff1321e5302c41a3f57a5edb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383741
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Chris Dalton <csmartdalton@google.com>
2021-03-12 16:02:16 +00:00
Brian Osman
1f64c80bff Update SPIR-V test outputs with latest SPIRV tools
Bug: skia:11738
Change-Id: I1dd5e99830f70d72c292379a45c4e39a55588858
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383706
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
2021-03-11 21:46:01 +00:00
Robert Phillips
e7a959d8fe Expand SkMessageBus to support different unique key types
Bug: skia:11728
Change-Id: I16fb8250fa5c04ce3fe369a50d0c61a0bee46811
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/383696
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-03-11 20:38:00 +00:00
John Stiles
f3a28db703 Eliminate control-flow analysis.
We no longer derive a performance benefit from this pass in practice,
and it is a very expensive compilation step. It is also prone to fuzz-
related errors.

Doc: http://go/optimization-in-sksl

Change-Id: Ief08ffac659a8fe7fe92c92b9a5da14c9f713bc2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381261
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-11 13:24:54 +00:00
Greg Daniel
80ef70e8f5 Make sure we check for abandoned when with getBackendSurface calls.
Previously even if we release/abanoned a resource we would still return
GrBackendSurfaces from these queries even if the actual backend api
objects are no longer valid.

This hopefully fixes the attached chrome bug, but can't know for sure.

Bug: chromium:1186623
Change-Id: Ib1c03699c2c7d81f6d305428dfbf39d647d28373
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382918
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-03-11 13:02:17 +00:00
Chris Dalton
2603c1fb14 Delete coverage counting backend from ccpr
This declutters the atlas generation code so there are fewer variables.
Next we will delete all the lower level rendering code and render the
atlas with stencilPath/stencilRect instead.

Bug: chromium:1158093
Change-Id: I36cff285d0f7de6f8ece4b027e62ae84aa01adc8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380656
Commit-Queue: Chris Dalton <csmartdalton@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-11 04:26:00 +00:00
Greg Daniel
9b6e30bd67 Fix deleting of GrGpuResource when abanonded and refs go to zero.
Abandoning would cause us to unref all the command buffer usages, but
then in notifyARefIsZero we would delete the abandoned resources even
though we still had a real ref. Then when that ref went away we would
crash.

Change-Id: I05dc2ba9a67c35c36a36704f4b81d6eef4e860e6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382916
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-03-10 20:53:23 +00:00
John Stiles
4d7ac49dca Declare outputColor and outputCoverage inside emitCode.
This is useful because it allows the variables to be declared as `const`
when they are trivial values like `half4(1)`. This enables the constant
folder to simplify or eliminate them. In most cases, this is only a
small benefit, as you'd expect a competent GPU driver to do the same.
However, Mali-400 can benefit significantly from optimizing away the
multiplication against a constant half4(1) coverage in Porter-Duff.

Mali-400 performance is back to normal: http://screen/3cDxdaGkYE8oBcS

Change-Id: I21fd23f91f747079cd05b082f7b3444aeabafb93
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382476
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-10 16:03:18 +00:00
John Stiles
0dd1a77e12 Add noinline keyword to SkSL.
As you might expect, a function tagged with `noinline` will never be
considered as a candidate for inlining.

Change-Id: Ia098f8974e6de251d78bb2a76cd71db8a86bc19c
Bug: skia:11362
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382337
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-10 15:39:48 +00:00
Brian Osman
86e85537fe Test that we propagate 'const' to the GPU backend of runtime effects
Currently, only one of three uses (local variables) does this correctly.

Bug: skia:11716
Change-Id: Iad11e8e5998fcc7caee4d438e0558c5d4e2b1821
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382277
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-09 23:26:00 +00:00
Michael Ludwig
a5c90588b1 Consistently fail writePixels when rowbytes not a multiple of bpp
With skia-review.googlesource.com/c/skia/+/363782, both cpu and gpu
backends would gracefully fail readPixel requests where dst row bytes
wasn't a multiple of dst bpp. It also updated the cpu backend's
writePixels behavior to gracefully reject writePixels requests where
the src row bytes wasn't a multiple of src bpp.

GPU writePixels would not detect this and later trigger an assert
in debug builds in GrConvertPixels (caught by the linked fuzzer bug).

This adds tests to mirror the read pixels bad-row-bytes tests and
updates GrSurfaceContext::writePixels to check src row bytes vs. bpp.
I confirmed it fixes the fuzzer crash.

Bug: chromium:1185266
Change-Id: I7cd8406c65a9ba35a55d695b2f65410a1edd2a19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/382276
Auto-Submit: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-03-09 21:54:50 +00:00
John Stiles
c3ce43be8e Replace the vector<Statement> in SwitchCase with a Block.
Change-Id: Ic2d1240ab785101365b0fd934562505fb5a3e599
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381816
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-09 20:46:10 +00:00
Ethan Nicholas
24c1772ea4 Fixed an issue with DSL includes
It turned out that everywhere we were using or testing DSL code either
directly or indirectly imported big chunks of the SkSL library. These
imports turned out to be necessary; code written using just DSL.h would
fail with various template instantiation errors.

Change-Id: Iae72d15b0d6ef14614ac1a4ff08c36bc1876cd4d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381638
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-09 20:07:00 +00:00
Peng Huang
0408afc573 Fix GrGpuBuffer::onRelease() crash problem.
The crash is because method GrGpuBuffer::onRelease() is called on wrong
thread. SkMessageBus::Post(const Message& m) takes a const ref of the
message, so there is a little possibility the GPU thread received and
handled the message before SkMessageBus::Post() is returned. In that
case, the caller of SkMessageBus::Post() (AsyncReadResult) still holds
the last ref of the GrGpuBuffer, and then GrGpuBuffer() will just be
released on the wrong thread.

Bug: chromium:1185489
Change-Id: I28665dbb1db7925d59ec574e9e26385e845ff4df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380696
Commit-Queue: Peng Huang <penghuang@chromium.org>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Auto-Submit: Peng Huang <penghuang@chromium.org>
2021-03-09 18:10:00 +00:00
Ethan Nicholas
b14b63623c Added unary + and - DSL operators
These were accidentally omitted from the supported operator list.

Change-Id: Idecd17adb8b3f5043e36328c65ca12be33e990f4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381637
Auto-Submit: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-09 14:19:03 +00:00
Mike Reed
1cf56817f8 Remove supported for (removed) inheriting paint filter-quality
Bug: skia:7650
Change-Id: Ia4414d32b63b686b9987a7d1424c89fe57bd1afe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380836
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Auto-Submit: Mike Reed <reed@google.com>
2021-03-09 00:58:09 +00:00
John Stiles
5676c57220 Optimize away self-comparison in the constant folder.
Expressions like `value == value` or `color.a != color.a` can be
replaced by `true` or `false` on sight. The GLSL spec makes it clear
that checking for NaN is optional:

4.7.1 Range and Precision
"... NaNs are not required to be generated. Support for signaling NaNs
is not required and exceptions are never raised. Operations and built-in
functions that operate on a NaN are not required to return a NaN as the
result."

Change-Id: I2ad9f2dc505b638ea2904bef41b7a79a2b329551
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381262
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-08 23:38:29 +00:00
John Stiles
7a3f5506b6 Performance experiment: disable control-flow analysis.
This CL will be used to test for potential performance regressions (or
improvements) that we might cause by disabling this optimization pass.

It will be reverted in ~1 day.

Change-Id: I26b7687c341eb6d81231406381c39869cfccf6d6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/381259
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-08 19:41:19 +00:00
Adlai Holler
19fd5145a9 Simplify GrResourceAllocator API
There's really only one failure at this point, and it's a failed
instantiation. The original intention of the allocation failure
system was to only drop the ops that referred to the uninstantiated
proxies, but somewhere along the way that was lost and
we started dropping arbitrarily large chunks of ops. Lets just
bail and not crash instead.

Bug: skia:10877
Change-Id: I675358e8a1fbd2d75ea29b72ccfc50c7df90343e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371337
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-08 18:12:29 +00:00
John Stiles
51d33982ad Add Convert/Make factory functions to IndexExpression.
Change-Id: I7a7874e58bf53978afce8a41b26092406b6490ed
Bug: skia:11342
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380360
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-08 15:55:59 +00:00
Mike Reed
07ee548d5b Remove legacy picture-shader impl
Change-Id: Ibb4c49f101519b8290884f8da654b661e119de82
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380916
Auto-Submit: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Mike Reed <reed@google.com>
2021-03-07 20:55:34 +00:00
Greg Daniel
00d6cf4368 Reland "Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage."
This reverts commit 9ef3f2e3da.

Reason for revert: relanding with fix

Original change's description:
> Revert "Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage."
>
> This reverts commit 3dc6c190da.
>
> Reason for revert: hitting assert about RT having input attachment on mali bots
>
> Original change's description:
> > Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage.
> >
> > This change moves the color and resolve attachments used in a
> > GrVkRenderTarget to be a GrVkAttachment. These along with the msaa
> > attachment now mean that GrVkRenderTarget no longer needs to derive from
> > a GrVkImage.
> >
> > There are a couple ugly things in this CL since GrVkTexture still is a
> > GrVkImage since we can't share attachments between GrVkRT and GrVkTex.
> > But when that gets updated in the follow on CL things will look much nicer.
> >
> > Bug: skia:10727
> > Change-Id: I2f12674d7517c6d6dea389e2d1fb7296028bcc85
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379576
> > Reviewed-by: Brian Salomon <bsalomon@google.com>
> > Commit-Queue: Greg Daniel <egdaniel@google.com>
>
> TBR=egdaniel@google.com,jvanverth@google.com,bsalomon@google.com
>
> Change-Id: Ic46f3947ed9f7b2ca26e8418d643e7f89b6108d2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10727
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380459
> Reviewed-by: Greg Daniel <egdaniel@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

# Not skipping CQ checks because this is a reland.

Bug: skia:10727
Change-Id: I7a995ee9ad35bdac34cfcfd6b0d963c3e0bb90b8
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380460
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-03-06 18:57:11 +00:00
John Stiles
8f440b4e1e Optimize away no-op arithmetic in ConstantFolder.
Expressions like `x * 1`, `x *= 1`, `x + 0`, `x * 0`, or `0 / x` don't
actually do anything, and can be simplified to just `x` or `0`. (The
zero case must also check that `x` doesn't have side effects, because
`0 * myFunction()` still needs to call `myFunction`.)

`0 - x` is also detected and rewritten as `-x`.
`0 / 0` is left as-is.

This logic works for scalars and vectors; matrices are left as-is.

A similar optimization also occurs in the constant-propagation pass, so
we see almost no diffs in the tests. If control-flow analysis is turned
off, we do see some improvements. (I didn't reuse the existing code at
all, since it was designed around rewriting the CFG tree, but the
concept was identical.)

Change-Id: Ia99cd81f1d4cd3dafaa43ccac6a2261e3257a185
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380356
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-06 01:33:35 +00:00
Greg Daniel
9ef3f2e3da Revert "Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage."
This reverts commit 3dc6c190da.

Reason for revert: hitting assert about RT having input attachment on mali bots

Original change's description:
> Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage.
>
> This change moves the color and resolve attachments used in a
> GrVkRenderTarget to be a GrVkAttachment. These along with the msaa
> attachment now mean that GrVkRenderTarget no longer needs to derive from
> a GrVkImage.
>
> There are a couple ugly things in this CL since GrVkTexture still is a
> GrVkImage since we can't share attachments between GrVkRT and GrVkTex.
> But when that gets updated in the follow on CL things will look much nicer.
>
> Bug: skia:10727
> Change-Id: I2f12674d7517c6d6dea389e2d1fb7296028bcc85
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379576
> Reviewed-by: Brian Salomon <bsalomon@google.com>
> Commit-Queue: Greg Daniel <egdaniel@google.com>

TBR=egdaniel@google.com,jvanverth@google.com,bsalomon@google.com

Change-Id: Ic46f3947ed9f7b2ca26e8418d643e7f89b6108d2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10727
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380459
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-03-05 22:48:27 +00:00
Ethan Nicholas
fe5d6928d0 DSL var values are now specified at construction time rather than in
Declare

This solves several issues caused by the lack of ordering guarantees in
C++; it was possible for the SkSL backend to look for the value of a
variable before its Declare() call gets processed. Moving the initial
value out of Declare should fix this whole class of problems.

Change-Id: I428fe230f1c312a0128c1f00c2a36cb95f4590a6
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380358
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-05 21:47:28 +00:00
John Stiles
28054added Optimize ternary tests that check a const variable.
This enables the ternary to be optimized away in code like:
   const bool SHINY = true;
   color = SHINY ? add_shine(x) : x; // to --> `color = add_shine(x);`

Without constant propagation.

Also, I added a unit test for ternary expression simplification; I
wasn't able to find an existing one.

When the optimization flag is disabled, this CL actually removes the
optimization of `true ? x : y` --> `x` entirely; previously, this
substitution would be made regardless of optimization settings.

Change-Id: I93a8b9d4027902d35f8a19cfd6417170b209d056
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379297
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-05 21:41:05 +00:00
Greg Daniel
3dc6c190da Have GrVkRenderTarget only use GrVkAttachments and not derive from GrVkImage.
This change moves the color and resolve attachments used in a
GrVkRenderTarget to be a GrVkAttachment. These along with the msaa
attachment now mean that GrVkRenderTarget no longer needs to derive from
a GrVkImage.

There are a couple ugly things in this CL since GrVkTexture still is a
GrVkImage since we can't share attachments between GrVkRT and GrVkTex.
But when that gets updated in the follow on CL things will look much nicer.

Bug: skia:10727
Change-Id: I2f12674d7517c6d6dea389e2d1fb7296028bcc85
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379576
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
2021-03-05 20:52:15 +00:00
John Stiles
f04e09cd9b Optimize Swizzles inside Swizzle::Make.
Swizzle optimizations now occur at IR generation time. These
optimizations are redundant with the control-flow optimization phase so
they are mostly not visible in our test output, but they do affect DSL
test results. Interestingly, they do improve our test output slightly
as well, for various reasons (e.g. we do not fully optimize lvalues in
the control-flow pass).

Change-Id: I6ebe6d71a5c22d9823b5fa500e43078915cbfb45
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/372257
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-05 20:09:55 +00:00
Brian Salomon
d63638bb7b Copy on write for wrapped backend texture surfaces.
Makes SkImage_Gpu backed by two proxies, an original and a copy. The
image uses the original until a new render task is bound to it at which
point further uses of the image will use the copy. If the image is ever
used off a GrDirectContext we fall over to the copy. If the copy is
never used and never can be used by the next flush then the render
task that populates it is marked "skipped" and we don't perform the
copy.

Bug: skia:11208

Change-Id: Id255f4a733acc608c8a53c1a5633207aeafc404b
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/366282
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-03-05 19:50:05 +00:00
Herb Derby
bf2dd2af49 Reland "rename GrSDFTOptions to GrSDFTControl"
This is a reland of 40a9061203

Original change's description:
> rename GrSDFTOptions to GrSDFTControl
>
> Change-Id: Ie03fce7a99a9f71b18d54e3cd35e7675fb7f8912
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379616
> Commit-Queue: Herb Derby <herb@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>

Change-Id: Iba0816159a8ef99448a0040c487e56700b96be5d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379845
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2021-03-05 15:24:35 +00:00