From f9402f2242ade3a93ff259fe83266e70112e1bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 25 Sep 2023 16:19:57 +0200 Subject: [PATCH] TextureTools: don't allocate the output in atlasArrayPowerOfTwo(). I mean, yes, it was already SIGNIFICANTLY better than the atlas() that took a vector and returned one as well, but still. One of the usual use cases is that there's an array of structs containing both sizes offsets and the offsets get written back. The original versions are deprecated as I really don't see any convenient use case for these, especially given they return a pair and not just an array. --- src/Magnum/TextureTools/Atlas.cpp | 26 +++++-- src/Magnum/TextureTools/Atlas.h | 46 +++++++---- src/Magnum/TextureTools/Test/AtlasTest.cpp | 88 ++++++++++++++++------ 3 files changed, 117 insertions(+), 43 deletions(-) diff --git a/src/Magnum/TextureTools/Atlas.cpp b/src/Magnum/TextureTools/Atlas.cpp index 83d10a006..90a1eb18b 100644 --- a/src/Magnum/TextureTools/Atlas.cpp +++ b/src/Magnum/TextureTools/Atlas.cpp @@ -28,8 +28,8 @@ #include #include #include -#include #include +#include #include "Magnum/Math/Functions.h" #include "Magnum/Math/Range.h" @@ -65,15 +65,15 @@ std::vector atlas(const Vector2i& atlasSize, const std::vector> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes) { +Int atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes, const Containers::StridedArrayView1D& offsets) { + CORRADE_ASSERT(offsets.size() == sizes.size(), + "TextureTools::atlasArrayPowerOfTwo(): expected sizes and offsets views to have the same size, got" << sizes.size() << "and" << offsets.size(), {}); CORRADE_ASSERT(layerSize.product() && layerSize.x() == layerSize.y() && (layerSize & (layerSize - Vector2i{1})).isZero(), "TextureTools::atlasArrayPowerOfTwo(): expected layer size to be a non-zero power-of-two square, got" << Debug::packed << layerSize, {}); if(sizes.isEmpty()) return {}; - Containers::Array output{NoInit, sizes.size()}; - /* Copy the input to a sorted array, together with a mapping to the original order stored in Z. Can't really reuse the output allocation as it would be overwritten in random order. */ @@ -131,16 +131,30 @@ Containers::Pair> atlasArrayPowerOfTwo(const Ve } /* Save to the output in the original order */ - output[size.second()] = {coordinates, layer}; + offsets[size.second()] = {coordinates, layer}; previousSize = size.first(); --free; } - return {layer + 1, Utility::move(output)}; + return layer + 1; +} + +Int atlasArrayPowerOfTwo(const Vector2i& layerSize, const std::initializer_list sizes, const Containers::StridedArrayView1D& offsets) { + return atlasArrayPowerOfTwo(layerSize, Containers::stridedArrayView(sizes), offsets); +} + +#ifdef MAGNUM_BUILD_DEPRECATED +Containers::Pair> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes) { + Containers::Array offsets{NoInit, sizes.size()}; + Int layers = atlasArrayPowerOfTwo(layerSize, sizes, offsets); + return {layers, Utility::move(offsets)}; } Containers::Pair> atlasArrayPowerOfTwo(const Vector2i& layerSize, const std::initializer_list sizes) { + CORRADE_IGNORE_DEPRECATED_PUSH return atlasArrayPowerOfTwo(layerSize, Containers::stridedArrayView(sizes)); + CORRADE_IGNORE_DEPRECATED_POP } +#endif }} diff --git a/src/Magnum/TextureTools/Atlas.h b/src/Magnum/TextureTools/Atlas.h index d1c5a9f1e..4c0076978 100644 --- a/src/Magnum/TextureTools/Atlas.h +++ b/src/Magnum/TextureTools/Atlas.h @@ -54,32 +54,48 @@ std::vector MAGNUM_TEXTURETOOLS_EXPORT atlas(const Vector2i& atlasSize /** @brief Pack square power-of-two textures into a texture atlas array -@param layerSize Size of the texture layer -@param sizes Sizes of all textures in the atlas -@return Total layer count and offsets of all textures in the atlas, with the Z - coordinate being the layer index +@param[in] layerSize Size of a single layer in the texture atlas +@param[in] sizes Sizes of all textures in the atlas +@param[out] offsets Resulting offsets in the atlas +@return Total layer count @m_since_latest -The @p layerSize is expected to be non-zero, square and power-of-two. All items -in @p sizes are expected to be non-zero, square, power-of-two and not larger -than @p layerSize. With such constraints the packing is optimal with no wasted -space in all but the last layer. Setting @p layerSize to the size of the -largest texture in the set will lead to the least wasted space in the last -layer. +The @p sizes and @p offsets views are expected to have the same size. The +@p layerSize is expected to be non-zero, square and power-of-two. All items in +@p sizes are expected to be non-zero, square, power-of-two and not larger than +@p layerSize. With such constraints the packing is optimal with no wasted space +in all but the last layer. Setting @p layerSize to the size of the largest +texture in the set will lead to the least wasted space in the last layer. The algorithm first sorts the textures by size using @ref std::stable_sort(), which is usually @f$ \mathcal{O}(n \log{} n) @f$, and then performs the actual -atlasing in a single @f$ \mathcal{O}(n) @f$ operation. Due to the sort -involved, a temporary allocation holds the sorted array and additionally -@ref std::stable_sort() performs its own allocation. +atlasing in a single @f$ \mathcal{O}(n) @f$ operation. Memory complexity is +@f$ \mathcal{0}(n) @f$ with @f$ n @f$ being a sorted copy of the input size +array, additionally @ref std::stable_sort() performs its own allocation. */ -Containers::Pair> MAGNUM_TEXTURETOOLS_EXPORT atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes); +MAGNUM_TEXTURETOOLS_EXPORT Int atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes, const Containers::StridedArrayView1D& offsets); /** * @overload * @m_since_latest */ -Containers::Pair> MAGNUM_TEXTURETOOLS_EXPORT atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list sizes); +MAGNUM_TEXTURETOOLS_EXPORT Int atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list sizes, const Containers::StridedArrayView1D& offsets); + +#ifdef MAGNUM_BUILD_DEPRECATED +/** +@brief @copybrief atlasArrayPowerOfTwo(const Vector2i&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) +@m_deprecated_since_latest Use @ref atlasArrayPowerOfTwo(const Vector2i&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) + instead. +*/ +MAGNUM_TEXTURETOOLS_EXPORT CORRADE_DEPRECATED("use the overload taking offsets as an output view instead") Containers::Pair> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D& sizes); + +/** +@overload +@m_deprecated_since_latest Use @ref atlasArrayPowerOfTwo(const Vector2i&, std::initializer_list, const Containers::StridedArrayView1D&) + instead. +*/ +MAGNUM_TEXTURETOOLS_EXPORT CORRADE_DEPRECATED("use the overload taking offsets as an output view instead") Containers::Pair> atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list sizes); +#endif }} diff --git a/src/Magnum/TextureTools/Test/AtlasTest.cpp b/src/Magnum/TextureTools/Test/AtlasTest.cpp index d652a38cb..5fc34e25b 100644 --- a/src/Magnum/TextureTools/Test/AtlasTest.cpp +++ b/src/Magnum/TextureTools/Test/AtlasTest.cpp @@ -51,8 +51,12 @@ struct AtlasTest: TestSuite::Tester { void arrayPowerOfTwoAllSameElements(); void arrayPowerOfTwoOneLayer(); void arrayPowerOfTwoMoreLayers(); + void arrayPowerOfTwoInvalidViewSizes(); void arrayPowerOfTwoWrongLayerSize(); void arrayPowerOfTwoWrongSize(); + #ifdef MAGNUM_BUILD_DEPRECATED + void arrayPowerOfTwoDeprecated(); + #endif }; /* Could make order[15] and then Containers::arraySize(), but then it won't @@ -104,13 +108,18 @@ AtlasTest::AtlasTest() { addInstancedTests({&AtlasTest::arrayPowerOfTwoOneLayer}, Containers::arraySize(ArrayPowerOfTwoOneLayerData)); - addTests({&AtlasTest::arrayPowerOfTwoMoreLayers}); + addTests({&AtlasTest::arrayPowerOfTwoMoreLayers, + &AtlasTest::arrayPowerOfTwoInvalidViewSizes}); addInstancedTests({&AtlasTest::arrayPowerOfTwoWrongLayerSize}, Containers::arraySize(ArrayPowerOfTwoWrongLayerSizeData)); addInstancedTests({&AtlasTest::arrayPowerOfTwoWrongSize}, Containers::arraySize(ArrayPowerOfTwoWrongSizeData)); + + #ifdef MAGNUM_BUILD_DEPRECATED + addTests({&AtlasTest::arrayPowerOfTwoDeprecated}); + #endif } void AtlasTest::basic() { @@ -160,29 +169,27 @@ void AtlasTest::tooSmall() { } void AtlasTest::arrayPowerOfTwoEmpty() { - Containers::Pair> out = atlasArrayPowerOfTwo({128, 128}, {}); - CORRADE_COMPARE(out.first(), 0); - CORRADE_COMPARE_AS(out.second(), Containers::arrayView({ - }), TestSuite::Compare::Container); + Containers::ArrayView offsets; + CORRADE_COMPARE(atlasArrayPowerOfTwo({128, 128}, {}, offsets), 0); } void AtlasTest::arrayPowerOfTwoSingleElement() { - Containers::Pair> out = atlasArrayPowerOfTwo({128, 128}, {{128, 128}}); - CORRADE_COMPARE(out.first(), 1); - CORRADE_COMPARE_AS(out.second(), Containers::arrayView({ + Vector3i offsets[1]; + CORRADE_COMPARE(atlasArrayPowerOfTwo({128, 128}, {{128, 128}}, offsets), 1); + CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView({ {0, 0, 0} }), TestSuite::Compare::Container); } void AtlasTest::arrayPowerOfTwoAllSameElements() { - Containers::Pair> out = atlasArrayPowerOfTwo({128, 128}, { + Vector3i offsets[4]; + CORRADE_COMPARE(atlasArrayPowerOfTwo({128, 128}, { {64, 64}, {64, 64}, {64, 64}, {64, 64}, - }); - CORRADE_COMPARE(out.first(), 1); - CORRADE_COMPARE_AS(out.second(), Containers::arrayView({ + }, offsets), 1); + CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView({ {0, 0, 0}, {64, 0, 0}, {0, 64, 0}, @@ -245,15 +252,16 @@ void AtlasTest::arrayPowerOfTwoOneLayer() { expected[i] = expectedSorted[data.order[i]]; } - Containers::Pair> out = atlasArrayPowerOfTwo({2048, 2048}, input); - CORRADE_COMPARE(out.first(), 1); - CORRADE_COMPARE_AS(out.second(), - Containers::ArrayView{expected}, + Vector3i offsets[ArrayPowerOfTwoOneLayerImageCount]; + CORRADE_COMPARE(atlasArrayPowerOfTwo({2048, 2048}, input, offsets), 1); + CORRADE_COMPARE_AS(Containers::arrayView(offsets), + Containers::arrayView(expected), TestSuite::Compare::Container); } void AtlasTest::arrayPowerOfTwoMoreLayers() { - Containers::Pair> out = atlasArrayPowerOfTwo({2048, 2048}, { + Vector3i offsets[11]; + CORRADE_COMPARE(atlasArrayPowerOfTwo({2048, 2048}, { {2048, 2048}, {1024, 1024}, @@ -267,9 +275,8 @@ void AtlasTest::arrayPowerOfTwoMoreLayers() { {512, 512}, {256, 256}, {256, 256} - }); - CORRADE_COMPARE(out.first(), 3); - CORRADE_COMPARE_AS(out.second(), Containers::arrayView({ + }, offsets), 3); + CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView({ {0, 0, 0}, {0, 0, 1}, @@ -286,6 +293,19 @@ void AtlasTest::arrayPowerOfTwoMoreLayers() { }), TestSuite::Compare::Container); } +void AtlasTest::arrayPowerOfTwoInvalidViewSizes() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Vector2i sizes[2]; + Vector3i offsetsInvalid[3]; + + std::ostringstream out; + Error redirectError{&out}; + atlasArrayPowerOfTwo({}, sizes, offsetsInvalid); + CORRADE_COMPARE(out.str(), + "TextureTools::atlasArrayPowerOfTwo(): expected sizes and offsets views to have the same size, got 2 and 3\n"); +} + void AtlasTest::arrayPowerOfTwoWrongLayerSize() { auto&& data = ArrayPowerOfTwoWrongLayerSizeData[testCaseInstanceId()]; setTestCaseDescription(data.name); @@ -294,7 +314,7 @@ void AtlasTest::arrayPowerOfTwoWrongLayerSize() { std::ostringstream out; Error redirectError{&out}; - atlasArrayPowerOfTwo(data.size, {}); + atlasArrayPowerOfTwo(data.size, {}, {}); CORRADE_COMPARE(out.str(), Utility::formatString("TextureTools::atlasArrayPowerOfTwo(): expected layer size to be a non-zero power-of-two square, got {}\n", data.message)); } @@ -304,16 +324,40 @@ void AtlasTest::arrayPowerOfTwoWrongSize() { CORRADE_SKIP_IF_NO_ASSERT(); + Vector3i offsets[3]; + std::ostringstream out; Error redirectError{&out}; atlasArrayPowerOfTwo({256, 256}, { {64, 64}, {128, 128}, data.size - }); + }, offsets); CORRADE_COMPARE(out.str(), Utility::formatString("TextureTools::atlasArrayPowerOfTwo(): expected size 2 to be a non-zero power-of-two square not larger than {{256, 256}} but got {}\n", data.message)); } +#ifdef MAGNUM_BUILD_DEPRECATED +void AtlasTest::arrayPowerOfTwoDeprecated() { + /* Same as arrayPowerOfTwoAllSameElements(), but with the deprecated API */ + + CORRADE_IGNORE_DEPRECATED_PUSH + Containers::Pair> out = atlasArrayPowerOfTwo({128, 128}, { + {64, 64}, + {64, 64}, + {64, 64}, + {64, 64}, + }); + CORRADE_IGNORE_DEPRECATED_POP + CORRADE_COMPARE(out.first(), 1); + CORRADE_COMPARE_AS(out.second(), Containers::arrayView({ + {0, 0, 0}, + {64, 0, 0}, + {0, 64, 0}, + {64, 64, 0} + }), TestSuite::Compare::Container); +} +#endif + }}}} CORRADE_TEST_MAIN(Magnum::TextureTools::Test::AtlasTest)