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

Review URL: https://codereview.chromium.org/1684313002
This commit is contained in:
bsalomon 2016-02-11 11:42:31 -08:00 committed by Commit bot
parent e1745a1efc
commit 7ea5e28065
11 changed files with 224 additions and 51 deletions

View File

@ -278,24 +278,17 @@ public:
* @param src the surface to copy from.
* @param srcRect the rectangle of the src that should be copied.
* @param dstPoint the translation applied when writing the srcRect's pixels to the dst.
* @param pixelOpsFlags see PixelOpsFlags enum above. (kUnpremul_PixelOpsFlag is not allowed).
*/
void copySurface(GrSurface* dst,
bool copySurface(GrSurface* dst,
GrSurface* src,
const SkIRect& srcRect,
const SkIPoint& dstPoint,
uint32_t pixelOpsFlags = 0);
const SkIPoint& dstPoint);
/** Helper that copies the whole surface but fails when the two surfaces are not identically
sized. */
bool copySurface(GrSurface* dst, GrSurface* src) {
if (NULL == dst || NULL == src || dst->width() != src->width() ||
dst->height() != src->height()) {
return false;
}
this->copySurface(dst, src, SkIRect::MakeWH(dst->width(), dst->height()),
SkIPoint::Make(0,0));
return true;
return this->copySurface(dst, src, SkIRect::MakeWH(dst->width(), dst->height()),
SkIPoint::Make(0,0));
}
/**

View File

@ -47,7 +47,7 @@ class SK_API GrDrawContext : public SkRefCnt {
public:
~GrDrawContext() override;
void copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint);
bool copySurface(GrSurface* src, const SkIRect& srcRect, const SkIPoint& dstPoint);
// TODO: it is odd that we need both the SkPaint in the following 3 methods.
// We should extract the text parameters from SkPaint and pass them separately

View File

@ -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::prepareSurfaceForExternalIO(GrSurface* surface) {
}
}
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) {

View File

@ -97,13 +97,13 @@ GrDrawTarget* GrDrawContext::getDrawTarget() {
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,

View File

@ -406,19 +406,21 @@ void GrDrawTarget::discard(GrRenderTarget* renderTarget) {
////////////////////////////////////////////////////////////////////////////////
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) {

View File

@ -144,7 +144,7 @@ public:
* 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);

View File

@ -84,10 +84,10 @@ static SkGrPixelRef* copy_to_new_texture_pixelref(GrTexture* texture, SkColorTyp
}
// 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);

View File

@ -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 @@ GrBatch* GrCopySurfaceBatch::Create(GrSurface* dst, GrSurface* src, const SkIRec
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);

View File

@ -17,6 +17,16 @@ class GrCopySurfaceBatch final : public GrBatch {
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);

View File

@ -326,7 +326,8 @@ GrTexture* GrDeepCopyTexture(GrTexture* src, bool budgeted) {
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;
}

163
tests/CopySurfaceTest.cpp Normal file
View File

@ -0,0 +1,163 @@
/*
* Copyright 2016 Google Inc.
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include <initializer_list>
#include "Test.h"
#if SK_SUPPORT_GPU
#include "GrContext.h"
#include "GrTexture.h"
#include "GrTextureProvider.h"
#include "SkUtils.h"
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(CopySurface, reporter, context) {
static const int kW = 10;
static const int kH = 10;
static const size_t kRowBytes = sizeof(uint32_t) * kW;
GrSurfaceDesc baseDesc;
baseDesc.fConfig = kRGBA_8888_GrPixelConfig;
baseDesc.fWidth = kW;
baseDesc.fHeight = kH;
SkAutoTMalloc<uint32_t> srcPixels(kW * kH);
for (int i = 0; i < kW * kH; ++i) {
srcPixels.get()[i] = i;
}
SkAutoTMalloc<uint32_t> dstPixels(kW * kH);
for (int i = 0; i < kW * kH; ++i) {
dstPixels.get()[i] = ~i;
}
static const SkIRect kSrcRects[] {
{ 0, 0, kW , kH },
{-1, -1, kW+1, kH+1},
{ 1, 1, kW-1, kH-1},
{ 5, 5, 6 , 6 },
};
static const SkIPoint kDstPoints[] {
{ 0 , 0 },
{ 1 , 1 },
{ kW/2, kH/4},
{ kW-1, kH-1},
{ kW , kH },
{ kW+1, kH+2},
{-1 , -1 },
};
SkAutoTMalloc<uint32_t> read(kW * kH);
for (auto sOrigin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) {
for (auto dOrigin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) {
for (auto sFlags: {kRenderTarget_GrSurfaceFlag, kNone_GrSurfaceFlags}) {
for (auto dFlags: {kRenderTarget_GrSurfaceFlag, kNone_GrSurfaceFlags}) {
for (auto srcRect : kSrcRects) {
for (auto dstPoint : kDstPoints) {
GrSurfaceDesc srcDesc = baseDesc;
srcDesc.fOrigin = sOrigin;
srcDesc.fFlags = sFlags;
GrSurfaceDesc dstDesc = baseDesc;
dstDesc.fOrigin = dOrigin;
dstDesc.fFlags = dFlags;
SkAutoTUnref<GrTexture> src(
context->textureProvider()->createTexture(srcDesc, false,
srcPixels.get(),
kRowBytes));
SkAutoTUnref<GrTexture> dst(
context->textureProvider()->createTexture(dstDesc, false,
dstPixels.get(),
kRowBytes));
if (!src || !dst) {
ERRORF(reporter,
"Could not create surfaces for copy surface test.");
continue;
}
bool result = context->copySurface(dst, src, srcRect, dstPoint);
bool expectedResult = true;
SkIPoint dstOffset = { dstPoint.fX - srcRect.fLeft,
dstPoint.fY - srcRect.fTop };
SkIRect copiedDstRect = SkIRect::MakeXYWH(dstPoint.fX,
dstPoint.fY,
srcRect.width(),
srcRect.height());
SkIRect copiedSrcRect;
if (!copiedSrcRect.intersect(srcRect, SkIRect::MakeWH(kW, kH))) {
expectedResult = false;
} else {
// If the src rect was clipped, apply same clipping to each side of
// copied dst rect.
copiedDstRect.fLeft += copiedSrcRect.fLeft - srcRect.fLeft;
copiedDstRect.fTop += copiedSrcRect.fTop - srcRect.fTop;
copiedDstRect.fRight -= copiedSrcRect.fRight - srcRect.fRight;
copiedDstRect.fBottom -= copiedSrcRect.fBottom - srcRect.fBottom;
}
if (copiedDstRect.isEmpty() ||
!copiedDstRect.intersect(SkIRect::MakeWH(kW, kH))) {
expectedResult = false;
}
// To make the copied src rect correct we would apply any dst clipping
// back to the src rect, but we don't use it again so don't bother.
if (expectedResult != result) {
ERRORF(reporter, "Expected return value %d from copySurface, got "
"%d.", expectedResult, result);
continue;
}
if (!expectedResult || !result) {
continue;
}
sk_memset32(read.get(), 0, kW * kH);
if (!dst->readPixels(0, 0, kW, kH, baseDesc.fConfig, read.get(),
kRowBytes)) {
ERRORF(reporter, "Error calling readPixels");
continue;
}
bool abort = false;
// Validate that pixels inside copiedDstRect received the correct value
// from src and that those outside were not modified.
for (int y = 0; y < kH && !abort; ++y) {
for (int x = 0; x < kW; ++x) {
uint32_t r = read.get()[y * kW + x];
if (copiedDstRect.contains(x, y)) {
int sx = x - dstOffset.fX;
int sy = y - dstOffset.fY;
uint32_t s = srcPixels.get()[sy * kW + sx];
if (s != r) {
ERRORF(reporter, "Expected dst %d,%d to contain "
"0x%08x copied from src location %d,%d. Got "
"0x%08x", x, y, s, sx, sy, r);
abort = true;
break;
}
} else {
uint32_t d = dstPixels.get()[y * kW + x];
if (d != r) {
ERRORF(reporter, "Expected dst %d,%d to be unmodified ("
"0x%08x). Got 0x%08x", x, y, d, r);
abort = true;
break;
}
}
}
}
}
}
}
}
}
}
}
#endif