From e7d1e31f463d853df3a350b92688227871916885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 23 Apr 2023 22:56:32 +0200 Subject: [PATCH] SceneTools: orderClusterParents() -> parentsBreadthFirst(). Same reasoning as before, the verb suggests it's transforming the SceneData in some way, which isn't true, it just retrieves the data in a certain way. And if an API that actually operates on SceneData got added, it would be easily confused with this one. Plus, the "order" isn't just one, this orders objects so they're grouped with a common parent, but what if I wanted to instead order depth first? Thus it's explicitly saying this is a breadth-first order. The API got moved to the Hierarchy.h header, removing a need for a dedicated file and test. --- doc/snippets/MagnumSceneTools.cpp | 7 +- src/Magnum/SceneTools/CMakeLists.txt | 7 +- src/Magnum/SceneTools/Hierarchy.cpp | 104 +++++- src/Magnum/SceneTools/Hierarchy.h | 48 ++- src/Magnum/SceneTools/OrderClusterParents.cpp | 136 -------- src/Magnum/SceneTools/OrderClusterParents.h | 67 ++-- src/Magnum/SceneTools/Test/CMakeLists.txt | 1 - src/Magnum/SceneTools/Test/HierarchyTest.cpp | 280 +++++++++++++++ .../Test/OrderClusterParentsTest.cpp | 324 ------------------ 9 files changed, 461 insertions(+), 513 deletions(-) delete mode 100644 src/Magnum/SceneTools/OrderClusterParents.cpp delete mode 100644 src/Magnum/SceneTools/Test/OrderClusterParentsTest.cpp diff --git a/doc/snippets/MagnumSceneTools.cpp b/doc/snippets/MagnumSceneTools.cpp index 410686f38..f7c02ca88 100644 --- a/doc/snippets/MagnumSceneTools.cpp +++ b/doc/snippets/MagnumSceneTools.cpp @@ -33,7 +33,6 @@ #include "Magnum/MeshTools/Concatenate.h" #include "Magnum/MeshTools/Transform.h" #include "Magnum/SceneTools/Hierarchy.h" -#include "Magnum/SceneTools/OrderClusterParents.h" #include "Magnum/Trade/SceneData.h" #include "Magnum/Trade/MeshData.h" @@ -83,7 +82,7 @@ Trade::MeshData concatenated = MeshTools::concatenate(flattenedMeshes); } { -/* [orderClusterParents-transformations] */ +/* [parentsBreadthFirst-transformations] */ Trade::SceneData scene = DOXYGEN_ELLIPSIS(Trade::SceneData{{}, 0, nullptr, {}}); /* Put all transformations into an array indexed by object ID. Objects @@ -101,12 +100,12 @@ for(const Containers::Pair& transformation: function ensures that the parent transformation is already calculated when referenced by child nodes. */ for(const Containers::Pair& parent: - SceneTools::orderClusterParents(scene)) + SceneTools::parentsBreadthFirst(scene)) { transformations[parent.first() + 1] = transformations[parent.second() + 1]* transformations[parent.first() + 1]; } -/* [orderClusterParents-transformations] */ +/* [parentsBreadthFirst-transformations] */ } } diff --git a/src/Magnum/SceneTools/CMakeLists.txt b/src/Magnum/SceneTools/CMakeLists.txt index f0022a9f6..73680d5dc 100644 --- a/src/Magnum/SceneTools/CMakeLists.txt +++ b/src/Magnum/SceneTools/CMakeLists.txt @@ -45,14 +45,12 @@ set(MagnumSceneTools_SRCS ) set(MagnumSceneTools_GracefulAssert_SRCS Combine.cpp Filter.cpp - Hierarchy.cpp - OrderClusterParents.cpp) + Hierarchy.cpp) set(MagnumSceneTools_HEADERS Combine.h Filter.h Hierarchy.h - OrderClusterParents.h visibility.h) @@ -65,7 +63,8 @@ if(MAGNUM_BUILD_DEPRECATED) list(APPEND MagnumSceneTools_GracefulAssert_SRCS FlattenMeshHierarchy.cpp) list(APPEND MagnumSceneTools_HEADERS FlattenMeshHierarchy.h - FlattenTransformationHierarchy.h) + FlattenTransformationHierarchy.h + OrderClusterParents.h) endif() ## Objects shared between main and test library diff --git a/src/Magnum/SceneTools/Hierarchy.cpp b/src/Magnum/SceneTools/Hierarchy.cpp index b19efb489..e5016d58e 100644 --- a/src/Magnum/SceneTools/Hierarchy.cpp +++ b/src/Magnum/SceneTools/Hierarchy.cpp @@ -35,10 +35,108 @@ #include "Magnum/Math/Matrix3.h" #include "Magnum/Math/Matrix4.h" #include "Magnum/Trade/SceneData.h" -#include "Magnum/SceneTools/OrderClusterParents.h" namespace Magnum { namespace SceneTools { +Containers::Array> parentsBreadthFirst(const Trade::SceneData& scene) { + const Containers::Optional parentFieldId = scene.findFieldId(Trade::SceneField::Parent); + CORRADE_ASSERT(parentFieldId, + "SceneTools::parentsBreadthFirst(): the scene has no hierarchy", {}); + Containers::Array> out{NoInit, scene.fieldSize(*parentFieldId)}; + parentsBreadthFirstInto(scene, + stridedArrayView(out).slice(&decltype(out)::Type::first), + stridedArrayView(out).slice(&decltype(out)::Type::second)); + return out; +} + +void parentsBreadthFirstInto(const Trade::SceneData& scene, const Containers::StridedArrayView1D& mappingDestination, const Containers::StridedArrayView1D& parentDestination) { + const Containers::Optional parentFieldId = scene.findFieldId(Trade::SceneField::Parent); + CORRADE_ASSERT(parentFieldId, + "SceneTools::parentsBreadthFirstInto(): the scene has no hierarchy", ); + const std::size_t parentFieldSize = scene.fieldSize(*parentFieldId); + CORRADE_ASSERT(mappingDestination.size() == parentFieldSize, + "SceneTools::parentsBreadthFirstInto(): expected mapping destination view with" << parentFieldSize << "elements but got" << mappingDestination.size(), ); + CORRADE_ASSERT(parentDestination.size() == scene.fieldSize(*parentFieldId), + "SceneTools::parentsBreadthFirstInto(): expected parent destination view with" << parentFieldSize << "elements but got" << parentDestination.size(), ); + + /* Allocate a single storage for all temporary data */ + Containers::ArrayView> parents; + Containers::ArrayView childrenOffsets; + Containers::ArrayView children; + Containers::ArrayView parentsToProcess; + Containers::ArrayTuple storage{ + /* Output of scene.parentsInto() */ + {NoInit, parentFieldSize, parents}, + /* Running children offset (+1) for each node including root (+1), plus + one more element when we shift the array by one below */ + {ValueInit, std::size_t(scene.mappingBound() + 3), childrenOffsets}, + {NoInit, parentFieldSize, children}, + /* List of parents to process. Can't reuse mappingDestination because + this includes one more element for root objects. */ + {NoInit, parentFieldSize + 1, parentsToProcess} + }; + + /* Convert the parent list to a child list to sort them toplogically */ + scene.parentsInto( + stridedArrayView(parents).slice(&decltype(parents)::Type::first), + stridedArrayView(parents).slice(&decltype(parents)::Type::second) + ); + + /* Children offset for each node including root. First calculate the count + of children for each, skipping the first element (parent.second() can be + -1, accounting for that as well)... */ + for(const Containers::Pair& parent: parents) { + CORRADE_INTERNAL_ASSERT(parent.first() < scene.mappingBound() && (parent.second() == -1 || UnsignedInt(parent.second()) < scene.mappingBound())); + ++childrenOffsets[parent.second() + 2]; + } + + /* ... then convert the counts to a running offset. Now + `[childrenOffsets[i + 2], childrenOffsets[i + 3])` contains a range in + which the `children` array below contains a list of children for `i`. */ + UnsignedInt offset = 0; + for(UnsignedInt& i: childrenOffsets) { + UnsignedInt nextOffset = offset + i; + i = offset; + offset = nextOffset; + } + CORRADE_INTERNAL_ASSERT(offset == parents.size()); + + /* Go through the parent list again, convert that to child ranges. The + childrenOffsets array gets shifted by one element by the process, thus + now `[childrenOffsets[i + 1], childrenOffsets[i + 2])` contains a range + in which the `children` array below contains a list of children for + `i`. */ + for(const Containers::Pair& parent: parents) + children[childrenOffsets[parent.second() + 2]++] = parent.first(); + + /* Go breadth-first (so we have nodes sharing the same parent next to each + other) and build a list of (id, parent id) where a parent is always + before its children */ + std::size_t outputOffset = 0; + parentsToProcess[0] = -1; + for(std::size_t i = 0; i != outputOffset + 1; ++i) { + const Int objectId = parentsToProcess[i]; + for(std::size_t j = childrenOffsets[objectId + 1], jMax = childrenOffsets[objectId + 2]; j != jMax; ++j) { + /** @todo better diagnostic once we can use a BitArray to detect + which nodes are parented more than once (OTOH maybe that's + unnecessary extra work which isn't desired to be done here but + should be instead in a dedicated cycle/sparse checker utility?) */ + CORRADE_ASSERT(outputOffset < parents.size(), + "SceneTools::parentsBreadthFirst(): hierarchy is cyclic", ); + parentsToProcess[outputOffset + 1] = children[j]; + mappingDestination[outputOffset] = children[j]; + parentDestination[outputOffset] = objectId; + ++outputOffset; + } + } + + /** @todo better diagnostic once we can use a BitArray to detect which + nodes are unreachable from root (OTOH again maybe that's undesirable + extra work that doesn't belong here?) */ + CORRADE_ASSERT(outputOffset == parents.size(), + "SceneTools::parentsBreadthFirst(): hierarchy is sparse", ); +} + namespace { template struct SceneDataDimensionTraits; @@ -75,14 +173,14 @@ template void absoluteFieldTransformationsIntoImplementa Containers::ArrayView>> transformations; Containers::ArrayView> absoluteTransformations; Containers::ArrayTuple storage{ - /* Output of orderClusterParentsInto() */ + /* Output of parentsBreadthFirstInto() */ {NoInit, scene.fieldSize(*parentFieldId), orderedClusteredParents}, /* Output of scene.transformationsXDInto() */ {NoInit, scene.transformationFieldSize(), transformations}, /* Above transformations but indexed by object ID */ {ValueInit, std::size_t(scene.mappingBound() + 1), absoluteTransformations} }; - orderClusterParentsInto(scene, + parentsBreadthFirstInto(scene, stridedArrayView(orderedClusteredParents).slice(&decltype(orderedClusteredParents)::Type::first), stridedArrayView(orderedClusteredParents).slice(&decltype(orderedClusteredParents)::Type::second)); SceneDataDimensionTraits::transformationsInto(scene, diff --git a/src/Magnum/SceneTools/Hierarchy.h b/src/Magnum/SceneTools/Hierarchy.h index a7934cd41..e87930bff 100644 --- a/src/Magnum/SceneTools/Hierarchy.h +++ b/src/Magnum/SceneTools/Hierarchy.h @@ -26,7 +26,7 @@ */ /** @file - * @brief Function @ref Magnum::SceneTools::absoluteFieldTransformations2D(), @ref Magnum::SceneTools::absoluteFieldTransformations2DInto(), @ref Magnum::SceneTools::absoluteFieldTransformations3D(), @ref Magnum::SceneTools::absoluteFieldTransformations3DInto() + * @brief Function @ref Magnum::SceneTools::parentsBreadthFirst(), @ref Magnum::SceneTools::parentsBreadthFirstInto(), @ref Magnum::SceneTools::absoluteFieldTransformations2D(), @ref Magnum::SceneTools::absoluteFieldTransformations2DInto(), @ref Magnum::SceneTools::absoluteFieldTransformations3D(), @ref Magnum::SceneTools::absoluteFieldTransformations3DInto() * @m_since_latest */ @@ -36,6 +36,48 @@ namespace Magnum { namespace SceneTools { +/** +@brief Retrieve parents in a breadth-first order +@m_since_latest + +Extracts the @ref Trade::SceneField::Parent field mapping and data from +@p scene and converts it to match the following rules: + +- a parent object reference appears always before any of its children +- the array is clustered so children sharing the same parent are together + +This form is useful primarily for calculating absolute object transformations, +for example: + +@snippet MagnumSceneTools.cpp parentsBreadthFirst-transformations + +The operation is done in an @f$ \mathcal{O}(n) @f$ execution time and memory +complexity, with @f$ n @f$ being @ref Trade::SceneData::mappingBound(). The +@ref Trade::SceneField::Parent field is expected to be contained in the scene, +having no cycles (i.e., every node listed just once) and not being sparse +(i.e., every node listed in the field reachable from the root). + +@experimental + +@see @ref Trade::SceneData::hasField() +*/ +MAGNUM_SCENETOOLS_EXPORT Containers::Array> parentsBreadthFirst(const Trade::SceneData& scene); + +/** +@brief Retrieve parents in a breadth-first order into a pre-allocated view +@m_since_latest + +Like @ref parentsBreadthFirst(), but puts the result into @p mappingDestination +and @p parentDestination instead of allocating a new array. Expect that both +views have a size equal to size of the @ref Trade::SceneField::Parent view in +@p scene. + +@experimental + +@see @ref Trade::SceneData::fieldSize(SceneField) const +*/ +MAGNUM_SCENETOOLS_EXPORT void parentsBreadthFirstInto(const Trade::SceneData& scene, const Containers::StridedArrayView1D& mappingDestination, const Containers::StridedArrayView1D& parentDestination); + /** @brief Calculate absolute 2D transformations for given field @m_since_latest @@ -50,7 +92,7 @@ If the field is empty, the function returns an empty array. The operation is done in an @f$ \mathcal{O}(m + n) @f$ execution time and memory complexity, with @f$ m @f$ being size of @p fieldId and @f$ n @f$ being @ref Trade::SceneData::mappingBound(). The function calls -@ref orderClusterParents() internally. +@ref parentsBreadthFirst() internally. The returned data are in the same order as object mapping entries in @p fieldId. Fields attached to objects without a @ref Trade::SceneField::Parent @@ -148,7 +190,7 @@ If the field is empty, the function returns an empty array. The operation is done in an @f$ \mathcal{O}(m + n) @f$ execution time and memory complexity, with @f$ m @f$ being size of @p fieldId and @f$ n @f$ being @ref Trade::SceneData::mappingBound(). The function calls -@ref orderClusterParents() internally. +@ref parentsBreadthFirst() internally. The returned data are in the same order as object mapping entries in @p fieldId. Fields attached to objects without a @ref Trade::SceneField::Parent diff --git a/src/Magnum/SceneTools/OrderClusterParents.cpp b/src/Magnum/SceneTools/OrderClusterParents.cpp deleted file mode 100644 index 59c1d22ed..000000000 --- a/src/Magnum/SceneTools/OrderClusterParents.cpp +++ /dev/null @@ -1,136 +0,0 @@ -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, - 2020, 2021, 2022 Vladimír Vondruš - - Permission is hereby granted, free of charge, to any person obtaining a - copy of this software and associated documentation files (the "Software"), - to deal in the Software without restriction, including without limitation - the rights to use, copy, modify, merge, publish, distribute, sublicense, - and/or sell copies of the Software, and to permit persons to whom the - Software is furnished to do so, subject to the following conditions: - - The above copyright notice and this permission notice shall be included - in all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - DEALINGS IN THE SOFTWARE. -*/ - -#include "OrderClusterParents.h" - -#include -#include -#include -#include - -#include "Magnum/Trade/SceneData.h" - -namespace Magnum { namespace SceneTools { - -Containers::Array> orderClusterParents(const Trade::SceneData& scene) { - const Containers::Optional parentFieldId = scene.findFieldId(Trade::SceneField::Parent); - CORRADE_ASSERT(parentFieldId, - "SceneTools::orderClusterParents(): the scene has no hierarchy", {}); - Containers::Array> out{NoInit, scene.fieldSize(*parentFieldId)}; - orderClusterParentsInto(scene, - stridedArrayView(out).slice(&decltype(out)::Type::first), - stridedArrayView(out).slice(&decltype(out)::Type::second)); - return out; -} - -void orderClusterParentsInto(const Trade::SceneData& scene, const Containers::StridedArrayView1D& mappingDestination, const Containers::StridedArrayView1D& parentDestination) { - const Containers::Optional parentFieldId = scene.findFieldId(Trade::SceneField::Parent); - CORRADE_ASSERT(parentFieldId, - "SceneTools::orderClusterParentsInto(): the scene has no hierarchy", ); - const std::size_t parentFieldSize = scene.fieldSize(*parentFieldId); - CORRADE_ASSERT(mappingDestination.size() == parentFieldSize, - "SceneTools::orderClusterParentsInto(): expected mapping destination view with" << parentFieldSize << "elements but got" << mappingDestination.size(), ); - CORRADE_ASSERT(parentDestination.size() == scene.fieldSize(*parentFieldId), - "SceneTools::orderClusterParentsInto(): expected parent destination view with" << parentFieldSize << "elements but got" << parentDestination.size(), ); - - /* Allocate a single storage for all temporary data */ - Containers::ArrayView> parents; - Containers::ArrayView childrenOffsets; - Containers::ArrayView children; - Containers::ArrayView parentsToProcess; - Containers::ArrayTuple storage{ - /* Output of scene.parentsInto() */ - {NoInit, parentFieldSize, parents}, - /* Running children offset (+1) for each node including root (+1), plus - one more element when we shift the array by one below */ - {ValueInit, std::size_t(scene.mappingBound() + 3), childrenOffsets}, - {NoInit, parentFieldSize, children}, - /* List of parents to process. Can't reuse mappingDestination because - this includes one more element for root objects. */ - {NoInit, parentFieldSize + 1, parentsToProcess} - }; - - /* Convert the parent list to a child list to sort them toplogically */ - scene.parentsInto( - stridedArrayView(parents).slice(&decltype(parents)::Type::first), - stridedArrayView(parents).slice(&decltype(parents)::Type::second) - ); - - /* Children offset for each node including root. First calculate the count - of children for each, skipping the first element (parent.second() can be - -1, accounting for that as well)... */ - for(const Containers::Pair& parent: parents) { - CORRADE_INTERNAL_ASSERT(parent.first() < scene.mappingBound() && (parent.second() == -1 || UnsignedInt(parent.second()) < scene.mappingBound())); - ++childrenOffsets[parent.second() + 2]; - } - - /* ... then convert the counts to a running offset. Now - `[childrenOffsets[i + 2], childrenOffsets[i + 3])` contains a range in - which the `children` array below contains a list of children for `i`. */ - UnsignedInt offset = 0; - for(UnsignedInt& i: childrenOffsets) { - UnsignedInt nextOffset = offset + i; - i = offset; - offset = nextOffset; - } - CORRADE_INTERNAL_ASSERT(offset == parents.size()); - - /* Go through the parent list again, convert that to child ranges. The - childrenOffsets array gets shifted by one element by the process, thus - now `[childrenOffsets[i + 1], childrenOffsets[i + 2])` contains a range - in which the `children` array below contains a list of children for - `i`. */ - for(const Containers::Pair& parent: parents) - children[childrenOffsets[parent.second() + 2]++] = parent.first(); - - /* Go breadth-first (so we have nodes sharing the same parent next to each - other) and build a list of (id, parent id) where a parent is always - before its children */ - std::size_t outputOffset = 0; - parentsToProcess[0] = -1; - for(std::size_t i = 0; i != outputOffset + 1; ++i) { - const Int objectId = parentsToProcess[i]; - for(std::size_t j = childrenOffsets[objectId + 1], jMax = childrenOffsets[objectId + 2]; j != jMax; ++j) { - /** @todo better diagnostic once we can use a BitArray to detect - which nodes are parented more than once (OTOH maybe that's - unnecessary extra work which isn't desired to be done here but - should be instead in a dedicated cycle/sparse checker utility?) */ - CORRADE_ASSERT(outputOffset < parents.size(), - "SceneTools::orderClusterParents(): hierarchy is cyclic", ); - parentsToProcess[outputOffset + 1] = children[j]; - mappingDestination[outputOffset] = children[j]; - parentDestination[outputOffset] = objectId; - ++outputOffset; - } - } - - /** @todo better diagnostic once we can use a BitArray to detect which - nodes are unreachable from root (OTOH again maybe that's undesirable - extra work that doesn't belong here?) */ - CORRADE_ASSERT(outputOffset == parents.size(), - "SceneTools::orderClusterParents(): hierarchy is sparse", ); -} - -}} diff --git a/src/Magnum/SceneTools/OrderClusterParents.h b/src/Magnum/SceneTools/OrderClusterParents.h index 8780f19b0..9b4cbc813 100644 --- a/src/Magnum/SceneTools/OrderClusterParents.h +++ b/src/Magnum/SceneTools/OrderClusterParents.h @@ -25,59 +25,50 @@ DEALINGS IN THE SOFTWARE. */ +#ifdef MAGNUM_BUILD_DEPRECATED /** @file * @brief Function @ref Magnum::SceneTools::orderClusterParents(), @ref Magnum::SceneTools::orderClusterParentsInto() - * @m_since_latest + * @m_deprecated_since_latest Use @ref Magnum/SceneTools/Hierarchy.h and the + * @relativeref{Magnum,SceneTools::parentsBreadthFirst()} function + * instead. */ +#endif -#include "Magnum/Magnum.h" -#include "Magnum/SceneTools/visibility.h" -#include "Magnum/Trade/Trade.h" - -namespace Magnum { namespace SceneTools { - -/** -@brief Calculate ordered and clustered parents -@m_since_latest - -Extracts the @ref Trade::SceneField::Parent field from @p scene and converts it -to match the following rules: +#include "Magnum/configure.h" -- a parent object reference appears always before any of its children -- the array is clustered so children sharing the same parent are together +#ifdef MAGNUM_BUILD_DEPRECATED +#include +#include -This form is useful primarily for calculating absolute object transformations, -for example: +#include "Magnum/SceneTools/Hierarchy.h" -@snippet MagnumSceneTools.cpp orderClusterParents-transformations +CORRADE_DEPRECATED_FILE("use Magnum/SceneTools/Hierarchy.h and the SceneTools::parentsBreadthFirst() function instead") -The operation is done in an @f$ \mathcal{O}(n) @f$ execution time and memory -complexity, with @f$ n @f$ being @ref Trade::SceneData::mappingBound(). The -@ref Trade::SceneField::Parent field is expected to be contained in the scene, -having no cycles (i.e., every node listed just once) and not being sparse -(i.e., every node listed in the field reachable from the root). +namespace Magnum { namespace SceneTools { -@experimental +/* Made header-only to not have to maintain a deprecated source file */ -@see @ref Trade::SceneData::hasField() +/** +@brief Calculate ordered and clustered parents +@m_deprecated_since_latest Use @ref parentsBreadthFirst(const Trade::SceneData&) + instead. */ -MAGNUM_SCENETOOLS_EXPORT Containers::Array> orderClusterParents(const Trade::SceneData& scene); +inline CORRADE_DEPRECATED("use parentsBreadthFirst() instead") Containers::Array> orderClusterParents(const Trade::SceneData& scene) { + return parentsBreadthFirst(scene); +} /** -@brief Calculate ordered and clustered parents into a pre-allocated view -@m_since_latest - -Like @ref orderClusterParents(), but puts the result into -@p mappingDestination and @p parentDestination instead of allocating a new -array. Expect that both views have a size equal to size of the -@ref Trade::SceneField::Parent view in @p scene. - -@experimental - -@see @ref Trade::SceneData::fieldSize(SceneField) const +@brief Calculate ordered and clustered parents +@m_deprecated_since_latest Use @ref parentsBreadthFirstInto(const Trade::SceneData&, const Containers::StridedArrayView1D&, const Containers::StridedArrayView1D&) + instead. */ -MAGNUM_SCENETOOLS_EXPORT void orderClusterParentsInto(const Trade::SceneData& scene, const Containers::StridedArrayView1D& mappingDestination, const Containers::StridedArrayView1D& parentDestination); +inline CORRADE_DEPRECATED("use parentsBreadthFirstInto() instead") void orderClusterParentsInto(const Trade::SceneData& scene, const Containers::StridedArrayView1D& mappingDestination, const Containers::StridedArrayView1D& parentDestination) { + return parentsBreadthFirstInto(scene, mappingDestination, parentDestination); +} }} +#else +#error use Magnum/SceneTools/Hierarchy.h and the SceneTools::parentsBreadthFirst() function instead +#endif #endif diff --git a/src/Magnum/SceneTools/Test/CMakeLists.txt b/src/Magnum/SceneTools/Test/CMakeLists.txt index ffa97e179..654a12940 100644 --- a/src/Magnum/SceneTools/Test/CMakeLists.txt +++ b/src/Magnum/SceneTools/Test/CMakeLists.txt @@ -54,7 +54,6 @@ corrade_add_test(SceneToolsCombineTest CombineTest.cpp LIBRARIES MagnumSceneTool corrade_add_test(SceneToolsConvertToSingleFunc___Test ConvertToSingleFunctionObjectsTest.cpp LIBRARIES MagnumSceneToolsTestLib) corrade_add_test(SceneToolsFilterTest FilterTest.cpp LIBRARIES MagnumSceneToolsTestLib) corrade_add_test(SceneToolsHierarchyTest HierarchyTest.cpp LIBRARIES MagnumSceneToolsTestLib) -corrade_add_test(SceneToolsOrderClusterParentsTest OrderClusterParentsTest.cpp LIBRARIES MagnumSceneToolsTestLib) corrade_add_test(SceneToolsSceneConverterImple___Test SceneConverterImplementationTest.cpp LIBRARIES MagnumSceneTools diff --git a/src/Magnum/SceneTools/Test/HierarchyTest.cpp b/src/Magnum/SceneTools/Test/HierarchyTest.cpp index 84e374bcd..7d56addd5 100644 --- a/src/Magnum/SceneTools/Test/HierarchyTest.cpp +++ b/src/Magnum/SceneTools/Test/HierarchyTest.cpp @@ -24,6 +24,7 @@ */ #include +#include #include #include #include @@ -39,6 +40,19 @@ namespace Magnum { namespace SceneTools { namespace Test { namespace { struct HierarchyTest: TestSuite::Tester { explicit HierarchyTest(); + void parentsBreadthFirst(); + void parentsBreadthFirstNoParentField(); + void parentsBreadthFirstEmptyParentField(); + + void parentsBreadthFirstIntoNoParentField(); + void parentsBreadthFirstIntoEmptyParentField(); + void parentsBreadthFirstIntoWrongDestinationSize(); + + void parentsBreadthFirstSparse(); + void parentsBreadthFirstCyclic(); + void parentsBreadthFirstCyclicDeep(); + void parentsBreadthFirstSparseAndCyclic(); + void absoluteFieldTransformations2D(); void absoluteFieldTransformations3D(); @@ -103,6 +117,19 @@ const struct { }; HierarchyTest::HierarchyTest() { + addTests({&HierarchyTest::parentsBreadthFirst, + &HierarchyTest::parentsBreadthFirstNoParentField, + &HierarchyTest::parentsBreadthFirstEmptyParentField, + + &HierarchyTest::parentsBreadthFirstIntoNoParentField, + &HierarchyTest::parentsBreadthFirstIntoEmptyParentField, + &HierarchyTest::parentsBreadthFirstIntoWrongDestinationSize, + + &HierarchyTest::parentsBreadthFirstSparse, + &HierarchyTest::parentsBreadthFirstCyclic, + &HierarchyTest::parentsBreadthFirstCyclicDeep, + &HierarchyTest::parentsBreadthFirstSparseAndCyclic}); + addInstancedTests({&HierarchyTest::absoluteFieldTransformations2D, &HierarchyTest::absoluteFieldTransformations3D}, Containers::arraySize(TestData)); @@ -118,6 +145,259 @@ HierarchyTest::HierarchyTest() { addTests({&HierarchyTest::absoluteFieldTransformationsIntoInvalidSize}); } +void HierarchyTest::parentsBreadthFirst() { + struct Field { + /* To verify we don't have unnecessarily hardcoded 32-bit types */ + UnsignedShort mapping; + Byte parent; + } data[]{ + /* Backward parent reference */ + {5, 1}, + /* Forward parent reference */ + {6, 9}, + /* Root elements */ + {3, -1}, + {1, -1}, + /* Deep hierarchy */ + {9, 10}, + {10, 3}, + /* Multiple children */ + {7, 3}, + {157, 3}, + {143, 6}, + /* More root elements */ + {2, -1} + /* Elements 0, 4, 8, 11-142, 144-156 deliberately not used */ + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedShort, 158, {}, data, { + /* To verify it doesn't just pick the first field ever */ + Trade::SceneFieldData{Trade::SceneField::Mesh, + Trade::SceneMappingType::UnsignedShort, nullptr, + Trade::SceneFieldType::UnsignedInt, nullptr}, + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::mapping), + view.slice(&Field::parent)} + }}; + + CORRADE_COMPARE_AS(SceneTools::parentsBreadthFirst(scene), (Containers::arrayView>({ + /* Root objects first, in order as found */ + {3, -1}, + {1, -1}, + {2, -1}, + /* Then children of node 3, clustered together, in order as found */ + {10, 3}, + {7, 3}, + {157, 3}, + /* Then children of node 1 */ + {5, 1}, + /* Children of node 10 */ + {9, 10}, + /* Children of node 9 */ + {6, 9}, + /* Children of node 6 */ + {143, 6}, + })), TestSuite::Compare::Container); +} + +void HierarchyTest::parentsBreadthFirstNoParentField() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedByte, 0, nullptr, {}}; + + std::ostringstream out; + Error redirectError{&out}; + SceneTools::parentsBreadthFirst(scene); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirst(): the scene has no hierarchy\n"); +} + +void HierarchyTest::parentsBreadthFirstEmptyParentField() { + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 0, nullptr, { + Trade::SceneFieldData{Trade::SceneField::Parent, Trade::SceneMappingType::UnsignedInt, nullptr, Trade::SceneFieldType::Int, nullptr} + }}; + + CORRADE_COMPARE_AS(SceneTools::parentsBreadthFirst(scene), + (Containers::ArrayView>{}), + TestSuite::Compare::Container); +} + +void HierarchyTest::parentsBreadthFirstIntoNoParentField() { + CORRADE_SKIP_IF_NO_ASSERT(); + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedByte, 0, nullptr, {}}; + + std::ostringstream out; + Error redirectError{&out}; + parentsBreadthFirstInto(scene, nullptr, nullptr); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirstInto(): the scene has no hierarchy\n"); +} + +void HierarchyTest::parentsBreadthFirstIntoEmptyParentField() { + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 0, nullptr, { + Trade::SceneFieldData{Trade::SceneField::Parent, Trade::SceneMappingType::UnsignedInt, nullptr, Trade::SceneFieldType::Int, nullptr} + }}; + + parentsBreadthFirstInto(scene, nullptr, nullptr); + CORRADE_VERIFY(true); +} + +void HierarchyTest::parentsBreadthFirstIntoWrongDestinationSize() { + CORRADE_SKIP_IF_NO_ASSERT(); + + struct Field { + UnsignedInt mapping; + Int parent; + } data[]{ + {2, -1}, + {3, 2}, + {7, -1} + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 8, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::mapping), + view.slice(&Field::parent)} + }}; + + UnsignedInt mappingCorrect[3]; + UnsignedInt mapping[2]; + Int parentOffsetCorrect[3]; + Int parentOffset[2]; + + std::ostringstream out; + Error redirectError{&out}; + parentsBreadthFirstInto(scene, mappingCorrect, parentOffset); + parentsBreadthFirstInto(scene, mapping, parentOffsetCorrect); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirstInto(): expected parent destination view with 3 elements but got 2\n" + "SceneTools::parentsBreadthFirstInto(): expected mapping destination view with 3 elements but got 2\n"); +} + +void HierarchyTest::parentsBreadthFirstSparse() { + CORRADE_SKIP_IF_NO_ASSERT(); + + struct Field { + UnsignedInt object; + Int parent; + } data[]{ + {2, -1}, + {3, 2}, + {7, -1}, + /* Not reachable from root */ + {15, 6}, + {14, 6}, + {11, 15}, + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::object), + view.slice(&Field::parent)} + }}; + + std::ostringstream out; + Error redirectError{&out}; + SceneTools::parentsBreadthFirst(scene); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirst(): hierarchy is sparse\n"); +} + +void HierarchyTest::parentsBreadthFirstCyclic() { + CORRADE_SKIP_IF_NO_ASSERT(); + + struct Field { + UnsignedInt object; + Int parent; + } data[]{ + {2, -1}, + {3, 2}, + {7, -1}, + /* Cycle of length 1, which will be treated as sparse hierarchy */ + {13, 13} + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::object), + view.slice(&Field::parent)} + }}; + + std::ostringstream out; + Error redirectError{&out}; + SceneTools::parentsBreadthFirst(scene); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirst(): hierarchy is sparse\n"); +} + +void HierarchyTest::parentsBreadthFirstCyclicDeep() { + CORRADE_SKIP_IF_NO_ASSERT(); + + struct Field { + UnsignedInt object; + Int parent; + } data[]{ + {2, -1}, + {3, 2}, + {7, -1}, + /* Cycle of length 3 */ + {13, -1}, + {5, 13}, + {13, 3} + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::object), + view.slice(&Field::parent)} + }}; + + std::ostringstream out; + Error redirectError{&out}; + SceneTools::parentsBreadthFirst(scene); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirst(): hierarchy is cyclic\n"); +} + +void HierarchyTest::parentsBreadthFirstSparseAndCyclic() { + CORRADE_SKIP_IF_NO_ASSERT(); + + struct Field { + UnsignedInt object; + Int parent; + } data[]{ + {2, -1}, + {3, 2}, + {7, -1}, + /* Cycle of length 3 */ + {13, -1}, + {5, 13}, + {13, 3}, + /* Not reachable from root */ + {15, 6} + }; + Containers::StridedArrayView1D view = data; + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Parent, + view.slice(&Field::object), + view.slice(&Field::parent)} + }}; + + std::ostringstream out; + Error redirectError{&out}; + SceneTools::parentsBreadthFirst(scene); + CORRADE_EXPECT_FAIL("The implementation needs to track already visited objects with a BitArray to detect this, it'd also provide a much better diagnostic."); + CORRADE_COMPARE(out.str(), + "SceneTools::parentsBreadthFirst(): hierarchy is cyclic\n"); +} + const struct Scene { /* Using smaller types to verify we don't have unnecessarily hardcoded 32-bit types */ diff --git a/src/Magnum/SceneTools/Test/OrderClusterParentsTest.cpp b/src/Magnum/SceneTools/Test/OrderClusterParentsTest.cpp deleted file mode 100644 index 7a04e8893..000000000 --- a/src/Magnum/SceneTools/Test/OrderClusterParentsTest.cpp +++ /dev/null @@ -1,324 +0,0 @@ -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, - 2020, 2021, 2022 Vladimír Vondruš - - Permission is hereby granted, free of charge, to any person obtaining a - copy of this software and associated documentation files (the "Software"), - to deal in the Software without restriction, including without limitation - the rights to use, copy, modify, merge, publish, distribute, sublicense, - and/or sell copies of the Software, and to permit persons to whom the - Software is furnished to do so, subject to the following conditions: - - The above copyright notice and this permission notice shall be included - in all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING - FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER - DEALINGS IN THE SOFTWARE. -*/ - -#include -#include -#include -#include -#include - -#include "Magnum/SceneTools/OrderClusterParents.h" -#include "Magnum/Trade/SceneData.h" - -namespace Magnum { namespace SceneTools { namespace Test { namespace { - -struct OrderClusterParentsTest: TestSuite::Tester { - explicit OrderClusterParentsTest(); - - void test(); - void noParentField(); - void emptyParentField(); - - void intoNoParentField(); - void intoEmptyParentField(); - void intoWrongDestinationSize(); - - void sparse(); - void cyclic(); - void cyclicDeep(); - void sparseAndCyclic(); -}; - -OrderClusterParentsTest::OrderClusterParentsTest() { - addTests({&OrderClusterParentsTest::test, - &OrderClusterParentsTest::noParentField, - &OrderClusterParentsTest::emptyParentField, - - &OrderClusterParentsTest::intoNoParentField, - &OrderClusterParentsTest::intoEmptyParentField, - &OrderClusterParentsTest::intoWrongDestinationSize, - - &OrderClusterParentsTest::sparse, - &OrderClusterParentsTest::cyclic, - &OrderClusterParentsTest::cyclicDeep, - &OrderClusterParentsTest::sparseAndCyclic}); -} - -void OrderClusterParentsTest::test() { - struct Field { - /* To verify we don't have unnecessarily hardcoded 32-bit types */ - UnsignedShort mapping; - Byte parent; - } data[]{ - /* Backward parent reference */ - {5, 1}, - /* Forward parent reference */ - {6, 9}, - /* Root elements */ - {3, -1}, - {1, -1}, - /* Deep hierarchy */ - {9, 10}, - {10, 3}, - /* Multiple children */ - {7, 3}, - {157, 3}, - {143, 6}, - /* More root elements */ - {2, -1} - /* Elements 0, 4, 8, 11-142, 144-156 deliberately not used */ - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedShort, 158, {}, data, { - /* To verify it doesn't just pick the first field ever */ - Trade::SceneFieldData{Trade::SceneField::Mesh, - Trade::SceneMappingType::UnsignedShort, nullptr, - Trade::SceneFieldType::UnsignedInt, nullptr}, - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::mapping), - view.slice(&Field::parent)} - }}; - - CORRADE_COMPARE_AS(orderClusterParents(scene), (Containers::arrayView>({ - /* Root objects first, in order as found */ - {3, -1}, - {1, -1}, - {2, -1}, - /* Then children of node 3, clustered together, in order as found */ - {10, 3}, - {7, 3}, - {157, 3}, - /* Then children of node 1 */ - {5, 1}, - /* Children of node 10 */ - {9, 10}, - /* Children of node 9 */ - {6, 9}, - /* Children of node 6 */ - {143, 6}, - })), TestSuite::Compare::Container); -} - -void OrderClusterParentsTest::noParentField() { - CORRADE_SKIP_IF_NO_ASSERT(); - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedByte, 0, nullptr, {}}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParents(scene); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParents(): the scene has no hierarchy\n"); -} - -void OrderClusterParentsTest::emptyParentField() { - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 0, nullptr, { - Trade::SceneFieldData{Trade::SceneField::Parent, Trade::SceneMappingType::UnsignedInt, nullptr, Trade::SceneFieldType::Int, nullptr} - }}; - - CORRADE_COMPARE_AS(orderClusterParents(scene), - (Containers::ArrayView>{}), - TestSuite::Compare::Container); -} - -void OrderClusterParentsTest::intoNoParentField() { - CORRADE_SKIP_IF_NO_ASSERT(); - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedByte, 0, nullptr, {}}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParentsInto(scene, nullptr, nullptr); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParentsInto(): the scene has no hierarchy\n"); -} - -void OrderClusterParentsTest::intoEmptyParentField() { - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 0, nullptr, { - Trade::SceneFieldData{Trade::SceneField::Parent, Trade::SceneMappingType::UnsignedInt, nullptr, Trade::SceneFieldType::Int, nullptr} - }}; - - orderClusterParentsInto(scene, nullptr, nullptr); - CORRADE_VERIFY(true); -} - -void OrderClusterParentsTest::intoWrongDestinationSize() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct Field { - UnsignedInt mapping; - Int parent; - } data[]{ - {2, -1}, - {3, 2}, - {7, -1} - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 8, {}, data, { - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::mapping), - view.slice(&Field::parent)} - }}; - - UnsignedInt mappingCorrect[3]; - UnsignedInt mapping[2]; - Int parentOffsetCorrect[3]; - Int parentOffset[2]; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParentsInto(scene, mappingCorrect, parentOffset); - orderClusterParentsInto(scene, mapping, parentOffsetCorrect); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParentsInto(): expected parent destination view with 3 elements but got 2\n" - "SceneTools::orderClusterParentsInto(): expected mapping destination view with 3 elements but got 2\n"); -} - -void OrderClusterParentsTest::sparse() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct Field { - UnsignedInt object; - Int parent; - } data[]{ - {2, -1}, - {3, 2}, - {7, -1}, - /* Not reachable from root */ - {15, 6}, - {14, 6}, - {11, 15}, - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::object), - view.slice(&Field::parent)} - }}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParents(scene); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParents(): hierarchy is sparse\n"); -} - -void OrderClusterParentsTest::cyclic() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct Field { - UnsignedInt object; - Int parent; - } data[]{ - {2, -1}, - {3, 2}, - {7, -1}, - /* Cycle of length 1, which will be treated as sparse hierarchy */ - {13, 13} - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::object), - view.slice(&Field::parent)} - }}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParents(scene); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParents(): hierarchy is sparse\n"); -} - -void OrderClusterParentsTest::cyclicDeep() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct Field { - UnsignedInt object; - Int parent; - } data[]{ - {2, -1}, - {3, 2}, - {7, -1}, - /* Cycle of length 3 */ - {13, -1}, - {5, 13}, - {13, 3} - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::object), - view.slice(&Field::parent)} - }}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParents(scene); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParents(): hierarchy is cyclic\n"); -} - -void OrderClusterParentsTest::sparseAndCyclic() { - CORRADE_SKIP_IF_NO_ASSERT(); - - struct Field { - UnsignedInt object; - Int parent; - } data[]{ - {2, -1}, - {3, 2}, - {7, -1}, - /* Cycle of length 3 */ - {13, -1}, - {5, 13}, - {13, 3}, - /* Not reachable from root */ - {15, 6} - }; - Containers::StridedArrayView1D view = data; - - Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 16, {}, data, { - Trade::SceneFieldData{Trade::SceneField::Parent, - view.slice(&Field::object), - view.slice(&Field::parent)} - }}; - - std::ostringstream out; - Error redirectError{&out}; - orderClusterParents(scene); - CORRADE_EXPECT_FAIL("The implementation needs to track already visited objects with a BitArray to detect this, it'd also provide a much better diagnostic."); - CORRADE_COMPARE(out.str(), - "SceneTools::orderClusterParents(): hierarchy is cyclic\n"); -} - -}}}} - -CORRADE_TEST_MAIN(Magnum::SceneTools::Test::OrderClusterParentsTest)