Revert "Revert "New read pixels implementation that is simpler but does all conversions on CPU.""
This reverts commit be5947c2f38a79b7c709accfb1047d8fd06a0227.
Bug: skia:
Change-Id: I06dc15b31042d7827511d0ac2a7f4262c3f09622
Reviewed-on: https://skia-review.googlesource.com/115079
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/include/gpu/GrCaps.h b/include/gpu/GrCaps.h
index 27ad6a1..02a51a0 100644
--- a/include/gpu/GrCaps.h
+++ b/include/gpu/GrCaps.h
@@ -186,7 +186,15 @@
* If this returns false then the caller should implement a fallback where a temporary texture
* is created, pixels are written to it, and then that is copied or drawn into the the surface.
*/
- virtual bool surfaceSupportsWritePixels(const GrSurface* surface) const = 0;
+ virtual bool surfaceSupportsWritePixels(const GrSurface*) const = 0;
+
+ /**
+ * Backends may have restrictions on what types of surfaces support GrGpu::readPixels().
+ * If this returns false then the caller should implement a fallback where a temporary texture
+ * is created, the surface is drawn or copied into the temporary, and pixels are read from the
+ * temporary.
+ */
+ virtual bool surfaceSupportsReadPixels(const GrSurface*) const = 0;
/**
* Given a dst pixel config and a src color type what color type must the caller coax the
@@ -197,6 +205,15 @@
return GrPixelConfigToColorType(config);
}
+ /**
+ * Given a src pixel config and a dst color type what color type must the caller read to using
+ * GrGpu::readPixels() and then coax into dstColorType.
+ */
+ virtual GrColorType supportedReadPixelsColorType(GrPixelConfig config,
+ GrColorType /*dstColorType*/) const {
+ return GrPixelConfigToColorType(config);
+ }
+
bool suppressPrints() const { return fSuppressPrints; }
size_t bufferMapThreshold() const {
diff --git a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Debug-All.json b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Debug-All.json
index 33a33b7..c89b44b 100644
--- a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Debug-All.json
+++ b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Debug-All.json
@@ -282,6 +282,10 @@
"gm",
"_",
"dftext_blob_persp",
+ "gltestthreading",
+ "gm",
+ "_",
+ "orientation",
"_",
"svg",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Release-All-Vulkan.json b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Release-All-Vulkan.json
index 624027c..ba4c599 100644
--- a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Release-All-Vulkan.json
+++ b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC5PPYH-GPU-IntelHD405-x86_64-Release-All-Vulkan.json
@@ -279,6 +279,10 @@
"gm",
"_",
"dftext_blob_persp",
+ "gltestthreading",
+ "gm",
+ "_",
+ "orientation",
"_",
"svg",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Debug-All-Vulkan.json b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Debug-All-Vulkan.json
index 52fce15..c6eb1c4 100644
--- a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Debug-All-Vulkan.json
+++ b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUC7i5BNK-GPU-IntelIris640-x86_64-Debug-All-Vulkan.json
@@ -279,6 +279,10 @@
"gm",
"_",
"dftext_blob_persp",
+ "gltestthreading",
+ "gm",
+ "_",
+ "orientation",
"_",
"svg",
"_",
diff --git a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUCDE3815TYKHE-GPU-IntelBayTrail-x86_64-Debug-All.json b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUCDE3815TYKHE-GPU-IntelBayTrail-x86_64-Debug-All.json
index 6bfa088..952a27a 100644
--- a/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUCDE3815TYKHE-GPU-IntelBayTrail-x86_64-Debug-All.json
+++ b/infra/bots/recipes/test.expected/Test-Ubuntu16-Clang-NUCDE3815TYKHE-GPU-IntelBayTrail-x86_64-Debug-All.json
@@ -282,6 +282,10 @@
"gm",
"_",
"dftext_blob_persp",
+ "gltestthreading",
+ "gm",
+ "_",
+ "orientation",
"_",
"svg",
"_",
diff --git a/infra/bots/recipes/test.py b/infra/bots/recipes/test.py
index 78c0280..910747b 100644
--- a/infra/bots/recipes/test.py
+++ b/infra/bots/recipes/test.py
@@ -178,6 +178,8 @@
blacklist('gltestthreading gm _ savelayer_with_backdrop')
blacklist('gltestthreading gm _ persp_shaders_bw')
blacklist('gltestthreading gm _ dftext_blob_persp')
+ # skbug.com/7523 - Flaky on various GPUs
+ blacklist('gltestthreading gm _ orientation')
# The following devices do not support glessrgb.
if 'glessrgb' in configs:
diff --git a/src/core/SkConvertPixels.cpp b/src/core/SkConvertPixels.cpp
index 725730a..8a8f2ba 100644
--- a/src/core/SkConvertPixels.cpp
+++ b/src/core/SkConvertPixels.cpp
@@ -184,6 +184,30 @@
}
break;
}
+ case kRGBA_1010102_SkColorType: {
+ auto src32 = (const uint32_t*) src;
+ for (int y = 0; y < srcInfo.height(); y++) {
+ for (int x = 0; x < srcInfo.width(); x++) {
+ switch (src32[x] >> 30) {
+ case 0:
+ dst[x] = 0;
+ break;
+ case 1:
+ dst[x] = 0x55;
+ break;
+ case 2:
+ dst[x] = 0xAA;
+ break;
+ case 3:
+ dst[x] = 0xFF;
+ break;
+ }
+ }
+ dst = SkTAddOffset<uint8_t>(dst, dstRB);
+ src32 = SkTAddOffset<const uint32_t>(src32, srcRB);
+ }
+ break;
+ }
case kARGB_4444_SkColorType: {
auto src16 = (const uint16_t*) src;
for (int y = 0; y < srcInfo.height(); y++) {
@@ -225,9 +249,20 @@
case kRGBA_8888_SkColorType:
pipeline.append(SkRasterPipeline::load_8888, &src);
break;
+ case kRGB_888x_SkColorType:
+ pipeline.append(SkRasterPipeline::load_8888, &src);
+ pipeline.append(SkRasterPipeline::force_opaque);
+ break;
case kBGRA_8888_SkColorType:
pipeline.append(SkRasterPipeline::load_bgra, &src);
break;
+ case kRGBA_1010102_SkColorType:
+ pipeline.append(SkRasterPipeline::load_1010102, &src);
+ break;
+ case kRGB_101010x_SkColorType:
+ pipeline.append(SkRasterPipeline::load_1010102, &src);
+ pipeline.append(SkRasterPipeline::force_opaque);
+ break;
case kRGB_565_SkColorType:
pipeline.append(SkRasterPipeline::load_565, &src);
break;
@@ -325,9 +360,20 @@
case kRGBA_8888_SkColorType:
pipeline.append(SkRasterPipeline::store_8888, &dst);
break;
+ case kRGB_888x_SkColorType:
+ pipeline.append(SkRasterPipeline::force_opaque);
+ pipeline.append(SkRasterPipeline::store_8888, &dst);
+ break;
case kBGRA_8888_SkColorType:
pipeline.append(SkRasterPipeline::store_bgra, &dst);
break;
+ case kRGBA_1010102_SkColorType:
+ pipeline.append(SkRasterPipeline::store_1010102, &dst);
+ break;
+ case kRGB_101010x_SkColorType:
+ pipeline.append(SkRasterPipeline::force_opaque);
+ pipeline.append(SkRasterPipeline::store_1010102, &dst);
+ break;
case kRGB_565_SkColorType:
pipeline.append(SkRasterPipeline::store_565, &dst);
break;
@@ -345,6 +391,16 @@
pipeline.run(0,0, srcInfo.width(), srcInfo.height());
}
+static bool swizzle_and_multiply_color_type(SkColorType ct) {
+ switch (ct) {
+ case kRGBA_8888_SkColorType:
+ case kBGRA_8888_SkColorType:
+ return true;
+ default:
+ return false;
+ }
+}
+
void SkConvertPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
const SkImageInfo& srcInfo, const void* srcPixels, size_t srcRB,
SkColorTable* ctable, SkTransferFunctionBehavior behavior) {
@@ -361,7 +417,8 @@
SkASSERT(srcInfo.colorSpace() || !isColorAware);
// Fast Path 2: Simple swizzles and premuls.
- if (4 == srcInfo.bytesPerPixel() && 4 == dstInfo.bytesPerPixel() && !isColorAware) {
+ if (swizzle_and_multiply_color_type(srcInfo.colorType()) &&
+ swizzle_and_multiply_color_type(dstInfo.colorType()) && !isColorAware) {
swizzle_and_multiply(dstInfo, dstPixels, dstRB, srcInfo, srcPixels, srcRB);
return;
}
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index f9c9415..626b3f6 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -26,6 +26,7 @@
#include "GrTextureContext.h"
#include "GrTextureStripAtlas.h"
#include "GrTracing.h"
+#include "SkAutoPixmapStorage.h"
#include "SkConvertPixels.h"
#include "SkDeferredDisplayList.h"
#include "SkGr.h"
@@ -611,6 +612,10 @@
int height, GrColorType dstColorType,
SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
uint32_t flags) {
+#ifndef SK_LEGACY_GPU_PIXEL_OPS
+ return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace,
+ buffer, rowBytes, flags);
+#endif
// TODO: Color space conversion
ASSERT_SINGLE_OWNER_PRIV
@@ -618,6 +623,10 @@
SkASSERT(src);
ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy());
GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels", fContext);
+ SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
+ if (flags & kDontFlush_PixelOpsFlag) {
+ return false;
+ }
// MDB TODO: delay this instantiation until later in the method
if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) {
@@ -667,7 +676,7 @@
return false;
}
- if (!(kDontFlush_PixelOpsFlag & flags) && srcSurface->surfacePriv().hasPendingWrite()) {
+ if (srcSurface->surfacePriv().hasPendingWrite()) {
this->flush(nullptr); // MDB TODO: tighten this
}
@@ -799,6 +808,12 @@
}
if (!fContext->caps()->surfaceSupportsWritePixels(dstSurface)) {
+ // We don't expect callers that are skipping flushes to require an intermediate draw.
+ SkASSERT(!(pixelOpsFlags & kDontFlush_PixelOpsFlag));
+ if (pixelOpsFlags & kDontFlush_PixelOpsFlag) {
+ return false;
+ }
+
GrSurfaceDesc desc;
desc.fConfig = dstProxy->config();
desc.fWidth = width;
@@ -871,7 +886,7 @@
memcpy(tempSrc.writable_addr(0, y), tempSrc.addr(0, height - 1 - y), rowBytes);
memcpy(tempSrc.writable_addr(0, height - 1 - y), row.get(), rowBytes);
}
- top = dstProxy->height() - top - height;
+ top = dstSurface->height() - top - height;
}
} else if (dstProxy->origin() == kBottomLeft_GrSurfaceOrigin) {
size_t trimRowBytes = GrColorTypeBytesPerPixel(srcColorType) * width;
@@ -883,7 +898,7 @@
}
buffer = tempBuffer.get();
rowBytes = trimRowBytes;
- top = dstProxy->height() - top - height;
+ top = dstSurface->height() - top - height;
}
if (!(kDontFlush_PixelOpsFlag & pixelOpsFlags) && dstSurface->surfacePriv().hasPendingIO()) {
@@ -894,6 +909,136 @@
rowBytes);
}
+bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top, int width,
+ int height, GrColorType dstColorType,
+ SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
+ uint32_t flags) {
+ SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
+ if (flags & kDontFlush_PixelOpsFlag){
+ return false;
+ }
+ ASSERT_SINGLE_OWNER_PRIV
+ RETURN_FALSE_IF_ABANDONED_PRIV
+ SkASSERT(src);
+ SkASSERT(buffer);
+ ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy());
+ GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels2", fContext);
+
+ // MDB TODO: delay this instantiation until later in the method
+ if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) {
+ return false;
+ }
+
+ GrSurfaceProxy* srcProxy = src->asSurfaceProxy();
+ GrSurface* srcSurface = srcProxy->priv().peekSurface();
+
+ if (!GrSurfacePriv::AdjustReadPixelParams(srcSurface->width(), srcSurface->height(),
+ GrColorTypeBytesPerPixel(dstColorType), &left, &top,
+ &width, &height, &buffer, &rowBytes)) {
+ return false;
+ }
+
+ // TODO: Make GrSurfaceContext know its alpha type and pass dst buffer's alpha type.
+ bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags);
+ bool convert = unpremul;
+
+ if (!valid_pixel_conversion(dstColorType, srcProxy->config(), unpremul)) {
+ return false;
+ }
+
+ if (!fContext->caps()->surfaceSupportsReadPixels(srcSurface)) {
+ GrSurfaceDesc desc;
+ desc.fConfig = srcProxy->config();
+ desc.fWidth = width;
+ desc.fHeight = height;
+ desc.fSampleCnt = 1;
+ auto tempProxy = this->proxyProvider()->createProxy(
+ desc, kTopLeft_GrSurfaceOrigin, SkBackingFit::kApprox, SkBudgeted::kYes);
+ if (!tempProxy) {
+ return false;
+ }
+ auto tempCtx = this->drawingManager()->makeTextureContext(
+ tempProxy, src->colorSpaceInfo().refColorSpace());
+ if (!tempCtx) {
+ return false;
+ }
+ if (!tempCtx->copy(srcProxy, {left, top, width, height}, {0, 0})) {
+ return false;
+ }
+ return this->readSurfacePixels2(tempCtx.get(), 0, 0, width, height, dstColorType,
+ dstColorSpace, buffer, rowBytes, flags);
+ }
+
+ bool flip = srcProxy->origin() == kBottomLeft_GrSurfaceOrigin;
+ if (flip) {
+ top = srcSurface->height() - top - height;
+ }
+
+ GrColorType allowedColorType =
+ fContext->caps()->supportedReadPixelsColorType(srcProxy->config(), dstColorType);
+ convert = convert || (dstColorType != allowedColorType);
+
+ if (!src->colorSpaceInfo().colorSpace()) {
+ // "Legacy" mode - no color space conversions.
+ dstColorSpace = nullptr;
+ }
+ convert = convert || !SkColorSpace::Equals(dstColorSpace, src->colorSpaceInfo().colorSpace());
+
+ SkAutoPixmapStorage tempPixmap;
+ SkPixmap finalPixmap;
+ if (convert) {
+ SkColorType srcSkColorType = GrColorTypeToSkColorType(allowedColorType);
+ SkColorType dstSkColorType = GrColorTypeToSkColorType(dstColorType);
+ if (kUnknown_SkColorType == srcSkColorType || kUnknown_SkColorType == dstSkColorType) {
+ return false;
+ }
+ auto tempAT = SkColorTypeIsAlwaysOpaque(srcSkColorType) ? kOpaque_SkAlphaType
+ : kPremul_SkAlphaType;
+ auto tempII = SkImageInfo::Make(width, height, srcSkColorType, tempAT,
+ src->colorSpaceInfo().refColorSpace());
+ SkASSERT(!unpremul || !SkColorTypeIsAlwaysOpaque(dstSkColorType));
+ auto finalAT = SkColorTypeIsAlwaysOpaque(srcSkColorType)
+ ? kOpaque_SkAlphaType
+ : unpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType;
+ auto finalII =
+ SkImageInfo::Make(width, height, dstSkColorType, finalAT, sk_ref_sp(dstColorSpace));
+ if (!SkImageInfoValidConversion(finalII, tempII)) {
+ return false;
+ }
+ tempPixmap.alloc(tempII);
+ finalPixmap.reset(finalII, buffer, rowBytes);
+ buffer = tempPixmap.writable_addr();
+ rowBytes = tempPixmap.rowBytes();
+ }
+
+ if (srcSurface->surfacePriv().hasPendingWrite()) {
+ this->flush(nullptr); // MDB TODO: tighten this
+ }
+
+ if (!fContext->fGpu->readPixels(srcSurface, left, top, width, height, allowedColorType, buffer,
+ rowBytes)) {
+ return false;
+ }
+
+ if (flip) {
+ size_t trimRowBytes = GrColorTypeBytesPerPixel(allowedColorType) * width;
+ std::unique_ptr<char[]> row(new char[trimRowBytes]);
+ char* upper = reinterpret_cast<char*>(buffer);
+ char* lower = reinterpret_cast<char*>(buffer) + (height - 1) * rowBytes;
+ for (int y = 0; y < height / 2; ++y, upper += rowBytes, lower -= rowBytes) {
+ memcpy(row.get(), upper, trimRowBytes);
+ memcpy(upper, lower, trimRowBytes);
+ memcpy(lower, row.get(), trimRowBytes);
+ }
+ }
+ if (convert) {
+ if (!tempPixmap.readPixels(finalPixmap)) {
+ return false;
+ }
+ }
+ return true;
+}
+
void GrContextPriv::prepareSurfaceForExternalIO(GrSurfaceProxy* proxy) {
ASSERT_SINGLE_OWNER_PRIV
RETURN_IF_ABANDONED_PRIV
diff --git a/src/gpu/GrContextPriv.h b/src/gpu/GrContextPriv.h
index f7bd911..61ff3c1 100644
--- a/src/gpu/GrContextPriv.h
+++ b/src/gpu/GrContextPriv.h
@@ -128,13 +128,16 @@
};
/**
- * Reads a rectangle of pixels from a surface.
+ * Reads a rectangle of pixels from a surface. There are currently two versions of this.
+ * readSurfacePixels() is the older version which will be replaced by the more robust and
+ * maintainable (but perhaps slower) readSurfacePixels2().
+ *
* @param src the surface context to read from.
* @param left left edge of the rectangle to read (inclusive)
* @param top top edge of the rectangle to read (inclusive)
* @param width width of rectangle to read in pixels.
* @param height height of rectangle to read in pixels.
- * @param dstConfig the pixel config of the destination buffer
+ * @param dstColorType the color type of the destination buffer
* @param dstColorSpace color space of the destination buffer
* @param buffer memory to read the rectangle into.
* @param rowBytes number of bytes bewtween consecutive rows. Zero means rows are tightly
@@ -147,6 +150,9 @@
bool readSurfacePixels(GrSurfaceContext* src, int left, int top, int width, int height,
GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer,
size_t rowBytes = 0, uint32_t pixelOpsFlags = 0);
+ bool readSurfacePixels2(GrSurfaceContext* src, int left, int top, int width, int height,
+ GrColorType dstColorType, SkColorSpace* dstColorSpace, void* buffer,
+ size_t rowBytes = 0, uint32_t pixelOpsFlags = 0);
/**
* Writes a rectangle of pixels to a surface. There are currently two versions of this.
diff --git a/src/gpu/GrGpu.h b/src/gpu/GrGpu.h
index 1656141..95c5117 100644
--- a/src/gpu/GrGpu.h
+++ b/src/gpu/GrGpu.h
@@ -272,6 +272,15 @@
*/
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);
+ }
/**
* Updates the pixels in a rectangle of a surface. No sRGB/linear conversions are performed.
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 0a5c2fe..e05a4bb 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -890,11 +890,6 @@
return true;
}
break;
- case kInteger_FormatType:
- if (GR_GL_RGBA_INTEGER == readFormat && GR_GL_INT == readType) {
- return true;
- }
- break;
case kFloat_FormatType:
if (GR_GL_RGBA == readFormat && GR_GL_FLOAT == readType) {
return true;
@@ -1226,7 +1221,8 @@
bool memoryIsAlphaOnly = GrPixelConfigIsAlphaOnly(memoryConfig);
// We don't currently support moving RGBA data into and out of ALPHA surfaces. It could be
- // made to work in many cases using glPixelStore and what not but is not needed currently.
+ // made to work. However, this is complicated by the use of GL_RED for alpha-only textures but
+ // is not needed currently.
if (surfaceIsAlphaOnly && !memoryIsAlphaOnly) {
return false;
}
@@ -2449,8 +2445,7 @@
return false;
}
}
- }
- if (auto rt = surface->asRenderTarget()) {
+ } if (auto rt = surface->asRenderTarget()) {
if (fUseDrawInsteadOfAllRenderTargetWrites) {
return false;
}
@@ -2462,6 +2457,44 @@
return true;
}
+bool GrGLCaps::surfaceSupportsReadPixels(const GrSurface* surface) const {
+ if (auto tex = static_cast<const GrGLTexture*>(surface->asTexture())) {
+ // We don't support reading pixels directly from EXTERNAL textures as it would require
+ // binding the texture to a FBO.
+ if (tex->target() == GR_GL_TEXTURE_EXTERNAL) {
+ return false;
+ }
+ }
+ return true;
+}
+
+GrColorType GrGLCaps::supportedReadPixelsColorType(GrPixelConfig config,
+ GrColorType dstColorType) const {
+ // For now, we mostly report the read back format that is required by the ES spec without
+ // checking for implementation allowed formats or consider laxer rules in non-ES GL. TODO: Relax
+ // this as makes sense to increase performance and correctness.
+ switch (fConfigTable[config].fFormatType) {
+ case kNormalizedFixedPoint_FormatType:
+ return GrColorType::kRGBA_8888;
+ case kFloat_FormatType:
+ // We cheat a little here and allow F16 read back if the src and dst match.
+ if (kRGBA_half_GrPixelConfig == config && GrColorType::kRGBA_F16 == dstColorType) {
+ return GrColorType::kRGBA_F16;
+ }
+ if ((kAlpha_half_GrPixelConfig == config ||
+ kAlpha_half_as_Red_GrPixelConfig == config) &&
+ GrColorType::kAlpha_F16 == dstColorType) {
+ return GrColorType::kAlpha_F16;
+ }
+ // And similar for full float RG.
+ if (kRG_float_GrPixelConfig == config && GrColorType::kRG_F32 == dstColorType) {
+ return GrColorType::kRG_F32;
+ }
+ return GrColorType::kRGBA_F32;
+ }
+ return GrColorType::kUnknown;
+}
+
bool GrGLCaps::onIsWindowRectanglesSupportedForRT(const GrBackendRenderTarget& backendRT) const {
const GrGLFramebufferInfo* fbInfo = backendRT.getGLFramebufferInfo();
SkASSERT(fbInfo);
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index 5d3fa6a..eed3ab3 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -316,7 +316,9 @@
/// Use indices or vertices in CPU arrays rather than VBOs for dynamic content.
bool useNonVBOVertexAndIndexDynamicData() const { return fUseNonVBOVertexAndIndexDynamicData; }
- bool surfaceSupportsWritePixels(const GrSurface* surface) const override;
+ bool surfaceSupportsWritePixels(const GrSurface*) const override;
+ bool surfaceSupportsReadPixels(const GrSurface*) const override;
+ GrColorType supportedReadPixelsColorType(GrPixelConfig, GrColorType) const override;
/// Does ReadPixels support reading readConfig pixels from a FBO that is surfaceConfig?
bool readPixelsSupported(GrPixelConfig surfaceConfig,
@@ -503,7 +505,6 @@
enum FormatType {
kNormalizedFixedPoint_FormatType,
kFloat_FormatType,
- kInteger_FormatType,
};
struct ReadPixelsFormat {
diff --git a/src/gpu/mock/GrMockCaps.h b/src/gpu/mock/GrMockCaps.h
index 4bcd459..27d295e 100644
--- a/src/gpu/mock/GrMockCaps.h
+++ b/src/gpu/mock/GrMockCaps.h
@@ -67,7 +67,8 @@
return 0;
}
- bool surfaceSupportsWritePixels(const GrSurface* surface) const override { return true; }
+ bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; }
+ bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
bool initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc* desc, GrSurfaceOrigin*,
bool* rectsMustMatch, bool* disallowSubrect) const override {
diff --git a/src/gpu/mtl/GrMtlCaps.h b/src/gpu/mtl/GrMtlCaps.h
index 130af67..c43d596 100644
--- a/src/gpu/mtl/GrMtlCaps.h
+++ b/src/gpu/mtl/GrMtlCaps.h
@@ -31,7 +31,8 @@
int getRenderTargetSampleCount(int requestedCount, GrPixelConfig) const override;
int maxRenderTargetSampleCount(GrPixelConfig) const override;
- bool surfaceSupportsWritePixels(const GrSurface* surface) const override { return true; }
+ bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; }
+ bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
bool isConfigCopyable(GrPixelConfig config) const override {
return true;
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index 63bd4e9..881aacc 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -40,7 +40,8 @@
int getRenderTargetSampleCount(int requestedCount, GrPixelConfig config) const override;
int maxRenderTargetSampleCount(GrPixelConfig config) const override;
- bool surfaceSupportsWritePixels(const GrSurface* surface) const override;
+ bool surfaceSupportsWritePixels(const GrSurface*) const override;
+ bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
bool isConfigTexturableLinearly(GrPixelConfig config) const {
return SkToBool(ConfigInfo::kTextureable_Flag & fConfigTable[config].fLinearFlags);
diff --git a/tests/PackedConfigsTextureTest.cpp b/tests/PackedConfigsTextureTest.cpp
index b1fbc77..5f18ef3 100644
--- a/tests/PackedConfigsTextureTest.cpp
+++ b/tests/PackedConfigsTextureTest.cpp
@@ -97,7 +97,7 @@
}
static void run_test(skiatest::Reporter* reporter, GrContext* context, int arraySize,
- GrColorType colorType) {
+ SkColorType colorType) {
SkTDArray<uint16_t> controlPixelData;
// We will read back into an 8888 buffer since 565/4444 read backs aren't supported
SkTDArray<GrColor> readBuffer;
@@ -113,19 +113,24 @@
kRGBA_8888_SkColorType, kOpaque_SkAlphaType);
for (auto origin : { kTopLeft_GrSurfaceOrigin, kBottomLeft_GrSurfaceOrigin }) {
- auto proxy = sk_gpu_test::MakeTextureProxyFromData(context, false, DEV_W, DEV_H, colorType,
+ auto proxy = sk_gpu_test::MakeTextureProxyFromData(context, false, DEV_W, DEV_H,
+ SkColorTypeToGrColorType(colorType),
origin, controlPixelData.begin(), 0);
SkASSERT(proxy);
sk_sp<GrSurfaceContext> sContext = context->contextPriv().makeWrappedSurfaceContext(
std::move(proxy));
- SkAssertResult(sContext->readPixels(dstInfo, readBuffer.begin(), 0, 0, 0));
+ if (!sContext->readPixels(dstInfo, readBuffer.begin(), 0, 0, 0)) {
+ // We only require this to succeed if the format is renderable.
+ REPORTER_ASSERT(reporter, !context->colorTypeSupportedAsSurface(colorType));
+ return;
+ }
- if (GrColorType::kABGR_4444 == colorType) {
+ if (kARGB_4444_SkColorType == colorType) {
check_4444(reporter, controlPixelData, readBuffer);
} else {
- SkASSERT(GrColorType::kRGB_565 == colorType);
+ SkASSERT(kRGB_565_SkColorType == colorType);
check_565(reporter, controlPixelData, readBuffer);
}
}
@@ -134,11 +139,11 @@
static const int CONTROL_ARRAY_SIZE = DEV_W * DEV_H;
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(RGBA4444TextureTest, reporter, ctxInfo) {
- run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, GrColorType::kABGR_4444);
+ run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, kARGB_4444_SkColorType);
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(RGB565TextureTest, reporter, ctxInfo) {
- run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, GrColorType::kRGB_565);
+ run_test(reporter, ctxInfo.grContext(), CONTROL_ARRAY_SIZE, kRGB_565_SkColorType);
}
#endif
diff --git a/tests/ReadWriteAlphaTest.cpp b/tests/ReadWriteAlphaTest.cpp
index eaad411..b82c48f 100644
--- a/tests/ReadWriteAlphaTest.cpp
+++ b/tests/ReadWriteAlphaTest.cpp
@@ -99,6 +99,11 @@
// read the texture back
result = sContext->readPixels(ii, readback.get(), rowBytes, 0, 0);
+ // We don't require reading from kAlpha_8 to be supported. TODO: At least make this work
+ // when kAlpha_8 is renderable.
+ if (!result) {
+ continue;
+ }
REPORTER_ASSERT(reporter, result, "Initial A8 readPixels failed");
// make sure the original & read back versions match
diff --git a/tests/SRGBReadWritePixelsTest.cpp b/tests/SRGBReadWritePixelsTest.cpp
index 18ed738..569490c 100644
--- a/tests/SRGBReadWritePixelsTest.cpp
+++ b/tests/SRGBReadWritePixelsTest.cpp
@@ -139,8 +139,8 @@
uint32_t read = readData[j * w + i];
if (!checker(orig, read, error)) {
- ERRORF(reporter, "Expected 0x%08x, read back as 0x%08x in %s at %d, %d).",
- orig, read, subtestName, i, j);
+ ERRORF(reporter, "Original 0x%08x, read back as 0x%08x in %s at %d, %d).", orig,
+ read, subtestName, i, j);
return;
}
}
@@ -272,13 +272,20 @@
///////////////////////////////////////////////////////////////////////////////////////////////
// Write sRGB data to a sRGB context - no conversion on the write.
- // back to sRGB no conversion
+ // back to sRGB - no conversion.
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kSRGB, smallError,
check_no_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
// Untagged read from sRGB is treated as a conversion back to linear. TODO: Fail or don't
// convert?
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kUntagged, error,
check_srgb_to_linear_conversion, context, reporter);
+#else
+ // Reading back to untagged should be a pass through with no conversion.
+ test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kUntagged, error,
+ check_no_conversion, context, reporter);
+#endif
+
// Converts back to linear
test_write_read(Encoding::kSRGB, Encoding::kSRGB, Encoding::kLinear, error,
check_srgb_to_linear_conversion, context, reporter);
@@ -307,9 +314,15 @@
// are all the same as the above cases where the original data was untagged.
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
// TODO: Fail or don't convert?
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kUntagged, error,
check_linear_to_srgb_to_linear_conversion, context, reporter);
+#else
+ // When the dst buffer is untagged there should be no conversion on the read.
+ test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kUntagged, error,
+ check_linear_to_srgb_conversion, context, reporter);
+#endif
test_write_read(Encoding::kSRGB, Encoding::kLinear, Encoding::kLinear, error,
check_linear_to_srgb_to_linear_conversion, context, reporter);
@@ -317,13 +330,13 @@
// Write data to an untagged context. The write does no conversion no matter what encoding the
// src data has.
for (auto writeEncoding : {Encoding::kSRGB, Encoding::kUntagged, Encoding::kLinear}) {
- // The read from untagged to sRGB also does no conversion. TODO: Should it just fail?
+ // The read from untagged to sRGB also does no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kSRGB, error,
check_no_conversion, context, reporter);
// Reading untagged back as untagged should do no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kUntagged, error,
check_no_conversion, context, reporter);
- // Reading untagged back as linear does no conversion. TODO: Should it just fail?
+ // Reading untagged back as linear does no conversion.
test_write_read(Encoding::kUntagged, writeEncoding, Encoding::kLinear, error,
check_no_conversion, context, reporter);
}
@@ -334,18 +347,18 @@
// converts back to sRGB on read.
test_write_read(Encoding::kLinear, Encoding::kSRGB, Encoding::kSRGB, error,
check_srgb_to_linear_to_srgb_conversion, context, reporter);
- // Reading untagged data from linear currently does no conversion. TODO: Should it fail?
+ // Reading untagged data from linear currently does no conversion.
test_write_read(Encoding::kLinear, Encoding::kSRGB, Encoding::kUntagged, error,
check_srgb_to_linear_conversion, context, reporter);
// Stays linear when read.
test_write_read(Encoding::kLinear, Encoding::kSRGB, Encoding::kLinear, error,
check_srgb_to_linear_conversion, context, reporter);
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
///////////////////////////////////////////////////////////////////////////////////////////////
// Write untagged data to a linear context. Currently does no conversion. TODO: Should this
// fail?
-#ifdef SK_LEGACY_GPU_PIXEL_OPS
// Reading to sRGB does a conversion.
test_write_read(Encoding::kLinear, Encoding::kUntagged, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
@@ -366,7 +379,7 @@
// Reading to sRGB does a conversion.
test_write_read(Encoding::kLinear, Encoding::kLinear, Encoding::kSRGB, error,
check_linear_to_srgb_conversion, context, reporter);
- // Reading to untagged does no conversion. TODO: Should it fail?
+ // Reading to untagged does no conversion.
test_write_read(Encoding::kLinear, Encoding::kLinear, Encoding::kUntagged, error,
check_no_conversion, context, reporter);
// Stays linear when read.