GrSurface -> GrGpuBuffer transfer improvements:
GrCaps now only provides the offset alignment requirement. The row bytes
is always bpp * width.
GrGpu::transferPixelsFrom now just returns bool since row bytes value is
implicit. It now asserts offset is aligned with GrCap's provided value
in base class.
Implement caps for GL.
Bug: skia:8962
Change-Id: I3299b62efe9fe05bfe02f2a6a4c2704f647d0f8a
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/206686
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/include/gpu/gl/GrGLTypes.h b/include/gpu/gl/GrGLTypes.h
index f66a092..88d563a 100644
--- a/include/gpu/gl/GrGLTypes.h
+++ b/include/gpu/gl/GrGLTypes.h
@@ -68,6 +68,7 @@
typedef unsigned short GrGLushort;
typedef unsigned int GrGLuint;
typedef uint64_t GrGLuint64;
+typedef unsigned short int GrGLhalf;
typedef float GrGLfloat;
typedef float GrGLclampf;
typedef double GrGLdouble;
diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp
index 4624171..c4e24f0 100644
--- a/src/gpu/GrCaps.cpp
+++ b/src/gpu/GrCaps.cpp
@@ -277,13 +277,30 @@
return surface->readOnly() ? false : this->onSurfaceSupportsWritePixels(surface);
}
-bool GrCaps::transferFromBufferRequirements(GrColorType bufferColorType, int width,
- size_t* rowBytes, size_t* offsetAlignment) const {
+size_t GrCaps::transferFromOffsetAlignment(GrColorType bufferColorType) const {
if (!this->transferBufferSupport()) {
- return false;
+ return 0;
}
- return this->onTransferFromBufferRequirements(bufferColorType, width, rowBytes,
- offsetAlignment);
+ size_t result = this->onTransferFromOffsetAlignment(bufferColorType);
+ if (!result) {
+ return 0;
+ }
+ // It's very convenient to access 1 byte-per-channel 32 bitvRGB/RGBA color types as uint32_t.
+ // Make those aligned reads out of the buffer even if the underlying API doesn't require it.
+ auto componentFlags = GrColorTypeComponentFlags(bufferColorType);
+ if ((componentFlags == kRGBA_SkColorTypeComponentFlags ||
+ componentFlags == kRGB_SkColorTypeComponentFlags) &&
+ GrColorTypeBytesPerPixel(bufferColorType) == 4) {
+ switch (result & 0b11) {
+ // offset alignment already a multiple of 4
+ case 0: return result;
+ // offset alignment is a multiple of 2 but not 4.
+ case 2: return 2 * result;
+ // offset alignment is not a multiple of 2.
+ default: return 4 * result;
+ }
+ }
+ return result;
}
bool GrCaps::canCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* src,
@@ -329,12 +346,3 @@
GrBackendFormat GrCaps::getBackendFormatFromColorType(SkColorType ct) const {
return this->getBackendFormatFromGrColorType(SkColorTypeToGrColorType(ct), GrSRGBEncoded::kNo);
}
-
-bool GrCaps::onTransferFromBufferRequirements(GrColorType bufferColorType, int width,
- size_t* rowBytes, size_t* offsetAlignment) const {
- // TODO(bsalomon): Provide backend-specific overrides of this the return the true requirements.
- // Currently assuming tight row bytes rounded up to a multiple of 4 and 4 byte offset alignment.
- *rowBytes = GrSizeAlignUp(GrColorTypeBytesPerPixel(bufferColorType) * width, 4);
- *offsetAlignment = 4;
- return true;
-}
diff --git a/src/gpu/GrCaps.h b/src/gpu/GrCaps.h
index 4045e5f..2977ccd 100644
--- a/src/gpu/GrCaps.h
+++ b/src/gpu/GrCaps.h
@@ -224,21 +224,16 @@
bool transferBufferSupport() const { return fTransferBufferSupport; }
/**
- * Gets the requirements to use GrGpu::transferPixelsFrom for a given GrColorType. To check
- * whether a pixels as GrColorType can be read for a given surface see
- * supportedReadPixelsColorType() and surfaceSupportsReadPixels().
+ * Gets the alignment requirement for the buffer offset used with GrGpu::transferPixelsFrom for
+ * a given GrColorType. To check whether a pixels as GrColorType can be read for a given surface
+ * see supportedReadPixelsColorType() and surfaceSupportsReadPixels().
*
* @param bufferColorType The color type of the pixel data that will be stored in the transfer
* buffer.
- * @param width The number of color values per row that will be stored in the buffer.
- * @param rowBytes The number of bytes per row that will be used in the transfer buffer.
- * @param alignment The required alignment of the offset into the transfer buffer passed
- * to GrGpu::transferPixelsFrom.
- * @return true if transferPixelFrom is supported, false otherwise. If false then rowBytes and
- * alignment are not updated.
+ * @return minimum required alignment for the buffer offset or zero if reading to the color type
+ * is not supported.
*/
- bool transferFromBufferRequirements(GrColorType bufferColorType, int width, size_t* rowBytes,
- size_t* offsetAlignment) const;
+ size_t transferFromOffsetAlignment(GrColorType bufferColorType) const;
bool suppressPrints() const { return fSuppressPrints; }
@@ -426,8 +421,7 @@
virtual bool onSurfaceSupportsWritePixels(const GrSurface*) const = 0;
virtual bool onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* src,
const SkIRect& srcRect, const SkIPoint& dstPoint) const = 0;
- virtual bool onTransferFromBufferRequirements(GrColorType bufferColorType, int width,
- size_t* rowBytes, size_t* offsetAlignment) const;
+ virtual size_t onTransferFromOffsetAlignment(GrColorType bufferColorType) const = 0;
// Backends should implement this if they have any extra requirements for use of window
// rectangles for a specific GrBackendRenderTarget outside of basic support.
diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp
index df909b6..810457a 100644
--- a/src/gpu/GrGpu.cpp
+++ b/src/gpu/GrGpu.cpp
@@ -330,11 +330,13 @@
return false;
}
-size_t GrGpu::transferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
- size_t offset) {
+bool GrGpu::transferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
+ size_t offset) {
SkASSERT(surface);
SkASSERT(transferBuffer);
+ SkASSERT(this->caps()->transferFromOffsetAlignment(bufferColorType));
+ SkASSERT(offset % this->caps()->transferFromOffsetAlignment(bufferColorType) == 0);
// We require that the write region is contained in the texture
SkIRect subRect = SkIRect::MakeXYWH(left, top, width, height);
@@ -344,12 +346,12 @@
}
this->handleDirtyContext();
- if (auto result = this->onTransferPixelsFrom(surface, left, top, width, height, bufferColorType,
- transferBuffer, offset)) {
+ if (this->onTransferPixelsFrom(surface, left, top, width, height, bufferColorType,
+ transferBuffer, offset)) {
fStats.incTransfersFromSurface();
- return result;
+ return true;
}
- return 0;
+ return false;
}
bool GrGpu::regenerateMipMapLevels(GrTexture* texture) {
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 6b7ab84..a80fa12 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -227,10 +227,14 @@
/**
* Reads the pixels from a rectangle of a surface into a buffer. Use
- * GrCaps::transferFromBufferRequirements to determine the requirements for the buffer size
- * and offset alignment. If the surface is a MIP mapped texture, the base level is read.
+ * GrCaps::transferFromOffsetAlignment to determine the requirements for the buffer offset
+ * alignment. If the surface is a MIP mapped texture, the base level is read.
*
- * Returns the number of bytes per row in the buffer or zero on failure.
+ * If successful the row bytes in the buffer is always:
+ * GrColorTypeBytesPerPixel(bufferColorType) * width
+ *
+ * Asserts that the caller has passed a properly aligned offset and that the buffer is
+ * large enough to hold the result
*
* @param surface The surface to read from.
* @param left left edge of the rectangle to read (inclusive)
@@ -241,9 +245,9 @@
* @param transferBuffer GrBuffer to write pixels to (type must be "kXferGpuToCpu")
* @param offset offset from the start of the buffer
*/
- size_t transferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
- size_t offset);
+ bool transferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
+ size_t offset);
// After the client interacts directly with the 3D context state the GrGpu
// must resync its internal state and assumptions about 3D context state.
@@ -532,9 +536,9 @@
GrColorType colorType, GrGpuBuffer* transferBuffer,
size_t offset, size_t rowBytes) = 0;
// overridden by backend-specific derived class to perform the surface transfer
- virtual size_t onTransferPixelsFrom(GrSurface*, int left, int top, int width, int height,
- GrColorType colorType, GrGpuBuffer* transferBuffer,
- size_t offset) = 0;
+ virtual bool onTransferPixelsFrom(GrSurface*, int left, int top, int width, int height,
+ GrColorType colorType, GrGpuBuffer* transferBuffer,
+ size_t offset) = 0;
// overridden by backend-specific derived class to perform the resolve
virtual void onResolveRenderTarget(GrRenderTarget* target) = 0;
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 7e6f942..8e9952a 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -3259,3 +3259,44 @@
return GrBackendFormat::MakeGL(this->configSizedInternalFormat(config), GR_GL_TEXTURE_2D);
}
+size_t GrGLCaps::onTransferFromOffsetAlignment(GrColorType bufferColorType) const {
+ // This implementation is highly coupled with decisions made in initConfigTable and GrGLGpu's
+ // read pixels implementation and may not be correct for all types. TODO(bsalomon): This will be
+ // cleaned up/unified as part of removing GrPixelConfig.
+ // There are known problems with 24 vs 32 bit BPP with this color type. Just fail for now.
+ if (GrColorType::kRGB_888x == bufferColorType) {
+ return 0;
+ }
+ auto config = GrColorTypeToPixelConfig(bufferColorType, GrSRGBEncoded::kNo);
+ // This switch is derived from a table titled "Pixel data type parameter values and the
+ // corresponding GL data types" in the OpenGL spec (Table 8.2 in OpenGL 4.5).
+ switch (fConfigTable[config].fFormats.fExternalType) {
+ case GR_GL_UNSIGNED_BYTE: return sizeof(GrGLubyte);
+ case GR_GL_BYTE: return sizeof(GrGLbyte);
+ case GR_GL_UNSIGNED_SHORT: return sizeof(GrGLushort);
+ case GR_GL_SHORT: return sizeof(GrGLshort);
+ case GR_GL_UNSIGNED_INT: return sizeof(GrGLuint);
+ case GR_GL_INT: return sizeof(GrGLint);
+ case GR_GL_HALF_FLOAT: return sizeof(GrGLhalf);
+ case GR_GL_FLOAT: return sizeof(GrGLfloat);
+ case GR_GL_UNSIGNED_SHORT_5_6_5: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_SHORT_4_4_4_4: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_SHORT_5_5_5_1: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_INT_2_10_10_10_REV: return sizeof(GrGLuint);
+#if 0 // GL types we currently don't use. Here for future reference.
+ case GR_GL_UNSIGNED_BYTE_3_3_2: return sizeof(GrGLubyte);
+ case GR_GL_UNSIGNED_BYTE_2_3_3_REV: return sizeof(GrGLubyte);
+ case GR_GL_UNSIGNED_SHORT_5_6_5_REV: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_SHORT_4_4_4_4_REV: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_SHORT_1_5_5_5_REV: return sizeof(GrGLushort);
+ case GR_GL_UNSIGNED_INT_8_8_8_8: return sizeof(GrGLuint);
+ case GR_GL_UNSIGNED_INT_8_8_8_8_REV: return sizeof(GrGLuint);
+ case GR_GL_UNSIGNED_INT_10_10_10_2: return sizeof(GrGLuint);
+ case GR_GL_UNSIGNED_INT_24_8: return sizeof(GrGLuint);
+ case GR_GL_UNSIGNED_INT_10F_11F_11F_REV: return sizeof(GrGLuint);
+ case GR_GL_UNSIGNED_INT_5_9_9_9_REV: return sizeof(GrGLuint);
+ case GR_GL_FLOAT_32_UNSIGNED_INT_24_8_REV: return sizeof(GrGLuint);
+#endif
+ default: return 0;
+ }
+}
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index 4a97e84..a2c6ccf 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -465,6 +465,7 @@
bool onSurfaceSupportsWritePixels(const GrSurface*) const override;
bool onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* src,
const SkIRect& srcRect, const SkIPoint& dstPoint) const override;
+ size_t onTransferFromOffsetAlignment(GrColorType bufferColorType) const override;
GrGLStandard fStandard;
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 8a847ed..c81fe6b 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -959,31 +959,14 @@
return true;
}
-size_t GrGLGpu::onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType dstColorType, GrGpuBuffer* transferBuffer,
- size_t offset) {
- size_t offsetAlignment;
- size_t rowBytes;
- if (!this->glCaps().transferFromBufferRequirements(dstColorType, width, &rowBytes,
- &offsetAlignment)) {
- return false;
- }
- if (offset % offsetAlignment) {
- return false;
- }
- if (offset + rowBytes * height > transferBuffer->size()) {
- return false;
- }
-
+bool GrGLGpu::onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType dstColorType, GrGpuBuffer* transferBuffer,
+ size_t offset) {
auto* glBuffer = static_cast<GrGLBuffer*>(transferBuffer);
this->bindBuffer(GrGpuBufferType::kXferGpuToCpu, glBuffer);
-
auto offsetAsPtr = reinterpret_cast<void*>(offset);
- if (this->readOrTransferPixelsFrom(surface, left, top, width, height, dstColorType, offsetAsPtr,
- rowBytes)) {
- return rowBytes;
- }
- return 0;
+ return this->readOrTransferPixelsFrom(surface, left, top, width, height, dstColorType,
+ offsetAsPtr, width);
}
/**
@@ -2363,7 +2346,7 @@
bool GrGLGpu::readOrTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
GrColorType dstColorType, void* offsetOrPtr,
- size_t rowBytes) {
+ int rowWidthInPixels) {
SkASSERT(surface);
GrGLRenderTarget* renderTarget = static_cast<GrGLRenderTarget*>(surface->asRenderTarget());
@@ -2413,13 +2396,10 @@
GrGLIRect readRect;
readRect.setRelativeTo(glvp, left, top, width, height, kTopLeft_GrSurfaceOrigin);
- int bytesPerPixel = GrBytesPerPixel(dstAsConfig);
- size_t tightRowBytes = bytesPerPixel * width;
// determine if GL can read using the passed rowBytes or if we need a scratch buffer.
- if (rowBytes != tightRowBytes) {
+ if (rowWidthInPixels != width) {
SkASSERT(this->glCaps().packRowLengthSupport());
- SkASSERT(!(rowBytes % bytesPerPixel));
- GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, static_cast<GrGLint>(rowBytes / bytesPerPixel)));
+ GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, rowWidthInPixels));
}
GL_CALL(PixelStorei(GR_GL_PACK_ALIGNMENT, config_alignment(dstAsConfig)));
@@ -2444,7 +2424,7 @@
GR_GL_RENDERBUFFER, stencilAttachment->renderbufferID()));
}
- if (rowBytes != tightRowBytes) {
+ if (rowWidthInPixels != width) {
SkASSERT(this->glCaps().packRowLengthSupport());
GL_CALL(PixelStorei(GR_GL_PACK_ROW_LENGTH, 0));
}
@@ -2459,36 +2439,37 @@
GrColorType dstColorType, void* buffer, size_t rowBytes) {
SkASSERT(surface);
- // TODO: Avoid this conversion by making GrGLCaps work with color types.
- auto dstAsConfig = GrColorTypeToPixelConfig(dstColorType, GrSRGBEncoded::kNo);
+ int bytesPerPixel = GrColorTypeBytesPerPixel(dstColorType);
- int bytesPerPixel = GrBytesPerPixel(dstAsConfig);
- size_t tightRowBytes = bytesPerPixel * width;
-
- size_t readDstRowBytes = tightRowBytes;
+ // GL_PACK_ROW_LENGTH is in terms of pixels not bytes.
+ int rowPixelWidth;
void* readDst = 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 (!rowBytes || rowBytes == (size_t)(width * bytesPerPixel)) {
+ rowPixelWidth = width;
+ } else {
if (this->glCaps().packRowLengthSupport() && !(rowBytes % bytesPerPixel)) {
- readDstRowBytes = rowBytes;
+ rowPixelWidth = rowBytes / bytesPerPixel;
} else {
- scratch.reset(tightRowBytes * height);
+ scratch.reset(width * bytesPerPixel * height);
readDst = scratch.get();
+ rowPixelWidth = width;
}
}
if (!this->readOrTransferPixelsFrom(surface, left, top, width, height, dstColorType, readDst,
- readDstRowBytes)) {
+ rowPixelWidth)) {
return false;
}
if (readDst != buffer) {
SkASSERT(readDst != buffer);
- SkASSERT(rowBytes != tightRowBytes);
+ SkASSERT(rowBytes != (size_t)(rowPixelWidth * bytesPerPixel));
const char* src = reinterpret_cast<const char*>(readDst);
char* dst = reinterpret_cast<char*>(buffer);
- SkRectMemcpy(dst, rowBytes, src, readDstRowBytes, tightRowBytes, height);
+ SkRectMemcpy(dst, rowBytes, src, rowPixelWidth * bytesPerPixel, width * bytesPerPixel,
+ height);
}
return true;
}
diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index 86504d8..57505b4 100644
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -239,10 +239,10 @@
bool onTransferPixelsTo(GrTexture*, int left, int top, int width, int height, GrColorType,
GrGpuBuffer* transferBuffer, size_t offset, size_t rowBytes) override;
- size_t onTransferPixelsFrom(GrSurface*, int left, int top, int width, int height, GrColorType,
- GrGpuBuffer* transferBuffer, size_t offset) override;
+ bool onTransferPixelsFrom(GrSurface*, int left, int top, int width, int height, GrColorType,
+ GrGpuBuffer* transferBuffer, size_t offset) override;
bool readOrTransferPixelsFrom(GrSurface*, int left, int top, int width, int height, GrColorType,
- void* offsetOrPtr, size_t rowBytes);
+ void* offsetOrPtr, int rowWidthInPixels);
// Before calling any variation of TexImage, TexSubImage, etc..., call this to ensure that the
// PIXEL_UNPACK_BUFFER is unbound.
diff --git a/src/gpu/mock/GrMockCaps.h b/src/gpu/mock/GrMockCaps.h
index 4ce5f89..1a0a559 100644
--- a/src/gpu/mock/GrMockCaps.h
+++ b/src/gpu/mock/GrMockCaps.h
@@ -112,6 +112,10 @@
const SkIRect& srcRect, const SkIPoint& dstPoint) const override {
return true;
}
+ size_t onTransferFromOffsetAlignment(GrColorType bufferColorType) const override {
+ // arbitrary
+ return GrSizeAlignUp(GrColorTypeBytesPerPixel(bufferColorType), 4);
+ }
static const int kMaxSampleCnt = 16;
diff --git a/src/gpu/mock/GrMockGpu.h b/src/gpu/mock/GrMockGpu.h
index c1e0065..1e99683 100644
--- a/src/gpu/mock/GrMockGpu.h
+++ b/src/gpu/mock/GrMockGpu.h
@@ -96,8 +96,8 @@
size_t rowBytes) override {
return true;
}
- size_t onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType, GrGpuBuffer* transferBuffer, size_t offset) override {
+ bool onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType, GrGpuBuffer* transferBuffer, size_t offset) override {
return true;
}
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
diff --git a/src/gpu/mtl/GrMtlCaps.h b/src/gpu/mtl/GrMtlCaps.h
index 6ff4694..b80c349 100644
--- a/src/gpu/mtl/GrMtlCaps.h
+++ b/src/gpu/mtl/GrMtlCaps.h
@@ -85,6 +85,10 @@
bool onSurfaceSupportsWritePixels(const GrSurface*) const override;
bool onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* src,
const SkIRect& srcRect, const SkIPoint& dstPoint) const override;
+ size_t onTransferFromOffsetAlignment(GrColorType bufferColorType) const override {
+ // Transfer buffers not yet supported.
+ return 0;
+ }
struct ConfigInfo {
ConfigInfo() : fFlags(0) {}
diff --git a/src/gpu/mtl/GrMtlGpu.h b/src/gpu/mtl/GrMtlGpu.h
index 5e5c0a9..895866d 100644
--- a/src/gpu/mtl/GrMtlGpu.h
+++ b/src/gpu/mtl/GrMtlGpu.h
@@ -169,12 +169,10 @@
// TODO: not sure this is worth the work since nobody uses it
return false;
}
- size_t onTransferPixelsFrom(GrSurface*,
- int left, int top, int width, int height,
- GrColorType, GrGpuBuffer*,
- size_t offset) override {
+ bool onTransferPixelsFrom(GrSurface*, int left, int top, int width, int height, GrColorType,
+ GrGpuBuffer*, size_t offset) override {
// TODO: Will need to implement this to support async read backs.
- return 0;
+ return false;
}
bool onRegenerateMipMapLevels(GrTexture*) override;
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 59ac518..a207938 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -951,8 +951,7 @@
return GrBackendFormat::MakeVk(format);
}
-bool GrVkCaps::onTransferFromBufferRequirements(GrColorType bufferColorType, int width,
- size_t* rowBytes, size_t* offsetAlignment) const {
+size_t GrVkCaps::onTransferFromOffsetAlignment(GrColorType bufferColorType) const {
// This GrColorType has 32 bpp but the Vulkan pixel format we use for with may have 24bpp
// (VK_FORMAT_R8G8B8_...) or may be 32 bpp. We don't support post transforming the pixel data
// for transfer-from currently and don't want to have to pass info about the src surface here.
@@ -962,19 +961,11 @@
size_t bpp = GrColorTypeBytesPerPixel(bufferColorType);
// The VkBufferImageCopy bufferOffset field must be both a multiple of 4 and of a single texel.
switch (bpp & 0b11) {
- case 0: // bpp is a multiple of 4
- *offsetAlignment = bpp;
- break;
- case 2: // bpp is a multiple of 2
- *offsetAlignment = 2 * bpp;
- break;
- case 1:
- case 3: // bpp neither a multiple of 2 nor 4.
- *offsetAlignment = 4 * bpp;
- break;
+ // bpp is already a multiple of 4.
+ case 0: return bpp;
+ // bpp is a multiple of 2 but not 4.
+ case 2: return 2 * bpp;
+ // bpp is not a multiple of 2.
+ default: return 4 * bpp;
}
- // The bufferRowLength member of VkBufferImageCopy is in texel units and must be >= the extent
- // of the src VkImage region.
- *rowBytes = width * bpp;
- return true;
}
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index 34abe70..1d3929c 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -199,8 +199,7 @@
bool onSurfaceSupportsWritePixels(const GrSurface*) const override;
bool onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* src,
const SkIRect& srcRect, const SkIPoint& dstPoint) const override;
- bool onTransferFromBufferRequirements(GrColorType bufferColorType, int width, size_t* rowBytes,
- size_t* offsetAlignment) const override;
+ size_t onTransferFromOffsetAlignment(GrColorType bufferColorType) const override;
struct ConfigInfo {
ConfigInfo() : fOptimalFlags(0), fLinearFlags(0) {}
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 207e5a3..389a49a 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -493,23 +493,12 @@
return true;
}
-size_t GrVkGpu::onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
- size_t offset) {
+bool GrVkGpu::onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType bufferColorType, GrGpuBuffer* transferBuffer,
+ size_t offset) {
SkASSERT(surface);
SkASSERT(transferBuffer);
- size_t rowBytes;
- size_t offsetAlignment;
- if (!this->vkCaps().transferFromBufferRequirements(bufferColorType, width, &rowBytes,
- &offsetAlignment)) {
- return 0;
- }
-
- if (offset % offsetAlignment || offset + height * rowBytes > transferBuffer->size()) {
- return 0;
- }
-
GrVkTransferBuffer* vkBuffer = static_cast<GrVkTransferBuffer*>(transferBuffer);
GrVkImage* srcImage;
@@ -541,12 +530,7 @@
VkBufferImageCopy region;
memset(®ion, 0, sizeof(VkBufferImageCopy));
region.bufferOffset = offset;
- // We're assuming that GrVkCaps made the row bytes tight.
- region.bufferRowLength = 0;
-#ifdef SK_DEBUG
- int bpp = GrColorTypeBytesPerPixel(bufferColorType);
- SkASSERT(rowBytes == width * (size_t)bpp);
-#endif
+ region.bufferRowLength = width;
region.bufferImageHeight = 0;
region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, 0, 0, 1 };
region.imageOffset = { left, top, 0 };
@@ -572,7 +556,7 @@
// The caller is responsible for syncing.
this->submitCommandBuffer(kSkip_SyncQueue);
- return rowBytes;
+ return true;
}
void GrVkGpu::resolveImage(GrSurface* dst, GrVkRenderTarget* src, const SkIRect& srcRect,
diff --git a/src/gpu/vk/GrVkGpu.h b/src/gpu/vk/GrVkGpu.h
index 1f820b2..93f746e 100644
--- a/src/gpu/vk/GrVkGpu.h
+++ b/src/gpu/vk/GrVkGpu.h
@@ -213,8 +213,8 @@
bool onTransferPixelsTo(GrTexture*, int left, int top, int width, int height, GrColorType,
GrGpuBuffer* transferBuffer, size_t offset, size_t rowBytes) override;
- size_t onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
- GrColorType, GrGpuBuffer* transferBuffer, size_t offset) override;
+ bool onTransferPixelsFrom(GrSurface* surface, int left, int top, int width, int height,
+ GrColorType, GrGpuBuffer* transferBuffer, size_t offset) override;
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
diff --git a/tests/TransferPixelsTest.cpp b/tests/TransferPixelsTest.cpp
index c55e4ef..771f9c7 100644
--- a/tests/TransferPixelsTest.cpp
+++ b/tests/TransferPixelsTest.cpp
@@ -212,21 +212,16 @@
const int kPartialWidth = 10;
const int kPartialHeight = 2;
- size_t fullBufferRowBytes;
- size_t partialBufferRowBytes;
- size_t fullBufferOffsetAlignment;
- size_t partialBufferOffsetAlignment;
-
- SkAssertResult(context->priv().caps()->transferFromBufferRequirements(
- colorType, kTextureWidth, &fullBufferRowBytes, &fullBufferOffsetAlignment));
- SkAssertResult(context->priv().caps()->transferFromBufferRequirements(
- colorType, kPartialWidth, &partialBufferRowBytes, &partialBufferOffsetAlignment));
+ size_t bpp = GrColorTypeBytesPerPixel(readColorType);
+ size_t fullBufferRowBytes = kTextureWidth * bpp;
+ size_t partialBufferRowBytes = kPartialWidth * bpp;
+ size_t offsetAlignment = context->priv().caps()->transferFromOffsetAlignment(readColorType);
+ SkASSERT(offsetAlignment);
size_t bufferSize = fullBufferRowBytes * kTextureHeight;
// Arbitrary starting offset for the partial read.
- size_t partialReadOffset = GrSizeAlignUp(11, partialBufferOffsetAlignment);
- bufferSize =
- SkTMax(bufferSize, partialReadOffset + partialBufferRowBytes * partialBufferRowBytes);
+ size_t partialReadOffset = GrSizeAlignUp(11, offsetAlignment);
+ bufferSize = SkTMax(bufferSize, partialReadOffset + partialBufferRowBytes * kPartialHeight);
sk_sp<GrGpuBuffer> buffer(resourceProvider->createBuffer(
bufferSize, GrGpuBufferType::kXferGpuToCpu, kDynamic_GrAccessPattern));
@@ -269,10 +264,10 @@
//////////////////////////
// transfer full data
- auto bufferRowBytes = gpu->transferPixelsFrom(
- tex.get(), 0, 0, kTextureWidth, kTextureHeight, readColorType, buffer.get(), 0);
- REPORTER_ASSERT(reporter, bufferRowBytes = fullBufferRowBytes);
- if (!bufferRowBytes) {
+ bool result = gpu->transferPixelsFrom(tex.get(), 0, 0, kTextureWidth, kTextureHeight,
+ readColorType, buffer.get(), 0);
+ if (!result) {
+ ERRORF(reporter, "transferPixelsFrom failed.");
continue;
}
++expectedTransferCnt;
@@ -291,17 +286,17 @@
kTextureWidth,
kTextureHeight,
textureDataRowBytes,
- bufferRowBytes,
+ fullBufferRowBytes,
readColorType != colorType));
buffer->unmap();
///////////////////////
// Now test a partial read at an offset into the buffer.
- bufferRowBytes = gpu->transferPixelsFrom(tex.get(), kPartialLeft, kPartialTop,
- kPartialWidth, kPartialHeight, readColorType,
- buffer.get(), partialReadOffset);
- REPORTER_ASSERT(reporter, bufferRowBytes = partialBufferRowBytes);
- if (!bufferRowBytes) {
+ result = gpu->transferPixelsFrom(tex.get(), kPartialLeft, kPartialTop, kPartialWidth,
+ kPartialHeight, readColorType, buffer.get(),
+ partialReadOffset);
+ if (!result) {
+ ERRORF(reporter, "transferPixelsFrom failed.");
continue;
}
++expectedTransferCnt;
@@ -325,7 +320,7 @@
kPartialWidth,
kPartialHeight,
textureDataRowBytes,
- bufferRowBytes,
+ partialBufferRowBytes,
readColorType != colorType));
buffer->unmap();
}
