Make copySurface work for texture dsts, return a bool, & add unit test.
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1684313002
Committed: https://skia.googlesource.com/skia/+/7ea5e28065e5eb797e95f5d81c1a65cf3209d741
Review URL: https://codereview.chromium.org/1684313002
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 187a3ca..296dfb9 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -18,6 +18,7 @@
#include "SkConfig8888.h"
#include "SkGrPriv.h"
+#include "batches/GrCopySurfaceBatch.h"
#include "effects/GrConfigConversionEffect.h"
#include "text/GrTextBlobCache.h"
@@ -509,34 +510,42 @@
}
}
-void GrContext::copySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
- const SkIPoint& dstPoint, uint32_t pixelOpsFlags) {
+bool GrContext::copySurface(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
+ const SkIPoint& dstPoint) {
ASSERT_SINGLE_OWNER
- RETURN_IF_ABANDONED
+ RETURN_FALSE_IF_ABANDONED
GR_AUDIT_TRAIL_AUTO_FRAME(&fAuditTrail, "GrContext::copySurface");
if (!src || !dst) {
- return;
+ return false;
}
ASSERT_OWNED_RESOURCE(src);
ASSERT_OWNED_RESOURCE(dst);
- // Since we're going to the draw target and not GPU, no need to check kNoFlush
- // here.
if (!dst->asRenderTarget()) {
- return;
+ SkIRect clippedSrcRect;
+ SkIPoint clippedDstPoint;
+ if (!GrCopySurfaceBatch::ClipSrcRectAndDstPoint(dst, src, srcRect, dstPoint,
+ &clippedSrcRect, &clippedDstPoint)) {
+ return false;
+ }
+ // If we don't have an RT for the dst then we won't have a GrDrawContext to insert the
+ // the copy surface into. In the future we plan to have a more limited Context type
+ // (GrCopyContext?) that has the subset of GrDrawContext operations that should be
+ // allowed on textures that aren't render targets.
+ // For now we just flush any writes to the src and issue an immediate copy to the dst.
+ src->flushWrites();
+ return fGpu->copySurface(dst, src, clippedSrcRect, clippedDstPoint);
}
-
SkAutoTUnref<GrDrawContext> drawContext(this->drawContext(dst->asRenderTarget()));
if (!drawContext) {
- return;
+ return false;
}
- drawContext->copySurface(src, srcRect, dstPoint);
-
- if (kFlushWrites_PixelOp & pixelOpsFlags) {
- this->flush();
+ if (!drawContext->copySurface(src, srcRect, dstPoint)) {
+ return false;
}
+ return true;
}
void GrContext::flushSurfaceWrites(GrSurface* surface) {
diff --git a/src/gpu/GrDrawContext.cpp b/src/gpu/GrDrawContext.cpp
index f37c48c..d971f58 100644
--- a/src/gpu/GrDrawContext.cpp
+++ b/src/gpu/GrDrawContext.cpp
@@ -97,13 +97,13 @@
return fDrawTarget;
}
-void GrDrawContext::copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint) {
+bool GrDrawContext::copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint) {
ASSERT_SINGLE_OWNER
- RETURN_IF_ABANDONED
+ RETURN_FALSE_IF_ABANDONED
SkDEBUGCODE(this->validate();)
GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrDrawContext::copySurface");
- this->getDrawTarget()->copySurface(fRenderTarget, src, srcRect, dstPoint);
+ return this->getDrawTarget()->copySurface(fRenderTarget, src, srcRect, dstPoint);
}
void GrDrawContext::drawText(const GrClip& clip, const GrPaint& grPaint,
diff --git a/src/gpu/GrDrawTarget.cpp b/src/gpu/GrDrawTarget.cpp
index b9dc794..9f15c11 100644
--- a/src/gpu/GrDrawTarget.cpp
+++ b/src/gpu/GrDrawTarget.cpp
@@ -406,19 +406,21 @@
////////////////////////////////////////////////////////////////////////////////
-void GrDrawTarget::copySurface(GrSurface* dst,
+bool GrDrawTarget::copySurface(GrSurface* dst,
GrSurface* src,
const SkIRect& srcRect,
const SkIPoint& dstPoint) {
GrBatch* batch = GrCopySurfaceBatch::Create(dst, src, srcRect, dstPoint);
- if (batch) {
+ if (!batch) {
+ return false;
+ }
#ifdef ENABLE_MDB
- this->addDependency(src);
+ this->addDependency(src);
#endif
- this->recordBatch(batch);
- batch->unref();
- }
+ this->recordBatch(batch);
+ batch->unref();
+ return true;
}
template <class Left, class Right> static bool intersect(const Left& a, const Right& b) {
diff --git a/src/gpu/GrDrawTarget.h b/src/gpu/GrDrawTarget.h
index 55c11da..a850efd 100644
--- a/src/gpu/GrDrawTarget.h
+++ b/src/gpu/GrDrawTarget.h
@@ -144,7 +144,7 @@
* depending on the type of surface, configs, etc, and the backend-specific
* limitations.
*/
- void copySurface(GrSurface* dst,
+ bool copySurface(GrSurface* dst,
GrSurface* src,
const SkIRect& srcRect,
const SkIPoint& dstPoint);
diff --git a/src/gpu/SkGrPixelRef.cpp b/src/gpu/SkGrPixelRef.cpp
index 3876f17..e48cbf5 100644
--- a/src/gpu/SkGrPixelRef.cpp
+++ b/src/gpu/SkGrPixelRef.cpp
@@ -84,10 +84,10 @@
}
// Blink is relying on the above copy being sent to GL immediately in the case when the source
- // is a WebGL canvas backing store. We could have a TODO to remove this flush flag, but we have
+ // is a WebGL canvas backing store. We could have a TODO to remove this flush, but we have
// a larger TODO to remove SkGrPixelRef entirely.
- context->copySurface(dst->asRenderTarget(), texture, srcRect, SkIPoint::Make(0,0),
- GrContext::kFlushWrites_PixelOp);
+ context->copySurface(dst, texture, srcRect, SkIPoint::Make(0,0));
+ context->flushSurfaceWrites(dst);
SkImageInfo info = SkImageInfo::Make(desc.fWidth, desc.fHeight, dstCT, kPremul_SkAlphaType,
dstPT);
diff --git a/src/gpu/batches/GrCopySurfaceBatch.cpp b/src/gpu/batches/GrCopySurfaceBatch.cpp
index 098f7c7..a59ed38 100644
--- a/src/gpu/batches/GrCopySurfaceBatch.cpp
+++ b/src/gpu/batches/GrCopySurfaceBatch.cpp
@@ -9,12 +9,12 @@
#include "GrCopySurfaceBatch.h"
// returns true if the read/written rect intersects the src/dst and false if not.
-static bool clip_srcrect_and_dstpoint(const GrSurface* dst,
- const GrSurface* src,
- const SkIRect& srcRect,
- const SkIPoint& dstPoint,
- SkIRect* clippedSrcRect,
- SkIPoint* clippedDstPoint) {
+bool GrCopySurfaceBatch::ClipSrcRectAndDstPoint(const GrSurface* dst,
+ const GrSurface* src,
+ const SkIRect& srcRect,
+ const SkIPoint& dstPoint,
+ SkIRect* clippedSrcRect,
+ SkIPoint* clippedDstPoint) {
*clippedSrcRect = srcRect;
*clippedDstPoint = dstPoint;
@@ -67,12 +67,7 @@
SkIRect clippedSrcRect;
SkIPoint clippedDstPoint;
// If the rect is outside the src or dst then we've already succeeded.
- if (!clip_srcrect_and_dstpoint(dst,
- src,
- srcRect,
- dstPoint,
- &clippedSrcRect,
- &clippedDstPoint)) {
+ if (!ClipSrcRectAndDstPoint(dst, src, srcRect, dstPoint, &clippedSrcRect, &clippedDstPoint)) {
return nullptr;
}
return new GrCopySurfaceBatch(dst, src, clippedSrcRect, clippedDstPoint);
diff --git a/src/gpu/batches/GrCopySurfaceBatch.h b/src/gpu/batches/GrCopySurfaceBatch.h
index 7bf8d8d..3926643 100644
--- a/src/gpu/batches/GrCopySurfaceBatch.h
+++ b/src/gpu/batches/GrCopySurfaceBatch.h
@@ -17,6 +17,16 @@
public:
DEFINE_BATCH_CLASS_ID
+ /** This should not really be exposed as Create() will apply this clipping, but there is
+ * currently a workaround in GrContext::copySurface() for non-render target dsts that relies
+ * on it. */
+ static bool ClipSrcRectAndDstPoint(const GrSurface* dst,
+ const GrSurface* src,
+ const SkIRect& srcRect,
+ const SkIPoint& dstPoint,
+ SkIRect* clippedSrcRect,
+ SkIPoint* clippedDstPoint);
+
static GrBatch* Create(GrSurface* dst, GrSurface* src, const SkIRect& srcRect,
const SkIPoint& dstPoint);
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index 04939e3..c502fc0 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -326,7 +326,8 @@
const SkIRect srcR = SkIRect::MakeWH(desc.fWidth, desc.fHeight);
const SkIPoint dstP = SkIPoint::Make(0, 0);
- ctx->copySurface(dst, src, srcR, dstP, GrContext::kFlushWrites_PixelOp);
+ ctx->copySurface(dst, src, srcR, dstP);
+ ctx->flushSurfaceWrites(dst);
return dst;
}