srgb texture format cleanup.

We now think of them as regular formats that happen to have a different
data encoding, orthogonal to color space, but we still have some
unnecessary special handling.

Remove sRGB support from GrCaps.

Move sRGB write control to GrGLCaps.

Detect texture and render support separately.

Support in WebGL.

Remove workaround in Vulkan for x86 PowerVR.

Remove SkSurface_Gpu::Valid().

Change-Id: I2aaf4bdd4cd1caeeee04bfe8ab539924cdb35bd1
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/244506
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>

src/gpu/GrCaps.cpp

@@ -17,8 +17,6 @@
 GrCaps::GrCaps(const GrContextOptions& options) {
     fMipMapSupport = false;
     fNPOTTextureTileSupport = false;
-    fSRGBSupport = false;
-    fSRGBWriteControl = false;
     fReuseScratchTextures = true;
     fReuseScratchBuffers = true;
     fGpuTracingSupport = false;
@@ -167,8 +165,6 @@ void GrCaps::dumpJSON(SkJSONWriter* writer) const {
 
     writer->appendBool("MIP Map Support", fMipMapSupport);
     writer->appendBool("NPOT Texture Tile Support", fNPOTTextureTileSupport);
-    writer->appendBool("sRGB Support", fSRGBSupport);
-    writer->appendBool("sRGB Write Control", fSRGBWriteControl);
     writer->appendBool("Reuse Scratch Textures", fReuseScratchTextures);
     writer->appendBool("Reuse Scratch Buffers", fReuseScratchBuffers);
     writer->appendBool("Gpu Tracing Support", fGpuTracingSupport);

src/gpu/GrCaps.h

@@ -41,15 +41,6 @@ public:
         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.
-     */
-    bool srgbSupport() const { return fSRGBSupport; }
-    /**
-     * Is there support for enabling/disabling sRGB writes for sRGB-capable color buffers?
-     */
-    bool srgbWriteControl() const { return fSRGBWriteControl; }
     bool gpuTracingSupport() const { return fGpuTracingSupport; }
     bool oversizedStencilSupport() const { return fOversizedStencilSupport; }
     bool textureBarrierSupport() const { return fTextureBarrierSupport; }
@@ -449,8 +440,6 @@ protected:
 
     bool fNPOTTextureTileSupport                     : 1;
     bool fMipMapSupport                              : 1;
-    bool fSRGBSupport                                : 1;
-    bool fSRGBWriteControl                           : 1;
     bool fReuseScratchTextures                       : 1;
     bool fReuseScratchBuffers                        : 1;
     bool fGpuTracingSupport                          : 1;

src/gpu/GrContext.cpp

@@ -370,8 +370,6 @@ GrBackendTexture GrContext::createBackendTexture(int width, int height,
 }
 
 GrBackendTexture GrContext::createBackendTexture(const SkSurfaceCharacterization& c) {
-    const GrCaps* caps = this->caps();
-
     if (!this->asDirectContext() || !c.isValid()) {
         return GrBackendTexture();
     }
@@ -394,10 +392,6 @@ GrBackendTexture GrContext::createBackendTexture(const SkSurfaceCharacterization
         return GrBackendTexture();
     }
 
-    if (!SkSurface_Gpu::Valid(caps, format)) {
-        return GrBackendTexture();
-    }
-
     GrBackendTexture result = this->createBackendTexture(c.width(), c.height(), format,
                                                          GrMipMapped(c.isMipMapped()),
                                                          GrRenderable::kYes,
@@ -430,10 +424,6 @@ GrBackendTexture GrContext::createBackendTexture(const SkSurfaceCharacterization
         return GrBackendTexture();
     }
 
-    if (!SkSurface_Gpu::Valid(this->caps(), format)) {
-        return GrBackendTexture();
-    }
-
     GrBackendTexture result = this->createBackendTexture(c.width(), c.height(), format, color,
                                                          GrMipMapped(c.isMipMapped()),
                                                          GrRenderable::kYes,

src/gpu/GrContextThreadSafeProxy.cpp

@@ -54,10 +54,6 @@ SkSurfaceCharacterization GrContextThreadSafeProxy::createCharacterization(
         isMipMapped = false;
     }
 
-    if (!SkSurface_Gpu::Valid(this->caps(), backendFormat)) {
-        return SkSurfaceCharacterization(); // return an invalid characterization
-    }
-
     GrColorType grColorType = SkColorTypeToGrColorType(ii.colorType());
 
     if (!this->caps()->areColorTypeAndFormatCompatible(grColorType, backendFormat)) {

src/gpu/GrDrawingManager.cpp

@@ -903,13 +903,6 @@ std::unique_ptr<GrRenderTargetContext> GrDrawingManager::makeRenderTargetContext
         return nullptr;
     }
 
-    // SkSurface catches bad color space usage at creation. This check handles anything that slips
-    // by, including internal usage.
-    if (!SkSurface_Gpu::Valid(fContext->priv().caps(), sProxy->backendFormat())) {
-        SkDEBUGFAIL("Invalid config and colorspace combination");
-        return nullptr;
-    }
-
     sk_sp<GrRenderTargetProxy> renderTargetProxy(sk_ref_sp(sProxy->asRenderTargetProxy()));
 
     return std::unique_ptr<GrRenderTargetContext>(
@@ -930,13 +923,6 @@ std::unique_ptr<GrTextureContext> GrDrawingManager::makeTextureContext(
         return nullptr;
     }
 
-    // SkSurface catches bad color space usage at creation. This check handles anything that slips
-    // by, including internal usage.
-    if (!SkSurface_Gpu::Valid(fContext->priv().caps(), sProxy->backendFormat())) {
-        SkDEBUGFAIL("Invalid config and colorspace combination");
-        return nullptr;
-    }
-
     // GrTextureRenderTargets should always be using a GrRenderTargetContext
     SkASSERT(!sProxy->asRenderTargetProxy());
 

src/gpu/gl/GrGLCaps.cpp

@@ -61,6 +61,7 @@ GrGLCaps::GrGLCaps(const GrContextOptions& contextOptions,
     fProgramParameterSupport = false;
     fSamplerObjectSupport = false;
     fFBFetchRequiresEnablePerSample = false;
+    fSRGBWriteControl = false;
 
     fBlitFramebufferFlags = kNoSupport_BlitFramebufferFlag;
     fMaxInstancesPerDrawWithoutCrashing = 0;
@@ -323,43 +324,15 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
     fSupportsAHardwareBufferImages = true;
 #endif
 
-    // We only enable srgb support if both textures and FBOs support srgb.
     if (GR_IS_GR_GL(standard)) {
-        if (version >= GR_GL_VER(3,0)) {
+        fSRGBWriteControl = version >= GR_GL_VER(3, 0) ||
-            fSRGBSupport = true;
+                            ctxInfo.hasExtension("GL_ARB_framebuffer_sRGB") ||
-        } else if (ctxInfo.hasExtension("GL_EXT_texture_sRGB")) {
+                            ctxInfo.hasExtension("GL_EXT_framebuffer_sRGB");
-            if (ctxInfo.hasExtension("GL_ARB_framebuffer_sRGB") ||
-                ctxInfo.hasExtension("GL_EXT_framebuffer_sRGB")) {
-                fSRGBSupport = true;
-            }
-        }
-        // All the above srgb extensions support toggling srgb writes
-        if (fSRGBSupport) {
-            fSRGBWriteControl = true;
-        }
     } else if (GR_IS_GR_GL_ES(standard)) {
-        fSRGBSupport = version >= GR_GL_VER(3,0) || ctxInfo.hasExtension("GL_EXT_sRGB");
+        // ES through 3.2 requires EXT_srgb_write_control to support toggling
-        // ES through 3.1 requires EXT_srgb_write_control to support toggling
         // sRGB writing for destinations.
         fSRGBWriteControl = ctxInfo.hasExtension("GL_EXT_sRGB_write_control");
-    } else if (GR_IS_GR_WEBGL(standard)) {
+    }  // No WebGL support
-        // sRGB extension should be on most WebGL 1.0 contexts, although
-        // sometimes under 2 names.
-        fSRGBSupport = version >= GR_GL_VER(2,0) || ctxInfo.hasExtension("GL_EXT_sRGB") ||
-                                                    ctxInfo.hasExtension("EXT_sRGB");
-    }
-
-    // This is very conservative, if we're on a platform where N32 is BGRA, and using ES, disable
-    // all sRGB support. Too much code relies on creating surfaces with N32 + sRGB colorspace,
-    // and sBGRA is basically impossible to support on any version of ES (with our current code).
-    // In particular, ES2 doesn't support sBGRA at all, and even in ES3, there is no valid pair
-    // of formats that can be used for TexImage calls to upload BGRA data to sRGBA (which is what
-    // we *have* to use as the internal format, because sBGRA doesn't exist). This primarily
-    // affects Windows.
-    if (kSkia8888_GrPixelConfig == kBGRA_8888_GrPixelConfig &&
-        (GR_IS_GR_GL_ES(standard) || GR_IS_GR_WEBGL(standard))) {
-        fSRGBSupport = false;
-    }
 
     /**************************************************************************
     * GrShaderCaps fields
@@ -1185,6 +1158,12 @@ void GrGLCaps::onDumpJSON(SkJSONWriter* writer) const {
     writer->appendBool("BGRA to RGBA readback conversions are slow",
                        fRGBAToBGRAReadbackConversionsAreSlow);
     writer->appendBool("Use buffer data null hint", fUseBufferDataNullHint);
+    writer->appendBool("Clear texture support", fClearTextureSupport);
+    writer->appendBool("Program binary support", fProgramBinarySupport);
+    writer->appendBool("Program parameters support", fProgramParameterSupport);
+    writer->appendBool("Sampler object support", fSamplerObjectSupport);
+    writer->appendBool("FB fetch requires enable per sample", fFBFetchRequiresEnablePerSample);
+    writer->appendBool("sRGB Write Control", fSRGBWriteControl);
 
     writer->appendBool("Intermediate texture for partial updates of unorm textures ever bound to FBOs",
                        fDisallowTexSubImageForUnormConfigTexturesEverBoundToFBO);
@@ -2509,28 +2488,61 @@ void GrGLCaps::initFormatTable(const GrGLContextInfo& ctxInfo, const GrGLInterfa
         FormatInfo& info = this->getFormatInfo(GrGLFormat::kSRGB8_ALPHA8);
         info.fFormatType = FormatType::kNormalizedFixedPoint;
         info.fInternalFormatForRenderbuffer = GR_GL_SRGB8_ALPHA8;
-        bool srgbTexStorageSupported = texStorageSupported;
-        // See comment below about ES 2.0 + GL_EXT_sRGB.
-        if (GR_IS_GR_GL_ES(standard) && version < GR_GL_VER(3, 0)) {
-            // ES 2.0 requires that the external format matches the internal format.
-            info.fDefaultExternalFormat = GR_GL_SRGB_ALPHA;
-            // There is no defined interaction between GL_EXT_sRGB and GL_EXT_texture_storage.
-            srgbTexStorageSupported = false;
-        } else {
-            // On other GLs the expected external format is GL_RGBA, assuming this format
-            // is supported at all.
-            info.fDefaultExternalFormat = GR_GL_RGBA;
-        }
         info.fDefaultExternalType = GR_GL_UNSIGNED_BYTE;
         info.fTexSubImageZeroDataBpp = 4;
-        if (fSRGBSupport) {
-            uint32_t srgbRenderFlags =
-                    formatWorkarounds.fDisableSRGBRenderWithMSAAForMacAMD ? nonMSAARenderFlags
-                                                                          : msaaRenderFlags;
 
-            info.fFlags = FormatInfo::kTexturable_Flag | srgbRenderFlags;
+        // We may modify the default external format below.
+        info.fDefaultExternalFormat = GR_GL_RGBA;
+        bool srgb8Alpha8TexStorageSupported = texStorageSupported;
+        bool srgb8Alpha8TextureSupport = false;
+        bool srgb8Alpha8RenderTargetSupport = false;
+        if (GR_IS_GR_GL(standard)) {
+            if (version >= GR_GL_VER(3, 0)) {
+                srgb8Alpha8TextureSupport = true;
+                srgb8Alpha8RenderTargetSupport = true;
+            } else if (ctxInfo.hasExtension("GL_EXT_texture_sRGB")) {
+                srgb8Alpha8TextureSupport = true;
+                if (ctxInfo.hasExtension("GL_ARB_framebuffer_sRGB") ||
+                    ctxInfo.hasExtension("GL_EXT_framebuffer_sRGB")) {
+                    srgb8Alpha8RenderTargetSupport = true;
+                }
+            }
+        } else if (GR_IS_GR_GL_ES(standard)) {
+            if (version >= GR_GL_VER(3, 0) || ctxInfo.hasExtension("GL_EXT_sRGB")) {
+                srgb8Alpha8TextureSupport = true;
+                srgb8Alpha8RenderTargetSupport = true;
+            }
+            if (version < GR_GL_VER(3, 0)) {
+                // ES 2.0 requires that the external format matches the internal format.
+                info.fDefaultExternalFormat = GR_GL_SRGB_ALPHA;
+                // There is no defined interaction between GL_EXT_sRGB and GL_EXT_texture_storage.
+                srgb8Alpha8TexStorageSupported = false;
+            }
+        } else if (GR_IS_GR_WEBGL(standard)) {
+            // sRGB extension should be on most WebGL 1.0 contexts, although sometimes under 2
+            // names.
+            if (version >= GR_GL_VER(2, 0) || ctxInfo.hasExtension("GL_EXT_sRGB") ||
+                ctxInfo.hasExtension("EXT_sRGB")) {
+                srgb8Alpha8TextureSupport = true;
+                srgb8Alpha8RenderTargetSupport = true;
+            }
+            if (version < GR_GL_VER(2, 0)) {
+                // WebGL 1.0 requires that the external format matches the internal format.
+                info.fDefaultExternalFormat = GR_GL_SRGB_ALPHA;
+                // There is no extension to WebGL 1 that adds glTexStorage.
+                SkASSERT(!srgb8Alpha8TexStorageSupported);
+            }
         }
-        if (srgbTexStorageSupported) {
+
+        if (srgb8Alpha8TextureSupport) {
+            info.fFlags = FormatInfo::kTexturable_Flag;
+            if (srgb8Alpha8RenderTargetSupport) {
+                info.fFlags |= formatWorkarounds.fDisableSRGBRenderWithMSAAForMacAMD
+                                       ? nonMSAARenderFlags
+                                       : msaaRenderFlags;
+            }
+        }
+        if (srgb8Alpha8TexStorageSupported) {
             info.fFlags |= FormatInfo::kUseTexStorage_Flag;
             info.fInternalFormatForTexImageOrStorage = GR_GL_SRGB8_ALPHA8;
         } else {
@@ -2538,7 +2550,7 @@ void GrGLCaps::initFormatTable(const GrGLContextInfo& ctxInfo, const GrGLInterfa
                     texImageSupportsSizedInternalFormat ? GR_GL_SRGB8_ALPHA8 : GR_GL_SRGB_ALPHA;
         }
 
-        if (fSRGBSupport) {
+        if (srgb8Alpha8TextureSupport) {
             info.fColorTypeInfoCount = 1;
             info.fColorTypeInfos.reset(new ColorTypeInfo[info.fColorTypeInfoCount]());
             int ctIdx = 0;

src/gpu/gl/GrGLCaps.h

@@ -406,6 +406,9 @@ public:
 
     bool fbFetchRequiresEnablePerSample() const { return fFBFetchRequiresEnablePerSample; }
 
+    /* Is there support for enabling/disabling sRGB writes for sRGB-capable color buffers? */
+    bool srgbWriteControl() const { return fSRGBWriteControl; }
+
     GrColorType getYUVAColorTypeFromBackendFormat(const GrBackendFormat&,
                                                   bool isAlphaChannel) const override;
 
@@ -503,6 +506,7 @@ private:
     bool fProgramParameterSupport : 1;
     bool fSamplerObjectSupport : 1;
     bool fFBFetchRequiresEnablePerSample : 1;
+    bool fSRGBWriteControl : 1;
 
     // Driver workarounds
     bool fDoManualMipmapping : 1;

src/gpu/mtl/GrMtlCaps.mm

@@ -231,8 +231,6 @@ void GrMtlCaps::initGrCaps(const id<MTLDevice> device) {
 
     fOversizedStencilSupport = true;
 
-    fSRGBSupport = true;   // always available in Metal
-    fSRGBWriteControl = false;
     fMipMapSupport = true;   // always available in Metal
     fNPOTTextureTileSupport = true;  // always available in Metal
 

src/gpu/vk/GrVkCaps.cpp

@@ -32,7 +32,6 @@ GrVkCaps::GrVkCaps(const GrContextOptions& contextOptions, const GrVkInterface*
      * GrCaps fields
      **************************************************************************/
     fMipMapSupport = true;   // always available in Vulkan
-    fSRGBSupport = true;   // always available in Vulkan
     fNPOTTextureTileSupport = true;  // always available in Vulkan
     fReuseScratchTextures = true; //TODO: figure this out
     fGpuTracingSupport = false; //TODO: figure this out
@@ -358,15 +357,6 @@ void GrVkCaps::init(const GrContextOptions& contextOptions, const GrVkInterface*
     this->initGrCaps(vkInterface, physDev, properties, memoryProperties, features, extensions);
     this->initShaderCaps(properties, features);
 
-    if (!contextOptions.fDisableDriverCorrectnessWorkarounds) {
-#if defined(SK_CPU_X86)
-        // We need to do this before initing the config table since it uses fSRGBSupport
-        if (kImagination_VkVendor == properties.vendorID) {
-            fSRGBSupport = false;
-        }
-#endif
-    }
-
     if (kQualcomm_VkVendor == properties.vendorID) {
         // A "clear" load for the CCPR atlas runs faster on QC than a "discard" load followed by a
         // scissored clear.
@@ -959,9 +949,7 @@ void GrVkCaps::initFormatTable(const GrVkInterface* interface, VkPhysicalDevice
     {
         constexpr VkFormat format = VK_FORMAT_R8G8B8A8_SRGB;
         auto& info = this->getFormatInfo(format);
-        if (fSRGBSupport) {
+        info.init(interface, physDev, properties, format);
-            info.init(interface, physDev, properties, format);
-        }
         if (SkToBool(info.fOptimalFlags & FormatInfo::kTexturable_Flag)) {
             info.fColorTypeInfoCount = 1;
             info.fColorTypeInfos.reset(new ColorTypeInfo[info.fColorTypeInfoCount]());

src/image/SkSurface_Gpu.cpp

@@ -328,14 +328,6 @@ bool SkSurface_Gpu::onDraw(const SkDeferredDisplayList* ddl) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-bool SkSurface_Gpu::Valid(const GrCaps* caps, const GrBackendFormat& format) {
-    if (caps->isFormatSRGB(format)) {
-        return caps->srgbSupport();
-    }
-
-    return true;
-}
-
 sk_sp<SkSurface> SkSurface::MakeRenderTarget(GrRecordingContext* context,
                                              const SkSurfaceCharacterization& c,
                                              SkBudgeted budgeted) {
@@ -343,8 +335,6 @@ sk_sp<SkSurface> SkSurface::MakeRenderTarget(GrRecordingContext* context,
         return nullptr;
     }
 
-    const GrCaps* caps = context->priv().caps();
-
     if (c.usesGLFBO0()) {
         // If we are making the surface we will never use FBO0.
         return nullptr;
@@ -354,10 +344,6 @@ sk_sp<SkSurface> SkSurface::MakeRenderTarget(GrRecordingContext* context,
         return nullptr;
     }
 
-    if (!SkSurface_Gpu::Valid(caps, c.backendFormat())) {
-        return nullptr;
-    }
-
     GrColorType grColorType = SkColorTypeToGrColorType(c.colorType());
 
     auto rtc = context->priv().makeDeferredRenderTargetContext(SkBackingFit::kExact,
@@ -416,10 +402,6 @@ static bool validate_backend_texture(const GrCaps* caps, const GrBackendTexture&
         return false;
     }
 
-    if (!SkSurface_Gpu::Valid(caps, backendFormat)) {
-        return false;
-    }
-
     return true;
 }
 
@@ -611,10 +593,6 @@ bool validate_backend_render_target(const GrCaps* caps, const GrBackendRenderTar
     if (!caps->isFormatAsColorTypeRenderable(grCT, rt.getBackendFormat(), rt.sampleCnt())) {
         return false;
     }
-    if (!SkSurface_Gpu::Valid(caps, rt.getBackendFormat())) {
-        return false;
-    }
-
     return true;
 }
 

src/image/SkSurface_Gpu.h

@@ -57,8 +57,6 @@ public:
 
     SkGpuDevice* getDevice() { return fDevice.get(); }
 
-    static bool Valid(const GrCaps*, const GrBackendFormat&);
-
 private:
     sk_sp<SkGpuDevice> fDevice;