diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 17b4b74a39..ba02d60b11 100755 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -450,16 +450,6 @@ bool GrContext::writeSurfacePixels(GrSurface* surface, return true; } -// toggles between RGBA and BGRA -static SkColorType toggle_colortype32(SkColorType ct) { - if (kRGBA_8888_SkColorType == ct) { - return kBGRA_8888_SkColorType; - } else { - SkASSERT(kBGRA_8888_SkColorType == ct); - return kRGBA_8888_SkColorType; - } -} - bool GrContext::readSurfacePixels(GrSurface* src, int left, int top, int width, int height, GrPixelConfig dstConfig, void* buffer, size_t rowBytes, @@ -480,126 +470,87 @@ bool GrContext::readSurfacePixels(GrSurface* src, this->flush(); } - // Determine which conversions have to be applied: flipY, swapRAnd, and/or unpremul. - - // We ignore the preferred config if it is different than our config unless it is an R/B swap. - // In that case we'll perform an R and B swap while drawing to a scratch texture of the swapped - // config. Then we will call readPixels on the scratch with the swapped config. The swaps during - // the draw cancels out the fact that we call readPixels with a config that is R/B swapped from - // dstConfig. - GrPixelConfig readConfig = dstConfig; - bool swapRAndB = false; - if (GrPixelConfigSwapRAndB(dstConfig) == - fGpu->preferredReadPixelsConfig(dstConfig, src->config())) { - readConfig = GrPixelConfigSwapRAndB(readConfig); - swapRAndB = true; - } - - bool flipY = false; - GrRenderTarget* srcAsRT = src->asRenderTarget(); - if (srcAsRT) { - // If fGpu->readPixels would incur a y-flip cost then we will read the pixels upside down. - // We'll either do the flipY by drawing into a scratch with a matrix or on the cpu after the - // read. - flipY = fGpu->readPixelsWillPayForYFlip(srcAsRT, left, top, - width, height, dstConfig, - rowBytes); - } - bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags); if (unpremul && !GrPixelConfigIs8888(dstConfig)) { - // The unpremul flag is only allowed for these two configs. + // The unpremul flag is only allowed for 8888 configs. return false; } - SkAutoTUnref tempTexture; + GrGpu::DrawPreference drawPreference = unpremul ? GrGpu::kCallerPrefersDraw_DrawPreference : + GrGpu::kNoDraw_DrawPreference; + GrGpu::ReadPixelTempDrawInfo tempDrawInfo; + if (!fGpu->getReadPixelsInfo(src, width, height, rowBytes, dstConfig, &drawPreference, + &tempDrawInfo)) { + return false; + } - // If the src is a texture and we would have to do conversions after read pixels, we instead - // do the conversions by drawing the src to a scratch texture. If we handle any of the - // conversions in the draw we set the corresponding bool to false so that we don't reapply it - // on the read back pixels. We also do an intermediate draw if the src is not a render target as - // GrGpu currently supports reading from render targets but not textures. - GrTexture* srcAsTex = src->asTexture(); - GrRenderTarget* rtToRead = srcAsRT; - if (srcAsTex && (swapRAndB || unpremul || flipY || !srcAsRT)) { - // Make the scratch a render so we can read its pixels. - GrSurfaceDesc desc; - desc.fFlags = kRenderTarget_GrSurfaceFlag; - desc.fWidth = width; - desc.fHeight = height; - desc.fConfig = readConfig; - desc.fOrigin = kTopLeft_GrSurfaceOrigin; - - // When a full read back is faster than a partial we could always make the scratch exactly - // match the passed rect. However, if we see many different size rectangles we will trash - // our texture cache and pay the cost of creating and destroying many textures. So, we only - // request an exact match when the caller is reading an entire RT. + GrRenderTarget* rtToRead = src->asRenderTarget(); + bool didTempDraw = false; + if (GrGpu::kNoDraw_DrawPreference != drawPreference) { GrTextureProvider::ScratchTexMatch match = GrTextureProvider::kApprox_ScratchTexMatch; - if (0 == left && - 0 == top && - src->width() == width && - src->height() == height && - fGpu->fullReadPixelsIsFasterThanPartial()) { - match = GrTextureProvider::kExact_ScratchTexMatch; + if (tempDrawInfo.fUseExactScratch) { + // We only respect this when the entire src is being read. Otherwise we can trigger too + // many odd ball texture sizes and trash the cache. + if (width == src->width() && height == src->height()) { + match = GrTextureProvider::kExact_ScratchTexMatch; + } } - tempTexture.reset(this->textureProvider()->refScratchTexture(desc, match)); - if (tempTexture) { - // compute a matrix to perform the draw + SkAutoTUnref temp; + temp.reset(this->textureProvider()->refScratchTexture(tempDrawInfo.fTempSurfaceDesc, match)); + if (temp) { SkMatrix textureMatrix; - textureMatrix.setTranslate(SK_Scalar1 *left, SK_Scalar1 *top); + textureMatrix.setTranslate(SkIntToScalar(left), SkIntToScalar(top)); textureMatrix.postIDiv(src->width(), src->height()); - GrPaint paint; SkAutoTUnref fp; if (unpremul) { - fp.reset(this->createPMToUPMEffect(paint.getProcessorDataManager(), srcAsTex, - swapRAndB, textureMatrix)); + fp.reset(this->createPMToUPMEffect( + paint.getProcessorDataManager(), src->asTexture(), tempDrawInfo.fSwapRAndB, + textureMatrix)); if (fp) { unpremul = false; // we no longer need to do this on CPU after the read back. + } else if (GrGpu::kCallerPrefersDraw_DrawPreference == drawPreference) { + // We only wanted to do the draw in order to perform the unpremul so don't + // bother. + temp.reset(NULL); } } - // If we failed to create a PM->UPM effect and have no other conversions to perform then - // there is no longer any point to using the scratch. - if (fp || flipY || swapRAndB || !srcAsRT) { - if (!fp) { - fp.reset(GrConfigConversionEffect::Create(paint.getProcessorDataManager(), - srcAsTex, swapRAndB, GrConfigConversionEffect::kNone_PMConversion, - textureMatrix)); - } - swapRAndB = false; // we will handle the swap in the draw. - - // We protect the existing geometry here since it may not be - // clear to the caller that a draw operation (i.e., drawSimpleRect) - // can be invoked in this method - { - GrDrawContext* drawContext = this->drawContext(); - if (!drawContext) { - return false; - } - - paint.addColorProcessor(fp); - - SkRect rect = SkRect::MakeWH(SkIntToScalar(width), SkIntToScalar(height)); - - drawContext->drawRect(tempTexture->asRenderTarget(), GrClip::WideOpen(), paint, - SkMatrix::I(), rect, NULL); - - // we want to read back from the scratch's origin - left = 0; - top = 0; - rtToRead = tempTexture->asRenderTarget(); - } - this->flushSurfaceWrites(tempTexture); + if (!fp && temp) { + fp.reset(GrConfigConversionEffect::Create( + paint.getProcessorDataManager(), src->asTexture(), tempDrawInfo.fSwapRAndB, + GrConfigConversionEffect::kNone_PMConversion, textureMatrix)); + } + if (fp) { + paint.addColorProcessor(fp); + SkRect rect = SkRect::MakeWH(SkIntToScalar(width), SkIntToScalar(height)); + GrDrawContext* drawContext = this->drawContext(); + drawContext->drawRect(temp->asRenderTarget(), GrClip::WideOpen(), paint, + SkMatrix::I(), rect, NULL); + rtToRead = temp->asRenderTarget(); + left = 0; + top = 0; + didTempDraw = true; } } } - if (!rtToRead || - !fGpu->readPixels(rtToRead, left, top, width, height, readConfig, buffer, rowBytes)) { + if (GrGpu::kRequireDraw_DrawPreference == drawPreference && !didTempDraw) { return false; } - // Perform any conversions we weren't able to perform using a scratch texture. - if (unpremul || swapRAndB) { + GrPixelConfig configToRead = dstConfig; + if (didTempDraw) { + this->flushSurfaceWrites(rtToRead); + // We swapped R and B while doing the temp draw. Swap back on the read. + if (tempDrawInfo.fSwapRAndB) { + configToRead = GrPixelConfigSwapRAndB(dstConfig); + } + } + if (!fGpu->readPixels(rtToRead, left, top, width, height, configToRead, buffer, rowBytes)) { + return false; + } + + // Perform umpremul conversion if we weren't able to perform it as a draw. + if (unpremul) { SkDstPixelInfo dstPI; if (!GrPixelConfig2ColorAndProfileType(dstConfig, &dstPI.fColorType, NULL)) { return false; @@ -609,7 +560,7 @@ bool GrContext::readSurfacePixels(GrSurface* src, dstPI.fRowBytes = rowBytes; SkSrcPixelInfo srcPI; - srcPI.fColorType = swapRAndB ? toggle_colortype32(dstPI.fColorType) : dstPI.fColorType; + srcPI.fColorType = dstPI.fColorType; srcPI.fAlphaType = kPremul_SkAlphaType; srcPI.fPixels = buffer; srcPI.fRowBytes = rowBytes; diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h index 9365d8927f..857efd0d65 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -133,15 +133,58 @@ public: */ void resolveRenderTarget(GrRenderTarget* target); + /** Info struct returned by getReadPixelsInfo about performing intermediate draws before + reading pixels for performance or correctness. */ + struct ReadPixelTempDrawInfo { + /** If the GrGpu is requesting that the caller do a draw to an intermediate surface then + this is descriptor for the temp surface. The draw should always be a rect with + dst 0,0,w,h. */ + GrSurfaceDesc fTempSurfaceDesc; + /** Indicates whether there is a performance advantage to using an exact match texture + (in terms of width and height) for the intermediate texture instead of approximate. */ + bool fUseExactScratch; + /** The caller should swap the R and B channel in the temp draw and then instead of reading + the desired config back it should read GrPixelConfigSwapRAndB(readConfig). The swap + during the draw and the swap at readback time cancel and the client gets the correct + data. The swapped read back is either faster for or required by the underlying backend + 3D API. */ + bool fSwapRAndB; + }; + /** Describes why an intermediate draw must/should be performed before readPixels. */ + enum DrawPreference { + /** On input means that the caller would proceed without draw if the GrGpu doesn't request + one. + On output means that the GrGpu is not requesting a draw. */ + kNoDraw_DrawPreference, + /** Means that the client would prefer a draw for performance of the readback but + can satisfy a straight readPixels call on the inputs without an intermediate draw. + getReadPixelsInfo will never set the draw preference to this value but may leave + it set. */ + kCallerPrefersDraw_DrawPreference, + /** On output means that GrGpu would prefer a draw for performance of the readback but + can satisfy a straight readPixels call on the inputs without an intermediate draw. The + caller of getReadPixelsInfo should never specify this on intput. */ + kGpuPrefersDraw_DrawPreference, + /** On input means that the caller requires a draw to do a transformation and there is no + CPU fallback. + On output means that GrGpu can only satisfy the readPixels request if the intermediate + draw is performed. + */ + kRequireDraw_DrawPreference + }; + + /** Used to negotiates whether and how an intermediate draw should or must be performed before + a readPixels call. If this returns false then GrGpu could not deduce an intermediate draw + that would allow a successful readPixels call. */ + virtual bool getReadPixelsInfo(GrSurface* srcSurface, int readWidth, int readHeight, + size_t rowBytes, GrPixelConfig readConfig, DrawPreference*, + ReadPixelTempDrawInfo *) = 0; + /** - * Gets a preferred 8888 config to use for writing/reading pixel data to/from a surface with + * Gets a preferred 8888 config to use for writing pixel data to a surface with * config surfaceConfig. The returned config must have at least as many bits per channel as the - * readConfig or writeConfig param. + * writeConfig param. */ - virtual GrPixelConfig preferredReadPixelsConfig(GrPixelConfig readConfig, - GrPixelConfig surfaceConfig) const { - return readConfig; - } virtual GrPixelConfig preferredWritePixelsConfig(GrPixelConfig writeConfig, GrPixelConfig surfaceConfig) const { return writeConfig; @@ -153,35 +196,6 @@ public: */ virtual bool canWriteTexturePixels(const GrTexture*, GrPixelConfig srcConfig) const = 0; - /** - * OpenGL's readPixels returns the result bottom-to-top while the skia - * API is top-to-bottom. Thus we have to do a y-axis flip. The obvious - * solution is to have the subclass do the flip using either the CPU or GPU. - * However, the caller (GrContext) may have transformations to apply and can - * simply fold in the y-flip for free. On the other hand, the subclass may - * be able to do it for free itself. For example, the subclass may have to - * do memcpys to handle rowBytes that aren't tight. It could do the y-flip - * concurrently. - * - * This function returns true if a y-flip is required to put the pixels in - * top-to-bottom order and the subclass cannot do it for free. - * - * See read pixels for the params - * @return true if calling readPixels with the same set of params will - * produce bottom-to-top data - */ - virtual bool readPixelsWillPayForYFlip(GrRenderTarget* renderTarget, - int left, int top, - int width, int height, - GrPixelConfig config, - size_t rowBytes) const = 0; - /** - * This should return true if reading a NxM rectangle of pixels from a - * render target is faster if the target has dimensons N and M and the read - * rectangle has its top-left at 0,0. - */ - virtual bool fullReadPixelsIsFasterThanPartial() const { return false; }; - /** * Reads a rectangle of pixels from a render target. * diff --git a/src/gpu/GrTest.cpp b/src/gpu/GrTest.cpp index 34d75a84f5..b2fd99f6c3 100644 --- a/src/gpu/GrTest.cpp +++ b/src/gpu/GrTest.cpp @@ -150,11 +150,10 @@ public: return true; } - bool readPixelsWillPayForYFlip(GrRenderTarget* renderTarget, - int left, int top, - int width, int height, - GrPixelConfig config, - size_t rowBytes) const override { return false; } + bool getReadPixelsInfo(GrSurface* srcSurface, int readWidth, int readHeight, size_t rowBytes, + GrPixelConfig readConfig, DrawPreference*, + ReadPixelTempDrawInfo*) override { return false; } + void buildProgramDesc(GrProgramDesc*,const GrPrimitiveProcessor&, const GrPipeline&, const GrBatchTracker&) const override {} diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp index 784a62fa27..e8dc788415 100644 --- a/src/gpu/gl/GrGLGpu.cpp +++ b/src/gpu/gl/GrGLGpu.cpp @@ -267,28 +267,6 @@ void GrGLGpu::contextAbandoned() { } /////////////////////////////////////////////////////////////////////////////// -GrPixelConfig GrGLGpu::preferredReadPixelsConfig(GrPixelConfig readConfig, - GrPixelConfig surfaceConfig) const { - if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && kRGBA_8888_GrPixelConfig == readConfig) { - return kBGRA_8888_GrPixelConfig; - } else if (kMesa_GrGLDriver == this->glContext().driver() && - GrBytesPerPixel(readConfig) == 4 && - GrPixelConfigSwapRAndB(readConfig) == surfaceConfig) { - // Mesa 3D takes a slow path on when reading back BGRA from an RGBA surface and vice-versa. - // Perhaps this should be guarded by some compiletime or runtime check. - return surfaceConfig; - } else if (readConfig == kBGRA_8888_GrPixelConfig - && !this->glCaps().readPixelsSupported( - this->glInterface(), - GR_GL_BGRA, - GR_GL_UNSIGNED_BYTE, - surfaceConfig - )) { - return kRGBA_8888_GrPixelConfig; - } else { - return readConfig; - } -} GrPixelConfig GrGLGpu::preferredWritePixelsConfig(GrPixelConfig writeConfig, GrPixelConfig surfaceConfig) const { @@ -322,10 +300,6 @@ bool GrGLGpu::canWriteTexturePixels(const GrTexture* texture, GrPixelConfig srcC } } -bool GrGLGpu::fullReadPixelsIsFasterThanPartial() const { - return SkToBool(GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL); -} - void GrGLGpu::onResetContext(uint32_t resetBits) { // we don't use the zb at all if (resetBits & kMisc_GrGLBackendState) { @@ -1651,7 +1625,6 @@ void GrGLGpu::discard(GrRenderTarget* renderTarget) { renderTarget->flagAsResolved(); } - void GrGLGpu::clearStencil(GrRenderTarget* target) { if (NULL == target) { return; @@ -1705,35 +1678,91 @@ void GrGLGpu::onClearStencilClip(GrRenderTarget* target, const SkIRect& rect, bo fHWStencilSettings.invalidate(); } -bool GrGLGpu::readPixelsWillPayForYFlip(GrRenderTarget* renderTarget, - int left, int top, - int width, int height, - GrPixelConfig config, - size_t rowBytes) const { - // If this rendertarget is aready TopLeft, we don't need to flip. +static bool read_pixels_pays_for_y_flip(GrRenderTarget* renderTarget, const GrGLCaps& caps, + int width, int height, GrPixelConfig config, + size_t rowBytes) { + // If this render target is already TopLeft, we don't need to flip. if (kTopLeft_GrSurfaceOrigin == renderTarget->origin()) { return false; } // if GL can do the flip then we'll never pay for it. - if (this->glCaps().packFlipYSupport()) { + if (caps.packFlipYSupport()) { return false; } // If we have to do memcpy to handle non-trim rowBytes then we // get the flip for free. Otherwise it costs. - if (this->glCaps().packRowLengthSupport()) { - return true; - } - // If we have to do memcpys to handle rowBytes then y-flip is free - // Note the rowBytes might be tight to the passed in data, but if data - // gets clipped in x to the target the rowBytes will no longer be tight. - if (left >= 0 && (left + width) < renderTarget->width()) { - return 0 == rowBytes || - GrBytesPerPixel(config) * width == rowBytes; - } else { + // Note that we're assuming that 0 rowBytes has already been handled and that the width has been + // clipped. + return caps.packRowLengthSupport() || GrBytesPerPixel(config) * width == rowBytes; +} + +void elevate_draw_preference(GrGpu::DrawPreference* preference, GrGpu::DrawPreference elevation) { + GR_STATIC_ASSERT(GrGpu::kCallerPrefersDraw_DrawPreference > GrGpu::kNoDraw_DrawPreference); + GR_STATIC_ASSERT(GrGpu::kGpuPrefersDraw_DrawPreference > + GrGpu::kCallerPrefersDraw_DrawPreference); + GR_STATIC_ASSERT(GrGpu::kRequireDraw_DrawPreference > GrGpu::kGpuPrefersDraw_DrawPreference); + *preference = SkTMax(*preference, elevation); +} + +bool GrGLGpu::getReadPixelsInfo(GrSurface* srcSurface, int width, int height, size_t rowBytes, + GrPixelConfig readConfig, DrawPreference* drawPreference, + ReadPixelTempDrawInfo* tempDrawInfo) { + SkASSERT(drawPreference); + SkASSERT(tempDrawInfo); + SkASSERT(kGpuPrefersDraw_DrawPreference != *drawPreference); + + if (GrPixelConfigIsCompressed(readConfig)) { return false; } + + tempDrawInfo->fSwapRAndB = false; + + // These settings we will always want if a temp draw is performed. The config is set below + // depending on whether we want to do a R/B swap or not. + tempDrawInfo->fTempSurfaceDesc.fFlags = kRenderTarget_GrSurfaceFlag; + tempDrawInfo->fTempSurfaceDesc.fWidth = width; + tempDrawInfo->fTempSurfaceDesc.fHeight = height; + tempDrawInfo->fTempSurfaceDesc.fSampleCnt = 0; + tempDrawInfo->fTempSurfaceDesc.fOrigin = kTopLeft_GrSurfaceOrigin; // no CPU y-flip for TL. + tempDrawInfo->fUseExactScratch = SkToBool(GR_GL_FULL_READPIXELS_FASTER_THAN_PARTIAL); + + // Start off assuming that any temp draw should be to the readConfig, then check if that will + // be inefficient. + GrPixelConfig srcConfig = srcSurface->config(); + tempDrawInfo->fTempSurfaceDesc.fConfig = readConfig; + + if (GR_GL_RGBA_8888_PIXEL_OPS_SLOW && kRGBA_8888_GrPixelConfig == readConfig) { + tempDrawInfo->fTempSurfaceDesc.fConfig = kBGRA_8888_GrPixelConfig; + } else if (kMesa_GrGLDriver == this->glContext().driver() && + GrBytesPerPixel(readConfig) == 4 && + GrPixelConfigSwapRAndB(readConfig) == srcConfig) { + // Mesa 3D takes a slow path on when reading back BGRA from an RGBA surface and vice-versa. + // Better to do a draw with a R/B swap and then read as the original config. + tempDrawInfo->fTempSurfaceDesc.fConfig = srcConfig; + tempDrawInfo->fSwapRAndB = true; + elevate_draw_preference(drawPreference, kGpuPrefersDraw_DrawPreference); + } else if (readConfig == kBGRA_8888_GrPixelConfig && + !this->glCaps().readPixelsSupported(this->glInterface(), GR_GL_BGRA, + GR_GL_UNSIGNED_BYTE, srcConfig)) { + tempDrawInfo->fTempSurfaceDesc.fConfig = kRGBA_8888_GrPixelConfig; + tempDrawInfo->fSwapRAndB = true; + elevate_draw_preference(drawPreference, kRequireDraw_DrawPreference); + } + + GrRenderTarget* srcAsRT = srcSurface->asRenderTarget(); + if (!srcAsRT) { + elevate_draw_preference(drawPreference, kRequireDraw_DrawPreference); + } else if (read_pixels_pays_for_y_flip(srcAsRT, this->glCaps(), width, height, readConfig, + rowBytes)) { + elevate_draw_preference(drawPreference, kGpuPrefersDraw_DrawPreference); + } + + if (kRequireDraw_DrawPreference == *drawPreference && !srcSurface->asTexture()) { + return false; + } + return true; } bool GrGLGpu::onReadPixels(GrRenderTarget* target, @@ -1742,6 +1771,8 @@ bool GrGLGpu::onReadPixels(GrRenderTarget* target, GrPixelConfig config, void* buffer, size_t rowBytes) { + SkASSERT(target); + // We cannot read pixels into a compressed buffer if (GrPixelConfigIsCompressed(config)) { return false; diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h index 45337ddb3c..066a237e59 100644 --- a/src/gpu/gl/GrGLGpu.h +++ b/src/gpu/gl/GrGLGpu.h @@ -57,18 +57,15 @@ public: // Used by GrGLProgram to configure OpenGL state. void bindTexture(int unitIdx, const GrTextureParams& params, GrGLTexture* texture); + bool getReadPixelsInfo(GrSurface* srcSurface, int readWidth, int readHeight, size_t rowBytes, + GrPixelConfig readConfig, DrawPreference*, + ReadPixelTempDrawInfo*) override; + + // GrGpu overrides - GrPixelConfig preferredReadPixelsConfig(GrPixelConfig readConfig, - GrPixelConfig surfaceConfig) const override; GrPixelConfig preferredWritePixelsConfig(GrPixelConfig writeConfig, GrPixelConfig surfaceConfig) const override; bool canWriteTexturePixels(const GrTexture*, GrPixelConfig srcConfig) const override; - bool readPixelsWillPayForYFlip(GrRenderTarget* renderTarget, - int left, int top, - int width, int height, - GrPixelConfig config, - size_t rowBytes) const override; - bool fullReadPixelsIsFasterThanPartial() const override; bool initCopySurfaceDstDesc(const GrSurface* src, GrSurfaceDesc* desc) const override; diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp index 9e9f53c799..b94be0eba8 100644 --- a/tests/ReadPixelsTest.cpp +++ b/tests/ReadPixelsTest.cpp @@ -198,15 +198,17 @@ static bool check_read(skiatest::Reporter* reporter, SkPMColor canvasPixel = get_src_color(devx, devy); bool didPremul; SkPMColor pmPixel = convert_to_pmcolor(ct, at, pixel, &didPremul); - bool check = check_read_pixel(pmPixel, canvasPixel, didPremul); - REPORTER_ASSERT(reporter, check); - if (!check) { + if (!check_read_pixel(pmPixel, canvasPixel, didPremul)) { + ERRORF(reporter, "Expected readback pixel value 0x%08x, got 0x%08x. " + "Readback was unpremul: %d", canvasPixel, pmPixel, didPremul); return false; } } } else if (checkBitmapPixels) { - REPORTER_ASSERT(reporter, get_dst_bmp_init_color(bx, by, bw) == *pixel); - if (get_dst_bmp_init_color(bx, by, bw) != *pixel) { + uint32_t origDstPixel = get_dst_bmp_init_color(bx, by, bw); + if (origDstPixel != *pixel) { + ERRORF(reporter, "Expected clipped out area of readback to be unchanged. " + "Expected 0x%08x, got 0x%08x", origDstPixel, *pixel); return false; } }