MeshTools: add a MeshData interleave() overload for loose arrays.

It went for so long without such a thing, causing so much suffering.

doc/changelog.dox

@@ -318,6 +318,9 @@ See also:
 -   New @ref MeshTools::compileLines() utility for creating meshes compatible
     with the new @ref Shaders::LineGL. See also
     [mosra/magnum#601](https://github.com/mosra/magnum/pull/601).
+-   New @ref MeshTools::interleave(MeshPrimitive, const Trade::MeshIndexData&, Containers::ArrayView<const Trade::MeshAttributeData>)
+    overload for conveniently creating an interleaved mesh out of loose index
+    and attribute arrays
 
 @subsubsection changelog-latest-new-platform Platform libraries
 

doc/snippets/MeshTools.cpp

@@ -154,6 +154,21 @@ auto data = MeshTools::interleave(positions, weights, 2, vertexColors, 1);
 /* [interleave2] */
 }
 
+{
+/* [interleave-meshdata] */
+Containers::ArrayView<const UnsignedInt> indices = DOXYGEN_ELLIPSIS({});
+Containers::ArrayView<const Vector3> positions = DOXYGEN_ELLIPSIS({});
+Containers::ArrayView<const Vector3> normals = DOXYGEN_ELLIPSIS({});
+
+Trade::MeshData mesh = MeshTools::interleave(
+    MeshPrimitive::Triangles,
+    Trade::MeshIndexData{indices}, {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, positions},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Normal, normals},
+    });
+/* [interleave-meshdata] */
+}
+
 {
 Trade::MeshData data{MeshPrimitive::Lines, 0};
 UnsignedInt vertexCount{};

src/Magnum/MeshTools/Interleave.cpp

@@ -350,7 +350,7 @@ Trade::MeshData interleave(Trade::MeshData&& mesh, const Containers::ArrayView<c
                 "MeshTools::interleave(): extra attribute" << i << "is offset-only",
                 (Trade::MeshData{MeshPrimitive::Triangles, 0}));
 
-            /* Copy the attribute in, if it is non-empty, otherwise keep the
+            /* Copy the attribute in, if it is non-null, otherwise keep the
                memory uninitialized */
             if(extra[i].data()) {
                 CORRADE_ASSERT(extra[i].data().size() == vertexCount,
@@ -387,4 +387,52 @@ Trade::MeshData interleave(const Trade::MeshData& mesh, const std::initializer_l
     return interleave(Utility::move(mesh), Containers::arrayView(extra), flags);
 }
 
+Trade::MeshData interleave(const MeshPrimitive primitive, const Trade::MeshIndexData& indices, const Containers::ArrayView<const Trade::MeshAttributeData> attributes) {
+    /* Get vertex count from the first non-padding attribute. Checking that all
+       arrays have the same size etc is done in the delegated-to function. */
+    UnsignedInt vertexCount = ~UnsignedInt{};
+    for(const Trade::MeshAttributeData& attribute: attributes) {
+        if(attribute.format() != VertexFormat{}) {
+            vertexCount = attribute.data().size();
+            break;
+        }
+    }
+    CORRADE_ASSERT(vertexCount != ~UnsignedInt{},
+        "MeshTools::interleave(): only padding found among" << attributes.size() << "attributes, can't infer vertex count",
+        (Trade::MeshData{MeshPrimitive::Triangles, 0}));
+
+    /* Check that indices aren't implementation-specific. The assert inside the
+       delegated-to interleave() suggests PreserveStridedIndices, which would
+       be confusing as here it's no such argument */
+    CORRADE_ASSERT(indices.type() == MeshIndexType{} || !isMeshIndexTypeImplementationSpecific(indices.type()),
+        "MeshTools::interleave(): implementation-specific index type" << Debug::hex << meshIndexTypeUnwrap(indices.type()),
+        (Trade::MeshData{MeshPrimitive{}, 0}));
+
+    return interleave(Trade::MeshData{primitive,
+        /* Pass indices as non-owned so they get copied. We can say the index
+           data is the whole memory as it's not going to get used because the
+           indices get tightly packed. */
+        {},
+        indices.type() == MeshIndexType{} ?
+            nullptr : Containers::ArrayView<char>{nullptr, ~std::size_t{}},
+        indices,
+        vertexCount},
+        attributes,
+        /* Explicitly *not* PreserveStridedIndices to ensure the indices get
+           tightly packed */
+        InterleaveFlags{});
+}
+
+Trade::MeshData interleave(const MeshPrimitive primitive, const Trade::MeshIndexData& indices, const std::initializer_list<Trade::MeshAttributeData> attributes) {
+    return interleave(primitive, indices, Containers::arrayView(attributes));
+}
+
+Trade::MeshData interleave(const MeshPrimitive primitive, const Containers::ArrayView<const Trade::MeshAttributeData> attributes) {
+    return interleave(primitive, Trade::MeshIndexData{}, attributes);
+}
+
+Trade::MeshData interleave(const MeshPrimitive primitive, const std::initializer_list<Trade::MeshAttributeData> attributes) {
+    return interleave(primitive, Containers::arrayView(attributes));
+}
+
 }}

src/Magnum/MeshTools/Interleave.h

@@ -303,16 +303,18 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleavedLayout(Trade::MeshData&& mesh
 
 Returns a copy of @p mesh with all attributes interleaved. The @p extra
 attributes, if any, are interleaved together with existing attributes (or, in
-case the attribute view is empty, only the corresponding space for given
+case the attribute view is null, only the corresponding space for given
-attribute type is reserved, with memory left uninitialized). The data layouting
+attribute type is reserved, with memory left uninitialized). See the
-is done by @ref interleavedLayout() with the @p flags parameter propagated to
+@ref interleave(MeshPrimitive, const Trade::MeshIndexData&, Containers::ArrayView<const Trade::MeshAttributeData>)
-it, see its documentation for detailed behavior description. Note that
+overload if you only have loose attributes and want to interleave them
-offset-only @ref Trade::MeshAttributeData instances are not supported in the
+together.
-@p extra array.
+
-
+The data layouting is done by @ref interleavedLayout() with the @p flags
-Indices (if any) are kept as-is only if they're tightly packed and not with an
+parameter propagated to it, see its documentation for detailed behavior
-implementation-specific type. Otherwise the behavior depends on presence of
+description. Note that offset-only @ref Trade::MeshAttributeData instances are
-@ref InterleaveFlag::PreserveStridedIndices.
+not supported in the @p extra array. Indices (if any) are kept as-is only if
+they're tightly packed and not with an implementation-specific type. Otherwise
+the behavior depends on presence of @ref InterleaveFlag::PreserveStridedIndices.
 
 Expects that each attribute in @p extra has either the same amount of elements
 as @p mesh vertex count or has none. This function will unconditionally make a
@@ -356,6 +358,59 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& mesh, Conta
  */
 MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(Trade::MeshData&& mesh, std::initializer_list<Trade::MeshAttributeData> extra, InterleaveFlags flags = InterleaveFlag::PreserveInterleavedAttributes);
 
+/**
+@brief Create an indexed interleaved mesh
+@m_since_latest
+
+Creates a mesh data instance out of given indices and attributes. Usage
+example:
+
+@snippet MeshTools.cpp interleave-meshdata
+
+The @ref interleave(MeshPrimitive, Containers::ArrayView<const Trade::MeshAttributeData>)
+overload creates a non-indexed mesh. This function is a convenience shorthand
+for calling @ref interleave(const Trade::MeshData&, Containers::ArrayView<const Trade::MeshAttributeData>, InterleaveFlags)
+with a @ref Trade::MeshData instance created out of @p primitive and
+@p indices and vertex count matching @p attributes. If a particular attribute
+view is null, only the corresponding space for given attribute type is
+reserved, with memory left uninitialized. The attribute can also be a are a
+padding value created with @ref Trade::MeshAttributeData::MeshAttributeData(Int),
+see documentation of @ref interleavedLayout() for an example snippet.
+
+Expects that @p attributes all have the same amount of elements or have none,
+there's at least one non-padding attribute, none of them have an
+implementation-specific format and none of them are offset-only
+@ref Trade::MeshAttributeData instances. The @p indices, if present, are
+assumed to not have an implementation-specific type. Returned instance vertex
+and index data flags have both @ref Trade::DataFlag::Mutable and
+@ref Trade::DataFlag::Owned, so mutable attribute access is guaranteed.
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(MeshPrimitive primitive, const Trade::MeshIndexData& indices, Containers::ArrayView<const Trade::MeshAttributeData> attributes);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(MeshPrimitive primitive, const Trade::MeshIndexData& indices, std::initializer_list<Trade::MeshAttributeData> attributes);
+
+/**
+@brief Create a non-indexed interleaved mesh
+@m_since_latest
+
+Same as calling @ref interleave(MeshPrimitive, const Trade::MeshIndexData&, Containers::ArrayView<const Trade::MeshAttributeData>)
+with a default-constructed @ref Trade::MeshIndexData instance. See its
+documentation for more information and a usage example.
+*/
+/* No InterleaveFlags as there's no index array for PreserveStridedIndices,
+   and PreserveInterleavedAttributes makes sense only for an input mesh */
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(MeshPrimitive primitive, Containers::ArrayView<const Trade::MeshAttributeData> attributes);
+
+/**
+@overload
+@m_since_latest
+*/
+MAGNUM_MESHTOOLS_EXPORT Trade::MeshData interleave(MeshPrimitive primitive, std::initializer_list<Trade::MeshAttributeData> attributes);
+
 namespace Implementation {
 
 /* Used internally by interleavedLayout() and concatenate() */

src/Magnum/MeshTools/Test/InterleaveTest.cpp

@@ -27,12 +27,14 @@
 #include <sstream>
 #include <vector>
 #include <Corrade/Containers/Optional.h>
+#include <Corrade/Containers/StringStl.h> /** @todo remove once Debug is stream-free */
 #include <Corrade/TestSuite/Tester.h>
 #include <Corrade/TestSuite/Compare/Container.h>
+#include <Corrade/TestSuite/Compare/String.h>
 #include <Corrade/Utility/Algorithms.h>
 #include <Corrade/Utility/Endianness.h>
 #include <Corrade/Utility/Debug.h>
-#include <Corrade/Utility/DebugStl.h>
+#include <Corrade/Utility/DebugStl.h> /** @todo remove once Debug is stream-free */
 
 #include "Magnum/Math/Vector3.h"
 #include "Magnum/MeshTools/Interleave.h"
@@ -99,6 +101,10 @@ struct InterleaveTest: Corrade::TestSuite::Tester {
     void interleaveMeshDataAlreadyInterleavedMoveIndices();
     void interleaveMeshDataAlreadyInterleavedMoveNonOwned();
     void interleaveMeshDataNothing();
+
+    void interleaveMeshDataLooseAttributes();
+    void interleaveMeshDataLooseAttributesIndexed();
+    void interleaveMeshDataLooseAttributesInvalid();
 };
 
 const struct {
@@ -191,7 +197,11 @@ InterleaveTest::InterleaveTest() {
         Containers::arraySize(StridedIndicesData));
 
     addTests({&InterleaveTest::interleaveMeshDataAlreadyInterleavedMoveNonOwned,
-              &InterleaveTest::interleaveMeshDataNothing});
+              &InterleaveTest::interleaveMeshDataNothing,
+
+              &InterleaveTest::interleaveMeshDataLooseAttributes,
+              &InterleaveTest::interleaveMeshDataLooseAttributesIndexed,
+              &InterleaveTest::interleaveMeshDataLooseAttributesInvalid});
 }
 
 void InterleaveTest::attributeCount() {
@@ -1343,6 +1353,7 @@ void InterleaveTest::interleaveMeshDataExtraOriginalEmpty() {
     CORRADE_VERIFY(!interleaved.isIndexed());
     /* No reason to not be like this */
     CORRADE_COMPARE(interleaved.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
+    CORRADE_COMPARE(interleaved.attributeStride(0), sizeof(Vector2) + 4);
     CORRADE_COMPARE(interleaved.attributeCount(), 1);
     CORRADE_COMPARE_AS(interleaved.attribute<Vector2>(Trade::MeshAttribute::Position),
         Containers::stridedArrayView(positions),
@@ -1544,6 +1555,97 @@ void InterleaveTest::interleaveMeshDataNothing() {
     CORRADE_COMPARE(interleaved.vertexData().size(), 0);
 }
 
+void InterleaveTest::interleaveMeshDataLooseAttributes() {
+    /* Same as interleaveMeshDataExtraOriginalEmpty(), but testing the
+       convenience overload instead */
+
+    Vector2 positions[]{{1.3f, 0.3f}, {0.87f, 1.1f}, {1.0f, -0.5f}};
+    Trade::MeshData interleaved = MeshTools::interleave(MeshPrimitive::TriangleFan, {
+        Trade::MeshAttributeData{4},
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::arrayView(positions)}
+    });
+
+    CORRADE_VERIFY(MeshTools::isInterleaved(interleaved));
+    CORRADE_COMPARE(interleaved.primitive(), MeshPrimitive::TriangleFan);
+    CORRADE_VERIFY(!interleaved.isIndexed());
+    /* No reason to not be like this */
+    CORRADE_COMPARE(interleaved.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
+    CORRADE_COMPARE(interleaved.attributeCount(), 1);
+    CORRADE_COMPARE(interleaved.attributeStride(0), sizeof(Vector2) + 4);
+    CORRADE_COMPARE_AS(interleaved.attribute<Vector2>(Trade::MeshAttribute::Position),
+        Containers::stridedArrayView(positions),
+        TestSuite::Compare::Container);
+}
+
+void InterleaveTest::interleaveMeshDataLooseAttributesIndexed() {
+    /* Same as interleaveMeshDataExtraOriginalEmpty(), but testing the
+       convenience overload instead */
+
+    struct Index {
+        UnsignedShort index;
+        Short dummy; /* MSVC 2015 doesn't like Short:16 in local structs */
+    } indices[]{{3, 0}, {6, 0}, {7, 0}, {9, 0}};
+    Vector2 positions[]{{1.3f, 0.3f}, {0.87f, 1.1f}, {1.0f, -0.5f}};
+    Trade::MeshData interleaved = MeshTools::interleave(
+        MeshPrimitive::TriangleStrip,
+        Trade::MeshIndexData{Containers::stridedArrayView(indices).slice(&Index::index)}, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::arrayView(positions)},
+            Trade::MeshAttributeData{4}
+        });
+
+    CORRADE_VERIFY(MeshTools::isInterleaved(interleaved));
+    CORRADE_COMPARE(interleaved.primitive(), MeshPrimitive::TriangleStrip);
+
+    CORRADE_VERIFY(interleaved.isIndexed());
+    CORRADE_COMPARE(interleaved.indexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
+    /* Indices get copied and made tightly packed */
+    CORRADE_COMPARE(interleaved.indexStride(), 2);
+    CORRADE_COMPARE_AS(interleaved.indices<UnsignedShort>(),
+        Containers::stridedArrayView(indices).slice(&Index::index),
+        TestSuite::Compare::Container);
+
+    CORRADE_COMPARE(interleaved.vertexDataFlags(), Trade::DataFlag::Mutable|Trade::DataFlag::Owned);
+    CORRADE_COMPARE(interleaved.attributeCount(), 1);
+    CORRADE_COMPARE(interleaved.attributeStride(0), sizeof(Vector2) + 4);
+    CORRADE_COMPARE_AS(interleaved.attribute<Vector2>(Trade::MeshAttribute::Position),
+        Containers::stridedArrayView(positions),
+        TestSuite::Compare::Container);
+}
+
+void InterleaveTest::interleaveMeshDataLooseAttributesInvalid() {
+    CORRADE_SKIP_IF_NO_ASSERT();
+
+    UnsignedShort indices[]{3, 6, 7, 9};
+
+    /* Null views are fine */
+    CORRADE_COMPARE(MeshTools::interleave(MeshPrimitive::Triangles, {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::ArrayView<Vector3>{nullptr, 3}}
+    }).vertexCount(), 3);
+    CORRADE_COMPARE(MeshTools::interleave(MeshPrimitive::Triangles, {
+        Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::ArrayView<Vector3>{nullptr, 0}}
+    }).vertexCount(), 0);
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    MeshTools::interleave(MeshPrimitive::Triangles,
+        Trade::MeshIndexData{indices}, {
+            Trade::MeshAttributeData{4}
+        });
+    MeshTools::interleave(MeshPrimitive::Triangles, {
+            Trade::MeshAttributeData{4},
+            Trade::MeshAttributeData{4}
+        });
+    MeshTools::interleave(MeshPrimitive::Triangles,
+        Trade::MeshIndexData{meshIndexTypeWrap(0xcece), Containers::stridedArrayView(indices)}, {
+            Trade::MeshAttributeData{Trade::MeshAttribute::Position, Containers::ArrayView<Vector3>{nullptr, 3}}
+        });
+    CORRADE_COMPARE_AS(out.str(),
+        "MeshTools::interleave(): only padding found among 1 attributes, can't infer vertex count\n"
+        "MeshTools::interleave(): only padding found among 2 attributes, can't infer vertex count\n"
+        "MeshTools::interleave(): implementation-specific index type 0xcece\n",
+        TestSuite::Compare::String);
+}
+
 }}}}
 
 CORRADE_TEST_MAIN(Magnum::MeshTools::Test::InterleaveTest)