From 746134721a2646891acffdf3febe110a12f4bc1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 25 Jan 2025 15:34:05 +0100 Subject: [PATCH] GL: don't accidentally skip CubeMapTexture creation on no-assert builds. In all other cases this is called outside of #ifndef CORRADE_NO_ASSERT, so this is likely some yet-undiscovered copypaste error. OTOH it's such a corner case of a corner case that I don't think anyone ever hits that because it'd fail on just anything else anyway: GL::CubeMapTexture texture; // nothing done here, not even a format / size specified texture.compressedSubImage(...); This extra call originates in ae70a62644d0f05a46a7cc74d4f9d63cd24b396c (2015, again) and was used in all places where DSA functions were called directly without feature-aware dispatch. Since then, some of the cases were turned into feature-aware dispatches, which means the call isn't needed in those anymore, so I removed it. And to clarify why is it needed in the remaining cases I added extra comments. --- src/Magnum/GL/AbstractTexture.cpp | 10 ++++++++++ src/Magnum/GL/CubeMapTexture.cpp | 24 +++++++++++------------- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/Magnum/GL/AbstractTexture.cpp b/src/Magnum/GL/AbstractTexture.cpp index 2c810a3a4..f0d305dee 100644 --- a/src/Magnum/GL/AbstractTexture.cpp +++ b/src/Magnum/GL/AbstractTexture.cpp @@ -1912,6 +1912,8 @@ template void AbstractTexture::subImage(const GLint leve CORRADE_ASSERT(image.size() == range.size(), "GL::AbstractTexture::subImage(): expected image view size" << range.size() << "but got" << image.size(), ); + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); const Math::Vector size = range.size(); @@ -1928,6 +1930,8 @@ template void MAGNUM_GL_EXPORT AbstractTexture::subImage<2>(GLint, const Range2D template void MAGNUM_GL_EXPORT AbstractTexture::subImage<3>(GLint, const Range3Di&, const BasicMutableImageView<3>&); template void AbstractTexture::subImage(const GLint level, const RangeTypeFor& range, BufferImage& image, const BufferUsage usage) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); const Math::Vector size = range.size(); @@ -1963,6 +1967,8 @@ template std::size_t MAGNUM_GL_EXPORT AbstractTexture::compressedSubImageSize<2> template std::size_t MAGNUM_GL_EXPORT AbstractTexture::compressedSubImageSize<3>(TextureFormat format, const Math::Vector<3, Int>& size); template void AbstractTexture::compressedSubImage(const GLint level, const RangeTypeFor& range, CompressedImage& image, const ImageFlags flags) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); const Math::Vector size = range.size(); @@ -2002,6 +2008,8 @@ template void AbstractTexture::compressedSubImage(const CORRADE_ASSERT(image.size() == range.size(), "GL::AbstractTexture::compressedSubImage(): expected image view size" << range.size() << "but got" << image.size(), ); + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); const Math::Vector size = range.size(); @@ -2039,6 +2047,8 @@ template void MAGNUM_GL_EXPORT AbstractTexture::compressedSubImage<2>(GLint, con template void MAGNUM_GL_EXPORT AbstractTexture::compressedSubImage<3>(GLint, const Range3Di&, const BasicMutableCompressedImageView<3>&); template void AbstractTexture::compressedSubImage(const GLint level, const RangeTypeFor& range, CompressedBufferImage& image, const BufferUsage usage) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); const Math::Vector size = range.size(); diff --git a/src/Magnum/GL/CubeMapTexture.cpp b/src/Magnum/GL/CubeMapTexture.cpp index e67968d78..58af137c4 100644 --- a/src/Magnum/GL/CubeMapTexture.cpp +++ b/src/Magnum/GL/CubeMapTexture.cpp @@ -98,8 +98,6 @@ Vector2i CubeMapTexture::imageSize(const Int level) { #ifndef MAGNUM_TARGET_GLES void CubeMapTexture::image(const Int level, Image3D& image) { - createIfNotAlready(); - const Vector3i size{imageSize(level), 6}; const std::size_t dataSize = Magnum::Implementation::imageDataSizeFor(image, size); @@ -132,8 +130,6 @@ void CubeMapTexture::image(const Int level, const MutableImageView3D& image) { } void CubeMapTexture::image(const Int level, BufferImage3D& image, const BufferUsage usage) { - createIfNotAlready(); - const Vector3i size{imageSize(level), 6}; const std::size_t dataSize = Magnum::Implementation::imageDataSizeFor(image, size); @@ -154,8 +150,6 @@ BufferImage3D CubeMapTexture::image(const Int level, BufferImage3D&& image, cons } void CubeMapTexture::compressedImage(const Int level, CompressedImage3D& image) { - createIfNotAlready(); - const Vector3i size{imageSize(level), 6}; /* If the user-provided pixel storage doesn't tell us all properties about @@ -228,8 +222,6 @@ void CubeMapTexture::compressedImage(const Int level, const MutableCompressedIma } void CubeMapTexture::compressedImage(const Int level, CompressedBufferImage3D& image, const BufferUsage usage) { - createIfNotAlready(); - const Vector3i size{imageSize(level), 6}; /* If the user-provided pixel storage doesn't tell us all properties about @@ -428,6 +420,8 @@ BufferImage3D CubeMapTexture::subImage(const Int level, const Range3Di& range, B } void CubeMapTexture::compressedSubImage(const Int level, const Range3Di& range, CompressedImage3D& image) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); /* Internal texture format. Zero-init to avoid an assert about value @@ -464,14 +458,16 @@ CompressedImage3D CubeMapTexture::compressedSubImage(const Int level, const Rang } void CubeMapTexture::compressedSubImage(const Int level, const Range3Di& range, const MutableCompressedImageView3D& image) { - #ifndef CORRADE_NO_ASSERT CORRADE_ASSERT(image.data().data() != nullptr || !range.size().product(), "GL::CubeMapTexture::compressedSubImage(): image view is nullptr", ); CORRADE_ASSERT(image.size() == range.size(), "GL::CubeMapTexture::compressedSubImage(): expected image view size" << range.size() << "but got" << image.size(), ); + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); + #ifndef CORRADE_NO_ASSERT /* Internal texture format. Zero-init to avoid an assert about value already wrapped in compressedPixelFormatWrap() later if the drivers are extra shitty (Intel Windows drivers, I'm talking about you). */ @@ -499,6 +495,8 @@ void CubeMapTexture::compressedSubImage(const Int level, const Range3Di& range, } void CubeMapTexture::compressedSubImage(const Int level, const Range3Di& range, CompressedBufferImage3D& image, const BufferUsage usage) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); /* Internal texture format. Zero-init to avoid an assert about value @@ -533,8 +531,6 @@ CompressedBufferImage3D CubeMapTexture::compressedSubImage(const Int level, cons #endif CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& offset, const ImageView3D& image) { - createIfNotAlready(); - #ifndef MAGNUM_TARGET_GLES2 Buffer::unbindInternal(Buffer::TargetHint::PixelUnpack); #endif @@ -549,8 +545,6 @@ CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& off #ifndef MAGNUM_TARGET_GLES2 CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& offset, BufferImage3D& image) { - createIfNotAlready(); - image.buffer().bindInternal(Buffer::TargetHint::PixelUnpack); Context::current().state().renderer.applyPixelStorageUnpack(image.storage()); Context::current().state().texture.cubeSubImage3DImplementation(*this, level, offset, image.size(), image.format(), image.type(), nullptr, image.storage()); @@ -560,6 +554,8 @@ CubeMapTexture& CubeMapTexture::setSubImage(const Int level, const Vector3i& off #ifndef MAGNUM_TARGET_GLES CubeMapTexture& CubeMapTexture::setCompressedSubImage(const Int level, const Vector3i& offset, const CompressedImageView3D& image) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); Buffer::unbindInternal(Buffer::TargetHint::PixelUnpack); @@ -569,6 +565,8 @@ CubeMapTexture& CubeMapTexture::setCompressedSubImage(const Int level, const Vec } CubeMapTexture& CubeMapTexture::setCompressedSubImage(const Int level, const Vector3i& offset, CompressedBufferImage3D& image) { + /* Explicitly create if not already because the texture might have been + created w/ the DSA extension disabled but below a DSA API is used */ createIfNotAlready(); image.buffer().bindInternal(Buffer::TargetHint::PixelUnpack);