From a115b943b38404773567e3c189d008905e20c194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 10 Mar 2025 13:22:08 +0100 Subject: [PATCH] DebugTools: deprecate buffer overloads for textureSubImage(). I just don't see a point in those. PBOs are for when a roundtrip through a CPU memory would be wasteful, but these utils are mainly for use in tests. Definitely not for being called several times per frame, because the temporary framebuffer creation just doesn't make sense. Not sure what was I thinking in 2016 when I added those, apart from "feature parity for no practical reason". --- doc/snippets/DebugTools-gl.cpp | 8 +++- .../DebugTools/Test/TextureImageGLTest.cpp | 20 +++++++--- src/Magnum/DebugTools/TextureImage.cpp | 10 +++-- src/Magnum/DebugTools/TextureImage.h | 38 ++++++++++++++++--- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/doc/snippets/DebugTools-gl.cpp b/doc/snippets/DebugTools-gl.cpp index 785cd04a6..25a555742 100644 --- a/doc/snippets/DebugTools-gl.cpp +++ b/doc/snippets/DebugTools-gl.cpp @@ -171,14 +171,16 @@ Image2D image = DebugTools::textureSubImage(texture, 0, rect, /* [textureSubImage-2D-rvalue] */ } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) { GL::Texture2D texture; Range2Di rect; +CORRADE_IGNORE_DEPRECATED_PUSH /* [textureSubImage-2D-rvalue-buffer] */ GL::BufferImage2D image = DebugTools::textureSubImage(texture, 0, rect, {PixelFormat::RGBA8Unorm}, GL::BufferUsage::StaticRead); /* [textureSubImage-2D-rvalue-buffer] */ +CORRADE_IGNORE_DEPRECATED_POP } #endif @@ -191,15 +193,17 @@ Image2D image = DebugTools::textureSubImage(texture, /* [textureSubImage-cubemap-rvalue] */ } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) { GL::CubeMapTexture texture; Range2Di rect; +CORRADE_IGNORE_DEPRECATED_PUSH /* [textureSubImage-cubemap-rvalue-buffer] */ GL::BufferImage2D image = DebugTools::textureSubImage(texture, GL::CubeMapCoordinate::PositiveX, 0, rect, {PixelFormat::RGBA8Unorm}, GL::BufferUsage::StaticRead); /* [textureSubImage-cubemap-rvalue-buffer] */ +CORRADE_IGNORE_DEPRECATED_POP } #endif } diff --git a/src/Magnum/DebugTools/Test/TextureImageGLTest.cpp b/src/Magnum/DebugTools/Test/TextureImageGLTest.cpp index ff64955f2..ac87bfc69 100644 --- a/src/Magnum/DebugTools/Test/TextureImageGLTest.cpp +++ b/src/Magnum/DebugTools/Test/TextureImageGLTest.cpp @@ -56,7 +56,7 @@ struct TextureImageGLTest: GL::OpenGLTester { void subImage2D(); void subImage2DNotReadable(); - #ifndef MAGNUM_TARGET_GLES2 + #if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void subImage2DBuffer(); void subImage2DBufferNotReadable(); #endif @@ -64,7 +64,7 @@ struct TextureImageGLTest: GL::OpenGLTester { void subImageCube(); void subImageCubeNotReadable(); - #ifndef MAGNUM_TARGET_GLES2 + #if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void subImageCubeBuffer(); void subImageCubeBufferNotReadable(); #endif @@ -79,7 +79,7 @@ struct TextureImageGLTest: GL::OpenGLTester { TextureImageGLTest::TextureImageGLTest() { addTests({&TextureImageGLTest::subImage2D, &TextureImageGLTest::subImage2DNotReadable, - #ifndef MAGNUM_TARGET_GLES2 + #if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) &TextureImageGLTest::subImage2DBuffer, &TextureImageGLTest::subImage2DBufferNotReadable, #endif @@ -87,7 +87,7 @@ TextureImageGLTest::TextureImageGLTest() { &TextureImageGLTest::subImageCube, &TextureImageGLTest::subImageCubeNotReadable, - #ifndef MAGNUM_TARGET_GLES2 + #if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) &TextureImageGLTest::subImageCubeBuffer, &TextureImageGLTest::subImageCubeBufferNotReadable, #endif @@ -154,12 +154,14 @@ void TextureImageGLTest::subImage2DNotReadable() { #endif } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void TextureImageGLTest::subImage2DBuffer() { GL::Texture2D texture; texture.setImage(0, GL::TextureFormat::RGBA8, ImageView2D{GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte, Vector2i{2}, Data2D}); + CORRADE_IGNORE_DEPRECATED_PUSH GL::BufferImage2D image = textureSubImage(texture, 0, {{}, Vector2i{2}}, {GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte}, GL::BufferUsage::StaticRead); + CORRADE_IGNORE_DEPRECATED_POP Containers::Array data = bufferData(image.buffer()); MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_COMPARE(image.size(), Vector2i{2}); @@ -188,7 +190,9 @@ void TextureImageGLTest::subImage2DBufferNotReadable() { Containers::String out; Error redirectError{&out}; /* The read type doesn't have to match, it doesn't get that far */ + CORRADE_IGNORE_DEPRECATED_PUSH textureSubImage(texture, 0, {{}, Vector2i{2}}, {GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte}, GL::BufferUsage::StaticRead); + CORRADE_IGNORE_DEPRECATED_POP MAGNUM_VERIFY_NO_GL_ERROR(); #ifndef MAGNUM_TARGET_GLES CORRADE_COMPARE(out, "DebugTools::textureSubImage(): texture format not framebuffer-readable: GL::Framebuffer::Status::Unsupported\n"); @@ -283,7 +287,7 @@ void TextureImageGLTest::subImageCubeNotReadable() { #endif } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void TextureImageGLTest::subImageCubeBuffer() { ImageView2D view{GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte, Vector2i{2}, Data2D}; @@ -295,7 +299,9 @@ void TextureImageGLTest::subImageCubeBuffer() { .setImage(GL::CubeMapCoordinate::PositiveZ, 0, GL::TextureFormat::RGBA8, view) .setImage(GL::CubeMapCoordinate::NegativeZ, 0, GL::TextureFormat::RGBA8, view); + CORRADE_IGNORE_DEPRECATED_PUSH GL::BufferImage2D image = textureSubImage(texture, GL::CubeMapCoordinate::PositiveX, 0, {{}, Vector2i{2}}, {GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte}, GL::BufferUsage::StaticRead); + CORRADE_IGNORE_DEPRECATED_POP Containers::Array data = bufferData(image.buffer()); MAGNUM_VERIFY_NO_GL_ERROR(); CORRADE_COMPARE(image.size(), Vector2i{2}); @@ -332,7 +338,9 @@ void TextureImageGLTest::subImageCubeBufferNotReadable() { Containers::String out; Error redirectError{&out}; /* The read type doesn't have to match, it doesn't get that far */ + CORRADE_IGNORE_DEPRECATED_PUSH textureSubImage(texture, GL::CubeMapCoordinate::PositiveX, 0, {{}, Vector2i{2}}, {GL::PixelFormat::RGBA, GL::PixelType::UnsignedByte}, GL::BufferUsage::StaticRead); + CORRADE_IGNORE_DEPRECATED_POP MAGNUM_VERIFY_NO_GL_ERROR(); #ifndef MAGNUM_TARGET_GLES CORRADE_COMPARE(out, "DebugTools::textureSubImage(): texture format not framebuffer-readable: GL::Framebuffer::Status::Unsupported\n"); diff --git a/src/Magnum/DebugTools/TextureImage.cpp b/src/Magnum/DebugTools/TextureImage.cpp index b19ce3ab6..f874bcf04 100644 --- a/src/Magnum/DebugTools/TextureImage.cpp +++ b/src/Magnum/DebugTools/TextureImage.cpp @@ -27,7 +27,7 @@ #include "TextureImage.h" #include "Magnum/Image.h" -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) #include "Magnum/GL/BufferImage.h" #endif #include "Magnum/GL/Context.h" @@ -194,7 +194,7 @@ Image2D textureSubImage(GL::Texture2D& texture, const Int level, const Range2Di& return Utility::move(image); } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void textureSubImage(GL::Texture2D& texture, const Int level, const Range2Di& range, GL::BufferImage2D& image, const GL::BufferUsage usage) { #ifndef MAGNUM_TARGET_GLES if(GL::Context::current().isExtensionSupported()) { @@ -213,7 +213,9 @@ void textureSubImage(GL::Texture2D& texture, const Int level, const Range2Di& ra } GL::BufferImage2D textureSubImage(GL::Texture2D& texture, const Int level, const Range2Di& range, GL::BufferImage2D&& image, const GL::BufferUsage usage) { + CORRADE_IGNORE_DEPRECATED_PUSH textureSubImage(texture, level, range, image, usage); + CORRADE_IGNORE_DEPRECATED_POP return Utility::move(image); } #endif @@ -233,7 +235,7 @@ Image2D textureSubImage(GL::CubeMapTexture& texture, const GL::CubeMapCoordinate return Utility::move(image); } -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) void textureSubImage(GL::CubeMapTexture& texture, const GL::CubeMapCoordinate coordinate, const Int level, const Range2Di& range, GL::BufferImage2D& image, const GL::BufferUsage usage) { GL::Framebuffer fb{range}; fb.attachCubeMapTexture(GL::Framebuffer::ColorAttachment{0}, texture, coordinate, level); @@ -245,7 +247,9 @@ void textureSubImage(GL::CubeMapTexture& texture, const GL::CubeMapCoordinate co } GL::BufferImage2D textureSubImage(GL::CubeMapTexture& texture, const GL::CubeMapCoordinate coordinate, const Int level, const Range2Di& range, GL::BufferImage2D&& image, const GL::BufferUsage usage) { + CORRADE_IGNORE_DEPRECATED_PUSH textureSubImage(texture, coordinate, level, range, image, usage); + CORRADE_IGNORE_DEPRECATED_POP return Utility::move(image); } #endif diff --git a/src/Magnum/DebugTools/TextureImage.h b/src/Magnum/DebugTools/TextureImage.h index ba41ec104..7d75a3608 100644 --- a/src/Magnum/DebugTools/TextureImage.h +++ b/src/Magnum/DebugTools/TextureImage.h @@ -106,9 +106,16 @@ Convenience alternative to the above, example usage: */ MAGNUM_DEBUGTOOLS_EXPORT Image2D textureSubImage(GL::CubeMapTexture& texture, GL::CubeMapCoordinate coordinate, Int level, const Range2Di& range, Image2D&& image); -#ifndef MAGNUM_TARGET_GLES2 +#if defined(MAGNUM_BUILD_DEPRECATED) && !defined(MAGNUM_TARGET_GLES2) /** @brief Read range of given texture mip level to buffer image +@m_deprecated_since_latest As these APIs are mainly meant to be used for + testing and verification, there's little point in having a variant that + puts the data into a pixel buffer by creating a temporary framebuffer. Use + @ref textureSubImage(GL::Texture2D&, Int, const Range2Di&, Image2D&) + instead or populate the buffer using + @ref GL::Framebuffer::read(const Range2Di&, GL::BufferImage2D&, GL::BufferUsage) + on a non-temporary framebuffer. Emulates @ref GL::Texture2D::subImage() call on platforms that don't support it (such as OpenGL ES) by creating a framebuffer object and using @@ -127,10 +134,17 @@ it @ref MAGNUM_TARGET_GL "TARGET_GL" enabled (done by default). See @ref building-features for more information. */ -MAGNUM_DEBUGTOOLS_EXPORT void textureSubImage(GL::Texture2D& texture, Int level, const Range2Di& range, GL::BufferImage2D& image, GL::BufferUsage usage); +MAGNUM_DEBUGTOOLS_EXPORT CORRADE_DEPRECATED("use textureSubImage(GL::Texture2D&, Int, const Range2Di&, Image2D&) instead") void textureSubImage(GL::Texture2D& texture, Int level, const Range2Di& range, GL::BufferImage2D& image, GL::BufferUsage usage); /** @brief Read range of given texture mip level to buffer image +@m_deprecated_since_latest As these APIs are mainly meant to be used for + testing and verification, there's little point in having a variant that + puts the data into a buffer image by creating a temporary framebuffer. Use + @ref textureSubImage(GL::Texture2D&, Int, const Range2Di&, Image2D&&) + instead or populate the buffer image directly using + @ref GL::Framebuffer::read(const Range2Di&, GL::BufferImage2D&&, GL::BufferUsage) + on a non-temporary framebuffer. Convenience alternative to the above, example usage: @@ -140,10 +154,17 @@ Convenience alternative to the above, example usage: @ref MAGNUM_TARGET_GL "TARGET_GL" enabled (done by default). See @ref building-features for more information. */ -MAGNUM_DEBUGTOOLS_EXPORT GL::BufferImage2D textureSubImage(GL::Texture2D& texture, Int level, const Range2Di& range, GL::BufferImage2D&& image, GL::BufferUsage usage); +MAGNUM_DEBUGTOOLS_EXPORT CORRADE_DEPRECATED("use textureSubImage(GL::Texture2D&, Int, const Range2Di&, Image2D&&) instead") GL::BufferImage2D textureSubImage(GL::Texture2D& texture, Int level, const Range2Di& range, GL::BufferImage2D&& image, GL::BufferUsage usage); /** @brief Read range of given cube map texture coordinate mip level to buffer image +@m_deprecated_since_latest As these APIs are mainly meant to be used for + testing and verification, there's little point in having a variant that + puts the data into a buffer image by creating a temporary framebuffer. Use + @ref textureSubImage(GL::CubeMapTexture&, GL::CubeMapCoordinate, Int, const Range2Di&, Image2D&) + instead or populate the buffer image directly using + @ref GL::Framebuffer::read(const Range2Di&, GL::BufferImage2D&, GL::BufferUsage) + on a non-temporary framebuffer. Emulates @ref GL::CubeMapTexture::subImage() call on platforms that don't support it (such as OpenGL ES) by creating a framebuffer object and using @@ -160,10 +181,17 @@ it @ref MAGNUM_TARGET_GL "TARGET_GL" enabled (done by default). See @ref building-features for more information. */ -MAGNUM_DEBUGTOOLS_EXPORT void textureSubImage(GL::CubeMapTexture& texture, GL::CubeMapCoordinate coordinate, Int level, const Range2Di& range, GL::BufferImage2D& image, GL::BufferUsage usage); +MAGNUM_DEBUGTOOLS_EXPORT CORRADE_DEPRECATED("use textureSubImage(GL::CubeMapTexture2D&, GL::CubeMapCoordinate, Int, const Range2Di&, Image2D&) instead") void textureSubImage(GL::CubeMapTexture& texture, GL::CubeMapCoordinate coordinate, Int level, const Range2Di& range, GL::BufferImage2D& image, GL::BufferUsage usage); /** @brief Read range of given cube map texture coordinate mip level to buffer image +@m_deprecated_since_latest As these APIs are mainly meant to be used for + testing and verification, there's little point in having a variant that + puts the data into a buffer image by creating a temporary framebuffer. Use + @ref textureSubImage(GL::CubeMapTexture&, GL::CubeMapCoordinate, Int, const Range2Di&, Image2D&) + instead or populate the buffer image directly using + @ref GL::Framebuffer::read(const Range2Di&, GL::BufferImage2D&, GL::BufferUsage) + on a non-temporary framebuffer. Convenience alternative to the above, example usage: @@ -173,7 +201,7 @@ Convenience alternative to the above, example usage: @ref MAGNUM_TARGET_GL "TARGET_GL" enabled (done by default). See @ref building-features for more information. */ -MAGNUM_DEBUGTOOLS_EXPORT GL::BufferImage2D textureSubImage(GL::CubeMapTexture& texture, GL::CubeMapCoordinate coordinate, Int level, const Range2Di& range, GL::BufferImage2D&& image, GL::BufferUsage usage); +MAGNUM_DEBUGTOOLS_EXPORT CORRADE_DEPRECATED("use textureSubImage(GL::CubeMapTexture2D&, GL::CubeMapCoordinate, Int, const Range2Di&, Image2D&&) instead") GL::BufferImage2D textureSubImage(GL::CubeMapTexture& texture, GL::CubeMapCoordinate coordinate, Int level, const Range2Di& range, GL::BufferImage2D&& image, GL::BufferUsage usage); #endif #else #error this header is available only in the OpenGL build