From 929329b1f8363ac5e6f8688423c6c6854014865e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 21 Aug 2017 19:23:50 +0200 Subject: [PATCH] Fix the ability to create sRGB textures on ES2/WebGL1. Since 98a676ef653fc956b375712bc7765e622a6dd2e9, on ES2 and WebGL1 the Texture::setStorage() emulation passes the pixel format to both and of glTexImage() APIs. On desktop the go-to way to create a sRGB texture is by passing GL_SRGB to and GL_RGB to , but here GL_RGB was passed to both and thus the information about sRGB was lost. With the new PixelFormat::SRGB and PixelFormat::SRGBAlpha enums that are present only on ES2/WebGL1 this case is fixed -- sRGB texture format will get translated to sRGB pixel format and that used for both and . Another case is when EXT_texture_storage is available -- passing unsized GL_SRGB_EXT or GL_SRGB_ALPHA_EXT to glTexStorageEXT() is an error and there's apparently no mention of this in any extension, making it impossible to create sRGB textures using EXT_texture_storage. I bit the bullet and tried passing the (numerical value of) GL_SRGB8 and GL_SRGB8_ALPHA8 to it. At least on my NV it worked, so I enabled these two in TextureFormat for ES2. No EXT_texture_storage is on WebGL1, so they are only on ES2. These two sized TextureFormat::SRGB8 and TextureFormat::SRGB8Alpha8 formats are translated to GL_SRGB and GL_SRGB_ALPHA and so using them unconditionally for all platforms (except WebGL1) "just works". --- src/Magnum/AbstractTexture.cpp | 38 ++++++++++++------- src/Magnum/PixelFormat.cpp | 4 ++ src/Magnum/PixelFormat.h | 22 +++++++++++ src/Magnum/PixelStorage.cpp | 6 +++ src/Magnum/Test/TextureGLTest.cpp | 63 ++++++++++++++++++++++++++++++- src/Magnum/TextureFormat.h | 34 ++++++++++++----- 6 files changed, 143 insertions(+), 24 deletions(-) diff --git a/src/Magnum/AbstractTexture.cpp b/src/Magnum/AbstractTexture.cpp index 198baf574..e4ae0d276 100644 --- a/src/Magnum/AbstractTexture.cpp +++ b/src/Magnum/AbstractTexture.cpp @@ -662,12 +662,6 @@ PixelFormat pixelFormatForInternalFormat(const TextureFormat internalFormat) { case TextureFormat::R11FG11FB10F: case TextureFormat::RGB9E5: #endif - #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) - case TextureFormat::SRGB: - #endif - #ifndef MAGNUM_TARGET_GLES2 - case TextureFormat::SRGB8: - #endif #ifndef MAGNUM_TARGET_GLES case TextureFormat::CompressedRGB: case TextureFormat::CompressedRGBBptcUnsignedFloat: @@ -680,6 +674,18 @@ PixelFormat pixelFormatForInternalFormat(const TextureFormat internalFormat) { case TextureFormat::CompressedRGBS3tcDxt1: return PixelFormat::RGB; + #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) + case TextureFormat::SRGB: + #endif + #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) + case TextureFormat::SRGB8: + #endif + #ifndef MAGNUM_TARGET_GLES2 + return PixelFormat::RGB; + #else + return PixelFormat::SRGB; + #endif + #ifndef MAGNUM_TARGET_GLES2 case TextureFormat::RGB8UI: case TextureFormat::RGB8I: @@ -716,12 +722,6 @@ PixelFormat pixelFormatForInternalFormat(const TextureFormat internalFormat) { #ifndef MAGNUM_TARGET_GLES case TextureFormat::RGBA12: #endif - #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) - case TextureFormat::SRGBAlpha: - #endif - #ifndef MAGNUM_TARGET_GLES2 - case TextureFormat::SRGB8Alpha8: - #endif #ifndef MAGNUM_TARGET_GLES case TextureFormat::CompressedRGBA: case TextureFormat::CompressedRGBABptcUnorm: @@ -768,6 +768,18 @@ PixelFormat pixelFormatForInternalFormat(const TextureFormat internalFormat) { #endif return PixelFormat::RGBA; + #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) + case TextureFormat::SRGBAlpha: + #endif + #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) + case TextureFormat::SRGB8Alpha8: + #endif + #ifndef MAGNUM_TARGET_GLES2 + return PixelFormat::RGBA; + #else + return PixelFormat::SRGBAlpha; + #endif + #ifndef MAGNUM_TARGET_GLES2 case TextureFormat::RGBA8UI: case TextureFormat::RGBA8I: @@ -845,7 +857,7 @@ PixelType pixelTypeForInternalFormat(const TextureFormat internalFormat) { case TextureFormat::SRGB: case TextureFormat::SRGBAlpha: #endif - #ifndef MAGNUM_TARGET_GLES2 + #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) case TextureFormat::SRGB8: case TextureFormat::SRGB8Alpha8: #endif diff --git a/src/Magnum/PixelFormat.cpp b/src/Magnum/PixelFormat.cpp index 072fdf8a0..457a1b309 100644 --- a/src/Magnum/PixelFormat.cpp +++ b/src/Magnum/PixelFormat.cpp @@ -58,6 +58,10 @@ Debug& operator<<(Debug& debug, const PixelFormat value) { #ifndef MAGNUM_TARGET_WEBGL _c(BGRA) #endif + #ifdef MAGNUM_TARGET_GLES2 + _c(SRGB) + _c(SRGBAlpha) + #endif #ifndef MAGNUM_TARGET_GLES2 _c(RedInteger) #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/PixelFormat.h b/src/Magnum/PixelFormat.h index fe9cdedac..90f85f93f 100644 --- a/src/Magnum/PixelFormat.h +++ b/src/Magnum/PixelFormat.h @@ -161,6 +161,28 @@ enum class PixelFormat: GLenum { #endif #endif + #if defined(MAGNUM_TARGET_GLES2) || defined(DOXYGEN_GENERATING_OUTPUT) + /** + * Floating-point sRGB. + * @requires_gles20 Not available in ES 3.0, WebGL 2.0 or desktop OpenGL. + * Use @ref PixelFormat::RGB instead. + * @deprecated_gl Included only in order to make it possible to upload + * sRGB image data with the @extension{EXT,sRGB} ES2 extension, use + * @ref PixelFormat::RGB elsewhere instead. + */ + SRGB = GL_SRGB_EXT, + + /** + * Floating-point sRGB + alpha. + * @requires_gles20 Not available in ES 3.0, WebGL 2.0 or desktop OpenGL. + * Use @ref PixelFormat::RGBA instead. + * @deprecated_gl Included only in order to make it possible to upload + * sRGB image data with the @extension{EXT,sRGB} ES2 extension, use + * @ref PixelFormat::RGBA elsewhere instead. + */ + SRGBAlpha = GL_SRGB_ALPHA_EXT, + #endif + #ifndef MAGNUM_TARGET_GLES2 /** * Integer red channel. diff --git a/src/Magnum/PixelStorage.cpp b/src/Magnum/PixelStorage.cpp index 29f2dd597..ed8cbe354 100644 --- a/src/Magnum/PixelStorage.cpp +++ b/src/Magnum/PixelStorage.cpp @@ -134,6 +134,9 @@ std::size_t PixelStorage::pixelSize(PixelFormat format, PixelType type) { #ifndef MAGNUM_TARGET_GLES case PixelFormat::BGR: case PixelFormat::BGRInteger: + #endif + #ifdef MAGNUM_TARGET_GLES2 + case PixelFormat::SRGB: #endif return 3*size; case PixelFormat::RGBA: @@ -143,6 +146,9 @@ std::size_t PixelStorage::pixelSize(PixelFormat format, PixelType type) { #ifndef MAGNUM_TARGET_WEBGL case PixelFormat::BGRA: #endif + #ifdef MAGNUM_TARGET_GLES2 + case PixelFormat::SRGBAlpha: + #endif #ifndef MAGNUM_TARGET_GLES case PixelFormat::BGRAInteger: #endif diff --git a/src/Magnum/Test/TextureGLTest.cpp b/src/Magnum/Test/TextureGLTest.cpp index ad9eef264..4bfde1c0f 100644 --- a/src/Magnum/Test/TextureGLTest.cpp +++ b/src/Magnum/Test/TextureGLTest.cpp @@ -206,6 +206,9 @@ struct TextureGLTest: OpenGLTester { #endif void invalidateSubImage2D(); void invalidateSubImage3D(); + + void srgbStorage(); + void srgbAlphaStorage(); }; namespace { @@ -567,7 +570,9 @@ TextureGLTest::TextureGLTest() { #endif &TextureGLTest::invalidateSubImage2D, &TextureGLTest::invalidateSubImage3D, - }); + + &TextureGLTest::srgbStorage, + &TextureGLTest::srgbAlphaStorage}); } #ifndef MAGNUM_TARGET_GLES @@ -2425,6 +2430,62 @@ void TextureGLTest::invalidateSubImage3D() { MAGNUM_VERIFY_NO_ERROR(); } +void TextureGLTest::srgbStorage() { + #ifdef MAGNUM_TARGET_GLES2 + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::GL::EXT::sRGB::string() + std::string(" is not supported.")); + #endif + + Texture2D texture; + texture.setImage(0, + #ifndef MAGNUM_TARGET_GLES2 + TextureFormat::SRGB8 + #else + TextureFormat::SRGB + #endif + , ImageView2D{ + #ifndef MAGNUM_TARGET_GLES2 + PixelFormat::RGB + #else + PixelFormat::SRGB + #endif + , PixelType::UnsignedByte, Vector2i{32}, nullptr}); + + MAGNUM_VERIFY_NO_ERROR(); + + texture.setStorage(1, TextureFormat::SRGB8, Vector2i{32}); + + MAGNUM_VERIFY_NO_ERROR(); +} + +void TextureGLTest::srgbAlphaStorage() { + #ifdef MAGNUM_TARGET_GLES2 + if(!Context::current().isExtensionSupported()) + CORRADE_SKIP(Extensions::GL::EXT::sRGB::string() + std::string(" is not supported.")); + #endif + + Texture2D texture; + texture.setImage(0, + #ifndef MAGNUM_TARGET_GLES2 + TextureFormat::SRGB8Alpha8 + #else + TextureFormat::SRGBAlpha + #endif + , ImageView2D{ + #ifndef MAGNUM_TARGET_GLES2 + PixelFormat::RGBA + #else + PixelFormat::SRGBAlpha + #endif + , PixelType::UnsignedByte, Vector2i{32}, nullptr}); + + MAGNUM_VERIFY_NO_ERROR(); + + texture.setStorage(1, TextureFormat::SRGB8Alpha8, Vector2i{32}); + + MAGNUM_VERIFY_NO_ERROR(); +} + }} CORRADE_TEST_MAIN(Magnum::Test::TextureGLTest) diff --git a/src/Magnum/TextureFormat.h b/src/Magnum/TextureFormat.h index 70a912f11..7d64eb149 100644 --- a/src/Magnum/TextureFormat.h +++ b/src/Magnum/TextureFormat.h @@ -717,13 +717,20 @@ enum class TextureFormat: GLenum { #endif #endif - #ifndef MAGNUM_TARGET_GLES2 + #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) /** * sRGB, each component normalized unsigned byte. - * @requires_gles30 Use @ref TextureFormat::SRGB in OpenGL ES 2.0 instead. + * @requires_gles30 Extension @extension{EXT,sRGB} and + * @extension{EXT,texture_storage}, only for + * @ref Texture::setStorage() "*Texture::setStorage()" calls, + * otherwise use @ref TextureFormat::SRGB in OpenGL ES 2.0 instead. * @requires_webgl20 Use @ref TextureFormat::SRGB in WebGL 1.0 instead. */ + #ifndef MAGNUM_TARGET_GLES2 SRGB8 = GL_SRGB8, + #else + SRGB8 = 0x8C41, /* Not in any spec, but seems to work at least on NV */ + #endif #endif #ifndef MAGNUM_TARGET_GLES @@ -794,9 +801,10 @@ enum class TextureFormat: GLenum { #if !defined(MAGNUM_TARGET_GLES) || defined(MAGNUM_TARGET_GLES2) /** - * sRGBA, normalized unsigned, size implementation-dependent. Not allowed in - * unemulated @ref Texture::setStorage() "*Texture::setStorage()" calls, in - * that case use @ref TextureFormat::SRGB8Alpha8 "TextureFormat::SRGB8Alpha8" instead. + * sRGB + linear alpha, normalized unsigned, size implementation-dependent. + * Not allowed in unemulated @ref Texture::setStorage() "*Texture::setStorage()" + * calls, in that case use @ref TextureFormat::SRGB8Alpha8 "TextureFormat::SRGB8Alpha8" + * instead. * @requires_es_extension Extension @extension{EXT,sRGB} in OpenGL ES * 2.0. Use @ref TextureFormat::SRGB8Alpha8 in OpenGL ES 3.0 instead. * @requires_webgl_extension Extension @webgl_extension{EXT,sRGB} in WebGL @@ -811,15 +819,21 @@ enum class TextureFormat: GLenum { #endif #endif - #ifndef MAGNUM_TARGET_GLES2 + #if !(defined(MAGNUM_TARGET_WEBGL) && defined(MAGNUM_TARGET_GLES2)) /** - * sRGBA, each component normalized unsigned byte. - * @requires_gles30 Use @ref TextureFormat::SRGBAlpha in OpenGL ES 2.0 - * instead. - * @requires_webgl20 Use @ref TextureFormat::SRGBAlpha in WebGL 1.0 + * sRGB + linear alpha, each component normalized unsigned byte. + * @requires_gles30 Extension @extension{EXT,sRGB} and + * @extension{EXT,texture_storage}, only for + * @ref Texture::setStorage() "*Texture::setStorage()" calls, + * otherwise use @ref TextureFormat::SRGBAlpha in OpenGL ES 2.0 * instead. + * @requires_webgl20 Use @ref TextureFormat::SRGBAlpha in WebGL 1.0 instead. */ + #ifndef MAGNUM_TARGET_GLES2 SRGB8Alpha8 = GL_SRGB8_ALPHA8, + #else + SRGB8Alpha8 = GL_SRGB8_ALPHA8_EXT, /* Not in spec, works at least on NV */ + #endif #endif #ifndef MAGNUM_TARGET_GLES