From 48bd603236c39282d63a5b995be6e56828f54ef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 8 Nov 2022 16:51:25 +0100 Subject: [PATCH] SceneTools: pin down current flattenMeshHierarchy() behavior. It doesn't discard meshes that are not a part of the hierarchy, but that was the plan in the beginning. However, over the time I realized that a better property for it is that the output is guaranteed to be in the same order and size as the mesh field in the scene. Because that's what I relied on in every use case, and every time I had to dig that property out of the sources because it was deliberately not documented *because* it was meant to change. No longer. The compatibility with the mesh field ordering is now documented, the behavior regarding loose objects also, and if there's ever a need to discard everything that's not in the reachable hierarchy, it'll probably get its own API. Because it's useful for general asset cleanup and other use cases, not just meshes. I'm still keeping the experimental tag here though, tp be sure. --- .../SceneTools/FlattenMeshHierarchy.cpp | 2 - src/Magnum/SceneTools/FlattenMeshHierarchy.h | 30 ++++++--- .../Test/FlattenMeshHierarchyTest.cpp | 67 ++++++------------- 3 files changed, 41 insertions(+), 58 deletions(-) diff --git a/src/Magnum/SceneTools/FlattenMeshHierarchy.cpp b/src/Magnum/SceneTools/FlattenMeshHierarchy.cpp index f62ecfa3a..9c6edd490 100644 --- a/src/Magnum/SceneTools/FlattenMeshHierarchy.cpp +++ b/src/Magnum/SceneTools/FlattenMeshHierarchy.cpp @@ -113,8 +113,6 @@ Containers::Array>> out{NoInit, scene.fieldSize(Trade::SceneField::Mesh)}; const auto matrices = stridedArrayView(out).slice(&decltype(out)::Type::third); const auto mapping = Containers::arrayCast(matrices); diff --git a/src/Magnum/SceneTools/FlattenMeshHierarchy.h b/src/Magnum/SceneTools/FlattenMeshHierarchy.h index 3ce14cd0c..363150f5d 100644 --- a/src/Magnum/SceneTools/FlattenMeshHierarchy.h +++ b/src/Magnum/SceneTools/FlattenMeshHierarchy.h @@ -40,11 +40,11 @@ namespace Magnum { namespace SceneTools { @brief Flatten a 2D mesh hierarchy @m_since_latest -For all @ref Trade::SceneField::Mesh entries that are a part of a hierarchy -returns a triple of mesh ID, @ref Trade::SceneField::MeshMaterial and its -absolute transformation in the scene with @p globalTransformation prepended. -The @ref Trade::SceneField::Parent field is expected to be contained in the -scene, having no cycles or duplicates, and the scene is expected to be 2D. If +For all @ref Trade::SceneField::Mesh entries returns a triple of mesh ID, +@ref Trade::SceneField::MeshMaterial and its absolute transformation in the +scene with @p globalTransformation prepended. The +@ref Trade::SceneField::Parent field is expected to be contained in the scene, +having no cycles or duplicates, and the scene is expected to be 2D. If @ref Trade::SceneField::Mesh is not present or is empty, returns an empty array. You can then use @ref MeshTools::transform2D() to apply the transformations to actual meshes: @@ -56,6 +56,11 @@ memory complexity, with @f$ m @f$ being size of the @ref Trade::SceneField::Mesh field and @f$ n @f$ being @ref Trade::SceneData::mappingBound(). The function calls @ref orderClusterParents() internally. +The returned data are in the same order as the @ref Trade::SceneField::Mesh +attribute. Meshes attached to objects without a @ref Trade::SceneField::Parent +or to objects in loose hierarchy subtrees will have their transformation set to +an unspecified value. + @experimental @see @ref Trade::SceneData::hasField(), @ref Trade::SceneData::is2D(), @@ -73,11 +78,11 @@ MAGNUM_SCENETOOLS_EXPORT Containers::Arrayparents) - .slice(&Data::Parent::object) - .exceptSuffix(instanceData.parentsToExclude), + .slice(&Data::Parent::object), Containers::stridedArrayView(data->parents) - .slice(&Data::Parent::parent) - .exceptSuffix(instanceData.parentsToExclude)}, + .slice(&Data::Parent::parent)}, Trade::SceneFieldData{Trade::SceneField::Transformation, Containers::stridedArrayView(data->transforms) .slice(&Data::Transformation::object) @@ -187,17 +172,17 @@ void FlattenMeshHierarchyTest::test2D() { Trade::SceneFieldData{Trade::SceneField::Mesh, Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::object) - .exceptPrefix(instanceData.meshesToExclude), + .exceptSuffix(instanceData.meshesToExclude), Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::mesh) - .exceptPrefix(instanceData.meshesToExclude)}, + .exceptSuffix(instanceData.meshesToExclude)}, Trade::SceneFieldData{Trade::SceneField::MeshMaterial, Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::object) - .exceptPrefix(instanceData.meshesToExclude), + .exceptSuffix(instanceData.meshesToExclude), Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::meshMaterial) - .exceptPrefix(instanceData.meshesToExclude)}, + .exceptSuffix(instanceData.meshesToExclude)}, }}; Containers::Array> out; @@ -207,8 +192,6 @@ void FlattenMeshHierarchyTest::test2D() { else out = flattenMeshHierarchy2D(scene); - CORRADE_EXPECT_FAIL_IF(instanceData.meshesToExclude == 0 || instanceData.parentsToExclude != 0, - "Meshes that are not part of the hierarchy are not excluded at the moment."); CORRADE_COMPARE_AS(out, (Containers::arrayView>({ {113, 96, instanceData.globalTransformation2D* Matrix3::translation({1.0f, -1.5f})* @@ -250,7 +233,7 @@ void FlattenMeshHierarchyTest::test3D() { UnsignedShort object; UnsignedShort mesh; Short meshMaterial; - } meshes[8]; + } meshes[5]; } data[]{{ /* Cases to test: @@ -296,11 +279,7 @@ void FlattenMeshHierarchyTest::test3D() { {32, Matrix4::translation({1.0f, 0.5f, 2.0f})}, {17, Matrix4::translation({2.0f, 1.0f, 4.0f})}, }, - {{0, 262, 33}, - {32, 155, 47}, - {0, 127, -1}, - /* The above are not part of the hierarchy */ - {2, 113, 96}, + {{2, 113, 96}, {3, 266, 74}, {4, 525, 33}, {3, 422, -1}, @@ -312,11 +291,9 @@ void FlattenMeshHierarchyTest::test3D() { Trade::SceneFieldData{Trade::SceneField::Camera, Trade::SceneMappingType::UnsignedShort, nullptr, Trade::SceneFieldType::UnsignedInt, nullptr}, Trade::SceneFieldData{Trade::SceneField::Parent, Containers::stridedArrayView(data->parents) - .slice(&Data::Parent::object) - .exceptSuffix(instanceData.parentsToExclude), + .slice(&Data::Parent::object), Containers::stridedArrayView(data->parents) - .slice(&Data::Parent::parent) - .exceptSuffix(instanceData.parentsToExclude)}, + .slice(&Data::Parent::parent)}, Trade::SceneFieldData{Trade::SceneField::Transformation, Containers::stridedArrayView(data->transforms) .slice(&Data::Transformation::object) @@ -327,17 +304,17 @@ void FlattenMeshHierarchyTest::test3D() { Trade::SceneFieldData{Trade::SceneField::Mesh, Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::object) - .exceptPrefix(instanceData.meshesToExclude), + .exceptSuffix(instanceData.meshesToExclude), Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::mesh) - .exceptPrefix(instanceData.meshesToExclude)}, + .exceptSuffix(instanceData.meshesToExclude)}, Trade::SceneFieldData{Trade::SceneField::MeshMaterial, Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::object) - .exceptPrefix(instanceData.meshesToExclude), + .exceptSuffix(instanceData.meshesToExclude), Containers::stridedArrayView(data->meshes) .slice(&Data::Mesh::meshMaterial) - .exceptPrefix(instanceData.meshesToExclude)}, + .exceptSuffix(instanceData.meshesToExclude)}, }}; Containers::Array> out; @@ -347,8 +324,6 @@ void FlattenMeshHierarchyTest::test3D() { else out = flattenMeshHierarchy3D(scene); - CORRADE_EXPECT_FAIL_IF(instanceData.meshesToExclude == 0 || instanceData.parentsToExclude != 0, - "Meshes that are not part of the hierarchy are not excluded at the moment."); CORRADE_COMPARE_AS(out, (Containers::arrayView>({ {113, 96, instanceData.globalTransformation3D* Matrix4::translation({1.0f, -1.5f, 0.5f})*