From 04b084c10b54142767250e736ea44fc82f30d4d6 Mon Sep 17 00:00:00 2001
From: Thomas Smith <thomsmit@google.com>
Date: Fri, 29 May 2026 12:42:22 -0400
Subject: [PATCH] Reland "[graphite] BufferSubAllocator respects failed mapping
 on reset"

* `getMappedUniformBuffer` was changed after M148 but before M149. This caused comments in the unit test to be wrong, causing M148 cherry pick to fail CQ.

* Change the unit test to instead use `getMappedIndexBuffer`, which is stable across the relevant releases, and has been confirmed to trigger the bug.

* These changes purely affect the unit test and not the fix itself.

This reverts commit c329e877d1848877d459999a4ed43b3728892b7b.

Original change's description:
> Revert "[graphite] BufferSubAllocator respects failed mapping on reset"
>
> This reverts commit e7bff78bf5d2994f627742aaf5040b2ba55c4475.
>
> Fails clang-tidy
>
> Original change's description:
> > [graphite] BufferSubAllocator respects failed mapping on reset
> >
> > Bug: b/516981393
> > Change-Id: If6837e26ee520ad34c8df46a34850220ec5b538c
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1247536
> > Commit-Queue: Thomas Smith <thomsmit@google.com>
> > Reviewed-by: Michael Ludwig <michaelludwig@google.com>
>
> Bug: b/516981393
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Change-Id: I8cf69a8ba2ff318aab65033af821954e4493e5cb
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1249556
> Auto-Submit: Thomas Smith <thomsmit@google.com>
> Commit-Queue: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>

Bug: https://issues.chromium.org/issues/516981393
Change-Id: I3688a446d9042ffdb79a6c91d73f72bd5016716e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1249557
Commit-Queue: Thomas Smith <thomsmit@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
---
 src/gpu/graphite/BufferManager.cpp   | 14 ++++++----
 src/gpu/graphite/BufferManager.h     |  4 +++
 tests/graphite/BufferManagerTest.cpp | 41 ++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 5 deletions(-)

--- a/src/gpu/graphite/BufferManager.cpp
+++ b/src/gpu/graphite/BufferManager.cpp
@@ -253,6 +253,13 @@
     if (fBuffer) {
         SkASSERT(fOwner);
 
+        if (fOwner->fMappingFailed) {
+            fBuffer = nullptr;
+            fRemaining = 0;
+            fTransferBuffer = {};
+            return;
+        }
+
         DrawBufferManager::BufferState& state = fOwner->fCurrentBuffers[fStateIndex];
         if (fBuffer->shareable() == Shareable::kScratch) {
             // TODO: Merge this reuse of scratch resources with the ScratchResourceManager, but
@@ -261,11 +268,8 @@
             // The scratch buffer's availability for reuse (scoped to the owning DrawBufferManager)
             // was tied to this BufferSubAllocator, so when that is reset, we just remove the buffer
             // from the set of unavailable buffers.
-            SkASSERT((fOwner->fMappingFailed && state.fUnavailableScratchBuffers.empty()) ||
-                     state.fUnavailableScratchBuffers.contains(fBuffer.get()));
-            if (!fOwner->fMappingFailed) {
-                state.fUnavailableScratchBuffers.remove(fBuffer.get());
-            }
+            SkASSERT(state.fUnavailableScratchBuffers.contains(fBuffer.get()));
+            state.fUnavailableScratchBuffers.remove(fBuffer.get());
 
             SkASSERT(!fTransferBuffer); // Scratch buffers shouldn't be using transfer buffers
             fOwner->fUsedBuffers.emplace_back(std::move(fBuffer), BindBufferInfo{});
--- a/src/gpu/graphite/BufferManager.h
+++ b/src/gpu/graphite/BufferManager.h
@@ -327,6 +327,10 @@
     // for recording buffer data for the next Recording.
     [[nodiscard]] bool transferToRecording(Recording*);
 
+#if defined(GPU_TEST_UTILS)
+    void testingOnly_onFailedBuffer() { this->onFailedBuffer(); }
+#endif
+
 private:
     friend class BufferSubAllocator;
 
