Skip to content

Commit

Permalink
TextureTools: don't allocate the output in atlasArrayPowerOfTwo().
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mosra
mosra committed Sep 25, 2023
1 parent 752acbd commit f9402f2
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 43 deletions.
26 changes: 20 additions & 6 deletions src/Magnum/TextureTools/Atlas.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
#include <algorithm>
#include <vector>
#include <Corrade/Containers/Array.h>
#include <Corrade/Containers/StridedArrayView.h>
#include <Corrade/Containers/Pair.h>
#include <Corrade/Containers/StridedArrayView.h>

#include "Magnum/Math/Functions.h"
#include "Magnum/Math/Range.h"
Expand Down Expand Up @@ -65,15 +65,15 @@ std::vector<Range2Di> atlas(const Vector2i& atlasSize, const std::vector<Vector2
return atlas;
}

Containers::Pair<Int, Containers::Array<Vector3i>> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes) {
Int atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes, const Containers::StridedArrayView1D<Vector3i>& 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<Vector3i> 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. */
Expand Down Expand Up @@ -131,16 +131,30 @@ Containers::Pair<Int, Containers::Array<Vector3i>> 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<Vector2i> sizes, const Containers::StridedArrayView1D<Vector3i>& offsets) {
return atlasArrayPowerOfTwo(layerSize, Containers::stridedArrayView(sizes), offsets);
}

#ifdef MAGNUM_BUILD_DEPRECATED
Containers::Pair<Int, Containers::Array<Vector3i>> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes) {
Containers::Array<Vector3i> offsets{NoInit, sizes.size()};
Int layers = atlasArrayPowerOfTwo(layerSize, sizes, offsets);
return {layers, Utility::move(offsets)};
}

Containers::Pair<Int, Containers::Array<Vector3i>> atlasArrayPowerOfTwo(const Vector2i& layerSize, const std::initializer_list<Vector2i> sizes) {
CORRADE_IGNORE_DEPRECATED_PUSH
return atlasArrayPowerOfTwo(layerSize, Containers::stridedArrayView(sizes));
CORRADE_IGNORE_DEPRECATED_POP
}
#endif

}}
46 changes: 31 additions & 15 deletions src/Magnum/TextureTools/Atlas.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,32 +54,48 @@ std::vector<Range2Di> 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<Int, Containers::Array<Vector3i>> MAGNUM_TEXTURETOOLS_EXPORT atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes);
MAGNUM_TEXTURETOOLS_EXPORT Int atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes, const Containers::StridedArrayView1D<Vector3i>& offsets);

/**
* @overload
* @m_since_latest
*/
Containers::Pair<Int, Containers::Array<Vector3i>> MAGNUM_TEXTURETOOLS_EXPORT atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list<Vector2i> sizes);
MAGNUM_TEXTURETOOLS_EXPORT Int atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list<Vector2i> sizes, const Containers::StridedArrayView1D<Vector3i>& offsets);

#ifdef MAGNUM_BUILD_DEPRECATED
/**
@brief @copybrief atlasArrayPowerOfTwo(const Vector2i&, const Containers::StridedArrayView1D<const Vector2i>&, const Containers::StridedArrayView1D<Vector3i>&)
@m_deprecated_since_latest Use @ref atlasArrayPowerOfTwo(const Vector2i&, const Containers::StridedArrayView1D<const Vector2i>&, const Containers::StridedArrayView1D<Vector3i>&)
instead.
*/
MAGNUM_TEXTURETOOLS_EXPORT CORRADE_DEPRECATED("use the overload taking offsets as an output view instead") Containers::Pair<Int, Containers::Array<Vector3i>> atlasArrayPowerOfTwo(const Vector2i& layerSize, const Containers::StridedArrayView1D<const Vector2i>& sizes);

/**
@overload
@m_deprecated_since_latest Use @ref atlasArrayPowerOfTwo(const Vector2i&, std::initializer_list<Vector2i>, const Containers::StridedArrayView1D<Vector3i>&)
instead.
*/
MAGNUM_TEXTURETOOLS_EXPORT CORRADE_DEPRECATED("use the overload taking offsets as an output view instead") Containers::Pair<Int, Containers::Array<Vector3i>> atlasArrayPowerOfTwo(const Vector2i& layerSize, std::initializer_list<Vector2i> sizes);
#endif

}}

Expand Down
88 changes: 66 additions & 22 deletions src/Magnum/TextureTools/Test/AtlasTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -160,29 +169,27 @@ void AtlasTest::tooSmall() {
}

void AtlasTest::arrayPowerOfTwoEmpty() {
Containers::Pair<Int, Containers::Array<Vector3i>> out = atlasArrayPowerOfTwo({128, 128}, {});
CORRADE_COMPARE(out.first(), 0);
CORRADE_COMPARE_AS(out.second(), Containers::arrayView<Vector3i>({
}), TestSuite::Compare::Container);
Containers::ArrayView<Vector3i> offsets;
CORRADE_COMPARE(atlasArrayPowerOfTwo({128, 128}, {}, offsets), 0);
}

void AtlasTest::arrayPowerOfTwoSingleElement() {
Containers::Pair<Int, Containers::Array<Vector3i>> out = atlasArrayPowerOfTwo({128, 128}, {{128, 128}});
CORRADE_COMPARE(out.first(), 1);
CORRADE_COMPARE_AS(out.second(), Containers::arrayView<Vector3i>({
Vector3i offsets[1];
CORRADE_COMPARE(atlasArrayPowerOfTwo({128, 128}, {{128, 128}}, offsets), 1);
CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView<Vector3i>({
{0, 0, 0}
}), TestSuite::Compare::Container);
}

void AtlasTest::arrayPowerOfTwoAllSameElements() {
Containers::Pair<Int, Containers::Array<Vector3i>> 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<Vector3i>({
}, offsets), 1);
CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView<Vector3i>({
{0, 0, 0},
{64, 0, 0},
{0, 64, 0},
Expand Down Expand Up @@ -245,15 +252,16 @@ void AtlasTest::arrayPowerOfTwoOneLayer() {
expected[i] = expectedSorted[data.order[i]];
}

Containers::Pair<Int, Containers::Array<Vector3i>> out = atlasArrayPowerOfTwo({2048, 2048}, input);
CORRADE_COMPARE(out.first(), 1);
CORRADE_COMPARE_AS(out.second(),
Containers::ArrayView<const Vector3i>{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<Int, Containers::Array<Vector3i>> out = atlasArrayPowerOfTwo({2048, 2048}, {
Vector3i offsets[11];
CORRADE_COMPARE(atlasArrayPowerOfTwo({2048, 2048}, {
{2048, 2048},

{1024, 1024},
Expand All @@ -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<Vector3i>({
}, offsets), 3);
CORRADE_COMPARE_AS(Containers::arrayView(offsets), Containers::arrayView<Vector3i>({
{0, 0, 0},

{0, 0, 1},
Expand All @@ -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);
Expand All @@ -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));
}

Expand All @@ -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<Int, Containers::Array<Vector3i>> 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<Vector3i>({
{0, 0, 0},
{64, 0, 0},
{0, 64, 0},
{64, 64, 0}
}), TestSuite::Compare::Container);
}
#endif

}}}}

CORRADE_TEST_MAIN(Magnum::TextureTools::Test::AtlasTest)

0 comments on commit f9402f2

Please sign in to comment.