Require sRGB write control for sRGB support. Add flag to GrPaint to suppress linear...

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1830303002

Review URL: https://codereview.chromium.org/1830303002

diff --git a/include/gpu/GrCaps.h b/include/gpu/GrCaps.h
index 1f5b119..398517c 100644 (file)
--- a/include/gpu/GrCaps.h
+++ b/include/gpu/GrCaps.h
@@ -137,6 +137,13 @@ public:
     /** To avoid as-yet-unnecessary complexity we don't allow any partial support of MIP Maps (e.g.
         only for POT textures) */
     bool mipMapSupport() const { return fMipMapSupport; }
+
+    /**
+     * Skia convention is that a device only has sRGB support if it supports sRGB formats for both
+     * textures and framebuffers. In addition:
+     *   Decoding to linear of an sRGB texture can be disabled.
+     *   Encoding and gamma-correct blending to an sRGB framebuffer can be disabled.
+     */
     bool srgbSupport() const { return fSRGBSupport; }
     bool twoSidedStencilSupport() const { return fTwoSidedStencilSupport; }
     bool stencilWrapOpsSupport() const { return  fStencilWrapOpsSupport; }

diff --git a/include/gpu/GrPaint.h b/include/gpu/GrPaint.h
index 152cb51..3f06f09 100644 (file)
--- a/include/gpu/GrPaint.h
+++ b/include/gpu/GrPaint.h
@@ -56,6 +56,13 @@ public:
     void setAntiAlias(bool aa) { fAntiAlias = aa; }
     bool isAntiAlias() const { return fAntiAlias; }
 
+    /**
+     * Should shader output conversion from linear to sRGB be disabled.
+     * Only relevant if the destination is sRGB. Defaults to false.
+     */
+    void setDisableOutputConversionToSRGB(bool srgb) { fDisableOutputConversionToSRGB = srgb; }
+    bool getDisableOutputConversionToSRGB() const { return fDisableOutputConversionToSRGB; }
+
     const GrXPFactory* setXPFactory(const GrXPFactory* xpFactory) {
         fXPFactory.reset(SkSafeRef(xpFactory));
         return xpFactory;
@@ -112,6 +119,7 @@ public:
 
     GrPaint& operator=(const GrPaint& paint) {
         fAntiAlias = paint.fAntiAlias;
+        fDisableOutputConversionToSRGB = paint.fDisableOutputConversionToSRGB;
 
         fColor = paint.fColor;
         this->resetFragmentProcessors();
@@ -154,6 +162,7 @@ private:
     SkSTArray<2, const GrFragmentProcessor*, true>  fCoverageFragmentProcessors;
 
     bool                                            fAntiAlias;
+    bool                                            fDisableOutputConversionToSRGB;
 
     GrColor                                         fColor;
 };

diff --git a/src/gpu/GrPaint.cpp b/src/gpu/GrPaint.cpp
index 1ec8e50..c381562 100644 (file)
--- a/src/gpu/GrPaint.cpp
+++ b/src/gpu/GrPaint.cpp
@@ -15,6 +15,7 @@
 
 GrPaint::GrPaint()
     : fAntiAlias(false)
+    , fDisableOutputConversionToSRGB(false)
     , fColor(GrColor_WHITE) {}
 
 void GrPaint::setCoverageSetOpXPFactory(SkRegion::Op regionOp, bool invertCoverage) {

diff --git a/src/gpu/GrPipeline.cpp b/src/gpu/GrPipeline.cpp
index 8e542d2..e3c4864 100644 (file)
--- a/src/gpu/GrPipeline.cpp
+++ b/src/gpu/GrPipeline.cpp
@@ -81,6 +81,9 @@ GrPipeline* GrPipeline::CreateAt(void* memory, const CreateArgs& args,
     if (builder.snapVerticesToPixelCenters()) {
         pipeline->fFlags |= kSnapVertices_Flag;
     }
+    if (builder.getDisableOutputConversionToSRGB()) {
+        pipeline->fFlags |= kDisableOutputConversionToSRGB_Flag;
+    }
 
     int firstColorProcessorIdx = args.fOpts.fColorPOI.firstEffectiveProcessorIndex();
 

diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h
index db7cb5b..bdcc7d9 100644 (file)
--- a/src/gpu/GrPipeline.h
+++ b/src/gpu/GrPipeline.h
@@ -145,6 +145,9 @@ public:
 
     bool isHWAntialiasState() const { return SkToBool(fFlags & kHWAA_Flag); }
     bool snapVerticesToPixelCenters() const { return SkToBool(fFlags & kSnapVertices_Flag); }
+    bool getDisableOutputConversionToSRGB() const {
+        return SkToBool(fFlags & kDisableOutputConversionToSRGB_Flag);
+    }
 
     GrXferBarrierType xferBarrierType(const GrCaps& caps) const {
         return this->getXferProcessor().xferBarrierType(fRenderTarget.get(), caps);
@@ -184,8 +187,9 @@ private:
                             const GrCaps&);
 
     enum Flags {
-        kHWAA_Flag              = 0x1,
-        kSnapVertices_Flag      = 0x2,
+        kHWAA_Flag                          = 0x1,
+        kSnapVertices_Flag                  = 0x2,
+        kDisableOutputConversionToSRGB_Flag = 0x4,
     };
 
     typedef GrPendingIOResource<GrRenderTarget, kWrite_GrIOType> RenderTarget;

diff --git a/src/gpu/GrPipelineBuilder.cpp b/src/gpu/GrPipelineBuilder.cpp
index f1d5c26..2eba535 100644 (file)
--- a/src/gpu/GrPipelineBuilder.cpp
+++ b/src/gpu/GrPipelineBuilder.cpp
@@ -44,6 +44,8 @@ GrPipelineBuilder::GrPipelineBuilder(const GrPaint& paint, GrRenderTarget* rt, c
 
     this->setState(GrPipelineBuilder::kHWAntialias_Flag,
                    rt->isUnifiedMultisampled() && paint.isAntiAlias());
+    this->setState(GrPipelineBuilder::kDisableOutputConversionToSRGB_Flag,
+                   paint.getDisableOutputConversionToSRGB());
 }
 
 //////////////////////////////////////////////////////////////////////////////s

diff --git a/src/gpu/GrPipelineBuilder.h b/src/gpu/GrPipelineBuilder.h
index f66ced3..83ad05f 100644 (file)
--- a/src/gpu/GrPipelineBuilder.h
+++ b/src/gpu/GrPipelineBuilder.h
@@ -283,12 +283,19 @@ public:
          */
         kSnapVerticesToPixelCenters_Flag = 0x02,
 
-        kLast_Flag = kSnapVerticesToPixelCenters_Flag,
+        /**
+         * Suppress linear -> sRGB conversion when rendering to sRGB render targets.
+         */
+        kDisableOutputConversionToSRGB_Flag = 0x04,
+
+        kLast_Flag = kDisableOutputConversionToSRGB_Flag,
     };
 
     bool isHWAntialias() const { return SkToBool(fFlags & kHWAntialias_Flag); }
     bool snapVerticesToPixelCenters() const {
         return SkToBool(fFlags & kSnapVerticesToPixelCenters_Flag); }
+    bool getDisableOutputConversionToSRGB() const {
+        return SkToBool(fFlags & kDisableOutputConversionToSRGB_Flag); }
 
     /**
      * Enable render state settings.

diff --git a/src/gpu/GrYUVProvider.cpp b/src/gpu/GrYUVProvider.cpp
index f35c6df..90d5537 100644 (file)
--- a/src/gpu/GrYUVProvider.cpp
+++ b/src/gpu/GrYUVProvider.cpp
@@ -123,6 +123,9 @@ GrTexture* GrYUVProvider::refAsTexture(GrContext* ctx, const GrSurfaceDesc& desc
     SkASSERT(renderTarget);
 
     GrPaint paint;
+    // We may be decoding an sRGB image, but the result of our linear math on the YUV planes
+    // is already in sRGB in that case. Don't convert (which will make the image too bright).
+    paint.setDisableOutputConversionToSRGB(true);
     SkAutoTUnref<const GrFragmentProcessor> yuvToRgbProcessor(
                                         GrYUVEffect::CreateYUVToRGB(yuvTextures[0],
                                                                     yuvTextures[1],

diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 50a74fe..faccbd1 100644 (file)
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -46,7 +46,6 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions,
     fBindFragDataLocationSupport = false;
     fRectangleTextureSupport = false;
     fTextureSwizzleSupport = false;
-    fSRGBWriteControl = false;
     fRGBA8888PixelsOpsAreSlow = false;
     fPartialFBOReadIsSlow = false;
     fMipMapLevelAndLodControlSupport = false;
@@ -1080,7 +1079,6 @@ SkString GrGLCaps::dump() const {
     r.appendf("Base instance support: %s\n", (fBaseInstanceSupport ? "YES" : "NO"));
     r.appendf("Use non-VBO for dynamic data: %s\n",
              (fUseNonVBOVertexAndIndexDynamicData ? "YES" : "NO"));
-    r.appendf("SRGB write contol: %s\n", (fSRGBWriteControl ? "YES" : "NO"));
     r.appendf("RGBA 8888 pixel ops are slow: %s\n", (fRGBA8888PixelsOpsAreSlow ? "YES" : "NO"));
     r.appendf("Partial FBO read is slow: %s\n", (fPartialFBOReadIsSlow ? "YES" : "NO"));
     r.appendf("Bind uniform location support: %s\n", (fBindUniformLocationSupport ? "YES" : "NO"));
@@ -1434,7 +1432,8 @@ void GrGLCaps::initConfigTable(const GrGLContextInfo& ctxInfo, const GrGLInterfa
     fConfigTable[kBGRA_8888_GrPixelConfig].fSwizzle = GrSwizzle::RGBA();
 
     // We only enable srgb support if both textures and FBOs support srgb,
-    // *and* we can disable sRGB decode-on-read, to support "legacy" mode.
+    // *and* we can disable sRGB decode-on-read, to support "legacy" mode,
+    // *and* we can disable sRGB encode-on-write.
     if (kGL_GrGLStandard == standard) {
         if (ctxInfo.version() >= GR_GL_VER(3,0)) {
             fSRGBSupport = true;
@@ -1445,14 +1444,13 @@ void GrGLCaps::initConfigTable(const GrGLContextInfo& ctxInfo, const GrGLInterfa
             }
         }
         // All the above srgb extensions support toggling srgb writes
-        fSRGBWriteControl = fSRGBSupport;
     } else {
         // See https://bug.skia.org/4148 for PowerVR issue.
         fSRGBSupport = kPowerVRRogue_GrGLRenderer != ctxInfo.renderer() &&
             (ctxInfo.version() >= GR_GL_VER(3,0) || ctxInfo.hasExtension("GL_EXT_sRGB"));
         // ES through 3.1 requires EXT_srgb_write_control to support toggling
         // sRGB writing for destinations.
-        fSRGBWriteControl = ctxInfo.hasExtension("GL_EXT_sRGB_write_control");
+        fSRGBSupport = fSRGBSupport && ctxInfo.hasExtension("GL_EXT_sRGB_write_control");
     }
     if (!ctxInfo.hasExtension("GL_EXT_texture_sRGB_decode")) {
         // To support "legacy" L32 mode, we require the ability to turn off sRGB decode:

diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index bb3e231..e9a2969 100644 (file)
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -323,13 +323,6 @@ public:
     /// GL_ARB_texture_swizzle
     bool textureSwizzleSupport() const { return fTextureSwizzleSupport; }
 
-    /**
-     * Is there support for enabling/disabling sRGB writes for sRGB-capable color attachments?
-     * If false this does not mean sRGB is not supported but rather that if it is supported
-     * it cannot be turned off for configs that support it.
-     */
-    bool srgbWriteControl() const { return fSRGBWriteControl; }
-
     bool mipMapLevelAndLodControlSupport() const { return fMipMapLevelAndLodControlSupport; }
 
     /**
@@ -402,7 +395,6 @@ private:
     bool fUseNonVBOVertexAndIndexDynamicData : 1;
     bool fIsCoreProfile : 1;
     bool fBindFragDataLocationSupport : 1;
-    bool fSRGBWriteControl : 1;
     bool fRGBA8888PixelsOpsAreSlow : 1;
     bool fPartialFBOReadIsSlow : 1;
     bool fBindUniformLocationSupport : 1;

diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index 3a29361..3bbc77a 100644 (file)
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -2114,7 +2114,7 @@ bool GrGLGpu::flushGLState(const GrPipeline& pipeline, const GrPrimitiveProcesso
 
     // This must come after textures are flushed because a texture may need
     // to be msaa-resolved (which will modify bound FBO state).
-    this->flushRenderTarget(glRT, nullptr);
+    this->flushRenderTarget(glRT, nullptr, pipeline.getDisableOutputConversionToSRGB());
 
     return true;
 }
@@ -2833,7 +2833,7 @@ void GrGLGpu::finishDrawTarget() {
     }
 }
 
-void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bounds) {
+void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bounds, bool disableSRGB) {
     SkASSERT(target);
 
     uint32_t rtID = target->getUniqueID();
@@ -2855,17 +2855,19 @@ void GrGLGpu::flushRenderTarget(GrGLRenderTarget* target, const SkIRect* bounds)
 #endif
         fHWBoundRenderTargetUniqueID = rtID;
         this->flushViewport(target->getViewport());
-        if (this->glCaps().srgbWriteControl()) {
-            bool enableSRGBWrite = GrPixelConfigIsSRGB(target->config());
-            if (enableSRGBWrite && kYes_TriState != fHWSRGBFramebuffer) {
-                GL_CALL(Enable(GR_GL_FRAMEBUFFER_SRGB));
-                fHWSRGBFramebuffer = kYes_TriState;
-            } else if (!enableSRGBWrite && kNo_TriState != fHWSRGBFramebuffer) {
-                GL_CALL(Disable(GR_GL_FRAMEBUFFER_SRGB));
-                fHWSRGBFramebuffer = kNo_TriState;
-            }
+    }
+
+    if (this->glCaps().srgbSupport()) {
+        bool enableSRGBWrite = GrPixelConfigIsSRGB(target->config()) && !disableSRGB;
+        if (enableSRGBWrite && kYes_TriState != fHWSRGBFramebuffer) {
+            GL_CALL(Enable(GR_GL_FRAMEBUFFER_SRGB));
+            fHWSRGBFramebuffer = kYes_TriState;
+        } else if (!enableSRGBWrite && kNo_TriState != fHWSRGBFramebuffer) {
+            GL_CALL(Disable(GR_GL_FRAMEBUFFER_SRGB));
+            fHWSRGBFramebuffer = kNo_TriState;
         }
     }
+
     this->didWriteToSurface(target, bounds);
 }
 

diff --git a/src/gpu/gl/GrGLGpu.h b/src/gpu/gl/GrGLGpu.h
index bcb3c19..61fdb8f 100644 (file)
--- a/src/gpu/gl/GrGLGpu.h
+++ b/src/gpu/gl/GrGLGpu.h
@@ -328,7 +328,7 @@ private:
 
     // bounds is region that may be modified.
     // nullptr means whole target. Can be an empty rect.
-    void flushRenderTarget(GrGLRenderTarget*, const SkIRect* bounds);
+    void flushRenderTarget(GrGLRenderTarget*, const SkIRect* bounds, bool disableSRGB = false);
     // Handles cases where a surface will be updated without a call to flushRenderTarget
     void didWriteToSurface(GrSurface*, const SkIRect* bounds) const;