From ca9aecfbf4e27de60619a4db811575e3fd1e1af6 Mon Sep 17 00:00:00 2001
From: Greg Daniel <egdaniel@google.com>
Date: Mon, 29 Jun 2026 11:45:26 -0400
Subject: [PATCH] Check clipped renderpass bounds against cleared stencil area

* If only a portion of the stencil attachment is cleared, we should not mark the entire attachment as cleared. This CL makes it such that we check which area has most recently been cleared to make an informed decision.

Bug: https://issues.chromium.org/issues/514010477
Change-Id: I3800ca3125f25341dffd818f6c557692020027be
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1255256
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
---
 src/gpu/ganesh/GrAttachment.h  | 11 ++++--
 src/gpu/ganesh/GrCaps.cpp      |  7 +++-
 src/gpu/ganesh/GrCaps.h        | 16 ++++++---
 src/gpu/ganesh/gl/GrGLCaps.cpp | 13 +++++--
 src/gpu/ganesh/ops/OpsTask.cpp | 66 +++++++++++++++++++++++++---------
 src/gpu/ganesh/vk/GrVkCaps.cpp | 15 +++++---
 6 files changed, 96 insertions(+), 32 deletions(-)

--- a/src/gpu/ganesh/GrAttachment.h
+++ b/src/gpu/ganesh/GrAttachment.h
@@ -53,8 +53,11 @@
 
     skgpu::Mipmapped mipmapped() const { return fMipmapped; }
 
-    bool hasPerformedInitialClear() const { return fHasPerformedInitialClear; }
-    void markHasPerformedInitialClear() { fHasPerformedInitialClear = true; }
+    SkIRect clearedArea() const { return fClearedArea; }
+    bool hasAreaBeenCleared(SkIRect attachmentArea) const {
+        return fClearedArea.contains(attachmentArea);
+    }
+    void markAreaCleared(SkIRect area) { fClearedArea = area; }
 
     // This unique key is used for attachments of the same dimensions, usage, and sample cnt which
     // are shared between multiple render targets at the same time. Only one usage flag may be
@@ -117,7 +120,9 @@
     UsageFlags fSupportedUsages;
     int fSampleCnt;
     skgpu::Mipmapped fMipmapped;
-    bool fHasPerformedInitialClear = false;
+    // Track which area of the attachment has already been cleared to cut down on unnecessary clear
+    // operations, which can be more expensive on desktop GPUs than loads.
+    SkIRect fClearedArea = SkIRect::MakeEmpty();
     GrMemoryless fMemoryless;
 
     using INHERITED = GrSurface;
--- a/src/gpu/ganesh/GrCaps.cpp
+++ b/src/gpu/ganesh/GrCaps.cpp
@@ -43,6 +43,8 @@
     fUsePrimitiveRestart = false;
     fPreferClientSideDynamicBuffers = false;
     fPreferFullscreenClears = false;
+    fDiscardStencilValuesAfterRenderPass = false;
+    fClearsAreFasterThanLoads = false;
     fTwoSidedStencilRefsAndMasksMustMatch = false;
     fMustClearUploadedBufferData = false;
     fShouldInitializeTextures = false;
@@ -227,7 +229,10 @@
     writer->appendBool("MSAA Resolves Automatically", fMSAAResolvesAutomatically);
     writer->appendBool("Use primitive restart", fUsePrimitiveRestart);
     writer->appendBool("Prefer client-side dynamic buffers", fPreferClientSideDynamicBuffers);
-    writer->appendBool("Prefer fullscreen clears (and stencil discard)", fPreferFullscreenClears);
+    writer->appendBool("Prefer fullscreen clears", fPreferFullscreenClears);
+    writer->appendBool("Discard stencil values after renderpass",
+                       fDiscardStencilValuesAfterRenderPass);
+    writer->appendBool("Clears are faster than loads", fClearsAreFasterThanLoads);
     writer->appendBool("Two-sided Stencil Refs And Masks Must Match",
                        fTwoSidedStencilRefsAndMasksMustMatch);
     writer->appendBool("Must clear buffer memory", fMustClearUploadedBufferData);
--- a/src/gpu/ganesh/GrCaps.h
+++ b/src/gpu/ganesh/GrCaps.h
@@ -114,10 +114,11 @@
 
     bool preferClientSideDynamicBuffers() const { return fPreferClientSideDynamicBuffers; }
 
-    // On tilers, an initial fullscreen clear is an OPTIMIZATION. It allows the hardware to
-    // initialize each tile with a constant value rather than loading each pixel from memory.
+    // The following 3 methods provide information that enables performance optimizations for tiler
+    // GPUs. Full clear operations (followed by discarding the content when finished) are more
+    // performant than load operations on these GPUs as they enable the hardware to initialize each
+    // tile with a constant value as opposed to loading each pixel from memory.
     bool preferFullscreenClears() const { return fPreferFullscreenClears; }
-
     // Should we discard stencil values after a render pass? (Tilers get better performance if we
     // always load stencil buffers with a "clear" op, and then discard the content when finished.)
     bool discardStencilValuesAfterRenderPass() const {
@@ -126,10 +127,15 @@
 #if 0
         // This method is actually just a duplicate of preferFullscreenClears(), with a descriptive
         // name for the sake of readability.
-        return this->preferFullscreenClears();
+        return fDiscardStencilValuesAfterRenderPass;
 #endif
     }
 
+    // Returns whether clearing an attachment is faster than loading the attachment. This is useful
+    // when you know the values are already cleared so it doesn't matter if we load or clear at the
+    // start of a render pass.
+    bool clearsAreFasterThanLoads() const { return fClearsAreFasterThanLoads; }
+
     // D3D does not allow the refs or masks to differ on a two-sided stencil draw.
     bool twoSidedStencilRefsAndMasksMustMatch() const {
         return fTwoSidedStencilRefsAndMasksMustMatch;
@@ -605,6 +611,8 @@
     bool fUsePrimitiveRestart                        : 1;
     bool fPreferClientSideDynamicBuffers             : 1;
     bool fPreferFullscreenClears                     : 1;
+    bool fDiscardStencilValuesAfterRenderPass        : 1;
+    bool fClearsAreFasterThanLoads                   : 1;
     bool fTwoSidedStencilRefsAndMasksMustMatch       : 1;
     bool fMustClearUploadedBufferData                : 1;
     bool fBuffersAreInitiallyZero                    : 1;
--- a/src/gpu/ganesh/gl/GrGLCaps.cpp
+++ b/src/gpu/ganesh/gl/GrGLCaps.cpp
@@ -118,6 +118,13 @@
     return backend == GrGLANGLEBackend::kMetal;
 }
 
+namespace {
+bool is_tiler_gpu(GrGLVendor vendor) {
+    return vendor == GrGLVendor::kARM         ||
+           vendor == GrGLVendor::kImagination ||
+           vendor == GrGLVendor::kQualcomm;
+}
+} // anonymous namespace
 void GrGLCaps::init(const GrContextOptions& contextOptions,
                     const GrGLContextInfo& ctxInfo,
                     const GrGLInterface* gli) {
@@ -221,10 +228,10 @@
         }
     }
 
-    if (ctxInfo.vendor() == GrGLVendor::kARM         ||
-        ctxInfo.vendor() == GrGLVendor::kImagination ||
-        ctxInfo.vendor() == GrGLVendor::kQualcomm ) {
+    if (is_tiler_gpu(ctxInfo.vendor())) {
         fPreferFullscreenClears = true;
+        fDiscardStencilValuesAfterRenderPass = true;
+        fClearsAreFasterThanLoads = true;
     }
 
     if (GR_IS_GR_GL(standard)) {
--- a/src/gpu/ganesh/ops/OpsTask.cpp
+++ b/src/gpu/ganesh/ops/OpsTask.cpp
@@ -586,35 +586,67 @@
     }
 
     GrLoadOp stencilLoadOp;
+    // Determine whether the stencil attachment's cleared area should be updated.
+    bool updateClearedStencilArea = false;
+    SkIRect boundsRequiredByStencil = fClippedContentBounds;
     switch (fInitialStencilContent) {
         case StencilContent::kDontCare:
-            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 {
+            if (!stencil || caps.performStencilClearsAsDraws()) {
                 // This should only intentionally happen for the AtlasRenderTask which
                 // immediately inserts a clear.
                 stencilLoadOp = GrLoadOp::kDiscard;
+				break;
             }
             break;
+            // 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 fallthrough to clear it so
+            // that uninitialized data won't creep into the stencil buffer.
+            [[fallthrough]];
         case StencilContent::kUserBitsCleared:
             SkASSERT(!caps.performStencilClearsAsDraws());
             SkASSERT(stencil);
+            // Based upon Caps, determine which stencil load operation to use and perform any
+            // necessary updates to the renderpass's bounds and the stencil's cleared area.
+
+            // If the user bits are meant to be cleared we either do that via an initial clear the
+            // first time the area of the stencil is used, or we assume previous draws left the
+            // stencil cleared so we just load the current values. However, on some devices it is
+            // faster to just clear the stencil each time instead of doing a load.
+            //
+            // Since we'll never end up loading the stencil in this case there is no reason for us
+            // to do the clear rect tracking below, so we just break.
+            if (caps.clearsAreFasterThanLoads()) {
+                stencilLoadOp = GrLoadOp::kClear;
+                break;
+            }
+            // If stencil values are discarded after every renderpass, then we do not need to track
+            // which portion of the stencil attachment has already been cleared.
             if (caps.discardStencilValuesAfterRenderPass()) {
-                // Always clear the stencil if it is being discarded after render passes. This is
-                // also an optimization because we are on a tiler and it avoids loading the values
-                // from memory.
                 stencilLoadOp = GrLoadOp::kClear;
                 break;
             }
-            if (!stencil->hasPerformedInitialClear()) {
+
+            // If the area of the stencil attachment corresponding to this renderpass's
+            // boundsRequiredByStencil has not already been cleared, calculate new, expanded
+            // renderpass bounds by joining boundsRequiredByStencil with the cleared stencil area.
+            // Using this joint area simplifies tracking of cleared areas on the stencil attachment.
+            if (!stencil->hasAreaBeenCleared(boundsRequiredByStencil)) {
                 stencilLoadOp = GrLoadOp::kClear;
-                stencil->markHasPerformedInitialClear();
+                if (!stencil->clearedArea().isEmpty()) {
+                    boundsRequiredByStencil.join(stencil->clearedArea());
+                    // We need to also intersect the bounds with the color attachment's bounds since
+                    // the stencil may be bigger. This could mean that we end up "forgetting" we
+                    // cleared the user bits in the portion of the stencil that doesn't overlap with
+                    // the color attachment - but that shouldn't have a large performance impact if
+                    // we later on need to reclear that area.
+                    boundsRequiredByStencil.intersect(
+                        SkIRect::MakeSize(renderTarget->dimensions()));
+                }
+                // We should update the stencil's cleared area if renderpass creation succeeds.
+                updateClearedStencilArea = true;
                 break;
             }
+
             // SurfaceDrawContexts are required to leave the user stencil bits in a cleared state
             // once finished, meaning the stencil values will always remain cleared after the
             // initial clear. Just fall through to reloading the existing (cleared) stencil values
@@ -624,6 +656,8 @@
             SkASSERT(stencil);
             stencilLoadOp = GrLoadOp::kLoad;
             break;
+        default:
+            SkUNREACHABLE;
     }
 
     // NOTE: If fMustPreserveStencil is set, then we are executing a surfaceDrawContext that split
@@ -643,7 +677,7 @@
                                                      fUsesMSAASurface,
                                                      stencil,
                                                      fTargetOrigin,
-                                                     fClippedContentBounds,
+                                                     boundsRequiredByStencil,
                                                      fColorLoadOp,
                                                      fLoadClearColor,
                                                      stencilLoadOp,
@@ -654,6 +688,9 @@
     if (!renderPass) {
         return false;
     }
+    if (updateClearedStencilArea) {
+        stencil->markAreaCleared(boundsRequiredByStencil);
+	}
 
 #if defined(SK_DEBUG)
     if (stencilLoadOp == GrLoadOp::kDiscard) {
--- a/src/gpu/ganesh/vk/GrVkCaps.cpp
+++ b/src/gpu/ganesh/vk/GrVkCaps.cpp
@@ -302,6 +302,13 @@
 
 }
 
+namespace {
+bool is_tiler_gpu(uint32_t vendorId) {
+    return vendorId == skgpu::kARM_VkVendor ||
+           vendorId == skgpu::kQualcomm_VkVendor ||
+           vendorId == skgpu::kImagination_VkVendor;
+}
+} // anonymous namespace
 void GrVkCaps::init(const GrContextOptions& contextOptions,
                     const skgpu::VulkanInterface* vkInterface,
                     VkPhysicalDevice physDev,
@@ -415,12 +422,12 @@
     this->initGrCaps(vkInterface, physDev, properties, memoryProperties, features, extensions);
     this->initShaderCaps(properties, features);
 
-    if (skgpu::kQualcomm_VkVendor == properties.vendorID) {
-        // A "clear" load for atlases runs faster on QC than a "discard" load followed by a
+    if (is_tiler_gpu(properties.vendorID)) {
+        // A "clear" load for atlases runs faster on tiler GPUs than a "discard" load followed by a
         // scissored clear.
-        // On NVIDIA and Intel, the discard load followed by clear is faster.
-        // TODO: Evaluate on ARM, Imagination, and ATI.
         fPreferFullscreenClears = true;
+        fDiscardStencilValuesAfterRenderPass = true;
+        fClearsAreFasterThanLoads = true;
     }
 
     if (properties.vendorID == skgpu::kNvidia_VkVendor ||
