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

@@ -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<const UnsignedInt>&, 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(),

doc/snippets/MagnumMeshTools-gl.cpp

@@ -23,6 +23,9 @@
     DEALINGS IN THE SOFTWARE.
 */
 
+#include <tuple> /* for std::tie() :( */
+#include <Corrade/Containers/StridedArrayView.h>
+
 #include "Magnum/GL/Buffer.h"
 #include "Magnum/GL/Mesh.h"
 #include "Magnum/Math/Vector3.h"
@@ -35,6 +38,25 @@ int main() {
 
 {
 /* [compressIndices] */
+Containers::Array<UnsignedInt> indices;
+
+Containers::Array<char> 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<UnsignedInt> indices;
 
 Containers::Array<char> 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 {

src/Magnum/MeshTools/CompressIndices.cpp

@@ -35,7 +35,9 @@ namespace Magnum { namespace MeshTools {
 
 namespace {
 
-template<class T> inline Containers::Array<char> compress(const std::vector<UnsignedInt>& indices) {
+template<class T, class U> inline Containers::Array<char> compress(const Containers::StridedArrayView1D<const U>& indices) {
+    /* Can't use Utility::copy() here because we're copying from a larger type
+       to a smaller one */
     Containers::Array<char> buffer(indices.size()*sizeof(T));
     for(std::size_t i = 0; i != indices.size(); ++i) {
         T index = static_cast<T>(indices[i]);
@@ -45,34 +47,56 @@ template<class T> inline Containers::Array<char> compress(const std::vector<Unsi
     return buffer;
 }
 
+template<class T> std::pair<Containers::Array<char>, MeshIndexType> compressIndicesImplementation(const Containers::StridedArrayView1D<const T>& indices, const MeshIndexType atLeast) {
+    const T max = Math::max(indices);
+    Containers::Array<char> 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<UnsignedByte>(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<UnsignedShort>(indices);
+        type = MeshIndexType::UnsignedShort;
+
+    /* Otherwise pack into 32 */
+    } else {
+        out = compress<UnsignedInt>(indices);
+        type = MeshIndexType::UnsignedInt;
+    }
+
+    return {std::move(out), type};
+}
+
+}
+
+std::pair<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedInt>& indices, const MeshIndexType atLeast) {
+    return compressIndicesImplementation(indices, atLeast);
 }
 
+std::pair<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedShort>& indices, const MeshIndexType atLeast) {
+    return compressIndicesImplementation(indices, atLeast);
+}
+
+std::pair<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedByte>& indices, const MeshIndexType atLeast) {
+    return compressIndicesImplementation(indices, atLeast);
+}
+
+#ifdef MAGNUM_BUILD_DEPRECATED
 std::tuple<Containers::Array<char>, MeshIndexType, UnsignedInt, UnsignedInt> compressIndices(const std::vector<UnsignedInt>& indices) {
     /** @todo Performance hint when range can be represented by smaller value? */
     const auto minmax = Math::minmax(indices);
     Containers::Array<char> data;
     MeshIndexType type;
-    switch(Math::log(256, minmax.second)) {
+    std::tie(data, type) = compressIndices(indices, MeshIndexType::UnsignedByte);
-        case 0:
-            data = compress<UnsignedByte>(indices);
-            type = MeshIndexType::UnsignedByte;
-            break;
-        case 1:
-            data = compress<UnsignedShort>(indices);
-            type = MeshIndexType::UnsignedShort;
-            break;
-        case 2:
-        case 3:
-            data = compress<UnsignedInt>(indices);
-            type = MeshIndexType::UnsignedInt;
-            break;
-
-        default:
-            CORRADE_ASSERT(false, "MeshTools::compressIndices(): no type able to index" << minmax.second << "elements.", {}); /* LCOV_EXCL_LINE */
-    }
-
     return std::make_tuple(std::move(data), type, minmax.first, minmax.second);
 }
+#endif
 
 template<class T> Containers::Array<T> compressIndicesAs(const std::vector<UnsignedInt>& indices) {
     #if !defined(CORRADE_NO_ASSERT) || defined(CORRADE_GRACEFUL_ASSERT)

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 <tuple>
+#include <utility>
-#include <vector>
+#include <Corrade/Containers/Containers.h>
+#include <Corrade/Utility/StlForwardVector.h>
 
 #include "Magnum/Mesh.h"
 #include "Magnum/MeshTools/visibility.h"
 
+#ifdef MAGNUM_BUILD_DEPRECATED
+#include <tuple>
+#include <Corrade/Utility/Macros.h>
+#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<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedInt>& indices, MeshIndexType atLeast = MeshIndexType::UnsignedShort);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_MESHTOOLS_EXPORT std::pair<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedShort>& indices, MeshIndexType atLeast = MeshIndexType::UnsignedShort);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_MESHTOOLS_EXPORT std::pair<Containers::Array<char>, MeshIndexType> compressIndices(const Containers::StridedArrayView1D<const UnsignedByte>& 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<const UnsignedInt>&, MeshIndexType)
+    instead. The index range isn't returned anymore, use @ref Math::minmax(const Containers::StridedArrayView1D<const T>&)
+    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<Containers::Array<char>, MeshIndexType, UnsignedInt, UnsignedInt> MAGNUM_MESHTOOLS_EXPORT compressIndices(const std::vector<UnsignedInt>& indices);
+CORRADE_DEPRECATED("use compressIndices(const Containers::StridedArrayView1D<const UnsignedInt>&, MeshIndexType) instead") MAGNUM_MESHTOOLS_EXPORT std::tuple<Containers::Array<char>, MeshIndexType, UnsignedInt, UnsignedInt> compressIndices(const std::vector<UnsignedInt>& indices);
+#endif
 
 /**
 @brief Compress vertex indices as given type

src/Magnum/MeshTools/Test/CompressIndicesTest.cpp

@@ -24,12 +24,15 @@
 */
 
 #include <sstream>
+#include <vector>
 #include <Corrade/Containers/Array.h>
+#include <Corrade/Containers/StridedArrayView.h>
 #include <Corrade/TestSuite/Tester.h>
 #include <Corrade/TestSuite/Compare/Container.h>
 #include <Corrade/Utility/DebugStl.h>
 #include <Corrade/Utility/Endianness.h>
 
+#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();
+    template<class T> void compressUnsignedByte();
-    void compressShort();
+    template<class T> void compressUnsignedShort();
-    void compressInt();
+    template<class T> void compressUnsignedInt();
+    void compressUnsignedByteInflateToShort();
+    #ifdef MAGNUM_BUILD_DEPRECATED
+    void compressDeprecated();
+    #endif
 
     void compressAsShort();
 };
 
 CompressIndicesTest::CompressIndicesTest() {
-    addTests({&CompressIndicesTest::compressChar,
+    addTests({&CompressIndicesTest::compressUnsignedByte<UnsignedByte>,
-              &CompressIndicesTest::compressShort,
+              &CompressIndicesTest::compressUnsignedByte<UnsignedShort>,
-              &CompressIndicesTest::compressInt,
+              &CompressIndicesTest::compressUnsignedByte<UnsignedInt>,
+              &CompressIndicesTest::compressUnsignedShort<UnsignedShort>,
+              &CompressIndicesTest::compressUnsignedShort<UnsignedInt>,
+              &CompressIndicesTest::compressUnsignedInt<UnsignedInt>,
+              &CompressIndicesTest::compressUnsignedByteInflateToShort,
+
+              #ifdef MAGNUM_BUILD_DEPRECATED
+              &CompressIndicesTest::compressDeprecated,
+              #endif
 
               &CompressIndicesTest::compressAsShort});
 }
 
-void CompressIndicesTest::compressChar() {
+template<class T> void CompressIndicesTest::compressUnsignedByte() {
-    Containers::Array<char> data;
+    setTestCaseTemplateName(Math::TypeTraits<T>::name());
-    MeshIndexType type;
-    UnsignedInt start, end;
-    std::tie(data, type, start, end) = MeshTools::compressIndices(
-        std::vector<UnsignedInt>{1, 2, 3, 0, 4});
 
-    CORRADE_COMPARE(start, 0);
+    const T indices[]{1, 2, 3, 0, 4};
-    CORRADE_COMPARE(end, 4);
+    /* By default it has 16-byte type as minimum, override */
-    CORRADE_COMPARE(type, MeshIndexType::UnsignedByte);
+    std::pair<Containers::Array<char>, MeshIndexType> out =
-    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedByte>(data),
+        compressIndices(indices, MeshIndexType::UnsignedByte);
+
+    CORRADE_COMPARE(out.second, MeshIndexType::UnsignedByte);
+    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedByte>(out.first),
         Containers::arrayView<UnsignedByte>({1, 2, 3, 0, 4}),
         TestSuite::Compare::Container);
 }
 
-void CompressIndicesTest::compressShort() {
+template<class T> void CompressIndicesTest::compressUnsignedShort() {
+    setTestCaseTemplateName(Math::TypeTraits<T>::name());
+
+    const T indices[]{1, 256, 0, 5};
+    std::pair<Containers::Array<char>, MeshIndexType> out = compressIndices(indices);
+
+    CORRADE_COMPARE(out.second, MeshIndexType::UnsignedShort);
+    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedShort>(out.first),
+        Containers::arrayView<UnsignedShort>({1, 256, 0, 5}),
+        TestSuite::Compare::Container);
+}
+
+template<class T> void CompressIndicesTest::compressUnsignedInt() {
+    setTestCaseTemplateName(Math::TypeTraits<T>::name());
+
+    const T indices[]{65536, 3, 2};
+    std::pair<Containers::Array<char>, MeshIndexType> out = compressIndices(indices);
+
+    CORRADE_COMPARE(out.second, MeshIndexType::UnsignedInt);
+    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedInt>(out.first),
+        Containers::arrayView<UnsignedInt>({65536, 3, 2}),
+        TestSuite::Compare::Container);
+}
+
+void CompressIndicesTest::compressUnsignedByteInflateToShort() {
+    const UnsignedByte indices[]{1, 2, 3, 0, 4};
+    /* That's the default */
+    std::pair<Containers::Array<char>, MeshIndexType> out = compressIndices(indices);
+
+    CORRADE_COMPARE(out.second, MeshIndexType::UnsignedShort);
+    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedShort>(out.first),
+        Containers::arrayView<UnsignedShort>({1, 2, 3, 0, 4}),
+        TestSuite::Compare::Container);
+}
+
+#ifdef MAGNUM_BUILD_DEPRECATED
+void CompressIndicesTest::compressDeprecated() {
     Containers::Array<char> data;
     MeshIndexType type;
     UnsignedInt start, end;
+    CORRADE_IGNORE_DEPRECATED_PUSH
     std::tie(data, type, start, end) = MeshTools::compressIndices(
         std::vector<UnsignedInt>{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<UnsignedShort>({1, 256, 0, 5}),
         TestSuite::Compare::Container);
 }
-
+#endif
-void CompressIndicesTest::compressInt() {
-    Containers::Array<char> data;
-    MeshIndexType type;
-    UnsignedInt start, end;
-    std::tie(data, type, start, end) = MeshTools::compressIndices(
-        std::vector<UnsignedInt>{65536, 3, 2});
-
-    CORRADE_COMPARE(start, 2);
-    CORRADE_COMPARE(end, 65536);
-    CORRADE_COMPARE(type, MeshIndexType::UnsignedInt);
-    CORRADE_COMPARE_AS(Containers::arrayCast<UnsignedInt>(data),
-        Containers::arrayView<UnsignedInt>({65536, 3, 2}),
-        TestSuite::Compare::Container);
-}
 
 void CompressIndicesTest::compressAsShort() {
     CORRADE_COMPARE_AS(MeshTools::compressIndicesAs<UnsignedShort>({123, 456}),