From cee83412c5b59f9406ddc2e7147290ea9ac9d9aa Mon Sep 17 00:00:00 2001
From: Michael Ludwig <michaelludwig@google.com>
Date: Thu, 23 Jul 2026 16:23:44 -0400
Subject: [PATCH] [ganesh] Check results of internal flushes for
 internalWritePixels success

This moves the caps' dependent pre-flush out of newWritePixelsTask and
performs the flush inside internalWritePixels. Also checks for the
success of the flush at the end of internalWritePixels when the source
pixel data isn't owned. These use a shared helper function.

Adds a unit test that can trigger writing stale scratch texture contents
if the internalWritePixels' flush failed when performing an upload to
the scratch texture when taking a write-as-draw path for the primary
writePixels target.

Bug: https://issues.chromium.org/issues/536068737
Fixed: 536068737
Change-Id: I75c01c1db94c3c38ba302473a0ba502e5cff2c94
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/1302397
Reviewed-by: Thomas Smith <thomsmit@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Nathan Sanchez <nathanasanchez@google.com>
---
 src/gpu/ganesh/GrDrawingManager.cpp | 10 ----
 src/gpu/ganesh/SurfaceContext.cpp   | 20 ++++++-
 tests/ReadWritePixelsGpuTest.cpp    | 84 +++++++++++++++++++++++++++++
 3 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/src/gpu/ganesh/GrDrawingManager.cpp b/src/gpu/ganesh/GrDrawingManager.cpp
index 0b986ebff2..689f673602 100644
--- a/src/gpu/ganesh/GrDrawingManager.cpp
+++ b/src/gpu/ganesh/GrDrawingManager.cpp
@@ -995,16 +995,6 @@ bool GrDrawingManager::newWritePixelsTask(sk_sp<GrSurfaceProxy> dst,
     SkASSERT(fContext);
 
     this->closeActiveOpsTask();
-    const GrCaps& caps = *fContext->priv().caps();
-
-    // On platforms that prefer flushes over VRAM use (i.e., ANGLE) we're better off forcing a
-    // complete flush here.
-    if (!caps.preferVRAMUseOverFlushes()) {
-        this->flushSurfaces(SkSpan<GrSurfaceProxy*>{},
-                            SkSurfaces::BackendSurfaceAccess::kNoAccess,
-                            GrFlushInfo{},
-                            nullptr);
-    }
 
     GrRenderTask* task = this->appendTask(GrWritePixelsTask::Make(this,
                                                                   std::move(dst),
diff --git a/src/gpu/ganesh/SurfaceContext.cpp b/src/gpu/ganesh/SurfaceContext.cpp
index 32475beb1b..8f2e415591 100644
--- a/src/gpu/ganesh/SurfaceContext.cpp
+++ b/src/gpu/ganesh/SurfaceContext.cpp
@@ -585,6 +585,22 @@ bool SurfaceContext::internalWritePixels(GrDirectContext* dContext,
     }
     pt.fY = flip ? dstSurface->height() - pt.fY - src[0].height() : pt.fY;
 
+    auto flushSurfaceAndCheckSuccess = [dContext](GrSurfaceProxy* dstProxy, bool expectsTasks) {
+        const bool hasPendingTasks =
+                dContext->priv().drawingManager()->getLastRenderTask(dstProxy) != nullptr;
+        SkASSERT(!expectsTasks || hasPendingTasks);
+        GrSemaphoresSubmitted flushResult = dContext->priv().flushSurface(dstProxy);
+        return flushResult == GrSemaphoresSubmitted::kYes || !hasPendingTasks;
+    };
+
+    // On platforms that prefer flushes over VRAM use (i.e., ANGLE) we're better off forcing a
+    // complete flush here.
+    if (!caps->preferVRAMUseOverFlushes()) {
+        if (!flushSurfaceAndCheckSuccess(dstProxy, /*expectsTasks=*/false)) {
+            return false;
+        }
+    }
+
     if (!dContext->priv().drawingManager()->newWritePixelsTask(
                 sk_ref_sp(dstProxy),
                 SkIRect::MakePtSize(pt, src[0].dimensions()),
@@ -600,7 +616,9 @@ bool SurfaceContext::internalWritePixels(GrDirectContext* dContext,
     if (!ownAllStorage) {
         // If any pixmap doesn't own its pixels then we must flush so that the pixels are pushed to
         // the GPU before we return.
-        dContext->priv().flushSurface(dstProxy);
+        if (!flushSurfaceAndCheckSuccess(dstProxy, /*expectsTasks=*/true)) {
+            return false;
+        }
     }
     return true;
 }
-- 
2.53.0

