diff --git a/src/gpu/ganesh/GrDrawingManager.cpp b/src/gpu/ganesh/GrDrawingManager.cpp index 398bc35c5e2..d4dedefa189 100644 --- a/src/gpu/ganesh/GrDrawingManager.cpp +++ b/src/gpu/ganesh/GrDrawingManager.cpp @@ -175,6 +175,7 @@ bool GrDrawingManager::flush(SkSpan proxies, } bool cachePurgeNeeded = false; + bool renderTasksDropped = !preFlushSuccessful; if (preFlushSuccessful) { bool usingReorderedDAG = false; @@ -205,8 +206,8 @@ bool GrDrawingManager::flush(SkSpan proxies, resourceAllocator.assign(); } - cachePurgeNeeded = !resourceAllocator.failedInstantiation() && - this->executeRenderTasks(&flushState); + renderTasksDropped = resourceAllocator.failedInstantiation(); + cachePurgeNeeded = !renderTasksDropped && this->executeRenderTasks(&flushState); } this->removeRenderTasks(); @@ -226,7 +227,7 @@ bool GrDrawingManager::flush(SkSpan proxies, } fFlushing = false; - return true; + return !renderTasksDropped; } bool GrDrawingManager::submitToGpu() { diff --git a/src/gpu/ganesh/SurfaceContext.cpp b/src/gpu/ganesh/SurfaceContext.cpp index ba6bb936c47..fad3af5fdb7 100644 --- a/src/gpu/ganesh/SurfaceContext.cpp +++ b/src/gpu/ganesh/SurfaceContext.cpp @@ -254,6 +254,11 @@ bool SurfaceContext::readPixels(GrDirectContext* dContext, GrPixmap dst, SkIPoin tempCtx = dContext->priv().makeSC(std::move(view), this->colorInfo()); SkASSERT(tempCtx); } + // The intermediate surface may be backed by a recycled scratch resource, so make sure the + // draw/copy that fills it has actually been flushed before reading from it. + if (dContext->priv().flushSurface(tempCtx->asSurfaceProxy()) != GrSemaphoresSubmitted::kYes) { + return false; + } return tempCtx->readPixels(dContext, dst, pt); } diff --git a/tests/ReadWritePixelsGpuTest.cpp b/tests/ReadWritePixelsGpuTest.cpp index 143ff72d526..18465a06115 100644 --- a/tests/ReadWritePixelsGpuTest.cpp +++ b/tests/ReadWritePixelsGpuTest.cpp @@ -43,7 +43,9 @@ #include "src/gpu/ganesh/GrCaps.h" #include "src/gpu/ganesh/GrDataUtils.h" #include "src/gpu/ganesh/GrDirectContextPriv.h" +#include "src/gpu/ganesh/GrDrawingManager.h" #include "src/gpu/ganesh/GrImageInfo.h" +#include "src/gpu/ganesh/GrOnFlushResourceProvider.h" #include "src/gpu/ganesh/GrPixmap.h" #include "src/gpu/ganesh/GrSamplerState.h" #include "src/gpu/ganesh/GrSurfaceProxy.h" @@ -1513,3 +1515,72 @@ DEF_GANESH_TEST_FOR_GL_CONTEXT(GLReadPixelsUnbindPBO, ComparePixels(syncResult.pixmap(), asyncResult, tol, error); } + +namespace { +class FailingFlushCallback : public GrOnFlushCallbackObject { +public: + bool preFlush(GrOnFlushResourceProvider*) override { + ++fCount; + return false; + } + + int count() const { return fCount; } + +private: + int fCount = 0; +}; +} // namespace + +// readPixels() that uses an intermediate scratch surface should not return stale recycled scratch +// contents if the implicit flush of the queued draw into that surface drops its render tasks. +DEF_GANESH_TEST_FOR_RENDERING_CONTEXTS(ReadPixelsIntermediateFailedFlush, + reporter, + ctxInfo, + CtsEnforcement::kNextRelease) { + auto dContext = ctxInfo.directContext(); + + static constexpr int kSize = 100; + static constexpr SkColor kStaleColor = 0xFF112233; + static constexpr SkColor kSrcColor = 0xFF008800; + + auto srcII = SkImageInfo::Make({kSize, kSize}, kRGBA_8888_SkColorType, kPremul_SkAlphaType); + auto dstII = srcII.makeAlphaType(kUnpremul_SkAlphaType); + auto surf = SkSurfaces::RenderTarget(dContext, skgpu::Budgeted::kYes, srcII); + if (!surf) { + return; + } + + SkAutoPixmapStorage pixels; + pixels.alloc(dstII); + + // Reading the unpremul destination from the premul source draws through an intermediate + // approx-fit surface. Do this once so a matching scratch surface lands in the resource cache + // with known stale contents. + surf->getCanvas()->clear(kStaleColor); + if (!surf->readPixels(pixels, 0, 0)) { + return; + } + + surf->getCanvas()->clear(kSrcColor); + dContext->flushAndSubmit(GrSyncCpu::kYes); + + // From here on the flush-time callback fails so any queued render tasks are discarded. + FailingFlushCallback failingCallback; + dContext->priv().addOnFlushCallbackObject(&failingCallback); + + pixels.erase(SkColors::kTransparent); + bool ok = surf->readPixels(pixels, 0, 0); + + dContext->priv().drawingManager()->testingOnly_removeOnFlushCallbackObject(&failingCallback); + + if (ok) { + SkColor result = pixels.getColor(0, 0); + REPORTER_ASSERT(reporter, + result == kSrcColor, + "readPixels reported success but returned 0x%08x, expected 0x%08x", + result, + kSrcColor); + } else { + REPORTER_ASSERT(reporter, failingCallback.count() > 0); + } +}