commit 201b72a7f46fbebab3d60e641e277085cdc205bd
author Brian Osman <brianosman@google.com> Wed Sep 08 11:16:29 2021 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Wed Sep 08 17:03:43 2021 +0000
tree ab31d4145c19feb14341e96be233511d667c6f36
parent a0d61eb561992f196787d3822fb1011d3bc94b12

In runtime effects, treat all children as sampled

An unused child that used its coordinates would triggers asserts in the
GP - we expect that all FPs are sampled/invoked. This generates slightly
sub-optimal code (we compute and interpolate the local coordinates for
that child FP), but it's not likely to happen in real code.

Bug: skia:12429
Change-Id: Ic2ddc65d16a7e1f47af8c4192e5ff9ea329bf335
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/446836
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>

src/core/SkRuntimeEffect.cpp

diff --git a/src/core/SkRuntimeEffect.cpp b/src/core/SkRuntimeEffect.cpp
index aa84051..4b12857 100644
--- a/src/core/SkRuntimeEffect.cpp
+++ b/src/core/SkRuntimeEffect.cpp
@@ -317,8 +317,14 @@
                 c.type  = child_type(varType);
                 c.index = children.size();
                 children.push_back(c);
-                sampleUsages.push_back(SkSL::Analysis::GetSampleUsage(
-                        *program, var, sampleCoordsUsage.fWrite != 0, &elidedSampleCoords));
+                auto usage = SkSL::Analysis::GetSampleUsage(
+                        *program, var, sampleCoordsUsage.fWrite != 0, &elidedSampleCoords);
+                // If the child is never sampled, we pretend that it's actually in PassThrough mode.
+                // Otherwise, the GP code for collecting transforms and emitting transform code gets
+                // very confused, leading to asserts and bad (backend) shaders. There's an implicit
+                // assumption that every FP is used by its parent. (skbug.com/12429)
+                sampleUsages.push_back(usage.isSampled() ? usage
+                                                         : SkSL::SampleUsage::PassThrough());
             }
             // 'uniform' variables
             else if (var.modifiers().fFlags & SkSL::Modifiers::kUniform_Flag) {

src/gpu/GrFragmentProcessor.cpp

diff --git a/src/gpu/GrFragmentProcessor.cpp b/src/gpu/GrFragmentProcessor.cpp
index 0d5f537..390105d 100644
--- a/src/gpu/GrFragmentProcessor.cpp
+++ b/src/gpu/GrFragmentProcessor.cpp
@@ -142,6 +142,8 @@
 
 void GrFragmentProcessor::registerChild(std::unique_ptr<GrFragmentProcessor> child,
                                         SkSL::SampleUsage sampleUsage) {
+    SkASSERT(sampleUsage.isSampled());
+
     if (!child) {
         fChildProcessors.push_back(nullptr);
         return;

tests/SkRuntimeEffectTest.cpp

diff --git a/tests/SkRuntimeEffectTest.cpp b/tests/SkRuntimeEffectTest.cpp
index bf90812..428af17 100644
--- a/tests/SkRuntimeEffectTest.cpp
+++ b/tests/SkRuntimeEffectTest.cpp
@@ -498,6 +498,13 @@
     effect.child("child") = rgbwShader;
     effect.test({0xFF0000FF, 0xFFFF0000, 0xFF00FF00, 0xFFFFFFFF});
 
+    // Bind an image shader, but don't use it - ensure that we don't assert or generate bad shaders.
+    // (skbug.com/12429)
+    effect.build("uniform shader child;"
+                 "half4 main(float2 p) { return half4(0, 1, 0, 1); }");
+    effect.child("child") = rgbwShader;
+    effect.test(0xFF00FF00);
+
     //
     // Helper functions
     //