From 79c217ba3660695ed6267069675d6cf7ba95d1de Mon Sep 17 00:00:00 2001 From: Stephen White Date: Wed, 27 Apr 2022 19:45:33 +0000 Subject: [PATCH] OpenGL ES: implement support for BGRA textures and reads. ES requires GL_EXT_texture_format_BGRA8888 or GL_APPLE_texture_format_BGRA8888 to create BGRA8 internalFormat textures, and GL_EXT_read_format_bgra to read from them. Desktop GL can swizzle back and forth from RGBA8, so keep using RGBA8 if the extension is unavailable. Intel's implementation of GL_EXT_texture_format_BGRA8888 on ES is broken, and won't create GL_BGRA8_EXT or GL_BGRA internalFormat textures, so disable the test there and modify another test to not use BGRA textures. Change-Id: Ia81d9ff20e2849b00379f8e01fb5d2ecfa34bd53 Bug: dawn:596, dawn:1393 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/86744 Commit-Queue: Stephen White Reviewed-by: Austin Eng Kokoro: Kokoro --- src/dawn/native/Toggles.cpp | 4 ++++ src/dawn/native/Toggles.h | 1 + src/dawn/native/opengl/DeviceGL.cpp | 17 ++++++++++++++++- src/dawn/native/opengl/DeviceGL.h | 1 + src/dawn/native/opengl/GLFormat.cpp | 5 ++--- src/dawn/native/opengl/GLFormat.h | 2 +- .../native/opengl/supported_extensions.json | 4 +++- src/dawn/tests/end2end/BindGroupTests.cpp | 2 +- src/dawn/tests/end2end/TextureFormatTests.cpp | 8 ++++++-- 9 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/dawn/native/Toggles.cpp b/src/dawn/native/Toggles.cpp index eb473c2300..b1932141aa 100644 --- a/src/dawn/native/Toggles.cpp +++ b/src/dawn/native/Toggles.cpp @@ -147,6 +147,10 @@ namespace dawn::native { "Disables reading from depth/stencil textures which is unsupported on some " "platforms.", "https://crbug.com/dawn/667"}}, + {Toggle::DisableBGRARead, + {"disable_bgra_read", + "Disables reading from BGRA textures which is unsupported on some platforms.", + "https://crbug.com/dawn/1393"}}, {Toggle::DisableSampleVariables, {"disable_sample_variables", "Disables gl_SampleMask and related functionality which is unsupported on some " diff --git a/src/dawn/native/Toggles.h b/src/dawn/native/Toggles.h index 6e6c90f10a..988a991569 100644 --- a/src/dawn/native/Toggles.h +++ b/src/dawn/native/Toggles.h @@ -45,6 +45,7 @@ namespace dawn::native { DisableIndexedDrawBuffers, DisableSnormRead, DisableDepthStencilRead, + DisableBGRARead, DisableSampleVariables, UseD3D12SmallShaderVisibleHeapForTesting, UseDXC, diff --git a/src/dawn/native/opengl/DeviceGL.cpp b/src/dawn/native/opengl/DeviceGL.cpp index b3a8442e01..3d95b1b663 100644 --- a/src/dawn/native/opengl/DeviceGL.cpp +++ b/src/dawn/native/opengl/DeviceGL.cpp @@ -55,7 +55,7 @@ namespace dawn::native::opengl { MaybeError Device::Initialize(const DeviceDescriptor* descriptor) { InitTogglesFromDriver(); - mFormatTable = BuildGLFormatTable(); + mFormatTable = BuildGLFormatTable(GetBGRAInternalFormat()); return DeviceBase::Initialize(AcquireRef(new Queue(this, &descriptor->defaultQueue))); } @@ -74,6 +74,10 @@ namespace dawn::native::opengl { bool supportsDepthStencilRead = gl.IsAtLeastGL(3, 0) || gl.IsGLExtensionSupported("GL_NV_read_depth_stencil"); + // Desktop GL supports BGRA textures via swizzling in the driver; ES requires an extension. + bool supportsBGRARead = + gl.GetVersion().IsDesktop() || gl.IsGLExtensionSupported("GL_EXT_read_format_bgra"); + bool supportsSampleVariables = gl.IsAtLeastGL(4, 0) || gl.IsAtLeastGLES(3, 2) || gl.IsGLExtensionSupported("GL_OES_sample_variables"); @@ -97,6 +101,7 @@ namespace dawn::native::opengl { SetToggle(Toggle::DisableIndexedDrawBuffers, !supportsIndexedDrawBuffers); SetToggle(Toggle::DisableSnormRead, !supportsSnormRead); SetToggle(Toggle::DisableDepthStencilRead, !supportsDepthStencilRead); + SetToggle(Toggle::DisableBGRARead, !supportsBGRARead); SetToggle(Toggle::DisableSampleVariables, !supportsSampleVariables); SetToggle(Toggle::FlushBeforeClientWaitSync, gl.GetVersion().IsES()); // For OpenGL ES, we must use a placeholder fragment shader for vertex-only render pipeline. @@ -112,6 +117,16 @@ namespace dawn::native::opengl { return result; } + GLenum Device::GetBGRAInternalFormat() const { + if (gl.IsGLExtensionSupported("GL_EXT_texture_format_BGRA8888") || + gl.IsGLExtensionSupported("GL_APPLE_texture_format_BGRA8888")) { + return GL_BGRA8_EXT; + } else { + // Desktop GL will swizzle to/from RGBA8 for BGRA formats. + return GL_RGBA8; + } + } + ResultOrError> Device::CreateBindGroupImpl( const BindGroupDescriptor* descriptor) { DAWN_TRY(ValidateGLBindGroupDescriptor(descriptor)); diff --git a/src/dawn/native/opengl/DeviceGL.h b/src/dawn/native/opengl/DeviceGL.h index cb7c8cbc63..52ea056fe1 100644 --- a/src/dawn/native/opengl/DeviceGL.h +++ b/src/dawn/native/opengl/DeviceGL.h @@ -119,6 +119,7 @@ namespace dawn::native::opengl { const RenderPipelineDescriptor* descriptor) override; void InitTogglesFromDriver(); + GLenum GetBGRAInternalFormat() const; ResultOrError CheckAndUpdateCompletedSerials() override; void DestroyImpl() override; MaybeError WaitForIdleForDestruction() override; diff --git a/src/dawn/native/opengl/GLFormat.cpp b/src/dawn/native/opengl/GLFormat.cpp index dac02a6ad0..2d54426e14 100644 --- a/src/dawn/native/opengl/GLFormat.cpp +++ b/src/dawn/native/opengl/GLFormat.cpp @@ -16,7 +16,7 @@ namespace dawn::native::opengl { - GLFormatTable BuildGLFormatTable() { + GLFormatTable BuildGLFormatTable(GLenum internalFormatForBGRA) { GLFormatTable table; using Type = GLFormat::ComponentType; @@ -71,8 +71,7 @@ namespace dawn::native::opengl { AddFormat(wgpu::TextureFormat::RGBA8Uint, GL_RGBA8UI, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE, Type::Uint); AddFormat(wgpu::TextureFormat::RGBA8Sint, GL_RGBA8I, GL_RGBA_INTEGER, GL_BYTE, Type::Int); - // This doesn't have an enum for the internal format in OpenGL, so use RGBA8. - AddFormat(wgpu::TextureFormat::BGRA8Unorm, GL_RGBA8, GL_BGRA, GL_UNSIGNED_BYTE, Type::Float); + AddFormat(wgpu::TextureFormat::BGRA8Unorm, internalFormatForBGRA, GL_BGRA, GL_UNSIGNED_BYTE, Type::Float); AddFormat(wgpu::TextureFormat::RGB10A2Unorm, GL_RGB10_A2, GL_RGBA, GL_UNSIGNED_INT_2_10_10_10_REV, Type::Float); AddFormat(wgpu::TextureFormat::RG11B10Ufloat, GL_R11F_G11F_B10F, GL_RGB, GL_UNSIGNED_INT_10F_11F_11F_REV, Type::Float); AddFormat(wgpu::TextureFormat::RGB9E5Ufloat, GL_RGB9_E5, GL_RGB, GL_UNSIGNED_INT_5_9_9_9_REV, Type::Float); diff --git a/src/dawn/native/opengl/GLFormat.h b/src/dawn/native/opengl/GLFormat.h index 292fb4a6d3..a76f98946d 100644 --- a/src/dawn/native/opengl/GLFormat.h +++ b/src/dawn/native/opengl/GLFormat.h @@ -35,7 +35,7 @@ namespace dawn::native::opengl { }; using GLFormatTable = ityp::array; - GLFormatTable BuildGLFormatTable(); + GLFormatTable BuildGLFormatTable(GLenum internalFormatForBGRA); } // namespace dawn::native::opengl diff --git a/src/dawn/native/opengl/supported_extensions.json b/src/dawn/native/opengl/supported_extensions.json index 8e006337b9..8c110d012b 100644 --- a/src/dawn/native/opengl/supported_extensions.json +++ b/src/dawn/native/opengl/supported_extensions.json @@ -18,6 +18,8 @@ "supported_extensions": [ "GL_EXT_texture_compression_s3tc", "GL_EXT_texture_compression_s3tc_srgb", - "GL_OES_EGL_image" + "GL_OES_EGL_image", + "GL_EXT_texture_format_BGRA8888", + "GL_APPLE_texture_format_BGRA8888" ] } diff --git a/src/dawn/tests/end2end/BindGroupTests.cpp b/src/dawn/tests/end2end/BindGroupTests.cpp index fa5e01176f..837e071ff4 100644 --- a/src/dawn/tests/end2end/BindGroupTests.cpp +++ b/src/dawn/tests/end2end/BindGroupTests.cpp @@ -1601,7 +1601,7 @@ TEST_P(BindGroupTests, CreateWithDestroyedResource) { wgpu::TextureDescriptor textureDesc; textureDesc.usage = wgpu::TextureUsage::TextureBinding; textureDesc.size = {1, 1, 1}; - textureDesc.format = wgpu::TextureFormat::BGRA8Unorm; + textureDesc.format = wgpu::TextureFormat::RGBA8Unorm; // Create view, then destroy. { diff --git a/src/dawn/tests/end2end/TextureFormatTests.cpp b/src/dawn/tests/end2end/TextureFormatTests.cpp index 7260ac65d4..ca875bc2c8 100644 --- a/src/dawn/tests/end2end/TextureFormatTests.cpp +++ b/src/dawn/tests/end2end/TextureFormatTests.cpp @@ -458,8 +458,12 @@ TEST_P(TextureFormatTest, RGBA8Unorm) { // Test the BGRA8Unorm format TEST_P(TextureFormatTest, BGRA8Unorm) { - // TODO(crbug.com/dawn/596): BGRA is unsupported on OpenGL ES; add workaround or validation - DAWN_SUPPRESS_TEST_IF(IsOpenGLES()); + DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("disable_bgra_read")); + + // Intel's implementation of BGRA on ES is broken: it claims to support + // GL_EXT_texture_format_BGRA8888, but won't accept GL_BGRA or GL_BGRA8_EXT as internalFormat. + DAWN_SUPPRESS_TEST_IF(IsIntel() && IsOpenGLES() && IsLinux()); + uint8_t maxValue = std::numeric_limits::max(); std::vector textureData = {maxValue, 1, 0, maxValue}; std::vector uncompressedData = {0.0f, 1.0f / maxValue, 1.0f, 1.0f};