Commit Graph

64 Commits

Author SHA1 Message Date
Brian Osman
4f0098227e Make the generic SkRuntimeEffect::Make factories private
All clients have been moved to the stage-specific factories. The old
flexible factories are still used internally (for now), but this
prevents any new usage from creeping in accidentally.

Bug: skia:11813
Change-Id: I6c34dfd19b396541f9a0e2f9eab8a51591ed8b70
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/402156
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-28 14:49:46 +00:00
Brian Osman
4d5711192a Reland "Runtime effects: Detect passthrough sample calls automatically"
As explained, this is *very* conservative. It only works when the child
is sampled from within main, and using a direct reference to the coords
parameter. If that parameter is ever modified (even after being used),
the optimization doesn't happen. For most cases, this is fine.

Reland changes the logic in GrSkSLFP slightly, to avoid turning all
samples into pass-through when a child is sampled with both pass-through
and explicit coordinates.

Bug: skia:11869
Change-Id: Iec18f059b4e78df0d2f53449aa0c2945c58a58f7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401677
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2021-04-27 17:44:31 +00:00
Brian Osman
00f3041414 Revert "Runtime effects: Detect passthrough sample calls automatically"
This reverts commit cc3d2d25c5.

Reason for revert: Subtle bug snuck past testing.

Original change's description:
> Runtime effects: Detect passthrough sample calls automatically
>
> As explained, this is *very* conservative. It only works when the child
> is sampled from within main, and using a direct reference to the coords
> parameter. If that parameter is ever modified (even after being used),
> the optimization doesn't happen. For most cases, this is fine.
>
> Bug: skia:11869
> Change-Id: Ia06181730a6d07e2a4fe2de4cc8e8c3402f0dc52
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397320
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>
> Reviewed-by: John Stiles <johnstiles@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>

Bug: skia:11869
Change-Id: Icfc756cde745d450966eaa49f5067a73232635b1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401158
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-26 21:27:28 +00:00
Brian Osman
56ed7daeca Use MakeForShader/ColorFilter in runtime effect tests
Bug: skia:11813
Change-Id: I9748a2806fe4636111fbb5740a3ebdb0814cfc35
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/401018
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-26 14:44:38 +00:00
Brian Osman
af4e2331d1 Add tests of the runtime effect stage factories
We were testing this with .rtcf and .rts files, but those don't go
through the MakeForXXX factories. This would have caught the bug fixed
in https://skia-review.googlesource.com/c/skia/+/398229

Change-Id: Id824e7bf32982c043978b2981cfd91dd7a9b2585
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398183
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-21 16:05:37 +00:00
Brian Osman
8f1dff6a9e Remove layout(marker) from runtime effect SkSL
This is another strange, experimental feature that clutters the
implementation and isn't used by anyone (to my knowledge).

Change-Id: I538b7eca0cd28aab32f4739b23459731ade9105e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398226
Commit-Queue: John Stiles <johnstiles@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-04-19 18:48:45 +00:00
Brian Osman
cc3d2d25c5 Runtime effects: Detect passthrough sample calls automatically
As explained, this is *very* conservative. It only works when the child
is sampled from within main, and using a direct reference to the coords
parameter. If that parameter is ever modified (even after being used),
the optimization doesn't happen. For most cases, this is fine.

Bug: skia:11869
Change-Id: Ia06181730a6d07e2a4fe2de4cc8e8c3402f0dc52
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/397320
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-19 14:52:44 +00:00
Brian Osman
cdee120180 Cache runtime color filter alpha-unchanged flag on the effect
This reuses the work we're already doing to compute the one-at-a-time
program, and lets us share the result across color filter instances
created from the same effect.

When we added caching of the flag to filter creation, pinpoint found a
performance regression. I expect this to resolve that.

Bug: chromium:1187432
Change-Id: Ie4f8551fa432d298ce950cba8642b9de28f19b81
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/396758
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-04-14 14:35:28 +00:00
Brian Osman
962e6140ac Add ivec GLSL type aliases to runtime effects
Also add unit test of all GLSL type aliases.

Bug: skia:10679
Change-Id: I93e21621c11adfe3f114d0c55fb8043518e62696
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/395718
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-12 16:55:41 +00:00
Brian Osman
5c62582874 Require that runtime effect child variables are 'uniform'
Bug: skia:11374
Change-Id: I63d605eabbe514a0469d00d8a671969874f3edd4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393081
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-07 13:16:28 +00:00
Brian Osman
601abfacc7 Move 'shader' usage tests to an SkSL golden file
Also adds tests of non-uniform shader declarations. These are currently
allowed, but will be detected as an error in the next CL.

Bug: skia:11374
Change-Id: I3fee0a0c97ae590f7bc6952cb367f7e94436b891
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/393080
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2021-04-06 19:20:16 +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
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
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
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
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
Mike Klein
e0d9b86421 impl SkRuntimeColorFilter::getFlags()
Simply build the skvm program and look to see if in.a.id == out.a.id.

This obviously is conservative, with false negatives possible.  I
haven't done it here, but one possible improvement is to splat uniforms
so constant folding can peer through uniforms too.

Add some basic tests.

Change-Id: I5578ba38ff490b96bf84538025e08d9d352f0320
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/370825
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-17 15:55:49 +00:00
John Stiles
1f19ce2272 Reland "Remove deprecated form of SkRuntimeEffect::Make."
This reverts commit e89b50ae05.

Reason for revert: landed Android fix at http://ag/13544365 (master) and
http://ag/13554983 (sc-dev)

Original change's description:
> Android roll broke with a compilation error:
> frameworks/base/libs/hwui/jni/Shader.cpp:243:37: error: no matching function for call to 'get'
>     sk_sp<SkRuntimeEffect> effect = std::get<0>(result)
>
> Revert "Remove deprecated form of SkRuntimeEffect::Make."
>
> This reverts commit 1cda194366.
>
> Reason for revert: <INSERT REASONING HERE>
>
> Original change's description:
> > Remove deprecated form of SkRuntimeEffect::Make.
> >
> > Chromium has migrated to the new API at https://crrev.com/c/2675855.
> >
> > Change-Id: Id4af77db2c462348e8031d28f56e543ad619c19c
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367060
> > Reviewed-by: Brian Osman <brianosman@google.com>
> > Commit-Queue: Brian Osman <brianosman@google.com>
> > Commit-Queue: John Stiles <johnstiles@google.com>
> > Auto-Submit: John Stiles <johnstiles@google.com>
>
> TBR=mtklein@google.com,brianosman@google.com,johnstiles@google.com
>
> Change-Id: Ie18f865f3b7f5b0263db1e52b19cf6faa0500fdd
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368616
> Reviewed-by: Stan Iliev <stani@google.com>
> Commit-Queue: Stan Iliev <stani@google.com>

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

Change-Id: I9d679013cb275dc80aaaa977b7f1f4da31f36d1e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/369037
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
2021-02-12 20:57:50 +00:00
Brian Osman
8e756f379c Support structs in runtime effects
Uses the pipeline-stage callback mechanism. It mangles the type name
(with a test to verify that this works), and then calls defineStruct
with the entire SkSL struct definition string.

Bug: skia:10939
Change-Id: If14cf1b11faaa80ad8d4086cdacf68532bac43fc
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368809
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2021-02-11 21:09:15 +00:00
Stan Iliev
e89b50ae05 Android roll broke with a compilation error:
frameworks/base/libs/hwui/jni/Shader.cpp:243:37: error: no matching function for call to 'get'
    sk_sp<SkRuntimeEffect> effect = std::get<0>(result)

Revert "Remove deprecated form of SkRuntimeEffect::Make."

This reverts commit 1cda194366.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Remove deprecated form of SkRuntimeEffect::Make.
>
> Chromium has migrated to the new API at https://crrev.com/c/2675855.
>
> Change-Id: Id4af77db2c462348e8031d28f56e543ad619c19c
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367060
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Brian Osman <brianosman@google.com>
> Commit-Queue: John Stiles <johnstiles@google.com>
> Auto-Submit: John Stiles <johnstiles@google.com>

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

Change-Id: Ie18f865f3b7f5b0263db1e52b19cf6faa0500fdd
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/368616
Reviewed-by: Stan Iliev <stani@google.com>
Commit-Queue: Stan Iliev <stani@google.com>
2021-02-10 00:05:10 +00:00
John Stiles
1cda194366 Remove deprecated form of SkRuntimeEffect::Make.
Chromium has migrated to the new API at https://crrev.com/c/2675855.

Change-Id: Id4af77db2c462348e8031d28f56e543ad619c19c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367060
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
2021-02-09 21:27:34 +00:00
Mike Klein
827f8c0103 support skvm fallback for single color filtering
with unit test

Change-Id: I7f0e30435bf4e054fe7436daaadb3512936a58ac
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/367237
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-02-06 19:04:22 +00:00
Derek Sollenberger
9e1cedda63 Add generic uniform setter function to SkRuntimeShaderBuilder
Change-Id: Idab539a4b39fe5ceab54948c99c0dcd6d19fd345
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/352743
Commit-Queue: Derek Sollenberger <djsollen@google.com>
Auto-Submit: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2021-01-14 14:38:18 +00:00
John Stiles
7ce175117f Split apart SkRuntimeInvalid tests into multiple functions.
This will help us narrow down the failing test from
http://review.skia.org/353097

Change-Id: Ida3bbecb09c278913cfb302ba7e7766f1a2cbcb0
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/353418
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2021-01-13 14:11:42 +00:00
Brian Osman
977feec5d7 Add .rte -> .skvm unit test framework
Includes a handful of test cases to exercise the system

Change-Id: I98e73a8bca063f475d2ddb51778e395697392ddb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/346637
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-12-22 17:59:42 +00:00
Brian Osman
f2876b0b9e SkSL: Only allow 'discard' in fragment shaders
Allowing this in runtime effects lets people break our contracts around
SkShaders on the color side of the paint altering coverage, etc.

Bug: skia:11085
Change-Id: I1ec8e71581c8d50f681cb0ca6ca8416375b3f43f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/344760
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-12-15 21:34:15 +00:00
Mike Reed
b41bd15a4b Use samplingoptions for bitmap shader
Change-Id: I64c67ddcac29b68cdd0d25e4af4153358939b0ee
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/343736
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Reed <reed@google.com>
2020-12-12 17:08:11 +00:00
Brian Osman
b4ce94431f Runtime effects: Fix error when mutating main's coords
These often became direct references to a varying, which can't be
mutated in GLSL. Fix this by copying them to a local variable in the
FP, and pointing all references at that.

Bug: skia:10918
Change-Id: I705e3c966b1d44fc4dfc3d4b40eb8e46110febdd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334043
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-11 16:56:25 +00:00
Brian Osman
24c18526a5 Make runtime effect error tests simpler / more explicit
I've wanted to write a few of these lately, and (just like the shader
tests later in the file), the rigid structure was making things
difficult. It also forced some of these to be overly clever (like
commenting out main). Switch to just providing all of the SkSL for each
test case.

Change-Id: I8aff34224fa6efbdc4d67628744a9cf499eb53b5
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/333796
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-11-11 01:14:44 +00:00
Brian Osman
b06301ee12 Reland "Rearrange SkSL pre-include modules to hide things"
This reverts commit 4cb5c5e172, and fixes
the Chromium issue by declaring sign(x) in sksl_public.sksl.

Original description:

This makes numerous internal and GLSL types or intrinsics hidden from
public (runtime effect) SkSL. In particular:

- Only core numeric types are visible to all program types. GLSL types
  involving images, textures, and sampling are restricted to internal use.
- sk_Caps is no longer visible to runtime effects.
- The set of intrinsics available to runtime effects is now a separate,
  curated list in sksl_public.sksl. It exactly matches the GLSL ES 1.00
  spec order.
- The blend intrinsics are no longer visible, which also fixes a bug.
  These are nice, but we're not going to offer them yet - they involve
  enums, which creates complications.

Bug: skia:10680
Bug: skia:10709
Bug: skia:10913

Cq-Include-Trybots: luci.chromium.try:linux-chromeos-rel,linux-rel
Change-Id: I42deeeccd725a9fe18314d091ce253404e3572e2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332750
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-06 19:28:34 +00:00
Brian Osman
33316414c7 Allow public SkSL to return vec4 (aka float4)
We already allowed narrowing type conversions, so GLSL type aliases
could be used almost exclusively. This fixes the last spot that a
client would be forced to use half4 rather than vec4.

Bug: skia:10679
Change-Id: Ie9cfc161650b238678861b9b126ce586c229162d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332743
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-06 19:11:44 +00:00
Brian Osman
4cb5c5e172 Revert "Rearrange SkSL pre-include modules to hide things"
This reverts commit bea0dc67f4.

No-Try: true
Change-Id: I7b000de199dc23bd3719a4ee835b2a8d3c93fefd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332747
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
2020-11-06 16:30:35 +00:00
Brian Osman
bea0dc67f4 Rearrange SkSL pre-include modules to hide things
This makes numerous internal and GLSL types or intrinsics hidden from
public (runtime effect) SkSL. In particular:

- Only core numeric types are visible to all program types. GLSL types
  involving images, textures, and sampling are restricted to internal use.
- sk_Caps is no longer visible to runtime effects.
- The set of intrinsics available to runtime effects is now a separate,
  curated list in sksl_public.sksl. It exactly matches the GLSL ES 1.00
  spec order.
- The blend intrinsics are no longer visible, which also fixes a bug.
  These are nice, but we're not going to offer them yet - they involve
  enums, which creates complications.

Bug: skia:10680
Bug: skia:10709
Bug: skia:10913

Change-Id: I8fa1c94f6e4899f38530bb9cff33d147f6983ab3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332597
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-05 21:44:40 +00:00
Brian Osman
91292e9482 Runtime Effects: Support 'uniform shader' (vs. 'in shader')
The previous behavior leaked Skia-internal concepts into public SkSL.
Users coming from GLSL will expect that bindable/sampleable objects are
uniform (just like texture2D). This keeps the old support around (and
tested), but updates all of our examples to use 'uniform'.

Bug: skia:10679
Change-Id: I0c98162f5e21dad7014d9778ceb26143d2f6030e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/332376
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-11-04 22:29:53 +00:00
Brian Osman
f1319c3756 Add GLSL type aliases for SkRuntimeEffect SkSL
Adds vec[N], mat[N], and mat[NxM] aliases when building runtime effect
code. Also moves the "shader" alias for fragmentProcessor so it's only
usable in that context.

Bug: skia:10679
Change-Id: Ia337bb680c50da32639bb1d4ffc3bc01df306b0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/325620
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-10-13 14:57:06 +00:00
Brian Osman
8e2ef02855 Support variables in the intrinsic map, clone them into Programs
As a first step, convert sksl_pipeline.sksl to an IntrinsicMap (rather
than inherited element list). This makes the new code operate on
sk_FragCoord (which was previously being shared by all runtime effect
programs).

The new unit test angered TSAN, and now runs without complaint.

Also finish converting the .fp intrinsics over, so those don't need an
inherited element list either. And while doing that, refactor that
parsing to match all of the others. FP was uniquely implementing
processIncludeFile itself, rather than reusing the pattern of other
pre-include parsing.

The meat of the CL is the subtle changes in Compiler, and the logic in
cloneBuiltinVariables. Note that we need to clone the global variable
declaration element (because one of the goals is to get rid of shared
and inherited program elements), but also the variable itself (and the
new copy needs to live in the program's symbol table).

Bug: skia:10589
Bug: skia:10679
Bug: skia:10680
Change-Id: Ied352f8434dac2b8eacb4e515b014b6af7b57d20
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/319023
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-10-01 20:30:33 +00:00
Brian Osman
0acb5b50b3 Allow casting to lower precision types in runtime effects
Bug: skia:10679
Change-Id: If464c48b7c31d0d8440d1231d1983829d54ce598
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/315281
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-09-14 18:08:17 +00:00
Brian Osman
b6bd0d2094 Make SkRuntimeShaderBuilder safe for reuse
Previously, if you snapped off a shader and then changed uniforms
(without drawing & flushing), we'd trigger the SkData assert about
calling writeable_data when not-uniquely-owned. Now we lazily copy the
SkData when necessary.

Includes unit test that previously failed.

Bug: skia:10667
Change-Id: If8d9dd8106d41e66560d760cb36ed83371791fc7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/313678
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-08-27 16:09:37 +00:00
Brian Osman
767f444feb SkRuntimeEffect SkSL has a new signature for main()
There is no more 'inout half4 color'. Effects return their output color.
If an effect wants the input color, it must use the (already existing)
approach of sampling a nullptr input shader.

The change is guarded for Chromium (so we can update their runtime color
filters in skia_renderer.cc).

For the GPU backend, FPs can now override usesExplicitReturn to indicate
that their emitCode will generate a return statement. If that's true,
then writeProcessorFunction doesn't inject the automatic return of the
output color, and emitFragProc will *always* wrap that FP in a helper
function, even as a top-level FP. GrSkSLFP opts in to this behavior, so
that the user-supplied return becomes the actual return in the FP's
emitCode.

Adapting the skvm code to this wasn't too bad: It looks fragile (what
happens if there are multiple returns?), but that's not really possible
today, without varying control flow.

Bug: skia:10613

Change-Id: I205b81fd87dd32bab30b6d6d5fc78853485da036
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310756
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-08-25 13:36:28 +00:00
John Stiles
c1c3c6d70d Enable ClangTidy flag bugprone-suspicious-string-compare.
https://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-string-compare.html

Find suspicious usage of runtime string comparison functions.
This check is valid in C and C++.

Checks for calls with implicit comparator and proposed to
explicitly add it:

  if (strcmp(...))       // Implicitly compare to zero
  if (!strcmp(...))      // Won't warn
  if (strcmp(...) != 0)  // Won't warn

Checks that compare function results (i,e, strcmp) are compared to valid
constant. The resulting value is

  <  0    when lower than,
  >  0    when greater than,
  == 0    when equals.

A common mistake is to compare the result to 1 or -1:

  if (strcmp(...) == -1)  // Incorrect usage of the returned value.

Additionally, the check warns if the results value is implicitly cast
to a suspicious non-integer type. It’s happening when the returned
value is used in a wrong context:

  if (strcmp(...) < 0.)  // Incorrect usage of the returned value.

Change-Id: I001b88d06cc4f3eb5846103885be675f9b78e126
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/310761
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
2020-08-16 03:54:08 +00:00
Brian Osman
e12939f0d8 Unit tests for runtime effect SkSL errors found later in compilation
Forgot to add these when I fixed the bug. Verified that these tests
failed without the fix.

Bug: skia:10593
Change-Id: Ia20fad3cd8e5b0f63ca19946b8314eed49bec2bf
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309716
Auto-Submit: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-12 19:40:21 +00:00
Brian Osman
a4b9169fb6 Remove 'in' variables from SkRuntimeEffect
Runtime effects previously allowed two kinds of global input variables:
'in' variables could be bool, int, or float. 'uniform' could be float,
vector, or matrix. Uniform variables worked like you'd expect, but 'in'
variables were baked into the program statically. There was a large
amount of machinery to make this work, and it meant that 'in' variables
needed to have values before we could make decisions about program
caching, and before we could catch some errors. It was also essentially
syntactic sugar over the client just inserting the value into their SkSL
as a string. Finally: No one was using the feature.

To simplify the mental model, and make the API much more predictable,
this CL removes 'in' variables entirely. We no longer need to
"specialize" runtime effect programs, which means we can catch more
errors up front (those not detected until optimization). All of the API
that referred to "inputs" (the previous term that unified 'in' and
'uniform') now just refers to "uniforms".

Bug: skia:10593
Change-Id: I971f620d868b259e652b3114f0b497c2620f4b0c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/309050
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-08-10 22:00:44 +00:00
Brian Osman
269b21c501 SkRuntimeEffect: Apply uniform transforms in color filters, too
We were only respecting this feature in runtime shaders. Note that use
of any tagged matrices will cause color filter creation to fail, but
color transformation is a totally sensible thing to want in a color
filter.

Change-Id: I482226b287ab794cb341367fce453381cb581966
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308507
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-06 18:46:17 +00:00
Brian Osman
92aac1e1b3 Disallow runtime effect color filters that depend on position
Some of these checks are currently redundant (we don't allow color
filters to have children right now). But the next CL will re-add that
capability, and the unit tests here will ensure we don't re-break things
by allowing child-sampling to violate the color filter invariant.

Change-Id: I54c10d8b1d1e376c13347296765185d42b9f644a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308285
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-08-06 16:06:47 +00:00
Brian Osman
82329004d0 SkSL: Disallow fragmentProcessor variables in several places
We only want SkSL to contain fp variables at global scope, and to sample
directly from references to those global variables. Anything else will
confuse our sample-usage analysis, and often leads to asserts in the CPP
or pipeline-stage generators.

Bug: skia:10514
Change-Id: Ie1ef10821c1b2a946a92d050fea45d95569bc934
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304599
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
2020-07-31 20:09:14 +00:00
Robert Phillips
e94b4e1b49 Update more tests to use the GrDirectContext/GrRecordingContext pair
This should be the last batch of tests. All the remaining uses of
GrContext should be resolved when SkImage no longer requires a context.

Change-Id: I47eeb3b74c28f483c20d9bec4daecbdb6d2cb982
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305541
Reviewed-by: Adlai Holler <adlai@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
2020-07-27 15:08:31 +00:00
Brian Osman
b5f0f52831 SkSL: Salt inline identifiers in PipelineStage and FragmentProcessor passes
Fixes a bug with name conflicts in the final SkSL.

Bug: skia:10526
Change-Id: Ic238f89dd778c186e775ecbaabfbaed9e426274f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305563
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2020-07-24 19:23:20 +00:00
Brian Osman
6241961a32 Add several more unit tests of runtime effects
Did some cleanup to remove repetition that was distracting from the
thing being tested.

Change-Id: Ie385c6ec2d1325a1bd0ba5c2270e7f2ddd1d24b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/305076
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: Brian Osman <brianosman@google.com>
2020-07-22 21:12:21 +00:00
Brian Osman
182c92ebb3 Catch SkRuntimeEffects w/o "main" earlier
Previously, these would produce a "valid" effect object, but it wouldn't
draw anything.

Bug: oss-fuzz:24070
Change-Id: I17d0ed1710196853da0694cac9f4c260312700a9
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/304064
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
2020-07-20 20:48:04 +00:00
Brian Osman
e64ae86d27 Rearrange logic in SkRuntimeEffect::Make
To extract metadata and validate the shader, we've added several
iterations over all program elements. This CL rearranges things
to iterate once (*). Variable conversion is moved to a separate
loop later, to help with nesting and readability.

Removes hard-to-read asserts. These were validating things enforced
by both the IR generator and unit tests.

*: Technically, there are additional implicit iterations when we call
   SkSL::Analysis functions. Folding all of this into a single pass
   would be even better, but much more complicated.

Change-Id: I4f5aa649e74094e94c365ad20ef2ac96082285cd
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/303924
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
2020-07-20 19:33:33 +00:00