From be5947c2f38a79b7c709accfb1047d8fd06a0227 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Mon, 19 Mar 2018 18:42:25 +0000 Subject: Revert "New read pixels implementation that is simpler but does all conversions on CPU." This reverts commit c9a642edf2d1c7f5380fe829adbb1a692f9969a6. Reason for revert: 1010102 gms broke Original change's description: > New read pixels implementation that is simpler but does all conversions on CPU. > > Change-Id: Ia548cd24a8544b35a233311706faf48de353b7cf > Reviewed-on: https://skia-review.googlesource.com/109902 > Commit-Queue: Brian Salomon > Reviewed-by: Robert Phillips TBR=bsalomon@google.com,robertphillips@google.com,brianosman@google.com Change-Id: I7724a6eef79885ba2a32c1ac871e5b2a9a3c0c12 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/115140 Reviewed-by: Brian Salomon Commit-Queue: Brian Salomon --- src/gpu/GrContext.cpp | 151 +--------------------------------------------- src/gpu/GrContextPriv.h | 10 +-- src/gpu/GrGpu.h | 9 --- src/gpu/gl/GrGLCaps.cpp | 49 +++------------ src/gpu/gl/GrGLCaps.h | 5 +- src/gpu/mock/GrMockCaps.h | 3 +- src/gpu/mtl/GrMtlCaps.h | 3 +- src/gpu/vk/GrVkCaps.h | 3 +- 8 files changed, 18 insertions(+), 215 deletions(-) (limited to 'src/gpu') diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 626b3f6ecd..f9c941565c 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -26,7 +26,6 @@ #include "GrTextureContext.h" #include "GrTextureStripAtlas.h" #include "GrTracing.h" -#include "SkAutoPixmapStorage.h" #include "SkConvertPixels.h" #include "SkDeferredDisplayList.h" #include "SkGr.h" @@ -612,10 +611,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top, 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 @@ -623,10 +618,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top, 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())) { @@ -676,7 +667,7 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top, return false; } - if (srcSurface->surfacePriv().hasPendingWrite()) { + if (!(kDontFlush_PixelOpsFlag & flags) && srcSurface->surfacePriv().hasPendingWrite()) { this->flush(nullptr); // MDB TODO: tighten this } @@ -808,12 +799,6 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top } 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; @@ -886,7 +871,7 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top 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 = dstSurface->height() - top - height; + top = dstProxy->height() - top - height; } } else if (dstProxy->origin() == kBottomLeft_GrSurfaceOrigin) { size_t trimRowBytes = GrColorTypeBytesPerPixel(srcColorType) * width; @@ -898,7 +883,7 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top } buffer = tempBuffer.get(); rowBytes = trimRowBytes; - top = dstSurface->height() - top - height; + top = dstProxy->height() - top - height; } if (!(kDontFlush_PixelOpsFlag & pixelOpsFlags) && dstSurface->surfacePriv().hasPendingIO()) { @@ -909,136 +894,6 @@ bool GrContextPriv::writeSurfacePixels2(GrSurfaceContext* dst, int left, int top 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 row(new char[trimRowBytes]); - char* upper = reinterpret_cast(buffer); - char* lower = reinterpret_cast(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 61ff3c1e34..f7bd911b1e 100644 --- a/src/gpu/GrContextPriv.h +++ b/src/gpu/GrContextPriv.h @@ -128,16 +128,13 @@ public: }; /** - * 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(). - * + * Reads a rectangle of pixels from a surface. * @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 dstColorType the color type of the destination buffer + * @param dstConfig the pixel config 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 @@ -150,9 +147,6 @@ public: 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 95c5117799..16561418aa 100644 --- a/src/gpu/GrGpu.h +++ b/src/gpu/GrGpu.h @@ -272,15 +272,6 @@ public: */ 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 e05a4bb970..0a5c2fe7b7 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -890,6 +890,11 @@ bool GrGLCaps::readPixelsSupported(GrPixelConfig surfaceConfig, 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; @@ -1221,8 +1226,7 @@ bool GrGLCaps::getExternalFormat(GrPixelConfig surfaceConfig, GrPixelConfig memo bool memoryIsAlphaOnly = GrPixelConfigIsAlphaOnly(memoryConfig); // We don't currently support moving RGBA data into and out of ALPHA surfaces. It could be - // made to work. However, this is complicated by the use of GL_RED for alpha-only textures but - // is not needed currently. + // made to work in many cases using glPixelStore and what not but is not needed currently. if (surfaceIsAlphaOnly && !memoryIsAlphaOnly) { return false; } @@ -2445,7 +2449,8 @@ bool GrGLCaps::surfaceSupportsWritePixels(const GrSurface* surface) const { return false; } } - } if (auto rt = surface->asRenderTarget()) { + } + if (auto rt = surface->asRenderTarget()) { if (fUseDrawInsteadOfAllRenderTargetWrites) { return false; } @@ -2457,44 +2462,6 @@ bool GrGLCaps::surfaceSupportsWritePixels(const GrSurface* surface) const { return true; } -bool GrGLCaps::surfaceSupportsReadPixels(const GrSurface* surface) const { - if (auto tex = static_cast(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 eed3ab3a17..5d3fa6a6f6 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/src/gpu/gl/GrGLCaps.h @@ -316,9 +316,7 @@ public: /// Use indices or vertices in CPU arrays rather than VBOs for dynamic content. bool useNonVBOVertexAndIndexDynamicData() const { return fUseNonVBOVertexAndIndexDynamicData; } - bool surfaceSupportsWritePixels(const GrSurface*) const override; - bool surfaceSupportsReadPixels(const GrSurface*) const override; - GrColorType supportedReadPixelsColorType(GrPixelConfig, GrColorType) const override; + bool surfaceSupportsWritePixels(const GrSurface* surface) const override; /// Does ReadPixels support reading readConfig pixels from a FBO that is surfaceConfig? bool readPixelsSupported(GrPixelConfig surfaceConfig, @@ -505,6 +503,7 @@ private: 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 27d295ebd5..4bcd4599c5 100644 --- a/src/gpu/mock/GrMockCaps.h +++ b/src/gpu/mock/GrMockCaps.h @@ -67,8 +67,7 @@ public: return 0; } - bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; } - bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; } + bool surfaceSupportsWritePixels(const GrSurface* surface) 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 c43d596e7f..130af67c70 100644 --- a/src/gpu/mtl/GrMtlCaps.h +++ b/src/gpu/mtl/GrMtlCaps.h @@ -31,8 +31,7 @@ public: int getRenderTargetSampleCount(int requestedCount, GrPixelConfig) const override; int maxRenderTargetSampleCount(GrPixelConfig) const override; - bool surfaceSupportsWritePixels(const GrSurface*) const override { return true; } - bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; } + bool surfaceSupportsWritePixels(const GrSurface* surface) 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 881aacc9dc..63bd4e90d4 100644 --- a/src/gpu/vk/GrVkCaps.h +++ b/src/gpu/vk/GrVkCaps.h @@ -40,8 +40,7 @@ public: int getRenderTargetSampleCount(int requestedCount, GrPixelConfig config) const override; int maxRenderTargetSampleCount(GrPixelConfig config) const override; - bool surfaceSupportsWritePixels(const GrSurface*) const override; - bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; } + bool surfaceSupportsWritePixels(const GrSurface* surface) const override; bool isConfigTexturableLinearly(GrPixelConfig config) const { return SkToBool(ConfigInfo::kTextureable_Flag & fConfigTable[config].fLinearFlags); -- cgit v1.2.3