Fix some GPU image filter code to preserve precision and color space
authorbrianosman <brianosman@google.com>
Wed, 21 Sep 2016 13:45:09 +0000 (06:45 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 21 Sep 2016 13:45:09 +0000 (06:45 -0700)
On the pure-GPU path, we just have an SkSpecialImage (that's definitely
texture backed), and we need a renderable config for the draw context we
make. Added a helper function to pick - this is basically the high
precision analog of what we were doing before (always using 8888).

The assert that I added catches many other problems in image filter code,
but those fixes are coming in subsequent CLs.

12 GMs render correctly (or more correctly) in gpusrgb and gpuf16
configs. In most cases, they were drawing previously nothing.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2359443003

Review-Url: https://codereview.chromium.org/2359443003

src/core/SkImageFilter.cpp
src/effects/SkDisplacementMapEffect.cpp
src/effects/SkLightingImageFilter.cpp
src/effects/SkMorphologyImageFilter.cpp
src/effects/SkXfermodeImageFilter.cpp
src/gpu/GrDrawingManager.cpp
src/gpu/SkGr.cpp
src/gpu/SkGrPriv.h

index b2c04450fff42d12b112a4316b05fba3b3a528a3..64b62759e2c8b83dfd1ba0d61d161a61008b92bd 100644 (file)
@@ -22,6 +22,7 @@
 #include "GrContext.h"
 #include "GrDrawContext.h"
 #include "GrFixedClip.h"
+#include "SkGrPriv.h"
 #endif
 
 #ifndef SK_IGNORE_TO_STRING
@@ -282,9 +283,10 @@ sk_sp<SkSpecialImage> SkImageFilter::DrawWithFP(GrContext* context,
     paint.addColorFragmentProcessor(std::move(fp));
     paint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
 
+    GrPixelConfig config = GrRenderableConfigForColorSpace(colorSpace.get());
     sk_sp<GrDrawContext> drawContext(context->makeDrawContext(SkBackingFit::kApprox,
                                                               bounds.width(), bounds.height(),
-                                                              kRGBA_8888_GrPixelConfig,
+                                                              config,
                                                               std::move(colorSpace)));
     if (!drawContext) {
         return nullptr;
index e74ca4fe3fbe1a3475cb0975f11965c4688bab92..0f49a991c22e9a0c6c938d0b5409b7fb62042f47 100644 (file)
@@ -19,6 +19,7 @@
 #include "GrCoordTransform.h"
 #include "GrInvariantOutput.h"
 #include "SkGr.h"
+#include "SkGrPriv.h"
 #include "effects/GrTextureDomain.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
@@ -337,7 +338,8 @@ sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* sou
 
         sk_sp<GrDrawContext> drawContext(
             context->makeDrawContext(SkBackingFit::kApprox, bounds.width(), bounds.height(),
-                                     kSkia8888_GrPixelConfig, sk_ref_sp(source->getColorSpace())));
+                                     GrRenderableConfigForColorSpace(source->getColorSpace()),
+                                     sk_ref_sp(source->getColorSpace())));
         if (!drawContext) {
             return nullptr;
         }
index c8d2154b8669f4587e636e06dff851d5406652dd..3485bd867ced0ee6795736a09f1208cec3d3b43c 100644 (file)
@@ -22,6 +22,7 @@
 #include "GrInvariantOutput.h"
 #include "GrPaint.h"
 #include "SkGr.h"
+#include "SkGrPriv.h"
 #include "effects/GrSingleTextureEffect.h"
 #include "effects/GrTextureDomain.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
@@ -409,11 +410,10 @@ sk_sp<SkSpecialImage> SkLightingImageFilterInternal::filterImageGPU(SkSpecialIma
     sk_sp<GrTexture> inputTexture(input->asTextureRef(context));
     SkASSERT(inputTexture);
 
-    sk_sp<GrDrawContext> drawContext(context->makeDrawContext(SkBackingFit::kApprox,
-                                                              offsetBounds.width(),
-                                                              offsetBounds.height(),
-                                                              kRGBA_8888_GrPixelConfig,
-                                                              sk_ref_sp(source->getColorSpace())));
+    sk_sp<GrDrawContext> drawContext(
+        context->makeDrawContext(SkBackingFit::kApprox,offsetBounds.width(), offsetBounds.height(),
+                                 GrRenderableConfigForColorSpace(source->getColorSpace()),
+                                 sk_ref_sp(source->getColorSpace())));
     if (!drawContext) {
         return nullptr;
     }
index 4e6032fa5a18115840c1b8389e6bf4486ba24cfc..f6d400111c604a1c7384d01fd78b55753ca1aecd 100644 (file)
@@ -22,6 +22,7 @@
 #include "GrInvariantOutput.h"
 #include "GrTexture.h"
 #include "SkGr.h"
+#include "SkGrPriv.h"
 #include "effects/Gr1DKernelEffect.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
@@ -476,6 +477,7 @@ static sk_sp<SkSpecialImage> apply_morphology(GrContext* context,
     sk_sp<GrTexture> srcTexture(input->asTextureRef(context));
     SkASSERT(srcTexture);
     sk_sp<SkColorSpace> colorSpace = sk_ref_sp(input->getColorSpace());
+    GrPixelConfig config = GrRenderableConfigForColorSpace(colorSpace.get());
 
     // setup new clip
     const GrFixedClip clip(SkIRect::MakeWH(srcTexture->width(), srcTexture->height()));
@@ -488,8 +490,7 @@ static sk_sp<SkSpecialImage> apply_morphology(GrContext* context,
     if (radius.fWidth > 0) {
         sk_sp<GrDrawContext> dstDrawContext(context->makeDrawContext(SkBackingFit::kApprox,
                                                                      rect.width(), rect.height(),
-                                                                     kSkia8888_GrPixelConfig,
-                                                                     colorSpace));
+                                                                     config, colorSpace));
         if (!dstDrawContext) {
             return nullptr;
         }
@@ -510,8 +511,7 @@ static sk_sp<SkSpecialImage> apply_morphology(GrContext* context,
     if (radius.fHeight > 0) {
         sk_sp<GrDrawContext> dstDrawContext(context->makeDrawContext(SkBackingFit::kApprox,
                                                                      rect.width(), rect.height(),
-                                                                     kSkia8888_GrPixelConfig,
-                                                                     colorSpace));
+                                                                     config, colorSpace));
         if (!dstDrawContext) {
             return nullptr;
         }
index 558e29a82f996dbe4361651e1cd3c4b72b3dbcd7..537175bc4a021e78f738eb3c397e0cf913aba8be 100644 (file)
@@ -21,6 +21,7 @@
 #include "effects/GrTextureDomain.h"
 #include "effects/GrSimpleTextureEffect.h"
 #include "SkGr.h"
+#include "SkGrPriv.h"
 #endif
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -240,10 +241,10 @@ sk_sp<SkSpecialImage> SkXfermodeImageFilter::filterImageGPU(SkSpecialImage* sour
 
     paint.setPorterDuffXPFactory(SkXfermode::kSrc_Mode);
 
-    sk_sp<GrDrawContext> drawContext(context->makeDrawContext(SkBackingFit::kApprox,
-                                                              bounds.width(), bounds.height(),
-                                                              kSkia8888_GrPixelConfig,
-                                                              sk_ref_sp(source->getColorSpace())));
+    sk_sp<GrDrawContext> drawContext(
+        context->makeDrawContext(SkBackingFit::kApprox, bounds.width(), bounds.height(),
+                                 GrRenderableConfigForColorSpace(source->getColorSpace()),
+                                 sk_ref_sp(source->getColorSpace())));
     if (!drawContext) {
         return nullptr;
     }
index 606a9abd6e43c3530661e3e2c28e3949cdac6ed6..9dd5613f21bd54a3aa7786b9ee8961054521d73c 100644 (file)
@@ -222,6 +222,8 @@ sk_sp<GrDrawContext> GrDrawingManager::makeDrawContext(sk_sp<GrRenderTarget> rt,
     // by, including internal usage. We allow a null color space here, for read/write pixels and
     // other special code paths. If a color space is provided, though, enforce all other rules.
     if (colorSpace && !SkSurface_Gpu::Valid(fContext, rt->config(), colorSpace.get())) {
+        // SRGBTODO: Enable this assert once image filters are propagating color type and space
+//        SkDEBUGFAIL("Invalid config and colorspace combination");
         return nullptr;
     }
 
index c8839f3f1a49b0863df897e6f60de1a421ea0c54..9ee8722afa0eb213dd9d40709e7b761b74c1f6f0 100644 (file)
@@ -508,6 +508,19 @@ bool GrPixelConfigToColorType(GrPixelConfig config, SkColorType* ctOut) {
     return true;
 }
 
+GrPixelConfig GrRenderableConfigForColorSpace(SkColorSpace* colorSpace) {
+    if (!colorSpace) {
+        return kRGBA_8888_GrPixelConfig;
+    } else if (colorSpace->gammaIsLinear()) {
+        return kRGBA_half_GrPixelConfig;
+    } else if (colorSpace->gammaCloseToSRGB()) {
+        return kSRGBA_8888_GrPixelConfig;
+    } else {
+        SkDEBUGFAIL("No renderable config exists for color space with strange gamma");
+        return kUnknown_GrPixelConfig;
+    }
+}
+
 ////////////////////////////////////////////////////////////////////////////////////////////////
 
 static inline bool blend_requires_shader(const SkXfermode::Mode mode, bool primitiveIsSrc) {
index 26dc4aaa86af77093ef8370afe361199252514f7..c1069d1a207d19ea934e8622442b54e63c8edf27 100644 (file)
@@ -105,6 +105,11 @@ GrSurfaceDesc GrImageInfoToSurfaceDesc(const SkImageInfo&, const GrCaps&);
 
 bool GrPixelConfigToColorType(GrPixelConfig, SkColorType*);
 
+/** When image filter code needs to construct a draw context to do intermediate rendering, we need
+    a renderable pixel config. The source (SkSpecialImage) may not be in a renderable format, but
+    we want to preserve the color space of that source. This picks an appropriate format to use. */
+GrPixelConfig GrRenderableConfigForColorSpace(SkColorSpace*);
+
 /**
  *  If the compressed data in the SkData is supported (as a texture format, this returns
  *  the pixel-config that should be used, and sets outStartOfDataToUpload to the ptr into