ic
/
magnum
mirror of https://github.com/mosra/magnum.git
6
0
Fork 0
Code Issues Projects Releases Wiki Activity
Browse Source

MeshTools: change concatenate() to do only what it's designed for. 

This is a breaking change that changes the signature, sorry -- if you
were using concatenate() for mesh concatenation before, enjoy the new
less strange signature, if you were using it for making the mesh owned
before (which was a strange and not very well thought out use case),
please use the recently added owned() instead. I thought about adding an
overload for backwards compatibility, but it would need to allocate to
work. This way with the breakage it's ensured you actually change to the
right API.

This also cleans up a lot of ugly code in the internals and resolves one
XFAIL in removeDuplicates().
pull/454/head
Vladimír Vondruš 6 years ago
parent
fdb754bc27
commit
10f29d2c81
6 changed files with 93 additions and 163 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 9
      doc/snippets/MagnumMeshTools.cpp
  2. 85
      src/Magnum/MeshTools/Concatenate.cpp
  3. 46
      src/Magnum/MeshTools/Concatenate.h
  4. 20
      src/Magnum/MeshTools/RemoveDuplicates.cpp
  5. 57
      src/Magnum/MeshTools/Test/ConcatenateTest.cpp
  6. 39
      src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp

9
doc/snippets/MagnumMeshTools.cpp
Unescape View File

@ -80,15 +80,6 @@ std::pair<Containers::Array<char>, MeshIndexType> result =
/* [compressIndices-offset] */ /* [compressIndices-offset] */
} }
{
/* [concatenate-make-mutable] */
/* Flip triangles on a cube primitive so it's counterclockwise from the inside
in order to render a cube map */
Trade::MeshData mesh = MeshTools::concatenate(Primitives::cubeSolid());
MeshTools::flipFaceWindingInPlace(mesh.mutableIndices());
/* [concatenate-make-mutable] */
}
#ifdef MAGNUM_BUILD_DEPRECATED #ifdef MAGNUM_BUILD_DEPRECATED
{ {
CORRADE_IGNORE_DEPRECATED_PUSH CORRADE_IGNORE_DEPRECATED_PUSH

85
src/Magnum/MeshTools/Concatenate.cpp
Unescape View File

@ -33,10 +33,10 @@ namespace Magnum { namespace MeshTools {
namespace Implementation { namespace Implementation {
std::pair<UnsignedInt, UnsignedInt> concatenateIndexVertexCount(const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next) { std::pair<UnsignedInt, UnsignedInt> concatenateIndexVertexCount(Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes) {
UnsignedInt indexCount = first.isIndexed() ? first.indexCount() : 0; UnsignedInt indexCount = 0;
UnsignedInt vertexCount = first.vertexCount(); UnsignedInt vertexCount = 0;
for(const Trade::MeshData& mesh: next) { for(const Trade::MeshData& mesh: meshes) {
/* If the mesh is indexed, add to index count. If this is the first /* If the mesh is indexed, add to index count. If this is the first
indexed mesh, all previous meshes will have a trivial index buffer indexed mesh, all previous meshes will have a trivial index buffer
generated for all their vertices */ generated for all their vertices */
@ -62,10 +62,9 @@ struct MeshAttributeHash: std::hash<typename std::underlying_type<Trade::MeshAtt
} }
}; };
Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedInt vertexCount, Containers::Array<char>&& vertexData, Containers::Array<Trade::MeshAttributeData>&& attributeData, const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next, const char* const assertPrefix, const std::size_t meshIndexOffset) { Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedInt vertexCount, Containers::Array<char>&& vertexData, Containers::Array<Trade::MeshAttributeData>&& attributeData, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes, const char* const assertPrefix) {
#ifdef CORRADE_NO_ASSERT #ifdef CORRADE_NO_ASSERT
static_cast<void>(assertPrefix); static_cast<void>(assertPrefix);
static_cast<void>(meshIndexOffset);
#endif #endif
/* Convert the attributes from offset-only and zero vertex count to /* Convert the attributes from offset-only and zero vertex count to
@ -83,17 +82,17 @@ Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedI
/** @todo delegate to `indexTriangleStrip()` (`duplicate*()`?) etc when /** @todo delegate to `indexTriangleStrip()` (`duplicate*()`?) etc when
those are done */ those are done */
CORRADE_ASSERT( CORRADE_ASSERT(
first.primitive() != MeshPrimitive::LineStrip && meshes.front()->primitive() != MeshPrimitive::LineStrip &&
first.primitive() != MeshPrimitive::LineLoop && meshes.front()->primitive() != MeshPrimitive::LineLoop &&
first.primitive() != MeshPrimitive::TriangleStrip && meshes.front()->primitive() != MeshPrimitive::TriangleStrip &&
first.primitive() != MeshPrimitive::TriangleFan, meshes.front()->primitive() != MeshPrimitive::TriangleFan,
assertPrefix << first.primitive() << "is not supported, turn it into a plain indexed mesh first", assertPrefix << meshes.front()->primitive() << "is not supported, turn it into a plain indexed mesh first",
(Trade::MeshData{MeshPrimitive{}, 0})); (Trade::MeshData{MeshPrimitive{}, 0}));
/* Populate the resulting instance with what we have. It'll be used below /* Populate the resulting instance with what we have. It'll be used below
for convenient access to vertex / index data */ for convenient access to vertex / index data */
auto indices = Containers::arrayCast<UnsignedInt>(indexData); auto indices = Containers::arrayCast<UnsignedInt>(indexData);
Trade::MeshData out{first.primitive(), Trade::MeshData out{meshes.front()->primitive(),
/* If the index array is empty, we're creating a non-indexed mesh (not /* If the index array is empty, we're creating a non-indexed mesh (not
an indexed mesh with zero indices) */ an indexed mesh with zero indices) */
std::move(indexData), indices.empty() ? std::move(indexData), indices.empty() ?
@ -107,19 +106,16 @@ Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedI
for(UnsignedInt i = 0; i != out.attributeCount(); ++i) for(UnsignedInt i = 0; i != out.attributeCount(); ++i)
attributeMap.emplace(out.attributeName(i), std::make_pair(i, false)); attributeMap.emplace(out.attributeName(i), std::make_pair(i, false));
/* Go through all meshes and put all attributes and index arrays together. /* Go through all meshes and put all attributes and index arrays together. */
The first mesh might get separately and thus can't be a part of the
view, so abuse the *defined* unsigned integer overflow to add it to the
loop. This probably breaks all coding guidelines on earth tho. */
std::size_t indexOffset = 0; std::size_t indexOffset = 0;
std::size_t vertexOffset = 0; std::size_t vertexOffset = 0;
for(std::size_t i = ~std::size_t{}; i != next.size(); ++i) { for(std::size_t i = 0; i != meshes.size(); ++i) {
const Trade::MeshData& mesh = i == ~std::size_t{} ? first : next[i].get(); const Trade::MeshData& mesh = meshes[i];
/* This won't fire for i == ~std::size_t{}, as that's where /* This won't fire for i == ~std::size_t{}, as that's where
out.primitive() comes from */ out.primitive() comes from */
CORRADE_ASSERT(mesh.primitive() == out.primitive(), CORRADE_ASSERT(mesh.primitive() == out.primitive(),
assertPrefix << "expected" << out.primitive() << "but got" << mesh.primitive() << "in mesh" << i + meshIndexOffset, assertPrefix << "expected" << out.primitive() << "but got" << mesh.primitive() << "in mesh" << i,
(Trade::MeshData{MeshPrimitive{}, 0})); (Trade::MeshData{MeshPrimitive{}, 0}));
/* If the mesh is indexed, copy the indices over, expanded to 32bit */ /* If the mesh is indexed, copy the indices over, expanded to 32bit */
@ -167,10 +163,10 @@ Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedI
/* Check format compatibility. This won't fire for i == /* Check format compatibility. This won't fire for i ==
~std::size_t{}, as that's where out.primitive() comes from */ ~std::size_t{}, as that's where out.primitive() comes from */
CORRADE_ASSERT(out.attributeFormat(dst) == mesh.attributeFormat(src), CORRADE_ASSERT(out.attributeFormat(dst) == mesh.attributeFormat(src),
assertPrefix << "expected" << out.attributeFormat(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeFormat(src) << "in mesh" << i + meshIndexOffset << "attribute" << src, assertPrefix << "expected" << out.attributeFormat(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeFormat(src) << "in mesh" << i << "attribute" << src,
(Trade::MeshData{MeshPrimitive{}, 0})); (Trade::MeshData{MeshPrimitive{}, 0}));
CORRADE_ASSERT(out.attributeArraySize(dst) == mesh.attributeArraySize(src), CORRADE_ASSERT(out.attributeArraySize(dst) == mesh.attributeArraySize(src),
assertPrefix << "expected array size" << out.attributeArraySize(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeArraySize(src) << "in mesh" << i + meshIndexOffset << "attribute" << src, assertPrefix << "expected array size" << out.attributeArraySize(dst) << "for attribute" << dst << "(" << Debug::nospace << out.attributeName(dst) << Debug::nospace << ") but got" << mesh.attributeArraySize(src) << "in mesh" << i << "attribute" << src,
(Trade::MeshData{MeshPrimitive{}, 0})); (Trade::MeshData{MeshPrimitive{}, 0}));
/* Copy the data to a slice of the output, mark the attribute as /* Copy the data to a slice of the output, mark the attribute as
@ -189,16 +185,10 @@ Trade::MeshData concatenate(Containers::Array<char>&& indexData, const UnsignedI
} }
Trade::MeshData concatenate(Trade::MeshData&& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next) { Trade::MeshData concatenate(const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes) {
/* If there's just a single non-empty mesh and its data is owned, pass it CORRADE_ASSERT(!meshes.empty(),
through, as it passes the guarantee that the returned data is always "MeshTools::concatenate(): expected at least one mesh",
owned. If it's empty, it doesn't matter that we drag it through the rest (Trade::MeshData{MeshPrimitive::Points, 0}));
as there will be no heavy allocation / copy made (and that also makes
tests easier to write). */
if(first.indexDataFlags() & Trade::DataFlag::Owned &&
first.vertexDataFlags() & Trade::DataFlag::Owned &&
first.attributeCount() && first.vertexCount() && next.empty())
return std::move(first);
/* Calculate final attribute stride and offsets. Make a non-owning copy of /* Calculate final attribute stride and offsets. Make a non-owning copy of
the attribute data to avoid interleavedLayout() stealing the original the attribute data to avoid interleavedLayout() stealing the original
@ -206,40 +196,27 @@ Trade::MeshData concatenate(Trade::MeshData&& first, const Containers::ArrayView
no attributes in the original array, pass just vertex count --- no attributes in the original array, pass just vertex count ---
otherwise MeshData will assert on that to avoid it getting lost. */ otherwise MeshData will assert on that to avoid it getting lost. */
Containers::Array<Trade::MeshAttributeData> attributeData; Containers::Array<Trade::MeshAttributeData> attributeData;
if(first.attributeCount()) if(meshes.front()->attributeCount())
attributeData = Implementation::interleavedLayout(Trade::MeshData{first.primitive(), attributeData = Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(),
{}, first.vertexData(), {}, meshes.front()->vertexData(),
Trade::meshAttributeDataNonOwningArray(first.attributeData())}, {}); Trade::meshAttributeDataNonOwningArray(meshes.front()->attributeData())}, {});
else attributeData = else attributeData =
Implementation::interleavedLayout(Trade::MeshData{first.primitive(), Implementation::interleavedLayout(Trade::MeshData{meshes.front()->primitive(),
first.vertexCount()}, {}); meshes.front()->vertexCount()}, {});
/* Calculate total index/vertex count and allocate the target memory. /* Calculate total index/vertex count and allocate the target memory.
Index data are allocated with NoInit as the whole array will be written, Index data are allocated with NoInit as the whole array will be written,
however vertex data might have holes and thus it's zero-initialized. */ however vertex data might have holes and thus it's zero-initialized. */
const std::pair<UnsignedInt, UnsignedInt> indexVertexCount = Implementation::concatenateIndexVertexCount(first, next); const std::pair<UnsignedInt, UnsignedInt> indexVertexCount = Implementation::concatenateIndexVertexCount(meshes);
Containers::Array<char> indexData{Containers::NoInit, Containers::Array<char> indexData{Containers::NoInit,
indexVertexCount.first*sizeof(UnsignedInt)}; indexVertexCount.first*sizeof(UnsignedInt)};
Containers::Array<char> vertexData{Containers::ValueInit, Containers::Array<char> vertexData{Containers::ValueInit,
attributeData.empty() ? 0 : (attributeData[0].stride()*indexVertexCount.second)}; attributeData.empty() ? 0 : (attributeData[0].stride()*indexVertexCount.second)};
return Implementation::concatenate(std::move(indexData), indexVertexCount.second, std::move(vertexData), std::move(attributeData), first, next, "MeshTools::concatenate():", 0); return Implementation::concatenate(std::move(indexData), indexVertexCount.second, std::move(vertexData), std::move(attributeData), meshes, "MeshTools::concatenate():");
} }
Trade::MeshData concatenate(Trade::MeshData&& first, std::initializer_list<Containers::Reference<const Trade::MeshData>> next) { Trade::MeshData concatenate(std::initializer_list<Containers::Reference<const Trade::MeshData>> meshes) {
return concatenate(std::move(first), Containers::arrayView(next)); return concatenate(Containers::arrayView(meshes));
}
Trade::MeshData concatenate(const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next) {
return concatenate(Trade::MeshData{first.primitive(),
/* If data is not indexed, the reference will be also non-indexed */
{}, first.indexData(), Trade::MeshIndexData{first.indices()},
{}, first.vertexData(), Trade::meshAttributeDataNonOwningArray(first.attributeData()),
first.vertexCount(),
}, next);
}
Trade::MeshData concatenate(const Trade::MeshData& first, std::initializer_list<Containers::Reference<const Trade::MeshData>> next) {
return concatenate(first, Containers::arrayView(next));
} }
}} }}

46
src/Magnum/MeshTools/Concatenate.h
Unescape View File

@ -39,8 +39,8 @@
namespace Magnum { namespace MeshTools { namespace Magnum { namespace MeshTools {
namespace Implementation { namespace Implementation {
MAGNUM_MESHTOOLS_EXPORT std::pair<UnsignedInt, UnsignedInt> concatenateIndexVertexCount(const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next); MAGNUM_MESHTOOLS_EXPORT std::pair<UnsignedInt, UnsignedInt> concatenateIndexVertexCount(Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes);
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::Array<char>&& indexData, UnsignedInt vertexCount, Containers::Array<char>&& vertexData, Containers::Array<Trade::MeshAttributeData>&& attributeData, const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next, const char* assertPrefix, std::size_t meshIndexOffset); MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::Array<char>&& indexData, UnsignedInt vertexCount, Containers::Array<char>&& vertexData, Containers::Array<Trade::MeshAttributeData>&& attributeData, Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes, const char* assertPrefix);
} }
/** /**
@ -54,10 +54,11 @@ any mesh has indices out of bounds for its particular vertex count. Meshes with
@ref MeshPrimitive::LineStrip, @ref MeshPrimitive::LineLoop, @ref MeshPrimitive::LineStrip, @ref MeshPrimitive::LineLoop,
@ref MeshPrimitive::TriangleStrip and @ref MeshPrimitive::TriangleFan can't be @ref MeshPrimitive::TriangleStrip and @ref MeshPrimitive::TriangleFan can't be
concatenated --- use @ref generateIndices() to turn them into concatenated --- use @ref generateIndices() to turn them into
@ref MeshPrimitive::Lines or @ref MeshPrimitive::Triangles first. @ref MeshPrimitive::Lines or @ref MeshPrimitive::Triangles first. The @p meshes
array is expected to have at least one item.
All attributes from the @p first mesh are taken; for each mesh in @p next, All attributes from the first mesh are taken; for each following mesh
attributes present in @p first are copied, superfluous attributes ignored and attributes present in the first are copied, superfluous attributes ignored and
missing attributes zeroed out. Matching attributes are expected to have the missing attributes zeroed out. Matching attributes are expected to have the
same type, all meshes are expected to have the same primitive. The vertex data same type, all meshes are expected to have the same primitive. The vertex data
are concatenated in the same order as passed, with no duplicate removal. are concatenated in the same order as passed, with no duplicate removal.
@ -69,36 +70,15 @@ applying transformations.
If an index buffer is needed, @ref MeshIndexType::UnsignedInt is always used. If an index buffer is needed, @ref MeshIndexType::UnsignedInt is always used.
Call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result Call @ref compressIndices(const Trade::MeshData&, MeshIndexType) on the result
to compress it to a smaller type, if desired. to compress it to a smaller type, if desired.
@see @ref concatenate(Trade::MeshData&&, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>>), @see @ref concatenateInto()
@ref concatenateInto()
*/ */
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(const Trade::MeshData& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next = {}); MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> meshes);
/** /**
* @overload * @overload
* @m_since_latest * @m_since_latest
*/ */
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(const Trade::MeshData& first, std::initializer_list<Containers::Reference<const Trade::MeshData>> next); MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(std::initializer_list<Containers::Reference<const Trade::MeshData>> meshes);
/**
@brief Concatenate meshes together
@m_since_latest
Compared to @ref concatenate(const Trade::MeshData&, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>>),
if @p first has both vertex and index data owned and @p next is empty, it's
passed through without any extra allocations or other work. This can be used
for example to ensure a mesh is mutable in order to do various modifications on
its data:
@snippet MagnumMeshTools.cpp concatenate-make-mutable
*/
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Trade::MeshData&& first, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>> next = {});
/**
* @overload
* @m_since_latest
*/
MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Trade::MeshData&& first, std::initializer_list<Containers::Reference<const Trade::MeshData>> next);
/** /**
@brief Concatenate a list of meshes into a pre-existing destination, enlarging it if necessary @brief Concatenate a list of meshes into a pre-existing destination, enlarging it if necessary
@ -108,8 +88,8 @@ MAGNUM_MESHTOOLS_EXPORT Trade::MeshData concatenate(Trade::MeshData&& first, std
@param[in] meshes Meshes to concatenate @param[in] meshes Meshes to concatenate
@m_since_latest @m_since_latest
Compared to @ref concatenate(const Trade::MeshData&, const Containers::ArrayView<const Containers::Reference<const Trade::MeshData>>) this Compared to @ref concatenate(Containers::ArrayView<const Containers::Reference<const Trade::MeshData>>)
function resizes existing index and vertex buffers in @p destination using this function resizes existing index and vertex buffers in @p destination using
@ref Containers::arrayResize() and given @p allocator, and reuses its @ref Containers::arrayResize() and given @p allocator, and reuses its
atttribute data array instead of always allocating new ones. Only the attribute atttribute data array instead of always allocating new ones. Only the attribute
layout from @p destination is used, all vertex/index data are taken from layout from @p destination is used, all vertex/index data are taken from
@ -119,7 +99,7 @@ template<template<class> class Allocator = Containers::ArrayAllocator> void conc
CORRADE_ASSERT(!meshes.empty(), CORRADE_ASSERT(!meshes.empty(),
"MeshTools::concatenateInto(): no meshes passed", ); "MeshTools::concatenateInto(): no meshes passed", );
std::pair<UnsignedInt, UnsignedInt> indexVertexCount = Implementation::concatenateIndexVertexCount(meshes[0], meshes.suffix(1)); std::pair<UnsignedInt, UnsignedInt> indexVertexCount = Implementation::concatenateIndexVertexCount(meshes);
Containers::Array<char> indexData; Containers::Array<char> indexData;
if(indexVertexCount.first) { if(indexVertexCount.first) {
@ -141,7 +121,7 @@ template<template<class> class Allocator = Containers::ArrayAllocator> void conc
Containers::arrayResize<Allocator>(vertexData, Containers::ValueInit, attributeStride*indexVertexCount.second); Containers::arrayResize<Allocator>(vertexData, Containers::ValueInit, attributeStride*indexVertexCount.second);
} }
destination = Implementation::concatenate(std::move(indexData), indexVertexCount.second, std::move(vertexData), std::move(attributeData), meshes[0], meshes.suffix(1), "MeshTools::concatenateInto():", 1); destination = Implementation::concatenate(std::move(indexData), indexVertexCount.second, std::move(vertexData), std::move(attributeData), meshes, "MeshTools::concatenateInto():");
} }
/** /**

20
src/Magnum/MeshTools/RemoveDuplicates.cpp
Unescape View File

@ -36,7 +36,7 @@
#include "Magnum/Math/FunctionsBatch.h" #include "Magnum/Math/FunctionsBatch.h"
#include "Magnum/Math/Range.h" #include "Magnum/Math/Range.h"
#include "Magnum/MeshTools/Concatenate.h" #include "Magnum/MeshTools/Reference.h"
#include "Magnum/MeshTools/Duplicate.h" #include "Magnum/MeshTools/Duplicate.h"
#include "Magnum/MeshTools/Interleave.h" #include "Magnum/MeshTools/Interleave.h"
#include "Magnum/Trade/MeshData.h" #include "Magnum/Trade/MeshData.h"
@ -409,13 +409,11 @@ Trade::MeshData removeDuplicates(Trade::MeshData&& data) {
(Trade::MeshData{MeshPrimitive::Points, 0})); (Trade::MeshData{MeshPrimitive::Points, 0}));
/* Turn the passed data into an interleaved owned mutable instance we can /* Turn the passed data into an interleaved owned mutable instance we can
operate on -- concatenate() alone only makes the data owned, operate on -- owned() alone only makes the data owned, interleave()
interleave() alone only makes the data interleaved (but those can stay alone only makes the data interleaved (but those can stay non-owned).
non-owned). There's a chance the original data are already like this, in There's a chance the original data are already like this, in which case
which case this will be just a passthrough. */ this will be just a passthrough. */
/** @todo concatenate() causes the resulting index type to be UnsignedInt Trade::MeshData ownedInterleaved = owned(interleave(std::move(data)));
always, replace with owned() or some such when that's done */
Trade::MeshData ownedInterleaved = interleave(concatenate(std::move(data)));
const Containers::StridedArrayView2D<char> vertexData = MeshTools::interleavedMutableData(ownedInterleaved); const Containers::StridedArrayView2D<char> vertexData = MeshTools::interleavedMutableData(ownedInterleaved);
@ -467,9 +465,7 @@ Trade::MeshData removeDuplicatesFuzzy(const Trade::MeshData& data, const Float f
/* Turn the passed data into an owned mutable instance we can operate on. /* Turn the passed data into an owned mutable instance we can operate on.
There's a chance the original data are already like this, in which case There's a chance the original data are already like this, in which case
this will be just a passthrough. */ this will be just a passthrough. */
/** @todo concatenate() causes the resulting index type to be UnsignedInt Trade::MeshData owned = MeshTools::owned(std::move(data));
always, replace with owned() or some such when that's done */
Trade::MeshData owned = concatenate(std::move(data));
/* Allocate an interleaved index array for all attribs. If the mesh is /* Allocate an interleaved index array for all attribs. If the mesh is
already indexed, use the existing index count and copy the original already indexed, use the existing index count and copy the original
@ -491,7 +487,7 @@ Trade::MeshData removeDuplicatesFuzzy(const Trade::MeshData& data, const Float f
for(UnsignedInt i = 0; i != owned.attributeCount(); ++i) { for(UnsignedInt i = 0; i != owned.attributeCount(); ++i) {
const VertexFormat format = owned.attributeFormat(i); const VertexFormat format = owned.attributeFormat(i);
CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format), CORRADE_ASSERT(!isVertexFormatImplementationSpecific(format),
"MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in" << format, "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an implementation-specific format" << reinterpret_cast<void*>(vertexFormatUnwrap(format)),
(Trade::MeshData{MeshPrimitive::Points, 0})); (Trade::MeshData{MeshPrimitive::Points, 0}));
const Containers::StridedArrayView1D<UnsignedInt> outputIndices = perAttributeIndices[i]; const Containers::StridedArrayView1D<UnsignedInt> outputIndices = perAttributeIndices[i];

57
src/Magnum/MeshTools/Test/ConcatenateTest.cpp
Unescape View File

@ -41,7 +41,7 @@ struct ConcatenateTest: TestSuite::Tester {
void concatenateNoAttributes(); void concatenateNoAttributes();
void concatenateNoAttributesNotIndexed(); void concatenateNoAttributesNotIndexed();
void concatenateOne(); void concatenateOne();
void concatenateOneRvalue(); void concatenateNone();
void concatenateInto(); void concatenateInto();
void concatenateIntoNoIndexArray(); void concatenateIntoNoIndexArray();
void concatenateIntoNonOwnedAttributeArray(); void concatenateIntoNonOwnedAttributeArray();
@ -59,7 +59,7 @@ ConcatenateTest::ConcatenateTest() {
&ConcatenateTest::concatenateNoAttributes, &ConcatenateTest::concatenateNoAttributes,
&ConcatenateTest::concatenateNoAttributesNotIndexed, &ConcatenateTest::concatenateNoAttributesNotIndexed,
&ConcatenateTest::concatenateOne, &ConcatenateTest::concatenateOne,
&ConcatenateTest::concatenateOneRvalue, &ConcatenateTest::concatenateNone,
&ConcatenateTest::concatenateInto, &ConcatenateTest::concatenateInto,
&ConcatenateTest::concatenateIntoNoIndexArray, &ConcatenateTest::concatenateIntoNoIndexArray,
&ConcatenateTest::concatenateIntoNonOwnedAttributeArray, &ConcatenateTest::concatenateIntoNonOwnedAttributeArray,
@ -165,7 +165,7 @@ void ConcatenateTest::concatenate() {
&vertexDataC[0].texcoords3, 3, sizeof(VertexDataC))}, &vertexDataC[0].texcoords3, 3, sizeof(VertexDataC))},
}}; }};
Trade::MeshData dst = MeshTools::concatenate(a, {b, c}); Trade::MeshData dst = MeshTools::concatenate({a, b, c});
CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points);
CORRADE_COMPARE(dst.attributeCount(), 4); CORRADE_COMPARE(dst.attributeCount(), 4);
CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position), CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position),
@ -244,7 +244,7 @@ void ConcatenateTest::concatenateNotIndexed() {
Containers::arrayView(positionB)} Containers::arrayView(positionB)}
}}; }};
Trade::MeshData dst = MeshTools::concatenate(a, {b, b}); Trade::MeshData dst = MeshTools::concatenate({a, b, b});
CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points);
CORRADE_COMPARE(dst.attributeCount(), 1); CORRADE_COMPARE(dst.attributeCount(), 1);
CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position), CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position),
@ -272,7 +272,7 @@ void ConcatenateTest::concatenateNoAttributes() {
const UnsignedByte indicesC[]{1, 0, 1, 0}; const UnsignedByte indicesC[]{1, 0, 1, 0};
Trade::MeshData c{MeshPrimitive::Points, {}, indicesC, Trade::MeshIndexData{indicesC}, 2}; Trade::MeshData c{MeshPrimitive::Points, {}, indicesC, Trade::MeshIndexData{indicesC}, 2};
Trade::MeshData dst = MeshTools::concatenate(a, {b, c}); Trade::MeshData dst = MeshTools::concatenate({a, b, c});
CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points);
CORRADE_COMPARE(dst.attributeCount(), 0); CORRADE_COMPARE(dst.attributeCount(), 0);
CORRADE_COMPARE(dst.vertexCount(), 10); CORRADE_COMPARE(dst.vertexCount(), 10);
@ -292,7 +292,7 @@ void ConcatenateTest::concatenateNoAttributesNotIndexed() {
Trade::MeshData b{MeshPrimitive::Points, 6}; Trade::MeshData b{MeshPrimitive::Points, 6};
Trade::MeshData c{MeshPrimitive::Points, 2}; Trade::MeshData c{MeshPrimitive::Points, 2};
Trade::MeshData dst = MeshTools::concatenate(a, {b, c}); Trade::MeshData dst = MeshTools::concatenate({a, b, c});
CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points);
CORRADE_COMPARE(dst.attributeCount(), 0); CORRADE_COMPARE(dst.attributeCount(), 0);
CORRADE_COMPARE(dst.vertexCount(), 11); CORRADE_COMPARE(dst.vertexCount(), 11);
@ -330,7 +330,9 @@ void ConcatenateTest::concatenateOne() {
Containers::arrayView(vertexData[0].position)}, Containers::arrayView(vertexData[0].position)},
}}; }};
Trade::MeshData dst = MeshTools::concatenate(a); /* This is a rather pointless use case, but could happen in generic code
that filters the input meshes and ends up with just one */
Trade::MeshData dst = MeshTools::concatenate({a});
CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points); CORRADE_COMPARE(dst.primitive(), MeshPrimitive::Points);
CORRADE_COMPARE(dst.attributeCount(), 3); CORRADE_COMPARE(dst.attributeCount(), 3);
CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position), CORRADE_COMPARE_AS(dst.attribute<Vector3>(Trade::MeshAttribute::Position),
@ -362,26 +364,15 @@ void ConcatenateTest::concatenateOne() {
CORRADE_COMPARE(dst.vertexDataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(dst.vertexDataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable);
} }
void ConcatenateTest::concatenateOneRvalue() { void ConcatenateTest::concatenateNone() {
Containers::Array<char> vertexData{sizeof(Vector2)*4}; #ifdef CORRADE_NO_ASSERT
auto positions = Containers::arrayCast<Vector2>(vertexData); CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
Containers::Array<char> indexData{sizeof(UnsignedInt)*6}; #endif
auto indices = Containers::arrayCast<UnsignedInt>(indexData);
Trade::MeshAttributeData attributeData[]{
Trade::MeshAttributeData{Trade::MeshAttribute::Position, positions}
};
/* The result should be just a pass-through, as both index and vertex data std::ostringstream out;
are already owned */ Error redirectError{&out};
Trade::MeshData dst = MeshTools::concatenate(Trade::MeshData{ MeshTools::concatenate({});
MeshPrimitive::Triangles, CORRADE_COMPARE(out.str(), "MeshTools::concatenate(): expected at least one mesh\n");
std::move(indexData), Trade::MeshIndexData{indices},
std::move(vertexData), Trade::meshAttributeDataNonOwningArray(attributeData)},
/* Explicitly pass an empty init list to ensure this overload is
covered as well */
std::initializer_list<Containers::Reference<const Trade::MeshData>>{});
CORRADE_COMPARE(dst.indexData().data(), static_cast<void*>(indices.data()));
CORRADE_COMPARE(dst.vertexData().data(), static_cast<void*>(positions.data()));
} }
void ConcatenateTest::concatenateInto() { void ConcatenateTest::concatenateInto() {
@ -561,7 +552,7 @@ void ConcatenateTest::concatenateUnsupportedPrimitive() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
MeshTools::concatenate(a); MeshTools::concatenate({a});
MeshTools::concatenateInto(a, {a}); MeshTools::concatenateInto(a, {a});
CORRADE_COMPARE(out.str(), CORRADE_COMPARE(out.str(),
"MeshTools::concatenate(): MeshPrimitive::TriangleStrip is not supported, turn it into a plain indexed mesh first\n" "MeshTools::concatenate(): MeshPrimitive::TriangleStrip is not supported, turn it into a plain indexed mesh first\n"
@ -579,10 +570,10 @@ void ConcatenateTest::concatenateInconsistentPrimitive() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
MeshTools::concatenate(a, {a, b}); MeshTools::concatenate({a, a, b});
MeshTools::concatenateInto(a, {a, b}); MeshTools::concatenateInto(a, {a, b});
CORRADE_COMPARE(out.str(), CORRADE_COMPARE(out.str(),
"MeshTools::concatenate(): expected MeshPrimitive::Triangles but got MeshPrimitive::Lines in mesh 1\n" "MeshTools::concatenate(): expected MeshPrimitive::Triangles but got MeshPrimitive::Lines in mesh 2\n"
"MeshTools::concatenateInto(): expected MeshPrimitive::Triangles but got MeshPrimitive::Lines in mesh 1\n"); "MeshTools::concatenateInto(): expected MeshPrimitive::Triangles but got MeshPrimitive::Lines in mesh 1\n");
} }
@ -609,10 +600,10 @@ void ConcatenateTest::concatenateInconsistentAttributeType() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
MeshTools::concatenate(a, {a, a, a, b}); MeshTools::concatenate({a, a, a, a, b});
MeshTools::concatenateInto(a, {a, a, a, b}); MeshTools::concatenateInto(a, {a, a, a, b});
CORRADE_COMPARE(out.str(), CORRADE_COMPARE(out.str(),
"MeshTools::concatenate(): expected VertexFormat::Vector3ubNormalized for attribute 2 (Trade::MeshAttribute::Color) but got VertexFormat::Vector3usNormalized in mesh 3 attribute 1\n" "MeshTools::concatenate(): expected VertexFormat::Vector3ubNormalized for attribute 2 (Trade::MeshAttribute::Color) but got VertexFormat::Vector3usNormalized in mesh 4 attribute 1\n"
"MeshTools::concatenateInto(): expected VertexFormat::Vector3ubNormalized for attribute 2 (Trade::MeshAttribute::Color) but got VertexFormat::Vector3usNormalized in mesh 3 attribute 1\n"); "MeshTools::concatenateInto(): expected VertexFormat::Vector3ubNormalized for attribute 2 (Trade::MeshAttribute::Color) but got VertexFormat::Vector3usNormalized in mesh 3 attribute 1\n");
} }
@ -639,10 +630,10 @@ void ConcatenateTest::concatenateInconsistentAttributeArraySize() {
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
MeshTools::concatenate(a, {a, a, a, b}); MeshTools::concatenate({a, a, a, a, b});
MeshTools::concatenateInto(a, {a, a, a, b}); MeshTools::concatenateInto(a, {a, a, a, b});
CORRADE_COMPARE(out.str(), CORRADE_COMPARE(out.str(),
"MeshTools::concatenate(): expected array size 5 for attribute 2 (Trade::MeshAttribute::Custom(42)) but got 4 in mesh 3 attribute 1\n" "MeshTools::concatenate(): expected array size 5 for attribute 2 (Trade::MeshAttribute::Custom(42)) but got 4 in mesh 4 attribute 1\n"
"MeshTools::concatenateInto(): expected array size 5 for attribute 2 (Trade::MeshAttribute::Custom(42)) but got 4 in mesh 3 attribute 1\n"); "MeshTools::concatenateInto(): expected array size 5 for attribute 2 (Trade::MeshAttribute::Custom(42)) but got 4 in mesh 3 attribute 1\n");
} }

39
src/Magnum/MeshTools/Test/RemoveDuplicatesTest.cpp
Unescape View File

@ -673,13 +673,9 @@ void RemoveDuplicatesTest::removeDuplicatesMeshData() {
CORRADE_VERIFY(unique.isIndexed()); CORRADE_VERIFY(unique.isIndexed());
if(data.indexed) { if(data.indexed) {
CORRADE_COMPARE(unique.indexCount(), mesh.indexCount()); CORRADE_COMPARE(unique.indexCount(), mesh.indexCount());
{ CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedShort);
CORRADE_EXPECT_FAIL("The function currently doesn't preserve the index type."); CORRADE_COMPARE_AS(unique.indices<UnsignedShort>(),
CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedShort); Containers::arrayView<UnsignedShort>({1, 2, 5, 7, 1, 6, 4, 1, 5, 0, 3, 2, 3}),
}
CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedInt);
CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(),
Containers::arrayView<UnsignedInt>({1, 2, 5, 7, 1, 6, 4, 1, 5, 0, 3, 2, 3}),
TestSuite::Compare::Container); TestSuite::Compare::Container);
} else { } else {
CORRADE_COMPARE(unique.indexCount(), mesh.vertexCount()); CORRADE_COMPARE(unique.indexCount(), mesh.vertexCount());
@ -820,11 +816,13 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzy() {
CORRADE_COMPARE(unique.primitive(), MeshPrimitive::Lines); CORRADE_COMPARE(unique.primitive(), MeshPrimitive::Lines);
CORRADE_VERIFY(unique.isIndexed()); CORRADE_VERIFY(unique.isIndexed());
if(data.indexed) if(data.indexed) {
CORRADE_COMPARE(unique.indexCount(), mesh.indexCount()); CORRADE_COMPARE(unique.indexCount(), mesh.indexCount());
else CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedShort);
} else {
CORRADE_COMPARE(unique.indexCount(), mesh.vertexCount()); CORRADE_COMPARE(unique.indexCount(), mesh.vertexCount());
CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedInt); CORRADE_COMPARE(unique.indexType(), MeshIndexType::UnsignedInt);
}
CORRADE_COMPARE(unique.attributeCount(), 3 + data.attributes.size()); CORRADE_COMPARE(unique.attributeCount(), 3 + data.attributes.size());
@ -857,8 +855,8 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzy() {
/* The data differ depending on how much is actually removed */ /* The data differ depending on how much is actually removed */
if(data.vertexCount == 7) { if(data.vertexCount == 7) {
if(data.indexed) CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(), if(data.indexed) CORRADE_COMPARE_AS(unique.indices<UnsignedShort>(),
Containers::arrayView<UnsignedInt>({0, 1, 3, 6, 5, 4, 3, 5, 3, 0, 2, 5, 2}), Containers::arrayView<UnsignedShort>({0, 1, 3, 6, 5, 4, 3, 5, 3, 0, 2, 5, 2}),
TestSuite::Compare::Container); TestSuite::Compare::Container);
else CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(), else CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(),
Containers::arrayView<UnsignedInt>({0, 0, 1, 2, 3, 3, 4, 5, 5, 6}), Containers::arrayView<UnsignedInt>({0, 0, 1, 2, 3, 3, 4, 5, 5, 6}),
@ -912,8 +910,8 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzy() {
TestSuite::Compare::Container); TestSuite::Compare::Container);
} else if(data.vertexCount == 9) { } else if(data.vertexCount == 9) {
if(data.indexed) CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(), if(data.indexed) CORRADE_COMPARE_AS(unique.indices<UnsignedShort>(),
Containers::arrayView<UnsignedInt>({1, 2, 4, 8, 6, 5, 4, 6, 4, 0, 3, 7, 3}), Containers::arrayView<UnsignedShort>({1, 2, 4, 8, 6, 5, 4, 6, 4, 0, 3, 7, 3}),
TestSuite::Compare::Container); TestSuite::Compare::Container);
else CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(), else CORRADE_COMPARE_AS(unique.indices<UnsignedInt>(),
Containers::arrayView<UnsignedInt>({0, 1, 2, 3, 4, 4, 5, 6, 7, 8}), Containers::arrayView<UnsignedInt>({0, 1, 2, 3, 4, 4, 5, 6, 7, 8}),
@ -1059,14 +1057,11 @@ void RemoveDuplicatesTest::removeDuplicatesMeshDataFuzzyImplementationSpecific()
std::ostringstream out; std::ostringstream out;
Error redirectError{&out}; Error redirectError{&out};
CORRADE_EXPECT_FAIL("The function currently uses concatenate() to make data owned, which fails earlier with a different (and confusing) assert message."); MeshTools::removeDuplicatesFuzzy(Trade::MeshData{MeshPrimitive::Points,
CORRADE_VERIFY(false); nullptr, {Trade::MeshAttributeData{Trade::MeshAttribute::Position,
vertexFormatWrap(0x1234), nullptr}}});
// MeshTools::removeDuplicatesFuzzy(Trade::MeshData{MeshPrimitive::Points, CORRADE_COMPARE(out.str(),
// nullptr, {Trade::MeshAttributeData{Trade::MeshAttribute::Position, "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an implementation-specific format 0x1234\n");
// vertexFormatWrap(0x1234), nullptr}}});
// CORRADE_COMPARE(out.str(),
// "MeshTools::removeDuplicatesFuzzy(): can't remove duplicates in an implementation-specific format 0x1234\n");
} }
void RemoveDuplicatesTest::soakTest() { void RemoveDuplicatesTest::soakTest() {

Write Preview
Loading…
Cancel
Save