From 61f8b5da57ee5632f3d2e5abd2bba182c4fd615c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 12 Apr 2021 12:17:16 +0200 Subject: [PATCH] GL: enable cube map 3D image up/download on all platforms. There were slice-by-slice workarounds for certain drivers already, so why not reuse them to make this functionality available everywhere. --- doc/changelog.dox | 7 +++ src/Magnum/GL/CubeMapTexture.cpp | 14 ++++- src/Magnum/GL/CubeMapTexture.h | 52 +++++++++++++------ src/Magnum/GL/Implementation/TextureState.cpp | 19 ++++--- src/Magnum/GL/Implementation/TextureState.h | 2 +- src/Magnum/GL/Test/CubeMapTextureGLTest.cpp | 45 ++++++++++------ 6 files changed, 97 insertions(+), 42 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 80f29afce..637121113 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -220,6 +220,13 @@ See also: - The @ref GL::Context class got significantly optimized in terms of compile time, header size and runtime as well, significantly reducing the amount of allocations done at startup. +- To make working with cube map images easier, + @ref GL::CubeMapTexture::setSubImage() and + @ref GL::CubeMapTexture::subImage() taking 3D images are now exposed on all + platforms, not just desktop OpenGL 4.5+, with a fall back to slice-by-slice + upload/download where needed. The compressed variant is still only GL 4.5+, + as that needs the @ref CompressedImageView APIs to be aware of compression + format properties, which isn't implemented yet. - Added @ref GL::Framebuffer::Status::IncompleteDimensions for ES2. This enum isn't available on ES3 or desktop GL, but NVidia drivers are known to emit it, which is why it got added. diff --git a/src/Magnum/GL/CubeMapTexture.cpp b/src/Magnum/GL/CubeMapTexture.cpp index 9651f4ba0..c763dd346 100644 --- a/src/Magnum/GL/CubeMapTexture.cpp +++ b/src/Magnum/GL/CubeMapTexture.cpp @@ -493,16 +493,24 @@ CompressedBufferImage3D CubeMapTexture::compressedSubImage(const Int level, cons compressedSubImage(level, range, image, usage); return std::move(image); } +#endif CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& offset, const ImageView3D& image) { createIfNotAlready(); + #ifndef MAGNUM_TARGET_GLES2 Buffer::unbindInternal(Buffer::TargetHint::PixelUnpack); + #endif Context::current().state().renderer.applyPixelStorageUnpack(image.storage()); - (this->*Context::current().state().texture.cubeSubImage3DImplementation)(level, offset, image.size(), pixelFormat(image.format()), pixelType(image.format(), image.formatExtra()), image.data(), image.storage()); + (this->*Context::current().state().texture.cubeSubImage3DImplementation)(level, offset, image.size(), pixelFormat(image.format()), pixelType(image.format(), image.formatExtra()), image.data() + #ifdef MAGNUM_TARGET_GLES2 + + Magnum::Implementation::pixelStorageSkipOffset(image) + #endif + , image.storage()); return *this; } +#ifndef MAGNUM_TARGET_GLES2 CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& offset, BufferImage3D& image) { createIfNotAlready(); @@ -511,7 +519,9 @@ CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& off (this->*Context::current().state().texture.cubeSubImage3DImplementation)(level, offset, image.size(), image.format(), image.type(), nullptr, image.storage()); return *this; } +#endif +#ifndef MAGNUM_TARGET_GLES CubeMapTexture& CubeMapTexture::setCompressedSubImage(const Int level, const Vector3i& offset, const CompressedImageView3D& image) { createIfNotAlready(); @@ -689,13 +699,13 @@ void CubeMapTexture::subImageImplementationDSASliceBySlice(const GLint level, co for(Int i = 0; i != size.z(); ++i) subImageImplementationDSA(level, {offset.xy(), offset.z() + i}, {size.xy(), 1}, format, type, static_cast(data) + stride*i, storage); } +#endif void CubeMapTexture::subImageImplementationSliceBySlice(const GLint level, const Vector3i& offset, const Vector3i& size, const PixelFormat format, const PixelType type, const GLvoid* const data, const PixelStorage& storage) { const std::size_t stride = std::get<1>(storage.dataProperties(pixelSize(format, type), size)).xy().product(); for(Int i = 0; i != size.z(); ++i) subImageImplementationDefault(CubeMapCoordinate(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i), level, offset.xy(), size.xy(), format, type, static_cast(data) + stride*i); } -#endif void CubeMapTexture::subImageImplementationDefault(const CubeMapCoordinate coordinate, const GLint level, const Vector2i& offset, const Vector2i& size, const PixelFormat format, const PixelType type, const GLvoid* const data) { bindInternal(); diff --git a/src/Magnum/GL/CubeMapTexture.h b/src/Magnum/GL/CubeMapTexture.h index 38aec069b..9e0f6ff3b 100644 --- a/src/Magnum/GL/CubeMapTexture.h +++ b/src/Magnum/GL/CubeMapTexture.h @@ -559,10 +559,19 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture { * @ref imageSize(). The storage is not reallocated if it is large * enough to contain the new data. * - * The operation is protected from buffer overflow. - * @see @fn_gl2{GetTextureLevelParameter,GetTexLevelParameter} with - * @def_gl{TEXTURE_WIDTH}, @def_gl{TEXTURE_HEIGHT}, then - * @fn_gl{PixelStore}, then @fn_gl2_keyword{GetTextureImage,GetTexImage} + * If @gl_extension{ARB,direct_state_access} (part of OpenGL 4.5) is + * not available, the texture is bound before the operation (if not + * already) and the image is downloaded slice by slice. If either + * @gl_extension{ARB,get_texture_sub_image} or + * @gl_extension{ARB,robustness} is available, the operation is + * protected from buffer overflow. + * @see @fn_gl2{GetTextureLevelParameter,GetTexLevelParameter}, + * eventually @fn_gl{ActiveTexture}, @fn_gl{BindTexture} and + * @fn_gl{GetTexLevelParameter} with @def_gl{TEXTURE_WIDTH}, + * @def_gl{TEXTURE_HEIGHT}, then @fn_gl{PixelStore}, then + * @fn_gl2_keyword{GetTextureImage,GetTexImage}, + * @fn_gl_extension_keyword{GetnTexImage,ARB,robustness}, + * eventually @fn_gl_keyword{GetTexImage} * @requires_gl45 Extension @gl_extension{ARB,direct_state_access} * @requires_gl Texture image queries are not available in OpenGL ES or * WebGL. See @ref Framebuffer::read() or @ref DebugTools::textureSubImage() @@ -1024,7 +1033,6 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture { } #endif - #ifndef MAGNUM_TARGET_GLES /** * @brief Set image subdata * @param level Mip level @@ -1033,29 +1041,41 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture { * @ref Trade::ImageData3D * @return Reference to self (for method chaining) * - * @see @ref setStorage(), @fn_gl2{TextureSubImage3D,TexSubImage3D} - * @requires_gl45 Extension @gl_extension{ARB,direct_state_access} - * @requires_gl In OpenGL ES and WebGL you need to set image for each - * face separately. + * If @gl_extension{ARB,direct_state_access} (part of OpenGL 4.5) is + * not available, the texture is bound before the operation (if not + * already) and the image is uploaded slice by slice. + * @see @ref setStorage(), @fn_gl2{TextureSubImage3D,TexSubImage3D}, + * eventually @fn_gl{ActiveTexture}, @fn_gl{BindTexture} and + * @fn_gl_keyword{TexSubImage2D} + * @requires_gles30 Extension @gl_extension{EXT,unpack_subimage}/ + * @gl_extension{NV,pack_subimage} in OpenGL ES 2.0 if + * @ref PixelStorage::rowLength() is set to a non-zero value. + * @requires_webgl20 Non-zero @ref PixelStorage::rowLength() is not + * supported in WebGL 1.0. */ CubeMapTexture& setSubImage(Int level, const Vector3i& offset, const ImageView3D& image); + #ifndef MAGNUM_TARGET_GLES2 /** @overload - * @requires_gl45 Extension @gl_extension{ARB,direct_state_access} - * @requires_gl In OpenGL ES and WebGL you need to set image for each - * face separately. + * @requires_gles30 Pixel buffer objects are not available in OpenGL ES + * 2.0. + * @requires_webgl20 Pixel buffer objects are not available in WebGL + * 1.0. */ CubeMapTexture& setSubImage(Int level, const Vector3i& offset, BufferImage3D& image); /** @overload - * @requires_gl45 Extension @gl_extension{ARB,direct_state_access} - * @requires_gl In OpenGL ES and WebGL you need to set image for each - * face separately. + * @requires_gles30 Pixel buffer objects are not available in OpenGL ES + * 2.0. + * @requires_webgl20 Pixel buffer objects are not available in WebGL + * 1.0. */ CubeMapTexture& setSubImage(Int level, const Vector3i& offset, BufferImage3D&& image) { return setSubImage(level, offset, image); } + #endif + #ifndef MAGNUM_TARGET_GLES /** * @brief Set compressed image subdata * @param level Mip level @@ -1248,8 +1268,8 @@ class MAGNUM_GL_EXPORT CubeMapTexture: public AbstractTexture { #ifndef MAGNUM_TARGET_GLES void MAGNUM_GL_LOCAL subImageImplementationDSA(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&); void MAGNUM_GL_LOCAL subImageImplementationDSASliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&); - void MAGNUM_GL_LOCAL subImageImplementationSliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&); #endif + void MAGNUM_GL_LOCAL subImageImplementationSliceBySlice(GLint level, const Vector3i& offset, const Vector3i& size, PixelFormat format, PixelType type, const GLvoid* data, const PixelStorage&); void MAGNUM_GL_LOCAL subImageImplementationDefault(CubeMapCoordinate coordinate, GLint level, const Vector2i& offset, const Vector2i& size, PixelFormat format, PixelType type, const GLvoid* data); #ifndef MAGNUM_TARGET_GLES diff --git a/src/Magnum/GL/Implementation/TextureState.cpp b/src/Magnum/GL/Implementation/TextureState.cpp index 374ba2ff7..6f477557b 100644 --- a/src/Magnum/GL/Implementation/TextureState.cpp +++ b/src/Magnum/GL/Implementation/TextureState.cpp @@ -348,6 +348,9 @@ TextureState::TextureState(Context& context, getCubeImage3DImplementation = &CubeMapTexture::getImageImplementationSliceBySlice; else #endif + if(context.isExtensionSupported()) { + getCubeImage3DImplementation = &CubeMapTexture::getImageImplementationDSA; + } else { getCubeImage3DImplementation = &CubeMapTexture::getImageImplementationSliceBySlice; } #endif @@ -500,14 +503,14 @@ TextureState::TextureState(Context& context, /* SVGA3D and Intel workaround for cube map texture upload. Overrides the DSA / non-DSA function pointers set above. */ if((context.detectedDriver() & Context::DetectedDriver::Svga3D) && - !context.isDriverWorkaroundDisabled("svga3d-texture-upload-slice-by-slice"_s)) { - if(context.isExtensionSupported()) { - cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDSASliceBySlice; - } else { - cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice; - } + context.isExtensionSupported() && + !context.isDriverWorkaroundDisabled("svga3d-texture-upload-slice-by-slice"_s) + ) { + cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDSASliceBySlice; } else if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) && - !context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps"_s)) { + context.isExtensionSupported() && + !context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps"_s) + ) { cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice; } #ifdef CORRADE_TARGET_WINDOWS @@ -523,10 +526,10 @@ TextureState::TextureState(Context& context, else if(context.isExtensionSupported()) { cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationDSA; } else + #endif { cubeSubImage3DImplementation = &CubeMapTexture::subImageImplementationSliceBySlice; } - #endif #if defined(CORRADE_TARGET_APPLE) && !defined(MAGNUM_TARGET_GLES) if(!context.isDriverWorkaroundDisabled("apple-buffer-texture-unbind-on-buffer-modify"_s)) { diff --git a/src/Magnum/GL/Implementation/TextureState.h b/src/Magnum/GL/Implementation/TextureState.h index a0477a93f..686fd4d18 100644 --- a/src/Magnum/GL/Implementation/TextureState.h +++ b/src/Magnum/GL/Implementation/TextureState.h @@ -125,8 +125,8 @@ struct TextureState { void(CubeMapTexture::*getCubeImage3DImplementation)(GLint, const Vector3i&, PixelFormat, PixelType, std::size_t, GLvoid*, const PixelStorage&); void(CubeMapTexture::*getCompressedCubeImage3DImplementation)(GLint, const Vector2i&, std::size_t, std::size_t, GLvoid*); void(CubeMapTexture::*getCompressedCubeImageImplementation)(CubeMapCoordinate, GLint, const Vector2i&, std::size_t, GLvoid*); - void(CubeMapTexture::*cubeSubImage3DImplementation)(GLint, const Vector3i&, const Vector3i&, PixelFormat, PixelType, const GLvoid*, const PixelStorage&); #endif + void(CubeMapTexture::*cubeSubImage3DImplementation)(GLint, const Vector3i&, const Vector3i&, PixelFormat, PixelType, const GLvoid*, const PixelStorage&); void(CubeMapTexture::*cubeSubImageImplementation)(CubeMapCoordinate, GLint, const Vector2i&, const Vector2i&, PixelFormat, PixelType, const GLvoid*); void(CubeMapTexture::*cubeCompressedSubImageImplementation)(CubeMapCoordinate, GLint, const Vector2i&, const Vector2i&, CompressedPixelFormat, const GLvoid*, GLsizei); diff --git a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp index 41f78f076..d21cdb009 100644 --- a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp +++ b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp @@ -128,15 +128,21 @@ struct CubeMapTextureGLTest: OpenGLTester { void compressedSubImageQueryBuffer(); #endif - #ifndef MAGNUM_TARGET_GLES void image3D(); + #ifndef MAGNUM_TARGET_GLES2 void image3DBuffer(); + #endif + #ifndef MAGNUM_TARGET_GLES void image3DQueryView(); void image3DQueryViewNullptr(); void image3DQueryViewBadSize(); + #endif + #ifndef MAGNUM_TARGET_GLES void compressedImage3D(); + #ifndef MAGNUM_TARGET_GLES2 void compressedImage3DBuffer(); + #endif void compressedImage3DQueryView(); void compressedImage3DQueryViewNullptr(); void compressedImage3DQueryViewBadSize(); @@ -216,7 +222,6 @@ const struct { #endif }; -#ifndef MAGNUM_TARGET_GLES constexpr UnsignedByte FullData[]{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -253,6 +258,7 @@ const struct { #endif }; +#ifndef MAGNUM_TARGET_GLES /* Just 4x4 0x00 - 0x3f compressed using RGBA DXT3 by the driver, repeated six times */ constexpr UnsignedByte CompressedFullData[]{ @@ -348,13 +354,17 @@ CubeMapTextureGLTest::CubeMapTextureGLTest() { &CubeMapTextureGLTest::imageQueryViewBadSize}); #endif - #ifndef MAGNUM_TARGET_GLES addInstancedTests({ &CubeMapTextureGLTest::image3D, + #ifndef MAGNUM_TARGET_GLES2 &CubeMapTextureGLTest::image3DBuffer, - &CubeMapTextureGLTest::image3DQueryView}, - Containers::arraySize(FullPixelStorageData)); + #endif + #ifndef MAGNUM_TARGET_GLES + &CubeMapTextureGLTest::image3DQueryView + #endif + }, Containers::arraySize(FullPixelStorageData)); + #ifndef MAGNUM_TARGET_GLES addTests({&CubeMapTextureGLTest::image3DQueryViewNullptr, &CubeMapTextureGLTest::image3DQueryViewBadSize}); #endif @@ -1735,21 +1745,24 @@ void CubeMapTextureGLTest::compressedSubImageQueryBuffer() { } #endif -#ifndef MAGNUM_TARGET_GLES void CubeMapTextureGLTest::image3D() { setTestCaseDescription(FullPixelStorageData[testCaseInstanceId()].name); - if(!Context::current().isExtensionSupported()) - CORRADE_SKIP(Extensions::ARB::direct_state_access::string() << "is not supported."); + #if !(defined(MAGNUM_TARGET_GLES2) && defined(MAGNUM_TARGET_WEBGL)) + constexpr TextureFormat format = TextureFormat::RGBA8; + #else + constexpr TextureFormat format = TextureFormat::RGBA; + #endif CubeMapTexture texture; - texture.setStorage(1, TextureFormat::RGBA8, Vector2i{2, 2}) + texture.setStorage(1, format, Vector2i{2, 2}) .setSubImage(0, {}, ImageView3D{ PixelFormat::RGBA, PixelType::UnsignedByte, {2, 2, 6}, FullPixelStorageData[testCaseInstanceId()].data}); MAGNUM_VERIFY_NO_GL_ERROR(); + #ifndef MAGNUM_TARGET_GLES Image3D image = texture.image(0, {FullPixelStorageData[testCaseInstanceId()].storage, PixelFormat::RGBA, PixelType::UnsignedByte}); @@ -1763,14 +1776,13 @@ void CubeMapTextureGLTest::image3D() { FullPixelStorageData[testCaseInstanceId()].data, TestSuite::Compare::Container); } + #endif } +#ifndef MAGNUM_TARGET_GLES2 void CubeMapTextureGLTest::image3DBuffer() { setTestCaseDescription(FullPixelStorageData[testCaseInstanceId()].name); - if(!Context::current().isExtensionSupported()) - CORRADE_SKIP(Extensions::ARB::direct_state_access::string() << "is not supported."); - CubeMapTexture texture; texture.setStorage(1, TextureFormat::RGBA8, Vector2i{2}) .setSubImage(0, {}, BufferImage3D{ @@ -1780,6 +1792,7 @@ void CubeMapTextureGLTest::image3DBuffer() { MAGNUM_VERIFY_NO_GL_ERROR(); + #ifndef MAGNUM_TARGET_GLES BufferImage3D image = texture.image(0, {FullPixelStorageData[testCaseInstanceId()].storage, PixelFormat::RGBA, PixelType::UnsignedByte}, BufferUsage::StaticRead); @@ -1794,14 +1807,14 @@ void CubeMapTextureGLTest::image3DBuffer() { CORRADE_COMPARE_AS(Containers::arrayCast(imageData).suffix(FullPixelStorageData[testCaseInstanceId()].offset), FullPixelStorageData[testCaseInstanceId()].data, TestSuite::Compare::Container); } + #endif } +#endif +#ifndef MAGNUM_TARGET_GLES void CubeMapTextureGLTest::image3DQueryView() { setTestCaseDescription(FullPixelStorageData[testCaseInstanceId()].name); - if(!Context::current().isExtensionSupported()) - CORRADE_SKIP(Extensions::ARB::direct_state_access::string() << "is not supported."); - CubeMapTexture texture; texture.setStorage(1, TextureFormat::RGBA8, Vector2i{2, 2}) .setSubImage(0, {}, ImageView3D{ @@ -1864,7 +1877,9 @@ void CubeMapTextureGLTest::image3DQueryViewBadSize() { texture.image(0, image); CORRADE_COMPARE(out.str(), "GL::CubeMapTexture::image(): expected image view size Vector(2, 2, 6) but got Vector(2, 1, 6)\n"); } +#endif +#ifndef MAGNUM_TARGET_GLES void CubeMapTextureGLTest::compressedImage3D() { setTestCaseDescription(CompressedFullPixelStorageData[testCaseInstanceId()].name);