From f131e8403bd95289762d294886e0459f56f068bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 25 Jan 2025 13:07:36 +0100 Subject: [PATCH] GL: make CompressedBufferImage and BufferImage::setData() work the same. The uncompressed variant preserves the buffer contents if passed an empty view, which was done in 5ee461bdbe1fb17082a15a76b0a87c0349a1ef85 (2015!) but no corresponding change was ever done for the compressed variant. So let's make both do the same, and actually test the code path, which wasn't done until now either. --- src/Magnum/GL/BufferImage.cpp | 8 ++- src/Magnum/GL/BufferImage.h | 4 +- src/Magnum/GL/Test/BufferImageGLTest.cpp | 69 ++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/src/Magnum/GL/BufferImage.cpp b/src/Magnum/GL/BufferImage.cpp index 2d66eddad..095ec09c9 100644 --- a/src/Magnum/GL/BufferImage.cpp +++ b/src/Magnum/GL/BufferImage.cpp @@ -134,8 +134,12 @@ template void CompressedBufferImage::setData _storage = storage; _format = format; _size = size; - _buffer.setData(data, usage); - _dataSize = data.size(); + + /* Keep the old storage if zero-sized nullptr buffer was passed */ + if(!(data.data() == nullptr && data.size() == 0)) { + _buffer.setData(data, usage); + _dataSize = data.size(); + } } template void CompressedBufferImage::setData(const CompressedPixelStorage storage, const Magnum::CompressedPixelFormat format, const VectorTypeFor& size, const Containers::ArrayView data, const BufferUsage usage) { diff --git a/src/Magnum/GL/BufferImage.h b/src/Magnum/GL/BufferImage.h index 250197fc3..de0dc9684 100644 --- a/src/Magnum/GL/BufferImage.h +++ b/src/Magnum/GL/BufferImage.h @@ -640,7 +640,9 @@ template class CompressedBufferImage { * @param data Image data * @param usage Image buffer usage * - * Updates the image buffer with given data. + * Updates the image buffer with given data. Passing @cpp nullptr @ce + * zero-sized @p data will not reallocate current storage, but expects + * that current data size is large enough for the new parameters. * @see @ref Buffer::setData() * @requires_gl42 Extension @gl_extension{ARB,compressed_texture_pixel_storage} * @requires_gl Compressed pixel storage is hardcoded in OpenGL ES and diff --git a/src/Magnum/GL/Test/BufferImageGLTest.cpp b/src/Magnum/GL/Test/BufferImageGLTest.cpp index fc6b1d72d..145c5e772 100644 --- a/src/Magnum/GL/Test/BufferImageGLTest.cpp +++ b/src/Magnum/GL/Test/BufferImageGLTest.cpp @@ -62,8 +62,10 @@ struct BufferImageGLTest: OpenGLTester { void setData(); void setDataGeneric(); + void setDataKeepStorage(); void setDataCompressed(); void setDataCompressedGeneric(); + void setDataCompressedKeepStorage(); void setDataInvalidSize(); void setDataCompressedInvalidSize(); @@ -94,8 +96,10 @@ BufferImageGLTest::BufferImageGLTest() { &BufferImageGLTest::setData, &BufferImageGLTest::setDataGeneric, + &BufferImageGLTest::setDataKeepStorage, &BufferImageGLTest::setDataCompressed, &BufferImageGLTest::setDataCompressedGeneric, + &BufferImageGLTest::setDataCompressedKeepStorage, &BufferImageGLTest::setDataInvalidSize, &BufferImageGLTest::setDataCompressedInvalidSize, @@ -616,6 +620,39 @@ void BufferImageGLTest::setDataGeneric() { #endif } +void BufferImageGLTest::setDataKeepStorage() { + const char data[]{ + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 'l' + }; + BufferImage2D a{ + PixelFormat::Red, PixelType::UnsignedByte, {4, 1}, + data, BufferUsage::StaticDraw}; + + a.setData(PixelStorage{}.setAlignment(1), + PixelFormat::RGB, PixelType::UnsignedShort, {2, 1}, + nullptr, BufferUsage::StaticDraw); + + #ifndef MAGNUM_TARGET_GLES + const auto imageData = a.buffer().data(); + #endif + + MAGNUM_VERIFY_NO_GL_ERROR(); + + CORRADE_COMPARE(a.storage().alignment(), 1); + CORRADE_COMPARE(a.format(), PixelFormat::RGB); + CORRADE_COMPARE(a.type(), PixelType::UnsignedShort); + CORRADE_COMPARE(a.size(), (Vector2i{2, 1})); + CORRADE_COMPARE(a.pixelSize(), 6); + CORRADE_COMPARE(a.dataSize(), 12); + + /** @todo How to verify the contents in ES? */ + #ifndef MAGNUM_TARGET_GLES + CORRADE_COMPARE_AS(imageData, + Containers::arrayView(data), + TestSuite::Compare::Container); + #endif +} + void BufferImageGLTest::setDataCompressed() { const char data[] = { 'a', 0, 0, 0, 'b', 0, 0, 0 }; CompressedBufferImage2D a{CompressedPixelFormat::RGBAS3tcDxt1, {4, 4}, data, BufferUsage::StaticDraw}; @@ -678,6 +715,38 @@ void BufferImageGLTest::setDataCompressedGeneric() { #endif } +void BufferImageGLTest::setDataCompressedKeepStorage() { + const char data[]{ + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', + 'i', 'j', 'k', 'l', 'm', 'n', 'o', 'p' + }; + CompressedBufferImage2D a{ + CompressedPixelFormat::RGBAS3tcDxt1, {8, 3}, + data, BufferUsage::StaticDraw}; + + a.setData(CompressedPixelStorage{}.setRowLength(3), + CompressedPixelFormat::SRGB8Alpha8Astc4x4, {2, 4}, + nullptr, BufferUsage::StaticDraw); + + #ifndef MAGNUM_TARGET_GLES + const auto imageData = a.buffer().data(); + #endif + + MAGNUM_VERIFY_NO_GL_ERROR(); + + CORRADE_COMPARE(a.storage().rowLength(), 3); + CORRADE_COMPARE(a.format(), CompressedPixelFormat::SRGB8Alpha8Astc4x4); + CORRADE_COMPARE(a.size(), (Vector2i{2, 4})); + CORRADE_COMPARE(a.dataSize(), 16); + + /** @todo How to verify the contents in ES? */ + #ifndef MAGNUM_TARGET_GLES + CORRADE_COMPARE_AS(imageData, + Containers::arrayView(data), + TestSuite::Compare::Container); + #endif +} + void BufferImageGLTest::setDataInvalidSize() { CORRADE_SKIP_IF_NO_ASSERT();