Fix check in GrGLGpu for whether PBO 0 is bound
Found in OOP-R canvas in Chrome. Most likely async read followed by
sync read with an intervening resetContext().
Bug: chromium:1208212
Change-Id: Ibf85f070d0a4cd389f67e9b249655b0ef667c66e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435277
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 5e3a9c7..2473b49 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -952,9 +952,10 @@
void GrGLGpu::unbindXferBuffer(GrGpuBufferType type) {
SkASSERT(type == GrGpuBufferType::kXferCpuToGpu || type == GrGpuBufferType::kXferGpuToCpu);
auto* xferBufferState = this->hwBufferState(type);
- if (!xferBufferState->fBoundBufferUniqueID.isInvalid()) {
+ if (!xferBufferState->fBufferZeroKnownBound) {
GL_CALL(BindBuffer(xferBufferState->fGLTarget, 0));
- xferBufferState->invalidate();
+ xferBufferState->fBoundBufferUniqueID.makeInvalid();
+ xferBufferState->fBufferZeroKnownBound = true;
}
}
diff --git a/tests/ReadWritePixelsGpuTest.cpp b/tests/ReadWritePixelsGpuTest.cpp
index 890363e..fa381c1 100644
--- a/tests/ReadWritePixelsGpuTest.cpp
+++ b/tests/ReadWritePixelsGpuTest.cpp
@@ -1239,3 +1239,59 @@
}
}
}
+
+// Tests a bug found in OOP-R canvas2d in Chrome. The GPU backend would incorrectly not bind
+// buffer 0 to GL_PIXEL_PACK_BUFFER before a glReadPixels() that was supposed to read into
+// client memory if a GrDirectContext::resetContext() occurred.
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(GLReadPixelsUnbindPBO, reporter, ctxInfo) {
+ // Start with a async read so that we bind to GL_PIXEL_PACK_BUFFER.
+ auto info = SkImageInfo::Make(16, 16, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+ SkAutoPixmapStorage pmap = make_ref_data(info, /*forceOpaque=*/false);
+ auto image = SkImage::MakeFromRaster(pmap, nullptr, nullptr);
+ image = image->makeTextureImage(ctxInfo.directContext());
+ if (!image) {
+ ERRORF(reporter, "Couldn't make texture image.");
+ return;
+ }
+
+ AsyncContext asyncContext;
+ image->asyncRescaleAndReadPixels(info,
+ SkIRect::MakeSize(info.dimensions()),
+ SkImage::RescaleGamma::kSrc,
+ SkImage::RescaleMode::kNearest,
+ async_callback,
+ &asyncContext);
+
+ // This will force the async readback to finish.
+ ctxInfo.directContext()->flushAndSubmit(true);
+ if (!asyncContext.fCalled) {
+ ERRORF(reporter, "async_callback not called.");
+ }
+ if (!asyncContext.fResult) {
+ ERRORF(reporter, "async read failed.");
+ }
+
+ SkPixmap asyncResult(info, asyncContext.fResult->data(0), asyncContext.fResult->rowBytes(0));
+
+ // Bug was that this would cause GrGLGpu to think no buffer was left bound to
+ // GL_PIXEL_PACK_BUFFER even though async transfer did leave one bound. So the sync read
+ // wouldn't bind buffer 0.
+ ctxInfo.directContext()->resetContext();
+
+ SkBitmap syncResult;
+ syncResult.allocPixels(info);
+ syncResult.eraseARGB(0xFF, 0xFF, 0xFF, 0xFF);
+
+ image->readPixels(ctxInfo.directContext(), syncResult.pixmap(), 0, 0);
+
+ float tol[4] = {}; // expect exactly same pixels, no conversions.
+ auto error = std::function<ComparePixmapsErrorReporter>([&](int x, int y,
+ const float diffs[4]) {
+ SkASSERT(x >= 0 && y >= 0);
+ ERRORF(reporter, "Expect sync and async read to be the same. "
+ "Error at %d, %d. Diff in floats: (%f, %f, %f, %f)",
+ x, y, diffs[0], diffs[1], diffs[2], diffs[3]);
+ });
+
+ ComparePixels(syncResult.pixmap(), asyncResult, tol, error);
+}
