Issue 13737 in skia: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)

8 views
Skip to first unread message

johns… via monorail

unread,
Sep 13, 2022, 11:34:07 AM9/13/22
to bu...@skia.org
Status: Accepted
Owner: johns...@google.com
Area: SkSL
Priority: Medium
Type: Defect

New issue 13737 by johns...@google.com: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737

Every runtime-effect shader should use main(float2 coords).
The two-argument version is, thankfully, not documented in AGSL, so it's not public.
The color argument is only used in a few places and is never actually necessary.

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Git Watcher via monorail

unread,
Sep 13, 2022, 5:57:05 PM9/13/22
to bu...@skia.org

Comment #1 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c1

The following revision refers to this bug:
https://skia.googlesource.com/skia/+/9a4fd75426f404a3b66c02e04f3b566e6d17338d

commit 9a4fd75426f404a3b66c02e04f3b566e6d17338d
Author: John Stiles <johns...@google.com>
Date: Tue Sep 13 21:17:22 2022

Disallow two-argument public Runtime Shaders.

Public runtime shaders now require a signature of `half4 main(float2)`.
We still have a handful of private shaders which use the input color,
and keep the `half4 main(float2, half4)` signature. Those can be
cleaned up in followup CLs.

Bug: skia:13737
Change-Id: I4d352b04665a58d0e54be60bdd2d09fb61b7bef3
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580516
Auto-Submit: John Stiles <johns...@google.com>
Reviewed-by: Brian Osman <brian...@google.com>
Commit-Queue: John Stiles <johns...@google.com>

[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/tests/SkRuntimeEffectTest.cpp
[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/src/sksl/ir/SkSLFunctionDeclaration.cpp
[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/tests/sksl/runtime_errors/InvalidShaderMain.skvm
[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/RELEASE_NOTES.txt
[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/resources/sksl/runtime_errors/InvalidShaderMain.rts
[modify] https://crrev.com/9a4fd75426f404a3b66c02e04f3b566e6d17338d/include/effects/SkRuntimeEffect.h

Git Watcher via monorail

unread,
Sep 14, 2022, 2:06:06 PM9/14/22
to bu...@skia.org

Comment #2 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c2


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/0c7b0ffe512425f0477c2a42abd0568d16c5bd62

commit 0c7b0ffe512425f0477c2a42abd0568d16c5bd62
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 17:27:16 2022

Update GrFragmentProcessor::Circle to use main(float2).

Change-Id: I022ba3a032f2fadfa6246efbeb6deedf5fa46f2d
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580592
Commit-Queue: John Stiles <johns...@google.com>

Auto-Submit: John Stiles <johns...@google.com>
Reviewed-by: Brian Osman <brian...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 2:31:05 PM9/14/22
to bu...@skia.org

Comment #3 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c3


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/6a656bbba21fd79a4a10cbc073b70dce8c32d8fa

commit 6a656bbba21fd79a4a10cbc073b70dce8c32d8fa
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 17:49:37 2022

Update GrFragmentProcessor::Ellipse to use main(float2).

Change-Id: I2e450bd7b7a141b7badea25453205b3662c8ae8c
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580594
Reviewed-by: Michael Ludwig <michae...@google.com>
Auto-Submit: John Stiles <johns...@google.com>
Commit-Queue: John Stiles <johns...@google.com>
Commit-Queue: Michael Ludwig <michae...@google.com>

[modify] https://crrev.com/6a656bbba21fd79a4a10cbc073b70dce8c32d8fa/src/gpu/ganesh/GrFragmentProcessor.cpp

Git Watcher via monorail

unread,
Sep 14, 2022, 2:32:06 PM9/14/22
to bu...@skia.org

Comment #4 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c4


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/48a104714d9f5ac2723a8cdc4b9319ed769e98d5

commit 48a104714d9f5ac2723a8cdc4b9319ed769e98d5
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 17:39:54 2022

Update GrFragmentProcessor::Rect to use main(float2).

Change-Id: I9d906370827f3f29bf1b321261d8a6bbdc647492
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580593

Reviewed-by: Michael Ludwig <michae...@google.com>
Auto-Submit: John Stiles <johns...@google.com>
Commit-Queue: John Stiles <johns...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 2:43:11 PM9/14/22
to bu...@skia.org

Comment #5 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c5


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/025a6c7e7212c0654f82cbcd03768e69170973ff

commit 025a6c7e7212c0654f82cbcd03768e69170973ff
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 18:01:22 2022

Update CircleBlur FP to use main(float2).

Change-Id: Ia2106439fe314668b9b02f2ff4c7b7140235e3d4
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580595
Commit-Queue: Michael Ludwig <michae...@google.com>

Commit-Queue: John Stiles <johns...@google.com>
Auto-Submit: John Stiles <johns...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 2:57:10 PM9/14/22
to bu...@skia.org

Comment #6 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c6


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/d4940cfa079fca524661dbe3d384beaf8a4d8fad

commit d4940cfa079fca524661dbe3d384beaf8a4d8fad
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 18:15:20 2022

Update RRectBlur FP to use main(float2).

Change-Id: Iabf358a42e83b28d6e8480866be5aca9928550b2
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580956

Auto-Submit: John Stiles <johns...@google.com>
Reviewed-by: Michael Ludwig <michae...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 3:17:08 PM9/14/22
to bu...@skia.org

Comment #7 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c7


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/4c98ea839da82c4fe2013ab63808a80884bf4578

commit 4c98ea839da82c4fe2013ab63808a80884bf4578
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 18:34:50 2022

Update AlphaThreshold FP to use main(float2).

Change-Id: I923dcd200381eb7f8de0e94c10dd552339dfa277
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580958

Auto-Submit: John Stiles <johns...@google.com>
Commit-Queue: John Stiles <johns...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 4:02:06 PM9/14/22
to bu...@skia.org

Comment #8 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c8


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/357e4114b6fa8a8074d686c91574d8bea9a6f842

commit 357e4114b6fa8a8074d686c91574d8bea9a6f842
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 19:16:04 2022

Update Dither FP to use main(float2).

Change-Id: Ia43d30cdb497b5bbc4b3b235ca918361ee4253d4
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580960
Reviewed-by: Brian Osman <brian...@google.com>

Auto-Submit: John Stiles <johns...@google.com>
Commit-Queue: John Stiles <johns...@google.com>

Git Watcher via monorail

unread,
Sep 14, 2022, 4:07:05 PM9/14/22
to bu...@skia.org

Comment #9 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c9


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/b6efbd81f1543c3ca728f1e02b84e64b6da2e7d4

commit b6efbd81f1543c3ca728f1e02b84e64b6da2e7d4
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 19:14:32 2022

Update RectBlur FP to use main(float2).

Change-Id: Ifc8689f4f7cb323070341572443e1a2d14a4aed4
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580961
Commit-Queue: John Stiles <johns...@google.com>
Auto-Submit: John Stiles <johns...@google.com>
Commit-Queue: Michael Ludwig <michae...@google.com>
Reviewed-by: Michael Ludwig <michae...@google.com>

[modify] https://crrev.com/b6efbd81f1543c3ca728f1e02b84e64b6da2e7d4/src/core/SkBlurMF.cpp

Git Watcher via monorail

unread,
Sep 14, 2022, 5:10:08 PM9/14/22
to bu...@skia.org

Comment #10 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c10


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/79df441423aed9098cc38b06f4d4521750fd9f1d

commit 79df441423aed9098cc38b06f4d4521750fd9f1d
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 20:12:42 2022

Disallow two-argument Runtime Shaders entirely.

All the private FPs which used the input color have been fixed.

Change-Id: Ib046d62389e9e7e5d2067dd36abe93af7937f47b
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580965
Reviewed-by: Brian Osman <brian...@google.com>
Commit-Queue: John Stiles <johns...@google.com>

[modify] https://crrev.com/79df441423aed9098cc38b06f4d4521750fd9f1d/tests/SkRuntimeEffectTest.cpp
[modify] https://crrev.com/79df441423aed9098cc38b06f4d4521750fd9f1d/src/sksl/ir/SkSLFunctionDeclaration.cpp
[modify] https://crrev.com/79df441423aed9098cc38b06f4d4521750fd9f1d/include/effects/SkRuntimeEffect.h

Git Watcher via monorail

unread,
Sep 14, 2022, 5:40:08 PM9/14/22
to bu...@skia.org

Comment #11 on issue 13737 by Git Watcher: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c11


The following revision refers to this bug:
https://skia.googlesource.com/skia/+/f7ad27c03ee2c6d415cadad6e6b405c93b30745e

commit f7ad27c03ee2c6d415cadad6e6b405c93b30745e
Author: John Stiles <johns...@google.com>
Date: Wed Sep 14 20:55:11 2022

Reverse the order of inputs to Circle.

This shouldn't make any difference; a*b and b*a are the same. However,
this order gives us SkSL that looks closer to the original code. This
might give us better results on older Pixel 2/3 devices on Vulkan.
These devices started noise/garbage pixels after
http://review.skia.org/580595.

Change-Id: I05811bf0fe670d8821ef20f70f8dd6e65beea64b
Bug: skia:13737
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/580966
Commit-Queue: John Stiles <johns...@google.com>

Reviewed-by: Michael Ludwig <michae...@google.com>
Commit-Queue: Michael Ludwig <michae...@google.com>

johns… via monorail

unread,
Sep 14, 2022, 8:36:08 PM9/14/22
to bu...@skia.org
Updates:
Status: Fixed

Comment #12 on issue 13737 by johns...@google.com: Deprecate two-argument RuntimeShader main(float2 coords, half4 color)
https://bugs.chromium.org/p/skia/issues/detail?id=13737#c12

(No comment was entered for this change.)
Reply all
Reply to author
Forward
0 new messages