From 14c514d9f85015473aed5848f920079816836187 Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Thu, 26 Sep 2019 12:30:15 -0400 Subject: [PATCH] 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 Commit-Queue: Brian Salomon --- src/gpu/GrCaps.cpp | 4 - src/gpu/GrCaps.h | 11 --- src/gpu/GrContext.cpp | 10 --- src/gpu/GrContextThreadSafeProxy.cpp | 4 - src/gpu/GrDrawingManager.cpp | 14 ---- src/gpu/gl/GrGLCaps.cpp | 116 +++++++++++++++------------ src/gpu/gl/GrGLCaps.h | 4 + src/gpu/mtl/GrMtlCaps.mm | 2 - src/gpu/vk/GrVkCaps.cpp | 14 +--- src/image/SkSurface_Gpu.cpp | 22 ----- src/image/SkSurface_Gpu.h | 2 - 11 files changed, 69 insertions(+), 134 deletions(-) diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp index 7425157c9e..4b14a917fd 100644 --- a/src/gpu/GrCaps.cpp +++ b/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); diff --git a/src/gpu/GrCaps.h b/src/gpu/GrCaps.h index 112ba437f0..6785a0b84f 100644 --- a/src/gpu/GrCaps.h +++ b/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; diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp index 43b19017af..cd9db04147 100644 --- a/src/gpu/GrContext.cpp +++ b/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, diff --git a/src/gpu/GrContextThreadSafeProxy.cpp b/src/gpu/GrContextThreadSafeProxy.cpp index 7de2170599..64c7bf6a4c 100644 --- a/src/gpu/GrContextThreadSafeProxy.cpp +++ b/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)) { diff --git a/src/gpu/GrDrawingManager.cpp b/src/gpu/GrDrawingManager.cpp index 07d20666b9..3507f1b096 100644 --- a/src/gpu/GrDrawingManager.cpp +++ b/src/gpu/GrDrawingManager.cpp @@ -903,13 +903,6 @@ std::unique_ptr 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 renderTargetProxy(sk_ref_sp(sProxy->asRenderTargetProxy())); return std::unique_ptr( @@ -930,13 +923,6 @@ std::unique_ptr 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()); diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index 4524fe42a8..e2ad7b1b5e 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/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)) { - fSRGBSupport = true; - } else if (ctxInfo.hasExtension("GL_EXT_texture_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; - } + fSRGBWriteControl = version >= GR_GL_VER(3, 0) || + ctxInfo.hasExtension("GL_ARB_framebuffer_sRGB") || + ctxInfo.hasExtension("GL_EXT_framebuffer_sRGB"); } else if (GR_IS_GR_GL_ES(standard)) { - fSRGBSupport = version >= GR_GL_VER(3,0) || ctxInfo.hasExtension("GL_EXT_sRGB"); - // ES through 3.1 requires EXT_srgb_write_control to support toggling + // ES through 3.2 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)) { - // 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; - } + } // No WebGL support /************************************************************************** * 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; diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h index edf68a6baa..69a60ca7a6 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/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; diff --git a/src/gpu/mtl/GrMtlCaps.mm b/src/gpu/mtl/GrMtlCaps.mm index 6c211626b0..eff22a13f8 100644 --- a/src/gpu/mtl/GrMtlCaps.mm +++ b/src/gpu/mtl/GrMtlCaps.mm @@ -231,8 +231,6 @@ void GrMtlCaps::initGrCaps(const id device) { fOversizedStencilSupport = true; - fSRGBSupport = true; // always available in Metal - fSRGBWriteControl = false; fMipMapSupport = true; // always available in Metal fNPOTTextureTileSupport = true; // always available in Metal diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp index 7af5312509..003523631f 100644 --- a/src/gpu/vk/GrVkCaps.cpp +++ b/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]()); diff --git a/src/image/SkSurface_Gpu.cpp b/src/image/SkSurface_Gpu.cpp index 693be1e1c2..8968f0ab2f 100644 --- a/src/image/SkSurface_Gpu.cpp +++ b/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::MakeRenderTarget(GrRecordingContext* context, const SkSurfaceCharacterization& c, SkBudgeted budgeted) { @@ -343,8 +335,6 @@ sk_sp 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::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; } diff --git a/src/image/SkSurface_Gpu.h b/src/image/SkSurface_Gpu.h index fd3fb6d794..c1768111bb 100644 --- a/src/image/SkSurface_Gpu.h +++ b/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 fDevice;