diff --git a/doc/changelog.dox b/doc/changelog.dox index b7f865a69..041d0a40b 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -254,6 +254,10 @@ See also: - A new @cpp "nv-framebuffer-invalidation-wants-draw-binding" @ce workaround for @ref GL::Framebuffer::invalidate() affecting unrelated framebuffers on NVidia. See @ref opengl-workarounds for more information. +- A new @cpp "nv-cubemap-broken-dsa-compressed-subimage-upload" @ce + workaround for yet another broken case with compressed formats used in + @ref GL::CubeMapTexture on NVidia. See @ref opengl-workarounds for more + information. @subsubsection changelog-latest-new-math Math library diff --git a/src/Magnum/GL/Implementation/TextureState.cpp b/src/Magnum/GL/Implementation/TextureState.cpp index 4b1d442e9..80155f7d1 100644 --- a/src/Magnum/GL/Implementation/TextureState.cpp +++ b/src/Magnum/GL/Implementation/TextureState.cpp @@ -196,11 +196,10 @@ TextureState::TextureState(Context& context, #endif } - /* DSA/non-DSA implementation for cubemaps, because Intel (and AMD) Windows - drivers have to be broken in a special way */ + /* DSA/non-DSA implementation for cubemaps, because ... well, basically all + non-Mesa drivers have to be broken in a special way */ #ifndef MAGNUM_TARGET_GLES if(context.isExtensionSupported()) { - #ifdef CORRADE_TARGET_WINDOWS if((context.detectedDriver() & Context::DetectedDriver::IntelWindows) && !context.isDriverWorkaroundDisabled("intel-windows-broken-dsa-for-cubemaps"_s)) { getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDefault; @@ -213,7 +212,12 @@ TextureState::TextureState(Context& context, cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDefault; } else #endif - { + if((context.detectedDriver() & Context::DetectedDriver::NVidia) && !context.isDriverWorkaroundDisabled("nv-cubemap-broken-dsa-compressed-subimage-upload"_s)) { + getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDSA; + cubeSubImageImplementation = &CubeMapTexture::subImageImplementationDSA; + /* This one is broken, the others are not */ + cubeCompressedSubImageImplementation = &CubeMapTexture::compressedSubImageImplementationDefault; + } else { /* Extension name added above */ getCubeLevelParameterivImplementation = &CubeMapTexture::getLevelParameterImplementationDSA; diff --git a/src/Magnum/GL/Implementation/driverSpecific.cpp b/src/Magnum/GL/Implementation/driverSpecific.cpp index a693d8326..d26b346a8 100644 --- a/src/Magnum/GL/Implementation/driverSpecific.cpp +++ b/src/Magnum/GL/Implementation/driverSpecific.cpp @@ -196,6 +196,18 @@ constexpr Containers::StringView KnownWorkarounds[]{ image when querying all six slices using the ARB_DSA API */ "nv-cubemap-broken-full-compressed-image-query"_s, +/* NVidia drivers (575.64, but likely also any before) behave wrong when + uploading compressed 2D cubemap subimages when + - a DSA API is used, + - the cubemap doesn't have immutable storage, + - the query is done to client memory and not a buffer, + - *and* non-zero compressed block size is set in pixel storage + With just any of the four missing, it works well. As it's too restrictive to + require users to either always use setStorage() for cubemaps or to always + upload full slices, the workaround is simply to use the classic non-DSA + codepath. See CubeMapGLTest::compressedSubImage() for a repro case. */ +"nv-cubemap-broken-dsa-compressed-subimage-upload"_s, + /* NVidia drivers return 0 when asked for GL_CONTEXT_PROFILE_MASK, so it needs to be worked around by asking for GL_ARB_compatibility */ "nv-zero-context-profile-mask"_s, diff --git a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp index 1b9357c8d..4e3d7421c 100644 --- a/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp +++ b/src/Magnum/GL/Test/CubeMapTextureGLTest.cpp @@ -228,6 +228,40 @@ const struct { #endif }; +const struct { + const char* name; + Containers::ArrayView data; + CompressedPixelStorage storage; + Containers::ArrayView dataSparse; + std::size_t offset; + bool immutable; +} CompressedSubImageData[]{ + {"default pixel storage", + Containers::arrayView(CompressedData).exceptPrefix(16), + {}, + Containers::arrayView(CompressedData).exceptPrefix(16), 0, false}, + #ifndef MAGNUM_TARGET_GLES + {"skip Y", + Containers::arrayView(CompressedData).exceptPrefix(16), + CompressedPixelStorage{} + .setSkip({0, 4, 0}), + Containers::arrayView(CompressedData), 16, false}, + #endif + #ifndef MAGNUM_TARGET_GLES2 + {"immutable storage, default pixel storage", + Containers::arrayView(CompressedData).exceptPrefix(16), + {}, + Containers::arrayView(CompressedData).exceptPrefix(16), 0, true}, + #ifndef MAGNUM_TARGET_GLES + {"immutable storage, skip Y", + Containers::arrayView(CompressedData).exceptPrefix(16), + CompressedPixelStorage{} + .setSkip({0, 4, 0}), + Containers::arrayView(CompressedData), 16, true}, + #endif + #endif +}; + constexpr UnsignedByte FullData[]{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, @@ -403,7 +437,7 @@ CubeMapTextureGLTest::CubeMapTextureGLTest() { #ifndef MAGNUM_TARGET_GLES2 &CubeMapTextureGLTest::compressedSubImageBuffer, #endif - }, Containers::arraySize(CompressedPixelStorageData)); + }, Containers::arraySize(CompressedSubImageData)); addInstancedTests({ &CubeMapTextureGLTest::image3D, @@ -1411,7 +1445,7 @@ constexpr UnsignedByte CompressedSubDataComplete[]{ #endif void CubeMapTextureGLTest::compressedSubImage() { - auto&& data = CompressedPixelStorageData[testCaseInstanceId()]; + auto&& data = CompressedSubImageData[testCaseInstanceId()]; setTestCaseDescription(data.name); #ifndef MAGNUM_TARGET_GLES @@ -1431,18 +1465,37 @@ void CubeMapTextureGLTest::compressedSubImage() { #endif CubeMapTexture texture; - texture.setCompressedImage(CubeMapCoordinate::PositiveX, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeX, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::PositiveY, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeY, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::PositiveZ, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeZ, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + #ifndef MAGNUM_TARGET_GLES2 + if(data.immutable) { + texture.setStorage(1, TextureFormat::CompressedRGBAS3tcDxt3, Vector2i{12}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveX, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeX, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveY, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeY, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveZ, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeZ, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + } else + #endif + { + texture.setCompressedImage(CubeMapCoordinate::PositiveX, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeX, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::PositiveY, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeY, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::PositiveZ, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeZ, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + } texture.setCompressedSubImage(CubeMapCoordinate::PositiveX, 0, Vector2i{4}, CompressedImageView2D{ data.storage, CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{4}, @@ -1456,21 +1509,21 @@ void CubeMapTextureGLTest::compressedSubImage() { MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_COMPARE(image.size(), Vector2i{12}); - - { - CORRADE_EXPECT_FAIL_IF(data.storage != CompressedPixelStorage{} && Context::current().isExtensionSupported() && (Context::current().detectedDriver() & Context::DetectedDriver::NVidia), - "Non-default compressed pixel storage for cube map textures behaves weirdly on NVidia for client-memory images when using ARB_direct_state_access"); - - CORRADE_COMPARE_AS(Containers::arrayCast(image.data()), - Containers::arrayView(CompressedSubDataComplete), - TestSuite::Compare::Container); - } + /* This fails if the "nv-cubemap-broken-dsa-compressed-subimage-upload" is + disabled, but only if pixel storage is non-default and setStorage() + isn't used. Thus, the "skip Y" case will fail, and "default pixel + storage" case will fail if run after any other test that sets pixel + storage compressed block properties. Running it as a first test + works. */ + CORRADE_COMPARE_AS(Containers::arrayCast(image.data()), + Containers::arrayView(CompressedSubDataComplete), + TestSuite::Compare::Container); #endif } #ifndef MAGNUM_TARGET_GLES2 void CubeMapTextureGLTest::compressedSubImageBuffer() { - auto&& data = CompressedPixelStorageData[testCaseInstanceId()]; + auto&& data = CompressedSubImageData[testCaseInstanceId()]; setTestCaseDescription(data.name); #ifndef MAGNUM_TARGET_GLES @@ -1490,18 +1543,34 @@ void CubeMapTextureGLTest::compressedSubImageBuffer() { #endif CubeMapTexture texture; - texture.setCompressedImage(CubeMapCoordinate::PositiveX, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeX, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::PositiveY, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeY, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::PositiveZ, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); - texture.setCompressedImage(CubeMapCoordinate::NegativeZ, 0, - CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + if(data.immutable) { + texture.setStorage(1, TextureFormat::CompressedRGBAS3tcDxt3, Vector2i{12}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveX, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeX, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveY, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeY, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::PositiveZ, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedSubImage(CubeMapCoordinate::NegativeZ, 0, {}, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + } else { + texture.setCompressedImage(CubeMapCoordinate::PositiveX, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeX, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::PositiveY, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeY, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::PositiveZ, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + texture.setCompressedImage(CubeMapCoordinate::NegativeZ, 0, + CompressedImageView2D{CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{12}, CompressedZero}); + } texture.setCompressedSubImage(CubeMapCoordinate::PositiveX, 0, Vector2i{4}, CompressedBufferImage2D{ data.storage, CompressedPixelFormat::RGBAS3tcDxt3, Vector2i{4},