From 937689ea616eb32f287138e488e2c6c307645a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Wed, 22 Jan 2020 17:01:59 +0100 Subject: [PATCH] MeshTools: deSTLify compressIndices(). There's a lot to change with the current version -- the bloaty tuple, the useless min/max, and compressing all the way down to 8 bits is not desirable anymore either. The new function allows to specify a minimal type to compress to and works also on 8- and 16-byte types, which makes it possible to also inflate a smaller type into a larger one. The old function is now deprecated. --- doc/changelog.dox | 7 ++ doc/snippets/MagnumMeshTools-gl.cpp | 26 ++++- src/Magnum/MeshTools/CompressIndices.cpp | 64 +++++++---- src/Magnum/MeshTools/CompressIndices.h | 55 +++++++++- .../MeshTools/Test/CompressIndicesTest.cpp | 102 ++++++++++++------ 5 files changed, 196 insertions(+), 58 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index d8e348d1b..a9d6f36af 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -210,6 +210,10 @@ See also: @subsubsection changelog-latest-changes-meshtools MeshTools library +- Added @ref MeshTools::compressIndices() that takes a + @ref Containers::StridedArrayView instead of a @ref std::vector and + additionally allows you to specify the smallest allowed type and is working + with 8- and 16-byte index types as well - Added @ref MeshTools::subdivide() that operates on a (growable) @ref Corrade::Containers::Array instead of a @ref std::vector - Added @ref MeshTools::subdivideInPlace() that operates on a partially @@ -363,6 +367,9 @@ See also: @ref Text::FontConverterFeatures, @ref Trade::ImporterFeatures, @ref Trade::ImageConverterFeatures enum sets and their corresponding enums placed directly in the namespace to have them shorter and unambiguous +- @cpp MeshTools::compressIndices() @ce taking a @ref std::vector is + deprecated in favor of @ref MeshTools::compressIndices(const Containers::StridedArrayView1D&, MeshIndexType) and + its 8- and 16-byte overloads - @cpp MeshTools::flipNormals() @ce and @cpp MeshTools::flipFaceWinding() @ce and @cpp MeshTools::tipsify() @ce are deprecated in favor of @ref MeshTools::flipNormalsInPlace(), diff --git a/doc/snippets/MagnumMeshTools-gl.cpp b/doc/snippets/MagnumMeshTools-gl.cpp index bf17e95e7..39ad83a02 100644 --- a/doc/snippets/MagnumMeshTools-gl.cpp +++ b/doc/snippets/MagnumMeshTools-gl.cpp @@ -23,6 +23,9 @@ DEALINGS IN THE SOFTWARE. */ +#include /* for std::tie() :( */ +#include + #include "Magnum/GL/Buffer.h" #include "Magnum/GL/Mesh.h" #include "Magnum/Math/Vector3.h" @@ -35,6 +38,25 @@ int main() { { /* [compressIndices] */ +Containers::Array indices; + +Containers::Array indexData; +MeshIndexType indexType; +std::tie(indexData, indexType) = MeshTools::compressIndices(indices); + +GL::Buffer indexBuffer; +indexBuffer.setData(indexData); + +GL::Mesh mesh; +mesh.setCount(indices.size()) + .setIndexBuffer(indexBuffer, 0, indexType); +/* [compressIndices] */ +} + +#ifdef MAGNUM_BUILD_DEPRECATED +{ +CORRADE_IGNORE_DEPRECATED_PUSH +/* [compressIndices-stl] */ std::vector indices; Containers::Array indexData; @@ -49,8 +71,10 @@ indexBuffer.setData(indexData, GL::BufferUsage::StaticDraw); GL::Mesh mesh; mesh.setCount(indices.size()) .setIndexBuffer(indexBuffer, 0, indexType, indexStart, indexEnd); -/* [compressIndices] */ +/* [compressIndices-stl] */ } +CORRADE_IGNORE_DEPRECATED_POP +#endif { struct MyShader { diff --git a/src/Magnum/MeshTools/CompressIndices.cpp b/src/Magnum/MeshTools/CompressIndices.cpp index c3b56d32e..c2b2dd3c7 100644 --- a/src/Magnum/MeshTools/CompressIndices.cpp +++ b/src/Magnum/MeshTools/CompressIndices.cpp @@ -35,7 +35,9 @@ namespace Magnum { namespace MeshTools { namespace { -template inline Containers::Array compress(const std::vector& indices) { +template inline Containers::Array compress(const Containers::StridedArrayView1D& indices) { + /* Can't use Utility::copy() here because we're copying from a larger type + to a smaller one */ Containers::Array buffer(indices.size()*sizeof(T)); for(std::size_t i = 0; i != indices.size(); ++i) { T index = static_cast(indices[i]); @@ -45,34 +47,56 @@ template inline Containers::Array compress(const std::vector std::pair, MeshIndexType> compressIndicesImplementation(const Containers::StridedArrayView1D& indices, const MeshIndexType atLeast) { + const T max = Math::max(indices); + Containers::Array out; + MeshIndexType type; + const UnsignedInt log = Math::log(256, max); + + /* If it fits into 8 bytes and 8 bytes are allowed, pack into 8 */ + if(log == 0 && atLeast == MeshIndexType::UnsignedByte) { + out = compress(indices); + type = MeshIndexType::UnsignedByte; + + /* Otherwise, if it fits into either 8 or 16 bytes and we allow either 8 or + 16, pack into 16 */ + } else if(log <= 1 && atLeast != MeshIndexType::UnsignedInt) { + out = compress(indices); + type = MeshIndexType::UnsignedShort; + + /* Otherwise pack into 32 */ + } else { + out = compress(indices); + type = MeshIndexType::UnsignedInt; + } + + return {std::move(out), type}; +} + +} + +std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, const MeshIndexType atLeast) { + return compressIndicesImplementation(indices, atLeast); } +std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, const MeshIndexType atLeast) { + return compressIndicesImplementation(indices, atLeast); +} + +std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, const MeshIndexType atLeast) { + return compressIndicesImplementation(indices, atLeast); +} + +#ifdef MAGNUM_BUILD_DEPRECATED std::tuple, MeshIndexType, UnsignedInt, UnsignedInt> compressIndices(const std::vector& indices) { /** @todo Performance hint when range can be represented by smaller value? */ const auto minmax = Math::minmax(indices); Containers::Array data; MeshIndexType type; - switch(Math::log(256, minmax.second)) { - case 0: - data = compress(indices); - type = MeshIndexType::UnsignedByte; - break; - case 1: - data = compress(indices); - type = MeshIndexType::UnsignedShort; - break; - case 2: - case 3: - data = compress(indices); - type = MeshIndexType::UnsignedInt; - break; - - default: - CORRADE_ASSERT(false, "MeshTools::compressIndices(): no type able to index" << minmax.second << "elements.", {}); /* LCOV_EXCL_LINE */ - } - + std::tie(data, type) = compressIndices(indices, MeshIndexType::UnsignedByte); return std::make_tuple(std::move(data), type, minmax.first, minmax.second); } +#endif template Containers::Array compressIndicesAs(const std::vector& indices) { #if !defined(CORRADE_NO_ASSERT) || defined(CORRADE_GRACEFUL_ASSERT) diff --git a/src/Magnum/MeshTools/CompressIndices.h b/src/Magnum/MeshTools/CompressIndices.h index eceb614bd..ea2e0b658 100644 --- a/src/Magnum/MeshTools/CompressIndices.h +++ b/src/Magnum/MeshTools/CompressIndices.h @@ -26,21 +26,65 @@ */ /** @file - * @brief Function @ref Magnum::MeshTools::compressIndices() + * @brief Function @ref Magnum::MeshTools::compressIndices(), @ref Magnum::MeshTools::compressIndicesAs() */ -#include -#include +#include +#include +#include #include "Magnum/Mesh.h" #include "Magnum/MeshTools/visibility.h" +#ifdef MAGNUM_BUILD_DEPRECATED +#include +#include +#endif + namespace Magnum { namespace MeshTools { +/** +@brief Compress an index array +@param indices Index array +@param atLeast Smallest allowed type +@return Compressed index array and corresponding type +@m_since_latest + +This function compresses @p indices to the smallest possible size. For example +when your indices have the maximum vertex index 463, it's wasteful to store +them in array of 32bit integers, array of 16bit integers is sufficient. The +@p atLeast parameter allows you to specify the smallest type to use and it +defaults to @ref MeshIndexType::UnsignedShort as 8-bit types are not friendly +to many GPUs (and for example unextended Vulkan or D3D12 don't even support +them). It's also possible to choose a type larger than the input type to +"inflate" an index buffer of a smaller type. + +Example usage: + +@snippet MagnumMeshTools-gl.cpp compressIndices +*/ +MAGNUM_MESHTOOLS_EXPORT std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, MeshIndexType atLeast = MeshIndexType::UnsignedShort); + +/** +@overload +@m_since_latest +*/ +MAGNUM_MESHTOOLS_EXPORT std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, MeshIndexType atLeast = MeshIndexType::UnsignedShort); + +/** +@overload +@m_since_latest +*/ +MAGNUM_MESHTOOLS_EXPORT std::pair, MeshIndexType> compressIndices(const Containers::StridedArrayView1D& indices, MeshIndexType atLeast = MeshIndexType::UnsignedShort); + +#ifdef MAGNUM_BUILD_DEPRECATED /** @brief Compress vertex indices @param indices Index array @return Index range, type and compressed index array +@m_deprecated_since_latest Use @ref compressIndices(const Containers::StridedArrayView1D&, MeshIndexType) + instead. The index range isn't returned anymore, use @ref Math::minmax(const Containers::StridedArrayView1D&) + to get it if needed. This function takes index array and outputs them compressed to smallest possible size. For example when your indices have maximum number 463, it's @@ -49,11 +93,12 @@ sufficient. Example usage: -@snippet MagnumMeshTools-gl.cpp compressIndices +@snippet MagnumMeshTools-gl.cpp compressIndices-stl @see @ref compressIndicesAs() */ -std::tuple, MeshIndexType, UnsignedInt, UnsignedInt> MAGNUM_MESHTOOLS_EXPORT compressIndices(const std::vector& indices); +CORRADE_DEPRECATED("use compressIndices(const Containers::StridedArrayView1D&, MeshIndexType) instead") MAGNUM_MESHTOOLS_EXPORT std::tuple, MeshIndexType, UnsignedInt, UnsignedInt> compressIndices(const std::vector& indices); +#endif /** @brief Compress vertex indices as given type diff --git a/src/Magnum/MeshTools/Test/CompressIndicesTest.cpp b/src/Magnum/MeshTools/Test/CompressIndicesTest.cpp index 15889b561..0ed27bff5 100644 --- a/src/Magnum/MeshTools/Test/CompressIndicesTest.cpp +++ b/src/Magnum/MeshTools/Test/CompressIndicesTest.cpp @@ -24,12 +24,15 @@ */ #include +#include #include +#include #include #include #include #include +#include "Magnum/Math/TypeTraits.h" #include "Magnum/MeshTools/CompressIndices.h" namespace Magnum { namespace MeshTools { namespace Test { namespace { @@ -37,42 +40,91 @@ namespace Magnum { namespace MeshTools { namespace Test { namespace { struct CompressIndicesTest: TestSuite::Tester { explicit CompressIndicesTest(); - void compressChar(); - void compressShort(); - void compressInt(); + template void compressUnsignedByte(); + template void compressUnsignedShort(); + template void compressUnsignedInt(); + void compressUnsignedByteInflateToShort(); + #ifdef MAGNUM_BUILD_DEPRECATED + void compressDeprecated(); + #endif void compressAsShort(); }; CompressIndicesTest::CompressIndicesTest() { - addTests({&CompressIndicesTest::compressChar, - &CompressIndicesTest::compressShort, - &CompressIndicesTest::compressInt, + addTests({&CompressIndicesTest::compressUnsignedByte, + &CompressIndicesTest::compressUnsignedByte, + &CompressIndicesTest::compressUnsignedByte, + &CompressIndicesTest::compressUnsignedShort, + &CompressIndicesTest::compressUnsignedShort, + &CompressIndicesTest::compressUnsignedInt, + &CompressIndicesTest::compressUnsignedByteInflateToShort, + + #ifdef MAGNUM_BUILD_DEPRECATED + &CompressIndicesTest::compressDeprecated, + #endif &CompressIndicesTest::compressAsShort}); } -void CompressIndicesTest::compressChar() { - Containers::Array data; - MeshIndexType type; - UnsignedInt start, end; - std::tie(data, type, start, end) = MeshTools::compressIndices( - std::vector{1, 2, 3, 0, 4}); +template void CompressIndicesTest::compressUnsignedByte() { + setTestCaseTemplateName(Math::TypeTraits::name()); - CORRADE_COMPARE(start, 0); - CORRADE_COMPARE(end, 4); - CORRADE_COMPARE(type, MeshIndexType::UnsignedByte); - CORRADE_COMPARE_AS(Containers::arrayCast(data), + const T indices[]{1, 2, 3, 0, 4}; + /* By default it has 16-byte type as minimum, override */ + std::pair, MeshIndexType> out = + compressIndices(indices, MeshIndexType::UnsignedByte); + + CORRADE_COMPARE(out.second, MeshIndexType::UnsignedByte); + CORRADE_COMPARE_AS(Containers::arrayCast(out.first), Containers::arrayView({1, 2, 3, 0, 4}), TestSuite::Compare::Container); } -void CompressIndicesTest::compressShort() { +template void CompressIndicesTest::compressUnsignedShort() { + setTestCaseTemplateName(Math::TypeTraits::name()); + + const T indices[]{1, 256, 0, 5}; + std::pair, MeshIndexType> out = compressIndices(indices); + + CORRADE_COMPARE(out.second, MeshIndexType::UnsignedShort); + CORRADE_COMPARE_AS(Containers::arrayCast(out.first), + Containers::arrayView({1, 256, 0, 5}), + TestSuite::Compare::Container); +} + +template void CompressIndicesTest::compressUnsignedInt() { + setTestCaseTemplateName(Math::TypeTraits::name()); + + const T indices[]{65536, 3, 2}; + std::pair, MeshIndexType> out = compressIndices(indices); + + CORRADE_COMPARE(out.second, MeshIndexType::UnsignedInt); + CORRADE_COMPARE_AS(Containers::arrayCast(out.first), + Containers::arrayView({65536, 3, 2}), + TestSuite::Compare::Container); +} + +void CompressIndicesTest::compressUnsignedByteInflateToShort() { + const UnsignedByte indices[]{1, 2, 3, 0, 4}; + /* That's the default */ + std::pair, MeshIndexType> out = compressIndices(indices); + + CORRADE_COMPARE(out.second, MeshIndexType::UnsignedShort); + CORRADE_COMPARE_AS(Containers::arrayCast(out.first), + Containers::arrayView({1, 2, 3, 0, 4}), + TestSuite::Compare::Container); +} + +#ifdef MAGNUM_BUILD_DEPRECATED +void CompressIndicesTest::compressDeprecated() { Containers::Array data; MeshIndexType type; UnsignedInt start, end; + CORRADE_IGNORE_DEPRECATED_PUSH std::tie(data, type, start, end) = MeshTools::compressIndices( std::vector{1, 256, 0, 5}); + CORRADE_IGNORE_DEPRECATED_POP CORRADE_COMPARE(start, 0); CORRADE_COMPARE(end, 256); @@ -81,21 +133,7 @@ void CompressIndicesTest::compressShort() { Containers::arrayView({1, 256, 0, 5}), TestSuite::Compare::Container); } - -void CompressIndicesTest::compressInt() { - Containers::Array data; - MeshIndexType type; - UnsignedInt start, end; - std::tie(data, type, start, end) = MeshTools::compressIndices( - std::vector{65536, 3, 2}); - - CORRADE_COMPARE(start, 2); - CORRADE_COMPARE(end, 65536); - CORRADE_COMPARE(type, MeshIndexType::UnsignedInt); - CORRADE_COMPARE_AS(Containers::arrayCast(data), - Containers::arrayView({65536, 3, 2}), - TestSuite::Compare::Container); -} +#endif void CompressIndicesTest::compressAsShort() { CORRADE_COMPARE_AS(MeshTools::compressIndicesAs({123, 456}),