From d2ca59a1cd39f81d4fb05be253bc3803daa23210 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Thu, 13 Apr 2017 14:03:57 -0400 Subject: Further unify logic in readSurfacePixels and writeSurfacePixels Hoist simple failure checks to the beginning, apply logic in the same sequence in both functions. Factor out some common logic. Remove redundant checking inside other helper functions (that are only used from here). I'm inching towards better sRGB and color-conversion support in these functions, but I don't want that intertwined with the legacy premul software fallback. Next step is probably to diverge at the top-level, and keep the current code as "legacy" versions, with new versions that don't have the fallback logic, only used for configs and color space scenarios where it doesn't matter. Bug: skia:5853 Change-Id: I327be5f0186a16ceda9440670fb0646acaef7766 Reviewed-on: https://skia-review.googlesource.com/13337 Commit-Queue: Brian Osman Reviewed-by: Robert Phillips Reviewed-by: Matt Sarett --- src/gpu/GrContext.cpp | 70 ++++++++++++++++++++++++++++----------------------- src/gpu/GrGpu.cpp | 10 -------- 2 files changed, 39 insertions(+), 41 deletions(-) (limited to 'src') diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index c511310590..ec9ebf66b3 100644 --- a/src/gpu/GrContext.cpp +++ b/src/gpu/GrContext.cpp @@ -265,10 +265,25 @@ bool sw_convert_to_premul(GrPixelConfig srcConfig, int width, int height, size_t return true; } -static bool valid_unpremul_config(GrPixelConfig config) { +static bool valid_premul_config(GrPixelConfig config) { return GrPixelConfigIs8888Unorm(config) || kRGBA_half_GrPixelConfig == config; } +static bool valid_pixel_conversion(GrPixelConfig srcConfig, GrPixelConfig dstConfig, + bool premulConversion) { + // We don't allow conversion between integer configs and float/fixed configs. + if (GrPixelConfigIsSint(srcConfig) != GrPixelConfigIsSint(dstConfig)) { + return false; + } + + // We only allow premul <-> unpremul conversions for some formats + if (premulConversion && (!valid_premul_config(srcConfig) || !valid_premul_config(dstConfig))) { + return false; + } + + return true; +} + bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* dstColorSpace, int left, int top, int width, int height, GrPixelConfig srcConfig, SkColorSpace* srcColorSpace, @@ -287,6 +302,12 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d return false; } + // The src is unpremul but the dst is premul -> premul the src before or as part of the write + bool premul = SkToBool(kUnpremul_PixelOpsFlag & pixelOpsFlags); + if (!valid_pixel_conversion(srcConfig, surface->config(), premul)) { + return false; + } + fContext->testPMConversionsIfNecessary(pixelOpsFlags); // Trim the params here so that if we wind up making a temporary surface it can be as small as @@ -297,19 +318,10 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d return false; } - bool applyPremulToSrc = SkToBool(kUnpremul_PixelOpsFlag & pixelOpsFlags); - if (applyPremulToSrc && !valid_unpremul_config(srcConfig)) { - return false; - } - // We don't allow conversion between integer configs and float/fixed configs. - if (GrPixelConfigIsSint(surface->config()) != GrPixelConfigIsSint(srcConfig)) { - return false; - } - GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference; // Don't prefer to draw for the conversion (and thereby access a texture from the cache) when // we've already determined that there isn't a roundtrip preserving conversion processor pair. - if (applyPremulToSrc && fContext->validPMUPMConversionExists(srcConfig)) { + if (premul && fContext->validPMUPMConversionExists(srcConfig)) { drawPreference = GrGpu::kCallerPrefersDraw_DrawPreference; } @@ -340,11 +352,11 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d sk_sp texFP = GrSimpleTextureEffect::Make( fContext->resourceProvider(), tempProxy, nullptr, SkMatrix::I()); sk_sp fp; - if (applyPremulToSrc) { + if (premul) { fp = fContext->createUPMToPMEffect(texFP, tempProxy->config()); if (fp) { // We no longer need to do this on CPU before the upload. - applyPremulToSrc = false; + premul = false; } else if (GrGpu::kCallerPrefersDraw_DrawPreference == drawPreference) { // We only wanted to do the draw to perform the premul so don't bother. tempProxy.reset(nullptr); @@ -364,7 +376,7 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d if (!texture) { return false; } - if (applyPremulToSrc) { + if (premul) { size_t tmpRowBytes = 4 * width; tmpPixels.reset(width * height); if (!sw_convert_to_premul(srcConfig, width, height, rowBytes, buffer, tmpRowBytes, @@ -373,7 +385,7 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d } rowBytes = tmpRowBytes; buffer = tmpPixels.get(); - applyPremulToSrc = false; + premul = false; } if (!fContext->fGpu->writePixels(texture, 0, 0, width, height, tempDrawInfo.fWriteConfig, buffer, @@ -406,7 +418,7 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d } } if (!tempProxy) { - if (applyPremulToSrc) { + if (premul) { size_t tmpRowBytes = 4 * width; tmpPixels.reset(width * height); if (!sw_convert_to_premul(srcConfig, width, height, rowBytes, buffer, tmpRowBytes, @@ -415,7 +427,7 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceProxy* dstProxy, SkColorSpace* d } rowBytes = tmpRowBytes; buffer = tmpPixels.get(); - applyPremulToSrc = false; + premul = false; } return fContext->fGpu->writePixels(surface, left, top, width, height, srcConfig, buffer, rowBytes); @@ -441,6 +453,12 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceProxy* srcProxy, SkColorSpace* sr return false; } + // The src is premul but the dst is unpremul -> unpremul the src after or as part of the read + bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags); + if (!valid_pixel_conversion(src->config(), dstConfig, unpremul)) { + return false; + } + fContext->testPMConversionsIfNecessary(flags); // Adjust the params so that if we wind up using an intermediate surface we've already done @@ -451,20 +469,6 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceProxy* srcProxy, SkColorSpace* sr return false; } - if (!(kDontFlush_PixelOpsFlag & flags) && src->surfacePriv().hasPendingWrite()) { - this->flush(nullptr); // MDB TODO: tighten this - } - - bool unpremul = SkToBool(kUnpremul_PixelOpsFlag & flags); - if (unpremul && !valid_unpremul_config(dstConfig)) { - // The unpremul flag is only allowed for 8888 and F16 configs. - return false; - } - // We don't allow conversion between integer configs and float/fixed configs. - if (GrPixelConfigIsSint(src->config()) != GrPixelConfigIsSint(dstConfig)) { - return false; - } - GrGpu::DrawPreference drawPreference = GrGpu::kNoDraw_DrawPreference; // Don't prefer to draw for the conversion (and thereby access a texture from the cache) when // we've already determined that there isn't a roundtrip preserving conversion processor pair. @@ -478,6 +482,10 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceProxy* srcProxy, SkColorSpace* sr return false; } + if (!(kDontFlush_PixelOpsFlag & flags) && src->surfacePriv().hasPendingWrite()) { + this->flush(nullptr); // MDB TODO: tighten this + } + sk_sp proxyToRead = sk_ref_sp(srcProxy); bool didTempDraw = false; if (GrGpu::kNoDraw_DrawPreference != drawPreference) { diff --git a/src/gpu/GrGpu.cpp b/src/gpu/GrGpu.cpp index b1560f9a2d..d4fdb1960b 100644 --- a/src/gpu/GrGpu.cpp +++ b/src/gpu/GrGpu.cpp @@ -280,11 +280,6 @@ bool GrGpu::getReadPixelsInfo(GrSurface* srcSurface, int width, int height, size SkASSERT(srcSurface); SkASSERT(kGpuPrefersDraw_DrawPreference != *drawPreference); - // We don't allow conversion between integer configs and float/fixed configs. - if (GrPixelConfigIsSint(srcSurface->config()) != GrPixelConfigIsSint(readConfig)) { - return false; - } - // We currently do not support reading into a compressed buffer if (GrPixelConfigIsCompressed(readConfig)) { return false; @@ -326,11 +321,6 @@ bool GrGpu::getWritePixelsInfo(GrSurface* dstSurface, int width, int height, return false; } - // We don't allow conversion between integer configs and float/fixed configs. - if (GrPixelConfigIsSint(dstSurface->config()) != GrPixelConfigIsSint(srcConfig)) { - return false; - } - if (SkToBool(dstSurface->asRenderTarget())) { if (this->caps()->useDrawInsteadOfAllRenderTargetWrites()) { ElevateDrawPreference(drawPreference, kRequireDraw_DrawPreference); -- cgit v1.2.3