Remove origin from GrGpu::readPixels
Change-Id: I40d046c66240ab40794aa008861a1974f7f9182c
Reviewed-on: https://skia-review.googlesource.com/131620
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index 7f60129..5c2864d 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -214,8 +214,8 @@
canDiscardOutsideDstRect);
}
-bool GrGpu::readPixels(GrSurface* surface, GrSurfaceOrigin origin, int left, int top, int width,
- int height, GrColorType dstColorType, void* buffer, size_t rowBytes) {
+bool GrGpu::readPixels(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType dstColorType, void* buffer, size_t rowBytes) {
SkASSERT(surface);
int bpp = GrColorTypeBytesPerPixel(dstColorType);
@@ -228,8 +228,7 @@
this->handleDirtyContext();
- return this->onReadPixels(surface, origin, left, top, width, height, dstColorType, buffer,
- rowBytes);
+ return this->onReadPixels(surface, left, top, width, height, dstColorType, buffer, rowBytes);
}
bool GrGpu::writePixels(GrSurface* surface, int left, int top, int width, int height,
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 6958ea9..ad20d52 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -160,17 +160,8 @@
* because of a unsupported pixel config or because no render
* target is currently set.
*/
- bool readPixels(GrSurface* surface, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType dstColorType, void* buffer, size_t rowBytes);
- /**
- * This version of readPixels doesn't take an origin. TODO: Remove origin handling from
- * GrGpu::readPixels entirely.
- */
bool readPixels(GrSurface* surface, int left, int top, int width, int height,
- GrColorType dstColorType, void* buffer, size_t rowBytes) {
- return this->readPixels(surface, kTopLeft_GrSurfaceOrigin, left, top, width, height,
- dstColorType, buffer, rowBytes);
- }
+ GrColorType dstColorType, void* buffer, size_t rowBytes);
/**
* Updates the pixels in a rectangle of a surface. No sRGB/linear conversions are performed.
@@ -436,8 +427,8 @@
const void* data) = 0;
// overridden by backend-specific derived class to perform the surface read
- virtual bool onReadPixels(GrSurface*, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType, void* buffer, size_t rowBytes) = 0;
+ virtual bool onReadPixels(GrSurface*, int left, int top, int width, int height, GrColorType,
+ void* buffer, size_t rowBytes) = 0;
// overridden by backend-specific derived class to perform the surface write
virtual bool onWritePixels(GrSurface*, int left, int top, int width, int height, GrColorType,
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index aa5d2e7..e89439a 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -26,6 +26,7 @@
#include "GrTexturePriv.h"
#include "GrTypes.h"
#include "SkAutoMalloc.h"
+#include "SkConvertPixels.h"
#include "SkHalf.h"
#include "SkJSONWriter.h"
#include "SkMakeUnique.h"
@@ -1063,11 +1064,7 @@
// copy data into our new storage, skipping the trailing bytes
const char* src = (const char*)texelsShallowCopy[currentMipLevel].fPixels;
char* dst = buffer + individualMipOffsets[currentMipLevel];
- for (int y = 0; y < currentHeight; y++) {
- memcpy(dst, src, trimRowBytes);
- src += rowBytes;
- dst += trimRowBytes;
- }
+ SkRectMemcpy(dst, trimRowBytes, src, rowBytes, trimRowBytes, currentHeight);
// now point data to our copied version
texelsShallowCopy[currentMipLevel].fPixels = buffer +
individualMipOffsets[currentMipLevel];
@@ -2025,8 +2022,8 @@
}
}
-bool GrGLGpu::onReadPixels(GrSurface* surface, GrSurfaceOrigin origin, int left, int top, int width,
- int height, GrColorType dstColorType, void* buffer, size_t rowBytes) {
+bool GrGLGpu::onReadPixels(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType dstColorType, void* buffer, size_t rowBytes) {
SkASSERT(surface);
GrGLRenderTarget* renderTarget = static_cast<GrGLRenderTarget*>(surface->asRenderTarget());
@@ -2043,8 +2040,8 @@
if (kAlpha_8_GrPixelConfig == dstAsConfig &&
this->readPixelsSupported(surface, kRGBA_8888_GrPixelConfig)) {
std::unique_ptr<uint32_t[]> temp(new uint32_t[width * height * 4]);
- if (this->onReadPixels(surface, origin, left, top, width, height,
- GrColorType::kRGBA_8888, temp.get(), width * 4)) {
+ if (this->onReadPixels(surface, left, top, width, height, GrColorType::kRGBA_8888,
+ temp.get(), width * 4)) {
uint8_t* dst = reinterpret_cast<uint8_t*>(buffer);
for (int j = 0; j < height; ++j) {
for (int i = 0; i < width; ++i) {
@@ -2060,8 +2057,8 @@
if (kRGBA_half_GrPixelConfig == dstAsConfig &&
this->readPixelsSupported(surface, kRGBA_float_GrPixelConfig)) {
std::unique_ptr<float[]> temp(new float[width * height * 4]);
- if (this->onReadPixels(surface, origin, left, top, width, height,
- GrColorType::kRGBA_F32, temp.get(), width * sizeof(float) * 4)) {
+ if (this->onReadPixels(surface, left, top, width, height, GrColorType::kRGBA_F32,
+ temp.get(), width * sizeof(float) * 4)) {
uint8_t* dst = reinterpret_cast<uint8_t*>(buffer);
float* src = temp.get();
for (int j = 0; j < height; ++j) {
@@ -2085,7 +2082,6 @@
&externalType)) {
return false;
}
- bool flipY = kBottomLeft_GrSurfaceOrigin == origin;
GrGLIRect glvp;
if (renderTarget) {
@@ -2113,7 +2109,7 @@
// the read rect is viewport-relative
GrGLIRect readRect;
- readRect.setRelativeTo(glvp, left, top, width, height, origin);
+ readRect.setRelativeTo(glvp, left, top, width, height, kTopLeft_GrSurfaceOrigin);
int bytesPerPixel = GrBytesPerPixel(dstAsConfig);
size_t tightRowBytes = bytesPerPixel * width;
@@ -2121,8 +2117,7 @@
size_t readDstRowBytes = tightRowBytes;
void* readDst = buffer;
- // determine if GL can read using the passed rowBytes or if we need
- // a scratch buffer.
+ // determine if GL can read using the passed rowBytes or if we need a scratch buffer.
SkAutoSMalloc<32 * sizeof(GrColor)> scratch;
if (rowBytes != tightRowBytes) {
if (this->glCaps().packRowLengthSupport() && !(rowBytes % bytesPerPixel)) {
@@ -2134,9 +2129,6 @@
readDst = scratch.get();
}
}
- if (flipY && this->glCaps().packFlipYSupport()) {
- GL_CALL(PixelStorei(GR_GL_PACK_REVERSE_ROW_ORDER, 1));
- }
GL_CALL(PixelStorei(GR_GL_PACK_ALIGNMENT, config_alignment(dstAsConfig)));
GL_CALL(ReadPixels(readRect.fLeft, readRect.fBottom,
@@ -2146,50 +2138,13 @@
SkASSERT(this->glCaps().packRowLengthSupport());
GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, 0));
}
- if (flipY && this->glCaps().packFlipYSupport()) {
- GL_CALL(PixelStorei(GR_GL_PACK_REVERSE_ROW_ORDER, 0));
- flipY = false;
- }
- // now reverse the order of the rows, since GL's are bottom-to-top, but our
- // API presents top-to-bottom. We must preserve the padding contents. Note
- // that the above readPixels did not overwrite the padding.
- if (readDst == buffer) {
- SkASSERT(rowBytes == readDstRowBytes);
- if (flipY) {
- scratch.reset(tightRowBytes);
- void* tmpRow = scratch.get();
- // flip y in-place by rows
- const int halfY = height >> 1;
- char* top = reinterpret_cast<char*>(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;
- }
- }
- } else {
+ if (readDst != buffer) {
SkASSERT(readDst != buffer);
SkASSERT(rowBytes != tightRowBytes);
- // copy from readDst to buffer while flipping y
- // const int halfY = height >> 1;
const char* src = reinterpret_cast<const char*>(readDst);
char* dst = reinterpret_cast<char*>(buffer);
- if (flipY) {
- dst += (height-1) * rowBytes;
- }
- for (int y = 0; y < height; y++) {
- memcpy(dst, src, tightRowBytes);
- src += readDstRowBytes;
- if (!flipY) {
- dst += rowBytes;
- } else {
- dst -= rowBytes;
- }
- }
+ SkRectMemcpy(dst, rowBytes, src, readDstRowBytes, tightRowBytes, height);
}
if (!renderTarget) {
this->unbindTextureFBOForPixelOps(GR_GL_FRAMEBUFFER, surface);
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 508b0f8..7240f9d 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -231,8 +231,8 @@
// variations above, depending on whether the surface is a render target or not.
bool readPixelsSupported(GrSurface* surfaceForConfig, GrPixelConfig readConfig);
- bool onReadPixels(GrSurface*, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType, void* buffer, size_t rowBytes) override;
+ bool onReadPixels(GrSurface*, int left, int top, int width, int height, GrColorType,
+ void* buffer, size_t rowBytes) override;
bool onWritePixels(GrSurface*, int left, int top, int width, int height, GrColorType,
const GrMipLevel texels[], int mipLevelCount) override;
diff --git a/src/gpu/mock/GrMockGpu.h b/src/gpu/mock/GrMockGpu.h
index 0b874bb..3bb4a85 100644
--- a/src/gpu/mock/GrMockGpu.h
+++ b/src/gpu/mock/GrMockGpu.h
@@ -71,8 +71,8 @@
GrBuffer* onCreateBuffer(size_t sizeInBytes, GrBufferType, GrAccessPattern,
const void*) override;
- bool onReadPixels(GrSurface* surface, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType, void* buffer, size_t rowBytes) override {
+ bool onReadPixels(GrSurface* surface, int left, int top, int width, int height, GrColorType,
+ void* buffer, size_t rowBytes) override {
return true;
}
diff --git a/src/gpu/mtl/GrMtlGpu.h b/src/gpu/mtl/GrMtlGpu.h
index 4e1c5e7..e325ad6 100644
--- a/src/gpu/mtl/GrMtlGpu.h
+++ b/src/gpu/mtl/GrMtlGpu.h
@@ -96,8 +96,8 @@
return nullptr;
}
- bool onReadPixels(GrSurface* surface, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType, void* buffer, size_t rowBytes) override {
+ bool onReadPixels(GrSurface* surface, int left, int top, int width, int height, GrColorType,
+ void* buffer, size_t rowBytes) override {
return false;
}
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index f115a51..a999311 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1776,8 +1776,8 @@
return false;
}
-bool GrVkGpu::onReadPixels(GrSurface* surface, GrSurfaceOrigin origin, int left, int top, int width,
- int height, GrColorType dstColorType, void* buffer, size_t rowBytes) {
+bool GrVkGpu::onReadPixels(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType dstColorType, void* buffer, size_t rowBytes) {
if (GrPixelConfigToColorType(surface->config()) != dstColorType) {
return false;
}
@@ -1815,7 +1815,6 @@
int bpp = GrColorTypeBytesPerPixel(dstColorType);
size_t tightRowBytes = bpp * width;
- bool flipY = kBottomLeft_GrSurfaceOrigin == origin;
VkBufferImageCopy region;
memset(®ion, 0, sizeof(VkBufferImageCopy));
@@ -1823,16 +1822,9 @@
bool copyFromOrigin = this->vkCaps().mustDoCopiesFromOrigin();
if (copyFromOrigin) {
region.imageOffset = { 0, 0, 0 };
- region.imageExtent = { (uint32_t)(left + width),
- (uint32_t)(flipY ? surface->height() - top : top + height),
- 1
- };
+ region.imageExtent = { (uint32_t)(left + width), (uint32_t)(top + height), 1 };
} else {
- VkOffset3D offset = {
- left,
- flipY ? surface->height() - top - height : top,
- 0
- };
+ VkOffset3D offset = { left, top, 0 };
region.imageOffset = offset;
region.imageExtent = { (uint32_t)width, (uint32_t)height, 1 };
}
@@ -1877,17 +1869,7 @@
mappedMemory = (char*)mappedMemory + transBufferRowBytes * skipRows + bpp * left;
}
- if (flipY) {
- const char* srcRow = reinterpret_cast<const char*>(mappedMemory);
- char* dstRow = reinterpret_cast<char*>(buffer)+(height - 1) * rowBytes;
- for (int y = 0; y < height; y++) {
- memcpy(dstRow, srcRow, tightRowBytes);
- srcRow += transBufferRowBytes;
- dstRow -= rowBytes;
- }
- } else {
- SkRectMemcpy(buffer, rowBytes, mappedMemory, transBufferRowBytes, tightRowBytes, height);
- }
+ SkRectMemcpy(buffer, rowBytes, mappedMemory, transBufferRowBytes, tightRowBytes, height);
transferBuffer->unmap();
transferBuffer->unref();
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index 4284bbc..5df1675 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -165,8 +165,8 @@
GrBuffer* onCreateBuffer(size_t size, GrBufferType type, GrAccessPattern,
const void* data) override;
- bool onReadPixels(GrSurface* surface, GrSurfaceOrigin, int left, int top, int width, int height,
- GrColorType, void* buffer, size_t rowBytes) override;
+ bool onReadPixels(GrSurface* surface, int left, int top, int width, int height, GrColorType,
+ void* buffer, size_t rowBytes) override;
bool onWritePixels(GrSurface* surface, int left, int top, int width, int height, GrColorType,
const GrMipLevel texels[], int mipLevelCount) override;
diff --git a/tests/TransferPixelsTest.cpp b/tests/TransferPixelsTest.cpp
index 5bb3094..339a591 100755
--- a/tests/TransferPixelsTest.cpp
+++ b/tests/TransferPixelsTest.cpp
@@ -41,12 +41,9 @@
int width,
int height,
int bufferWidth,
- int bufferHeight,
- GrSurfaceOrigin origin) {
+ int bufferHeight) {
GrColor* srcPtr = srcBuffer;
- bool bottomUp = SkToBool(kBottomLeft_GrSurfaceOrigin == origin);
- GrColor* dstPtr = bottomUp ? dstBuffer + bufferWidth*(bufferHeight-1) : dstBuffer;
- int dstIncrement = bottomUp ? -bufferWidth : +bufferWidth;
+ GrColor* dstPtr = dstBuffer;
for (int j = 0; j < height; ++j) {
for (int i = 0; i < width; ++i) {
@@ -55,13 +52,13 @@
}
}
srcPtr += bufferWidth;
- dstPtr += dstIncrement;
+ dstPtr += bufferWidth;
}
return true;
}
void basic_transfer_test(skiatest::Reporter* reporter, GrContext* context, GrColorType colorType,
- GrSurfaceOrigin origin, bool renderTarget) {
+ bool renderTarget) {
if (GrCaps::kNone_MapFlags == context->contextPriv().caps()->mapBufferFlags()) {
return;
}
@@ -125,7 +122,7 @@
REPORTER_ASSERT(reporter, result);
memset(dstBuffer.get(), 0xCDCD, size);
- result = gpu->readPixels(tex.get(), origin, 0, 0, kTextureWidth, kTextureHeight, colorType,
+ result = gpu->readPixels(tex.get(), 0, 0, kTextureWidth, kTextureHeight, colorType,
dstBuffer.get(), rowBytes);
if (result) {
REPORTER_ASSERT(reporter, does_full_buffer_contain_correct_values(srcBuffer,
@@ -133,8 +130,7 @@
kTextureWidth,
kTextureHeight,
kBufferWidth,
- kBufferHeight,
- origin));
+ kBufferHeight));
}
//////////////////////////
@@ -157,7 +153,7 @@
REPORTER_ASSERT(reporter, result);
memset(dstBuffer.get(), 0xCDCD, size);
- result = gpu->readPixels(tex.get(), origin, 0, 0, kTextureWidth, kTextureHeight, colorType,
+ result = gpu->readPixels(tex.get(), 0, 0, kTextureWidth, kTextureHeight, colorType,
dstBuffer.get(), rowBytes);
if (result) {
REPORTER_ASSERT(reporter, does_full_buffer_contain_correct_values(srcBuffer,
@@ -165,30 +161,17 @@
kTextureWidth,
kTextureHeight,
kBufferWidth,
- kBufferHeight,
- origin));
+ kBufferHeight));
}
}
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TransferPixelsTest, reporter, ctxInfo) {
// RGBA
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888,
- kTopLeft_GrSurfaceOrigin, false);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888,
- kTopLeft_GrSurfaceOrigin, true);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888,
- kBottomLeft_GrSurfaceOrigin, false);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888,
- kBottomLeft_GrSurfaceOrigin, true);
+ basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888, false);
+ basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kRGBA_8888, true);
// BGRA
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888,
- kTopLeft_GrSurfaceOrigin, false);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888,
- kTopLeft_GrSurfaceOrigin, true);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888,
- kBottomLeft_GrSurfaceOrigin, false);
- basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888,
- kBottomLeft_GrSurfaceOrigin, true);
+ basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888, false);
+ basic_transfer_test(reporter, ctxInfo.grContext(), GrColorType::kBGRA_8888, true);
}