From f8926fbdbdd63b6cbe182f6045ec263cb1a755e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 30 Jun 2025 17:10:08 +0200 Subject: [PATCH] Fix pixel storage data size calculation for compressed images. The tests now pass, but this damn GL pixel storage API is so convoluted with so many redundant and mutually conflicting degrees of freedom that it's impossible to be sure. Looking forward to when I can finally drop that thing and calculate it on the fly from just size + stride, like it should have been from the start. --- .../GL/Implementation/imageProperties.h | 11 ++++++-- src/Magnum/Implementation/ImageProperties.h | 28 ++++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/Magnum/GL/Implementation/imageProperties.h b/src/Magnum/GL/Implementation/imageProperties.h index 07ebe0467..a02ae0fe8 100644 --- a/src/Magnum/GL/Implementation/imageProperties.h +++ b/src/Magnum/GL/Implementation/imageProperties.h @@ -26,7 +26,7 @@ DEALINGS IN THE SOFTWARE. */ -#include "Magnum/Implementation/ImageProperties.h" +#include "Magnum/PixelStorage.h" namespace Magnum { namespace GL { namespace Implementation { @@ -45,8 +45,13 @@ namespace Magnum { namespace GL { namespace Implementation { In case the block size properties aren't set, the actual image data size is used as a backup, which might still be correct in most cases. */ template std::size_t occupiedCompressedImageDataSize(const T& image, std::size_t dataSize) { - return image.storage().compressedBlockSize().product() && image.storage().compressedBlockDataSize() - ? Magnum::Implementation::compressedImageDataOffsetSizeFor(image, image.size()).second : dataSize; + if(image.storage().compressedBlockSize().product() && image.storage().compressedBlockDataSize()) { + const auto realBlockCount = Math::Vector3{(Vector3i::pad(image.size(), 1) + image.storage().compressedBlockSize() - Vector3i{1})/image.storage().compressedBlockSize()}; + + return realBlockCount.product()*image.storage().compressedBlockDataSize(); + } + + return dataSize; } }}} diff --git a/src/Magnum/Implementation/ImageProperties.h b/src/Magnum/Implementation/ImageProperties.h index fedf30280..3a1fdc26c 100644 --- a/src/Magnum/Implementation/ImageProperties.h +++ b/src/Magnum/Implementation/ImageProperties.h @@ -97,7 +97,12 @@ template std::pair std::size_t imageDataSizeFor(const T& image, const Math::Vector& size) { std::pair, Math::Vector3> dataProperties = image.storage().dataProperties(image.pixelSize(), Vector3i::pad(size, 1)); - /* Smallest line/rectangle/cube that covers the area */ + /* Smallest line/rectangle/cube that covers the area. In other words, make + it so that it matches what can be practically achieved by slicing a + larger image. For example, if an image of 100x100 is sliced to a 50x50 + rectangle at offset (25, 25), the data size is 100x75. I.e., including + the extra 25 padding pixels until the end of the last row, because the + original image would have them anyway. */ std::size_t dataOffset = 0; if(dataProperties.first.z()) dataOffset += dataProperties.first.z(); @@ -116,14 +121,29 @@ template inline std::size_t imageDataSize(const T& image) { return imageDataSizeFor(image, image.size()); } +/* Unlike imageDataSizeFor() this produces separate offset and size because + because compressedImageDataSizeFor() it's also used in GL image queries, + where the nv-cubemap-broken-full-compressed-image-query workaround needs to + go slice by slice, taking offset and incrementing it by size divided by the + Z dimension. */ template std::pair compressedImageDataOffsetSizeFor(const T& image, const Math::Vector& size) { CORRADE_INTERNAL_ASSERT(image.storage().compressedBlockSize().product() && image.storage().compressedBlockDataSize()); std::pair, Math::Vector3> dataProperties = image.storage().dataProperties(Vector3i::pad(size, 1)); - const auto realBlockCount = Math::Vector3{(Vector3i::pad(size, 1) + image.storage().compressedBlockSize() - Vector3i{1})/image.storage().compressedBlockSize()}; - - return {dataProperties.first.sum(), (dataProperties.second.product() - (dataProperties.second.x() - realBlockCount.x()) - (dataProperties.second.y() - realBlockCount.y())*dataProperties.second.x())*image.storage().compressedBlockDataSize()}; + /* Smallest line/rectangle/cube that covers the area. Same logic as in + imageDataSizeFor() above. */ + std::size_t dataOffset = 0; + if(dataProperties.first.z()) + dataOffset += dataProperties.first.z(); + else if(dataProperties.first.y()) { + if(!image.storage().imageHeight()) + dataOffset += dataProperties.first.y(); + } else if(dataProperties.first.x()) { + if(!image.storage().rowLength()) + dataOffset += dataProperties.first.x(); + } + return {dataOffset, dataProperties.second.product()*image.storage().compressedBlockDataSize()}; } /* Used in image query functions */