From: egdaniel Date: Mon, 27 Jun 2016 21:34:55 +0000 (-0700) Subject: Fix Vulkan readPixels X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~129^2~45 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=88e8aef3916454e5f6916cc8b3420345b1cf0584;p=platform%2Fupstream%2FlibSkiaSharp.git Fix Vulkan readPixels Fixed bug with setting up tempReadTexture from previous CL. Also, previously we were not correctly handling the rowBytes during the read. BUG=skia:5461 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2102633003 Review-Url: https://codereview.chromium.org/2102633003 --- diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp index bfda30a..ca6ccde 100644 --- a/src/gpu/vk/GrVkGpu.cpp +++ b/src/gpu/vk/GrVkGpu.cpp @@ -414,8 +414,8 @@ bool GrVkGpu::uploadTexDataLinear(GrVkTexture* tex, if (trimRowBytes == rowBytes && trimRowBytes == layout.rowPitch) { memcpy(mapPtr, data, trimRowBytes * height); } else { - SkRectMemcpy(mapPtr, static_cast(layout.rowPitch), data, rowBytes, - trimRowBytes, height); + SkRectMemcpy(mapPtr, static_cast(layout.rowPitch), data, rowBytes, trimRowBytes, + height); } } @@ -1356,6 +1356,24 @@ void GrVkGpu::onGetMultisampleSpecs(GrRenderTarget* rt, const GrStencilSettings& bool GrVkGpu::onGetReadPixelsInfo(GrSurface* srcSurface, int width, int height, size_t rowBytes, GrPixelConfig readConfig, DrawPreference* drawPreference, ReadPixelTempDrawInfo* tempDrawInfo) { + // These settings we will always want if a temp draw is performed. + 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 = false; + + // For now assume no swizzling, we may change that below. + tempDrawInfo->fSwizzle = GrSwizzle::RGBA(); + + // Depends on why we need/want a temp draw. Start off assuming no change, the surface we read + // from will be srcConfig and we will read readConfig pixels from it. + // Not that if we require a draw and return a non-renderable format for the temp surface the + // base class will fail for us. + tempDrawInfo->fTempSurfaceDesc.fConfig = srcSurface->config(); + tempDrawInfo->fReadConfig = readConfig; + if (srcSurface->config() == readConfig) { return true; } @@ -1408,7 +1426,7 @@ bool GrVkGpu::onReadPixels(GrSurface* surface, VkBufferImageCopy region; memset(®ion, 0, sizeof(VkBufferImageCopy)); region.bufferOffset = 0; - region.bufferRowLength = 0; // Forces RowLength to be imageExtent.width + region.bufferRowLength = 0; // Forces RowLength to be width. We handle the rowBytes below. region.bufferImageHeight = 0; // Forces height to be tightly packed. Only useful for 3d images. region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1 }; region.imageOffset = offset; @@ -1435,29 +1453,26 @@ bool GrVkGpu::onReadPixels(GrSurface* surface, void* mappedMemory = transferBuffer->map(); - memcpy(buffer, mappedMemory, rowBytes*height); - - transferBuffer->unmap(); - transferBuffer->unref(); - + size_t tightRowBytes = GrBytesPerPixel(config) * width; if (flipY) { - SkAutoSMalloc<32 * sizeof(GrColor)> scratch; - size_t tightRowBytes = GrBytesPerPixel(config) * width; - scratch.reset(tightRowBytes); - void* tmpRow = scratch.get(); - // flip y in-place by rows - const int halfY = height >> 1; - char* top = reinterpret_cast(buffer); - char* bottom = top + (height - 1) * rowBytes; - for (int y = 0; y < halfY; y++) { - memcpy(tmpRow, top, tightRowBytes); - memcpy(top, bottom, tightRowBytes); - memcpy(bottom, tmpRow, tightRowBytes); - top += rowBytes; - bottom -= rowBytes; + const char* srcRow = reinterpret_cast(mappedMemory); + char* dstRow = reinterpret_cast(buffer)+(height - 1) * rowBytes; + for (int y = 0; y < height; y++) { + memcpy(dstRow, srcRow, tightRowBytes); + srcRow += tightRowBytes; + dstRow -= rowBytes; + } + } else { + if (tightRowBytes == rowBytes) { + memcpy(buffer, mappedMemory, rowBytes*height); + } else { + SkRectMemcpy(buffer, rowBytes, mappedMemory, tightRowBytes, tightRowBytes, height); } } + transferBuffer->unmap(); + transferBuffer->unref(); + return true; } diff --git a/tests/ReadPixelsTest.cpp b/tests/ReadPixelsTest.cpp index a4dbae9..20305cc 100644 --- a/tests/ReadPixelsTest.cpp +++ b/tests/ReadPixelsTest.cpp @@ -200,8 +200,8 @@ static bool check_read(skiatest::Reporter* reporter, bool didPremul; SkPMColor pmPixel = convert_to_pmcolor(ct, at, pixel, &didPremul); 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); + ERRORF(reporter, "Expected readback pixel (%d, %d) value 0x%08x, got 0x%08x. " + "Readback was unpremul: %d", bx, by, canvasPixel, pmPixel, didPremul); return false; } } @@ -387,7 +387,7 @@ DEF_TEST(ReadPixels, reporter) { test_readpixels(reporter, surface, kLastAligned_BitmapInit); } #if SK_SUPPORT_GPU -DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) { +DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Gpu, reporter, ctxInfo) { for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) { GrSurfaceDesc desc; desc.fFlags = kRenderTarget_GrSurfaceFlag; @@ -442,7 +442,7 @@ static void test_readpixels_texture(skiatest::Reporter* reporter, GrTexture* tex } } } -DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) { +DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Texture, reporter, ctxInfo) { // On the GPU we will also try reading back from a non-renderable texture. for (auto& origin : {kBottomLeft_GrSurfaceOrigin, kTopLeft_GrSurfaceOrigin}) { SkAutoTUnref texture; @@ -577,7 +577,7 @@ static void dump_to_file(const char name[], SkData* data) { * * https://bug.skia.org/4351 */ -DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(ReadPixels_Subset_Gpu, reporter, ctxInfo) { +DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ReadPixels_Subset_Gpu, reporter, ctxInfo) { SkBitmap bitmap; make_ringed_bitmap(&bitmap, 6, 6); const SkIRect subset = SkIRect::MakeLTRB(2, 2, 4, 4);