From e6496d127941b51f348f4c880032547d4c440c37 Mon Sep 17 00:00:00 2001
From: Robert Phillips <robertphillips@google.com>
Date: Thu, 25 Jun 2026 13:21:59 +0000
Subject: [PATCH] Fix Ganesh stencil UMR

When, for a given OpsTask, the stencil ops are discard/store there was a possibility of uninitialized values to creep into the stencil buffer.

This CL reduces the cases in which discard will be used, mapping the problematic cases to clear/store.

Bug: https://issues.chromium.org/issues/502351526
Change-Id: Ic5d7e352d4c59426f103f2cfc39d5cf3fd879213
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1269136
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
---
 gn/tests.gni                   |   1 +
 src/gpu/ganesh/ops/OpsTask.cpp |  27 ++++++-
 tests/StencilClearTest.cpp     | 131 +++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+), 3 deletions(-)
 create mode 100644 tests/StencilClearTest.cpp

--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -493,6 +493,7 @@
   "$_tests/Skbug6653.cpp",
   "$_tests/SlugTest.cpp",
   "$_tests/SrcSrcOverBatchTest.cpp",
+  "$_tests/StencilClearTest.cpp",
   "$_tests/SurfaceDrawContextTest.cpp",
   "$_tests/SurfaceSemaphoreTest.cpp",
   "$_tests/TextBlobCacheTest.cpp",
--- a/src/gpu/ganesh/ops/OpsTask.cpp
+++ b/src/gpu/ganesh/ops/OpsTask.cpp
@@ -588,7 +588,17 @@
     GrLoadOp stencilLoadOp;
     switch (fInitialStencilContent) {
         case StencilContent::kDontCare:
-            stencilLoadOp = GrLoadOp::kDiscard;
+            if (stencil && !caps.performStencilClearsAsDraws()) {
+                // This OpTask has a stencil, doesn't care about its contents,
+                // isn't clearing it with draws, and is going to store the result.
+                // In that case, we proactively clear it so that uninitialized data won't
+                // creep into the stencil buffer.
+                stencilLoadOp = GrLoadOp::kClear;
+            } else {
+                // This should only intentionally happen for the AtlasRenderTask which
+                // immediately inserts a clear.
+                stencilLoadOp = GrLoadOp::kDiscard;
+            }
             break;
         case StencilContent::kUserBitsCleared:
             SkASSERT(!caps.performStencilClearsAsDraws());
@@ -623,6 +633,7 @@
     // their store op might be "discard", and we currently make the assumption that a discard will
     // not invalidate what's already in main memory. This is probably ok for now, but certainly
     // something we want to address soon.
+    // b/160958008 forces discardStencilValuesAfterRenderPass to always return false.
     GrStoreOp stencilStoreOp = (caps.discardStencilValuesAfterRenderPass() && !fMustPreserveStencil)
             ? GrStoreOp::kDiscard
             : GrStoreOp::kStore;
@@ -643,6 +654,16 @@
     if (!renderPass) {
         return false;
     }
+
+#if defined(SK_DEBUG)
+    if (stencilLoadOp == GrLoadOp::kDiscard) {
+        // The only time we should have a stencil discard load-op is when either:
+        //    there is no stencil buffer
+        //    or stencil clears are being performed by draws
+        SkASSERT(!stencil || caps.performStencilClearsAsDraws());
+    }
+#endif
+
     flushState->setOpsRenderPass(renderPass);
     renderPass->begin();
 
@@ -732,10 +753,10 @@
         fTotalBounds.join(toMerge->fTotalBounds);
         fRenderPassXferBarriers |= toMerge->fRenderPassXferBarriers;
         if (fInitialStencilContent == StencilContent::kDontCare) {
-            // Propogate the first stencil content that isn't kDontCare.
+            // Propagate the first stencil content that isn't kDontCare.
             //
             // Once the stencil has any kind of initial content that isn't kDontCare, then the
-            // inital contents of subsequent opsTasks that get merged in don't matter.
+            // initial contents of subsequent opsTasks that get merged in don't matter.
             //
             // (This works because the opsTask all target the same render target and are in
             // painter's order. kPreserved obviously happens automatically with a merge, and kClear
