Commit Graph

9213 Commits

Author SHA1 Message Date
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
John Stiles
b9e4f649b4 Simplify constructors at IR generation time.
This performs the same simplifications as the control-flow phase, but at
IR generation time. There's no visible difference in the tests (besides
DSL) because these aren't new optimizations; they're just happening
at a different phase of compilation.

Change-Id: I26d241167b0e690b23f8f4370339714783c8d6fd
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/371482
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-05 15:07:35 +00:00
Mike Reed
9edff53a73 Revert "rename GrSDFTOptions to GrSDFTControl"
This reverts commit 40a9061203.

Reason for revert: breaking chrome roll (android-pie)?

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>

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

Change-Id: I0483cbada23c76bdd5ccdb51936bb20a819343cb
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379839
Reviewed-by: Mike Reed <reed@google.com>
2021-03-05 11:15:22 +00:00
John Stiles
b3dcbb12ef Detect functions that fail to return a value, without using CFG.
This check now runs at function finalization time, before constant
propagation has occurred; this affected the "DeadIfStatement" test.

Our detection isn't smart enough to realize that a loop will run zero
times, so it treats `for` and `while` loops as always running at least
once. This isn't strictly correct, but it actually mirrors how the CFG
implementation works anyway. The only downside is that we would not flag
code like `for (i=0; i<0; ++i) { return x; }` as an error.

Change-Id: I5e43a6ee3a3993045559f0fb0646d36112543a94
Bug: skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379056
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-04 22:47:05 +00:00
Ethan Nicholas
daed2592bb Made SkSL DSL into public API
In addition to the unsurprising changes to eliminate references to
src/, we also had to tighten up some C++17-isms as they are not
permitted in public headers.

Change-Id: Ie5005a33d7a135e69fb66beca5e7a5f960dbd453
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378496
Reviewed-by: Brian Osman <brianosman@google.com>
2021-03-04 21:03:58 +00:00
Mike Reed
ff83dda8cd Cache image behind picture-shader
PictureShader = picture + tiling + depth/colorspace + filtering [+ scale]

Today we cache the imageshader that is used to rendering. However, the
key for that cache is the pictureshader's ID itself... which means if
we have several, all using the same picture (but maybe diff tiling) we
would create dup cache entries.

Idea:
1. only cache the image (rastered picture), not an imageShader
2. key the cache on the picture's ID, not the shader's

Several implications of this:

1. Should get more cache reuse, since we don't care about the
shader's ID (which is just wrapping a picture+tiling, etc.)

2. We also eliminate the indirection of creating a PictureImage. Instead
we're creating real (pixel) images, and caching those. This removes one
extra layer of "cache".


Idea: when we cache something for pict
Change-Id: I51cf4e9bff3c91ce1872876597d3d565039d8c7a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377844
Commit-Queue: Mike Reed <reed@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
2021-03-04 20:48:30 +00:00
Herb Derby
40a9061203 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>
2021-03-04 19:49:10 +00:00
John Stiles
2b6ec98a82 Disallow unscoped for blocks which declare a variable.
This should be legal, and we support this, but some versions of Android
do not: http://screen/3bkQewHF3xUMn5v There's no point in allowing
these shaders to exist; they can't compile on real-world clients, and
these vardecls are borderline meaningless (as the variables being
declared aren't reachable by any other statements).

Change-Id: Ie1351933c90caee9124eeab8983364ec030b2653
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379584
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-04 17:04:40 +00:00
John Stiles
03467a53e6 Revert "Disable control-flow analysis in SkSL. (Performance experiment)"
This reverts commit 50b1b2b90d.

Reason for revert: ending experiment

Original change's description:
> Disable control-flow analysis in SkSL. (Performance experiment)
>
> This CL will be used to test for potential performance regressions (or
> improvements?) that we might incur by disabling this optimization pass.
>
> It will be reverted in ~1 day.
>
> Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
> 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: Ie385a82db237ff5651348d82b9651f8ba09375b9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379581
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-04 15:20:09 +00:00
Brian Osman
48d7f7ca20 Add new style key information to several core effects
Covers some common geometry processors, texture effect, etc.

This also rearranges how fp keys are arranged in the overall key. We no
longer include the key size as part of the key - this made no sense.
Instead, we explicitly include the number of children. We also put all
data for one fp before any children, so the tree can be reconstructed
more-or-less top-down.

Finally, added an "addBool" helper that reads nicer than addBits(1)
everywhere.

Bug: skia:11372
Change-Id: I4e35257fb5923d88fe6d7522109a0b3f4c4017d4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379059
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-04 14:49:07 +00:00
Adlai Holler
55aaefe687 Reland "Support sharing promise images between DDLs"
This reverts commit 38b9a4bc3e.

Reason for revert: Fixed ASAN, TSAN, and other bugs via other CLs.

Original change's description:
> Revert "Support sharing promise images between DDLs"
>
> This reverts commit 07e11d48cb.
>
> Reason for revert: Broke DDL3_ASAN and DDL3_TSAN
>
> Original change's description:
> > Support sharing promise images between DDLs
> >
> > - Migrate our code to SkImage::MakePromiseTexture
> > - Have DDLTileHelper share one SKP and one set of promise images across all tiles.
> > - Disallow on-the-fly allocation of mips for promise textures.
> >
> > Bug: skia:10286
> > Change-Id: Ie35976958454fc520f3c9d860e6285441260c9f7
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291938
> > Commit-Queue: Adlai Holler <adlai@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,adlai@google.com
>
> Change-Id: I939b14875d1a20e4a92eab94680adcfe9596ad81
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10286
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375738
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>


Bug: skia:10286
Change-Id: Ibfd7dfcd72f10a4e29a87fa8c610f2dfd018e0db
Cq-Include-Trybots: luci.skia.skia.primary:Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-DDL3_ASAN,Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Release-All-DDL3_TSAN
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375739
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
2021-03-04 13:10:37 +00:00
John Stiles
4633c9149b # Enter a description of the change.
Reland "Migrate if-statement simplifyStatement logic to IfStatement::Make."

This reverts commit 7e685f0377.

Reason for revert: fixed SkSLBench perf test

Original change's description:
> Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
>
> This reverts commit e4da7b672f.
>
> Reason for revert: breaks SkSLBench perf test
>
> Original change's description:
> > Migrate if-statement simplifyStatement logic to IfStatement::Make.
> >
> > This performs essentially the same simplifications as before, just at
> > a different phase of compilation.
> >
> > Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> > Bug: skia:11343, skia:11319
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
> > Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
>
> TBR=brianosman@google.com,ethannicholas@google.com,johnstiles@google.com
>
> Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:11343
> Bug: skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>

Bug: skia:11343
Bug: skia:11319
Change-Id: I74cc3295004133e9fdcf16e388106eb83603f526
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379063
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 22:21:36 +00:00
John Stiles
50b1b2b90d Disable control-flow analysis in SkSL. (Performance experiment)
This CL will be used to test for potential performance regressions (or
improvements?) that we might incur by disabling this optimization pass.

It will be reverted in ~1 day.

Change-Id: I775cdb0c95df81fa25ebbd66e4ff01f64c660f68
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378456
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-03 22:08:56 +00:00
John Stiles
7e685f0377 Revert "Migrate if-statement simplifyStatement logic to IfStatement::Make."
This reverts commit e4da7b672f.

Reason for revert: breaks SkSLBench perf test

Original change's description:
> Migrate if-statement simplifyStatement logic to IfStatement::Make.
>
> This performs essentially the same simplifications as before, just at
> a different phase of compilation.
>
> Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
> Bug: skia:11343, skia:11319
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>
> Reviewed-by: Ethan Nicholas <ethannicholas@google.com>

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

Change-Id: I0051188ffe69426904066eb60a932435efdc2af8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:11343
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/379062
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 20:50:37 +00:00
Brian Osman
4c5943781e Emit new style key information in generated effects
All layout(key) fields include the field name meta-data, and use as few
bits as possible.

Bug: skia:11372
Change-Id: Ie12b3e0d01148457e5ea078cbf7d0a4bff35302e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378596
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-03-03 20:39:36 +00:00
John Stiles
e4da7b672f Migrate if-statement simplifyStatement logic to IfStatement::Make.
This performs essentially the same simplifications as before, just at
a different phase of compilation.

Change-Id: Ia88df6857d4089962505cd1281798fda74fd0b02
Bug: skia:11343, skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376177
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-03-03 18:12:35 +00:00
John Stiles
2dda50dd50 Add return-value check to the function finalizer.
Rather than have the inliner own this responsibility, the function
finalizer now detects if a function is supposed to return a value but
never actually does. This will allow us to detect this error case even
if the inliner is disabled. The inliner should no longer encounter
functions that claim to return a value but don't, so it will now assert
if one is encountered. (The inliner still has the logic to handle this
case gracefully, just in case.)

The check is currently very simple and doesn't analyze the structure of
the function, so it won't report cases where some paths return a value
and others don't, e.g. this will pass the test:

    int func() { if (something()) return 123; }

(This is good enough to resolve the inliner issue, though, as it only
occurred in functions with no value-returns at all.)

Change-Id: I21f13daffe66c8f2e72932b320ee268ba9207bfa
Bug: oss-fuzz:31469, oss-fuzz:31525, skia:11377
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377196
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-03 16:29:22 +00:00
Herb Derby
9ad09829f5 encapsulate GrSDFTOptions better
The logic for picking what type of sub run to create is spread
over several locations. Gather that altogether in drawingType().

Have GrSDFT close over all the data needed to calculate the
drawing type. This reduces plumbing to the processGlyphRun
routine.

The next CL should rename GrSDFTOptions to GrSDFTControl.

Change-Id: I99e74c11af6d3b3d9919e54fe1e7286fcfbf1bfb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/378036
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Herb Derby <herb@google.com>
2021-03-03 14:27:32 +00:00
Brian Osman
dc4f8cd7ea Add early_fragment_tests layout qualifier to SkSL
Bug: skia:11356
Change-Id: I16322e6396dc7e7c8c50ba1d39e07311cf3bd346
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376116
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-03-02 16:45:03 +00:00
John Stiles
95d0badecf Support self-assignment elimination in the constant-folder.
Interestingly, this improves our codegen even with the optimizer fully
enabled, as apparently statement chains like:
	`x = true; x = x; x = x;`
were getting transformed by constant-propagation into:
	`x = true; x = true; x = true;`
making them no longer candidates for self-assignment elimination.

Change-Id: I6d94a809e94b01a00fd92459fcbce898b3cbbb11
Bug: skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377100
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-03-01 22:46:01 +00:00
John Stiles
0c2d14a1b9 Fix fuzzer-discovered error with inlining.
In http://review.skia.org/375776, an optimization was added to the
Inliner, causing it to skip generation of unnecessary temporary
variables. The fuzzer immediately discovered a flaw in this logic: the
"unnecessary" variable was actually used in the rare case that a
function failed to actually return a value. The inliner didn't detect
this case. Of course, this isn't a valid program either, so now we
report the error and cleanly fail.

Change-Id: I1f201cfd33f45cace3be93765a4e214e43a46e69
Bug: oss-fuzz:31469, oss-fuzz:31525
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/377101
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-03-01 20:06:50 +00:00
John Stiles
b0eb20f83a Convert IRGenerator::convertPrefixExpr to PrefixExpression::Make.
Surprisingly, this actually improved our error detection slightly.
The expression `- -half4(0)` can now be simplified to `half4(0)` at
IR generation time, which allows the constant-folder to detect a
constant zero (and from there, a division by constant zero).

Change-Id: I8c4f6ab522efab5bf98913f9c6a1487b7af39a99
Bug: skia:11342, skia:11343
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376842
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-26 22:34:05 +00:00
Brian Osman
a654faaf7f Add permittedLayoutFlags to checkModifiers
For now, just use this to prevent *any* layout qualifiers from appearing
on functions, or their parameters.

Bug: skia:11301
Change-Id: I05d8118c7121048c6ef49695a54e3714a8f8687e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376796
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-26 18:38:31 +00:00
Brian Osman
7b361499c9 Align SkSL const rules more closely with GLSL
This adds Analysis::IsConstantExpression, to determine if an expression
is a constant-expression. It now expands to cover 'const' local and
global variables, because we also enforce that the initializer on those
variables is - in turn - a constant expression.

This fixes 10837 - previously you could initialize a const variable with
a non-constant expression, and we'd emit GLSL that contained that same
pattern, which would fail to compile at the driver level. That should
not be possible any longer.

Bug: skia:10679
Bug: skia:10837
Change-Id: I517820ef4da57fff45768c0b04c55aebc18d3272
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375856
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-02-26 17:44:11 +00:00
John Stiles
bb8542f086 Fix fuzzer-discovered error with ~ prefix on literals.
The expression `~123` was making a PrefixExpression of type $intLiteral.
It should be converted to type `int` when the ~ prefix is applied.

This change also changes the output from oss-fuzz:27614. Both programs
are essentially nonsense expressions with no real behavior, so this is
fine.

Change-Id: I586be149ce95136fabee72fdd3473814d54948cf
Bug: oss-fuzz:31410
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376620
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-26 17:30:26 +00:00
Mike Klein
1f77cd802c always work from fBitmap in table CF
No point holding two copies of our data, an SkBitmap and fStorage array.
The GPU backend wants a bitmap, so make the CPU backend work from that.

Updating the flattened format removed the last use of SkPackBits.

Change-Id: I97f75ebe9324ec5eed37dfe6282e10d0fe08711f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376599
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Adlai Holler <adlai@google.com>
2021-02-26 14:48:19 +00:00
Ethan Nicholas
34c7e11492 Revert "Revert "SkSL DSL now reports the C++ line numbers of errors""
This reverts commit 3a4e19da8e.

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

Change-Id: I5ae97aba2df5b9c3b3dcca8e30da4e5ce9009109
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/376117
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-02-26 12:19:45 +00:00
Brian Salomon
76d13bbdf8 Remove texture idle proc mechanism
Bug: skia:11369
Change-Id: I0d1a74cc64dd74486acbb69df529ccc0f8b63ef3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375202
Reviewed-by: Greg Daniel <egdaniel@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
2021-02-25 22:23:20 +00:00
Ethan Nicholas
b956304bc7 Revert "Revert "Add DSL PossibleExpression & PossibleStatement""
This reverts commit 4903482a2a.

Change-Id: Ie56583344897fcb2c072bcac4d97d582edeadf07
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375660
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-02-25 19:50:20 +00:00
John Stiles
511c500ad4 Avoid generating unused variables in the Inliner.
These variables were later being eliminated by the dead-code-elimination
pass, so you can't see them directly in the final output, but removing
them affects the name mangling off all future symbols, so it causes an
enormous ripple effect in the diff. And of course, it's a waste of time
and memory to synthesize IRNodes just to destroy them later.

If we disable control-flow analysis, we lose the dead-code-elimination
pass entirely; this change is also beneficial for emitting better code
when optimizations are turned off.

Change-Id: I882b3be4f3fd99b77d99b6abe128f26bb9252c89
Bug: skia:11319
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375776
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
2021-02-25 19:18:19 +00:00
Adlai Holler
38b9a4bc3e Revert "Support sharing promise images between DDLs"
This reverts commit 07e11d48cb.

Reason for revert: Broke DDL3_ASAN and DDL3_TSAN

Original change's description:
> Support sharing promise images between DDLs
>
> - Migrate our code to SkImage::MakePromiseTexture
> - Have DDLTileHelper share one SKP and one set of promise images across all tiles.
> - Disallow on-the-fly allocation of mips for promise textures.
>
> Bug: skia:10286
> Change-Id: Ie35976958454fc520f3c9d860e6285441260c9f7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291938
> Commit-Queue: Adlai Holler <adlai@google.com>
> Reviewed-by: Robert Phillips <robertphillips@google.com>

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

Change-Id: I939b14875d1a20e4a92eab94680adcfe9596ad81
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:10286
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375738
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
2021-02-25 17:55:00 +00:00
Adlai Holler
07e11d48cb Support sharing promise images between DDLs
- Migrate our code to SkImage::MakePromiseTexture
- Have DDLTileHelper share one SKP and one set of promise images across all tiles.
- Disallow on-the-fly allocation of mips for promise textures.

Bug: skia:10286
Change-Id: Ie35976958454fc520f3c9d860e6285441260c9f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291938
Commit-Queue: Adlai Holler <adlai@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
2021-02-25 17:00:19 +00:00
Florin Malita
4903482a2a Revert "Add DSL PossibleExpression & PossibleStatement"
This reverts commit d42932a1b5.

Reason for revert: Breaking CMake and SKQP builds.

Original change's description:
> Add DSL PossibleExpression & PossibleStatement
>
> These are currently unused, but in future CLs they will be used to
> capture line number information in DSL error handling.
>
> Change-Id: Ieee730e0ad8323043437972fedb5bec471c367e4
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375069
> Reviewed-by: John Stiles <johnstiles@google.com>

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

Change-Id: I2062c9964569c9129a2145f9e45f849310129687
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375658
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-02-25 13:19:19 +00:00
Florin Malita
3a4e19da8e Revert "SkSL DSL now reports the C++ line numbers of errors"
This reverts commit 8a43a2889e.

Reason for revert: Prereq revert for https://skia-review.googlesource.com/c/skia/+/375069

Original change's description:
> SkSL DSL now reports the C++ line numbers of errors
>
> This is done a best-effort basis. Positions will only be reported in
> Clang and GCC, and may not be reported in every possible situation.
>
> Change-Id: I7efb15781546b95e667fee34e488366174bb5f36
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374600
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

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

Change-Id: I173cb355533d6f99c9561177b68f56924fe8a98f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375657
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Florin Malita <fmalita@google.com>
2021-02-25 13:17:59 +00:00
Chris Dalton
bb33be21cf Use SIMD to update hw tessellation tolerances
Bug: chromium:1172543
Change-Id: I223566197d1f2fd5fea07302f48ab89f50a36187
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374840
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-25 00:33:48 +00:00
Ethan Nicholas
8a43a2889e SkSL DSL now reports the C++ line numbers of errors
This is done a best-effort basis. Positions will only be reported in
Clang and GCC, and may not be reported in every possible situation.

Change-Id: I7efb15781546b95e667fee34e488366174bb5f36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/374600
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
2021-02-24 22:20:31 +00:00