Support Canvas Clip on Blit Framebuffer
The previous fix to blit framebuffer didn't take cases where
the canvas had a clip applied into account. Fix and update
the unit test to add this case.
AND
Respect kRectsMustMatchForMSAASrc_BlitFramebufferFlag in dst setup
Crurently, when preparing a texture for blitFramebuffer, we ignore the
kRectsMustMatchForMSAASrc_BlitFramebufferFlag, and may attempt to
copy from one src rect to a different dst rect.
This change updates initDescForDstCopy and setupDstTexture to allocate
larger textures if necessary and accomodate this flags requirements.
Bug: 658277, 658277
NOTREECHECKS=true
NOTRY=true
NOPRESUBMIT=true
Change-Id: I36a54072ee15d035249ef0b0f6cf512e791dd218
Reviewed-on: https://skia-review.googlesource.com/13085
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 201b22c..faf1dce 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -311,9 +311,12 @@
* This is can be called before allocating a texture to be a dst for copySurface. This is only
* used for doing dst copies needed in blends, thus the src is always a GrRenderTarget. It will
* populate the origin, config, and flags fields of the desc such that copySurface can
- * efficiently succeed.
+ * efficiently succeed. rectsMustMatch will be set to true if the copy operation must ensure
+ * that the src and dest rects are identical. disallowSubrect will be set to true if copy rect
+ * must equal src's bounds.
*/
- virtual bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const = 0;
+ virtual bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc,
+ bool* rectsMustMatch, bool* disallowSubrect) const = 0;
// After the client interacts directly with the 3D context state the GrGpu
// must resync its internal state and assumptions about 3D context state.
diff --git a/src/gpu/GrRenderTargetOpList.cpp b/src/gpu/GrRenderTargetOpList.cpp
index 4fc898c..5c1deab 100644
--- a/src/gpu/GrRenderTargetOpList.cpp
+++ b/src/gpu/GrRenderTargetOpList.cpp
@@ -99,9 +99,6 @@
const GrClip& clip,
const SkRect& opBounds,
GrXferProcessor::DstTexture* dstTexture) {
- SkRect bounds = opBounds;
- bounds.outset(0.5f, 0.5f);
-
if (this->caps()->textureBarrierSupport()) {
if (GrTexture* rtTex = rt->asTexture()) {
// The render target is a texture, so we can read from it directly in the shader. The XP
@@ -112,12 +109,15 @@
}
}
- SkIRect copyRect;
- clip.getConservativeBounds(rt->width(), rt->height(), ©Rect);
+ SkIRect copyRect = SkIRect::MakeWH(rt->width(), rt->height());
+ SkIRect clippedRect;
+ clip.getConservativeBounds(rt->width(), rt->height(), &clippedRect);
SkIRect drawIBounds;
- bounds.roundOut(&drawIBounds);
- if (!copyRect.intersect(drawIBounds)) {
+ opBounds.roundOut(&drawIBounds);
+ // Cover up for any precision issues by outsetting the op bounds a pixel in each direction.
+ drawIBounds.outset(1, 1);
+ if (!clippedRect.intersect(drawIBounds)) {
#ifdef SK_DEBUG
GrCapsDebugf(this->caps(), "Missed an early reject. "
"Bailing on draw from setupDstTexture.\n");
@@ -128,26 +128,45 @@
// MSAA consideration: When there is support for reading MSAA samples in the shader we could
// have per-sample dst values by making the copy multisampled.
GrSurfaceDesc desc;
- if (!fGpu->initDescForDstCopy(rt, &desc)) {
+ bool rectsMustMatch = false;
+ bool disallowSubrect = false;
+ if (!fGpu->initDescForDstCopy(rt, &desc, &rectsMustMatch, &disallowSubrect)) {
desc.fOrigin = kDefault_GrSurfaceOrigin;
desc.fFlags = kRenderTarget_GrSurfaceFlag;
desc.fConfig = rt->config();
}
- desc.fWidth = copyRect.width();
- desc.fHeight = copyRect.height();
+ if (!disallowSubrect) {
+ copyRect = clippedRect;
+ }
+ SkIPoint dstPoint;
+ SkIPoint dstOffset;
static const uint32_t kFlags = 0;
- sk_sp<GrTexture> copy(fResourceProvider->createApproxTexture(desc, kFlags));
+ sk_sp<GrTexture> copy;
+ if (rectsMustMatch) {
+ SkASSERT(desc.fOrigin == rt->origin());
+ desc.fWidth = rt->width();
+ desc.fHeight = rt->height();
+ dstPoint = {copyRect.fLeft, copyRect.fTop};
+ dstOffset = {0, 0};
+ copy.reset(fContext->resourceProvider()->createTexture(desc, SkBudgeted::kYes, kFlags));
+ } else {
+ desc.fWidth = copyRect.width();
+ desc.fHeight = copyRect.height();
+ dstPoint = {0, 0};
+ dstOffset = {copyRect.fLeft, copyRect.fTop};
+ copy.reset(fContext->resourceProvider()->createApproxTexture(desc, kFlags));
+ }
if (!copy) {
SkDebugf("Failed to create temporary copy of destination texture.\n");
return;
}
- SkIPoint dstPoint = {0, 0};
- this->copySurface(copy.get(), rt, copyRect, dstPoint);
+
+ fGpu->copySurface(copy.get(), rt, copyRect, dstPoint);
dstTexture->setTexture(std::move(copy));
- dstTexture->setOffset(copyRect.fLeft, copyRect.fTop);
+ dstTexture->setOffset(dstOffset);
}
void GrRenderTargetOpList::prepareOps(GrOpFlushState* flushState) {
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 0bcb7e8..3d0cebf 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -1029,6 +1029,7 @@
// is available.
if (ctxInfo.version() >= GR_GL_VER(3, 0)) {
fBlitFramebufferFlags = kNoFormatConversionForMSAASrc_BlitFramebufferFlag |
+ kNoMSAADst_BlitFramebufferFlag |
kRectsMustMatchForMSAASrc_BlitFramebufferFlag;
} else if (ctxInfo.hasExtension("GL_CHROMIUM_framebuffer_multisample") ||
ctxInfo.hasExtension("GL_ANGLE_framebuffer_blit")) {
@@ -1037,7 +1038,8 @@
fBlitFramebufferFlags = kNoScalingOrMirroring_BlitFramebufferFlag |
kResolveMustBeFull_BlitFrambufferFlag |
kNoMSAADst_BlitFramebufferFlag |
- kNoFormatConversion_BlitFramebufferFlag;
+ kNoFormatConversion_BlitFramebufferFlag |
+ kRectsMustMatchForMSAASrc_BlitFramebufferFlag;
}
} else {
if (fUsesMixedSamples) {
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index cfbef9f..bc4e7d6 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -3574,8 +3574,13 @@
}
}
if (GrGLCaps::kResolveMustBeFull_BlitFrambufferFlag & blitFramebufferFlags) {
- if (srcRT && srcRT->numColorSamples() && dstRT && !dstRT->numColorSamples()) {
- return false;
+ if (srcRT && srcRT->numColorSamples()) {
+ if (dstRT && !dstRT->numColorSamples()) {
+ return false;
+ }
+ if (SkRect::Make(srcRect) != srcRT->getBoundsRect()) {
+ return false;
+ }
}
}
if (GrGLCaps::kNoMSAADst_BlitFramebufferFlag & blitFramebufferFlags) {
@@ -3594,9 +3599,13 @@
}
}
if (GrGLCaps::kRectsMustMatchForMSAASrc_BlitFramebufferFlag & blitFramebufferFlags) {
- if (srcRT && srcRT->numColorSamples() &&
- (dstPoint.fX != srcRect.fLeft || dstPoint.fY != srcRect.fTop)) {
- return false;
+ if (srcRT && srcRT->numColorSamples()) {
+ if (dstPoint.fX != srcRect.fLeft || dstPoint.fY != srcRect.fTop) {
+ return false;
+ }
+ if (dst->origin() != src->origin()) {
+ return false;
+ }
}
}
return true;
@@ -3693,7 +3702,14 @@
}
}
-bool GrGLGpu::initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const {
+bool GrGLGpu::initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc,
+ bool* rectsMustMatch, bool* disallowSubrect) const {
+ // By default, we don't require rects to match.
+ *rectsMustMatch = false;
+
+ // By default, we allow subrects.
+ *disallowSubrect = false;
+
// If the src is a texture, we can implement the blit as a draw assuming the config is
// renderable.
if (src->asTexture() && this->caps()->isConfigRenderable(src->config(), false)) {
@@ -3713,21 +3729,35 @@
// possible and we return false to fallback to creating a render target dst for render-to-
// texture. This code prefers CopyTexSubImage to fbo blit and avoids triggering temporary fbo
// creation. It isn't clear that avoiding temporary fbo creation is actually optimal.
-
GrSurfaceOrigin originForBlitFramebuffer = kDefault_GrSurfaceOrigin;
- if (this->glCaps().blitFramebufferSupportFlags() &
- GrGLCaps::kNoScalingOrMirroring_BlitFramebufferFlag) {
+ bool rectsMustMatchForBlitFramebuffer = false;
+ bool disallowSubrectForBlitFramebuffer = false;
+ if (src->numColorSamples() && (this->glCaps().blitFramebufferSupportFlags() &
+ GrGLCaps::kResolveMustBeFull_BlitFrambufferFlag)) {
+ rectsMustMatchForBlitFramebuffer = true;
+ disallowSubrectForBlitFramebuffer = true;
+ // Mirroring causes rects to mismatch later, don't allow it.
+ originForBlitFramebuffer = src->origin();
+ } else if (src->numColorSamples() &&
+ (this->glCaps().blitFramebufferSupportFlags() &
+ GrGLCaps::kRectsMustMatchForMSAASrc_BlitFramebufferFlag)) {
+ rectsMustMatchForBlitFramebuffer = true;
+ // Mirroring causes rects to mismatch later, don't allow it.
+ originForBlitFramebuffer = src->origin();
+ } else if (this->glCaps().blitFramebufferSupportFlags() &
+ GrGLCaps::kNoScalingOrMirroring_BlitFramebufferFlag) {
originForBlitFramebuffer = src->origin();
}
// Check for format issues with glCopyTexSubImage2D
- if (kGLES_GrGLStandard == this->glStandard() && this->glCaps().bgraIsInternalFormat() &&
- kBGRA_8888_GrPixelConfig == src->config()) {
+ if (this->glCaps().bgraIsInternalFormat() && kBGRA_8888_GrPixelConfig == src->config()) {
// glCopyTexSubImage2D doesn't work with this config. If the bgra can be used with fbo blit
// then we set up for that, otherwise fail.
if (this->glCaps().canConfigBeFBOColorAttachment(kBGRA_8888_GrPixelConfig)) {
desc->fOrigin = originForBlitFramebuffer;
desc->fConfig = kBGRA_8888_GrPixelConfig;
+ *rectsMustMatch = rectsMustMatchForBlitFramebuffer;
+ *disallowSubrect = disallowSubrectForBlitFramebuffer;
return true;
}
return false;
@@ -3740,6 +3770,8 @@
if (this->glCaps().canConfigBeFBOColorAttachment(src->config())) {
desc->fOrigin = originForBlitFramebuffer;
desc->fConfig = src->config();
+ *rectsMustMatch = rectsMustMatchForBlitFramebuffer;
+ *disallowSubrect = disallowSubrectForBlitFramebuffer;
return true;
}
return false;
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 57068fb..a4f3784 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -75,7 +75,8 @@
GrPixelConfig srcConfig, DrawPreference*,
WritePixelTempDrawInfo*) override;
- bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const override;
+ bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc, bool* rectsMustMatch,
+ bool* disallowSubrect) const override;
// These functions should be used to bind GL objects. They track the GL state and skip redundant
// bindings. Making the equivalent glBind calls directly will confuse the state tracking.
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index b85aa2d..24a6dcd 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1590,14 +1590,18 @@
return false;
}
-bool GrVkGpu::initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const {
+bool GrVkGpu::initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc,
+ bool* rectsMustMatch, bool* disallowSubrect) const {
+ // Vk doesn't use rectsMustMatch or disallowSubrect. Always return false.
+ *rectsMustMatch = false;
+ *disallowSubrect = false;
+
// We can always succeed here with either a CopyImage (none msaa src) or ResolveImage (msaa).
// For CopyImage we can make a simple texture, for ResolveImage we require the dst to be a
// render target as well.
desc->fOrigin = src->origin();
desc->fConfig = src->config();
- if (src->numColorSamples() > 1 ||
- (src->asTexture() && this->vkCaps().supportsCopiesAsDraws())) {
+ if (src->numColorSamples() > 1 || (src->asTexture() && this->supportsCopiesAsDraws())) {
desc->fFlags = kRenderTarget_GrSurfaceFlag;
} else {
// Just going to use CopyImage here
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index 7203bf1..4bfd5de 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -79,7 +79,8 @@
void onQueryMultisampleSpecs(GrRenderTarget* rt, const GrStencilSettings&,
int* effectiveSampleCnt, SamplePattern*) override;
- bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const override;
+ bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc, bool* rectsMustMatch,
+ bool* disallowSubrect) const override;
void xferBarrier(GrRenderTarget*, GrXferBarrierType) override {}
diff --git a/tests/BlendTest.cpp b/tests/BlendTest.cpp
index 785acb86..193c37d 100644
--- a/tests/BlendTest.cpp
+++ b/tests/BlendTest.cpp
@@ -5,11 +5,24 @@
* found in the LICENSE file.
*/
-#include "Test.h"
+#include <functional>
+#include "SkBitmap.h"
+#include "SkCanvas.h"
#include "SkColor.h"
#include "SkColorPriv.h"
+#include "SkSurface.h"
#include "SkTaskGroup.h"
-#include <functional>
+#include "SkUtils.h"
+#include "Test.h"
+
+#if SK_SUPPORT_GPU
+#include "GrContext.h"
+#include "GrContextPriv.h"
+#include "GrResourceProvider.h"
+#include "GrSurfaceContext.h"
+#include "GrSurfaceProxy.h"
+#include "GrTexture.h"
+#endif
struct Results { int diffs, diffs_0x00, diffs_0xff, diffs_by_1; };
@@ -66,3 +79,120 @@
};
for (auto multiply : perfect) { REPORTER_ASSERT(r, test(multiply).diffs == 0); }
}
+
+#if SK_SUPPORT_GPU
+namespace {
+static sk_sp<SkSurface> create_gpu_surface_backend_texture_as_render_target(
+ GrContext* context, int sampleCnt, int width, int height, GrPixelConfig config,
+ sk_sp<GrTexture>* backingSurface) {
+ GrSurfaceDesc backingDesc;
+ backingDesc.fHeight = height;
+ backingDesc.fWidth = width;
+ backingDesc.fConfig = config;
+ backingDesc.fOrigin = kDefault_GrSurfaceOrigin;
+ backingDesc.fFlags = kRenderTarget_GrSurfaceFlag;
+
+ (*backingSurface)
+ .reset(context->resourceProvider()->createTexture(backingDesc, SkBudgeted::kNo));
+
+ GrBackendTextureDesc desc;
+ desc.fConfig = config;
+ desc.fWidth = width;
+ desc.fHeight = height;
+ desc.fFlags = kRenderTarget_GrBackendTextureFlag;
+ desc.fTextureHandle = (*backingSurface)->getTextureHandle();
+ desc.fSampleCnt = sampleCnt;
+ sk_sp<SkSurface> surface =
+ SkSurface::MakeFromBackendTextureAsRenderTarget(context, desc, nullptr);
+ return surface;
+}
+}
+
+// Tests blending to a surface with no texture available.
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ES2BlendWithNoTexture, reporter, ctxInfo) {
+ GrContext* context = ctxInfo.grContext();
+ const int kWidth = 10;
+ const int kHeight = 10;
+ const GrPixelConfig kConfig = kRGBA_8888_GrPixelConfig;
+ const SkColorType kColorType = kRGBA_8888_SkColorType;
+
+ // Build our test cases:
+ struct RectAndSamplePoint {
+ SkRect rect;
+ SkIPoint outPoint;
+ SkIPoint inPoint;
+ } allRectsAndPoints[3] = {
+ {SkRect::MakeXYWH(0, 0, 5, 5), SkIPoint::Make(7, 7), SkIPoint::Make(2, 2)},
+ {SkRect::MakeXYWH(2, 2, 5, 5), SkIPoint::Make(1, 1), SkIPoint::Make(4, 4)},
+ {SkRect::MakeXYWH(5, 5, 5, 5), SkIPoint::Make(2, 2), SkIPoint::Make(7, 7)},
+ };
+
+ struct TestCase {
+ RectAndSamplePoint rectAndPoints;
+ SkRect clip;
+ int sampleCnt;
+ };
+ std::vector<TestCase> testCases;
+
+ for (int sampleCnt : {0, 4}) {
+ for (auto rectAndPoints : allRectsAndPoints) {
+ for (auto clip : {SkRect::MakeXYWH(0, 0, 10, 10), SkRect::MakeXYWH(1, 1, 8, 8)}) {
+ testCases.push_back({rectAndPoints, clip, sampleCnt});
+ }
+ }
+ }
+
+ // Run each test case:
+ for (auto testCase : testCases) {
+ int sampleCnt = testCase.sampleCnt;
+ SkRect paintRect = testCase.rectAndPoints.rect;
+ SkIPoint outPoint = testCase.rectAndPoints.outPoint;
+ SkIPoint inPoint = testCase.rectAndPoints.inPoint;
+
+ sk_sp<GrTexture> backingSurface;
+ // BGRA forces a framebuffer blit on ES2.
+ sk_sp<SkSurface> surface = create_gpu_surface_backend_texture_as_render_target(
+ context, sampleCnt, kWidth, kHeight, kConfig, &backingSurface);
+
+ if (!surface && sampleCnt > 0) {
+ // Some platforms don't support MSAA.
+ continue;
+ }
+ REPORTER_ASSERT(reporter, !!surface);
+
+ // Fill our canvas with 0xFFFF80
+ SkCanvas* canvas = surface->getCanvas();
+ canvas->clipRect(testCase.clip, false);
+ SkPaint black_paint;
+ black_paint.setColor(SkColorSetRGB(0xFF, 0xFF, 0x80));
+ canvas->drawRect(SkRect::MakeXYWH(0, 0, kWidth, kHeight), black_paint);
+
+ // Blend 2x2 pixels at 5,5 with 0x80FFFF. Use multiply blend mode as this will trigger
+ // a copy of the destination.
+ SkPaint white_paint;
+ white_paint.setColor(SkColorSetRGB(0x80, 0xFF, 0xFF));
+ white_paint.setBlendMode(SkBlendMode::kMultiply);
+ canvas->drawRect(paintRect, white_paint);
+
+ // Read the result into a bitmap.
+ SkBitmap bitmap;
+ REPORTER_ASSERT(reporter, bitmap.tryAllocPixels(SkImageInfo::Make(
+ kWidth, kHeight, kColorType, kPremul_SkAlphaType)));
+ bitmap.lockPixels();
+ REPORTER_ASSERT(
+ reporter,
+ surface->readPixels(bitmap.info(), bitmap.getPixels(), bitmap.rowBytes(), 0, 0));
+
+ // Check the in/out pixels.
+ REPORTER_ASSERT(reporter, bitmap.getColor(outPoint.x(), outPoint.y()) ==
+ SkColorSetRGB(0xFF, 0xFF, 0x80));
+ REPORTER_ASSERT(reporter, bitmap.getColor(inPoint.x(), inPoint.y()) ==
+ SkColorSetRGB(0x80, 0xFF, 0x80));
+
+ // Clean up - surface depends on backingSurface and must be released first.
+ bitmap.unlockPixels();
+ surface.reset();
+ backingSurface.reset();
+ }
+}
+#endif
diff --git a/tools/gpu/GrTest.cpp b/tools/gpu/GrTest.cpp
index 4df328c..f23f40d 100644
--- a/tools/gpu/GrTest.cpp
+++ b/tools/gpu/GrTest.cpp
@@ -310,7 +310,8 @@
*effectiveSampleCnt = rt->desc().fSampleCnt;
}
- bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc) const override {
+ bool initDescForDstCopy(const GrRenderTarget* src, GrSurfaceDesc* desc, bool* rectsMustMatch,
+ bool* disallowSubrect) const override {
return false;
}
