diff --git a/src/Magnum/Trade/Implementation/sceneTools.h b/src/Magnum/Trade/Implementation/sceneTools.h index a4242f6c3..9c1e05605 100644 --- a/src/Magnum/Trade/Implementation/sceneTools.h +++ b/src/Magnum/Trade/Implementation/sceneTools.h @@ -185,7 +185,7 @@ inline SceneData sceneCombine(const SceneMappingType mappingType, const Unsigned /* Map the fields to the new data */ Containers::Array outFields{fields.size()}; for(std::size_t i = 0; i != fields.size(); ++i) { - outFields[i] = SceneFieldData{fields[i].name(), itemViews[itemViewMappings[i].first()], fields[i].fieldType(), itemViews[itemViewMappings[i].second()], fields[i].fieldArraySize()}; + outFields[i] = SceneFieldData{fields[i].name(), itemViews[itemViewMappings[i].first()], fields[i].fieldType(), itemViews[itemViewMappings[i].second()], fields[i].fieldArraySize(), fields[i].flags()}; } return SceneData{mappingType, mappingBound, std::move(outData), std::move(outFields)}; @@ -239,14 +239,26 @@ inline SceneData sceneConvertToSingleFunctionObjects(const SceneData& scene, Con for(std::size_t i = 0; i != scene.fieldCount(); ++i) { const SceneFieldData& field = scene.fieldData(i); - /* If this is a parent, enlarge it for the newly added objects */ + /* If this is a parent, enlarge it for the newly added objects, and if + it was implicit make it ordered */ if(field.name() == SceneField::Parent) { /** @todo some nicer constructor for placeholders once this is in public interest */ - fields[i] = SceneFieldData{SceneField::Parent, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}}; - - /* All other fields are copied as-is */ - } else fields[i] = field; + fields[i] = SceneFieldData{SceneField::Parent, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}, Containers::ArrayView{nullptr, std::size_t(field.size() + objectsToAdd)}, + /* If the field is ordered, we preserve that. But if it's + implicit, we can't. */ + field.flags() & ~(SceneFieldFlag::ImplicitMapping & ~SceneFieldFlag::OrderedMapping) + }; + + /* All other fields are copied as-is, but lose the implicit/ordered + flags */ + /** @todo the flags could get preserved for + - fields that don't share their object mapping with any fields in + the fieldsToConvert list + - fields that don't actually get their object mapping touched + during the process (and then all fields that share object + mapping with them) */ + } else fields[i] = SceneFieldData{field.name(), field.mappingType(), field.mappingData(), field.fieldType(), field.fieldData(), field.fieldArraySize(), field.flags() & ~SceneFieldFlag::ImplicitMapping}; } /* Combine the fields into a new SceneData */ @@ -292,6 +304,11 @@ inline SceneData sceneConvertToSingleFunctionObjects(const SceneData& scene, Con fieldObject = newParentMapping[newParentIndex]; /* Move to the next reserved object */ ++newParentIndex; + + /** @todo mark this field as touched here and the restore + the original flags for all fields that didn't have + their object mapping touched */ + } else ++objectAttachmentCount[fieldObject]; } } diff --git a/src/Magnum/Trade/Test/SceneToolsTest.cpp b/src/Magnum/Trade/Test/SceneToolsTest.cpp index 2cb3ae373..807b1839e 100644 --- a/src/Magnum/Trade/Test/SceneToolsTest.cpp +++ b/src/Magnum/Trade/Test/SceneToolsTest.cpp @@ -59,9 +59,17 @@ struct { const char* name; UnsignedLong originalObjectCount; UnsignedLong expectedObjectCount; + SceneFieldFlags parentFieldFlagsInput; + SceneFieldFlags parentFieldFlagsExpected; } ConvertToSingleFunctionObjectsData[]{ - {"original object count smaller than new", 64, 67}, - {"original object count larger than new", 96, 96} + {"original object count smaller than new", 64, 70, {}, {}}, + {"original object count larger than new", 96, 96, {}, {}}, + {"parent field with ordered mapping", 64, 70, + SceneFieldFlag::OrderedMapping, SceneFieldFlag::OrderedMapping}, + {"parent field with implicit mapping", 64, 70, + /* The mapping is *not* implicit but we're not using the flag for + anything so this should work */ + SceneFieldFlag::ImplicitMapping, SceneFieldFlag::OrderedMapping} }; SceneToolsTest::SceneToolsTest() { @@ -89,8 +97,8 @@ void SceneToolsTest::combine() { const UnsignedInt meshMappingData[]{45, 78, 23}; const UnsignedByte meshFieldData[]{3, 5, 17}; - const UnsignedShort parentMappingData[]{33, 25}; - const Short parentData[]{-1, 33}; + const UnsignedShort parentMappingData[]{0, 1}; + const Short parentData[]{-1, 0}; const UnsignedByte translationMappingData[]{16}; const Vector2d translationFieldData[]{{1.5, -0.5}}; @@ -100,10 +108,10 @@ void SceneToolsTest::combine() { SceneData scene = Implementation::sceneCombine(data.objectType, 167, Containers::arrayView({ SceneFieldData{SceneField::Mesh, Containers::arrayView(meshMappingData), Containers::arrayView(meshFieldData)}, - SceneFieldData{SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentData)}, + SceneFieldData{SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentData), SceneFieldFlag::ImplicitMapping}, SceneFieldData{SceneField::Translation, Containers::arrayView(translationMappingData), Containers::arrayView(translationFieldData)}, /* Array field */ - SceneFieldData{sceneFieldCustom(15), Containers::arrayView(fooMappingData), Containers::StridedArrayView2D{fooFieldData, {2, 2}}}, + SceneFieldData{sceneFieldCustom(15), Containers::arrayView(fooMappingData), Containers::StridedArrayView2D{fooFieldData, {2, 2}}, SceneFieldFlag::OrderedMapping}, /* Empty field */ SceneFieldData{SceneField::Camera, Containers::ArrayView{}, Containers::ArrayView{}} })); @@ -114,6 +122,7 @@ void SceneToolsTest::combine() { CORRADE_COMPARE(scene.fieldCount(), 5); CORRADE_COMPARE(scene.fieldName(0), SceneField::Mesh); + CORRADE_COMPARE(scene.fieldFlags(0), SceneFieldFlags{}); CORRADE_COMPARE(scene.fieldType(0), SceneFieldType::UnsignedByte); CORRADE_COMPARE(scene.fieldArraySize(0), 0); CORRADE_COMPARE_AS(scene.mappingAsArray(0), Containers::arrayView({ @@ -124,16 +133,18 @@ void SceneToolsTest::combine() { TestSuite::Compare::Container); CORRADE_COMPARE(scene.fieldName(1), SceneField::Parent); + CORRADE_COMPARE(scene.fieldFlags(1), SceneFieldFlag::ImplicitMapping); CORRADE_COMPARE(scene.fieldType(1), SceneFieldType::Short); CORRADE_COMPARE(scene.fieldArraySize(1), 0); CORRADE_COMPARE_AS(scene.mappingAsArray(1), Containers::arrayView({ - 33, 25 + 0, 1 }), TestSuite::Compare::Container); CORRADE_COMPARE_AS(scene.field(1), Containers::arrayView(parentData), TestSuite::Compare::Container); CORRADE_COMPARE(scene.fieldName(2), SceneField::Translation); + CORRADE_COMPARE(scene.fieldFlags(2), SceneFieldFlags{}); CORRADE_COMPARE(scene.fieldType(2), SceneFieldType::Vector2d); CORRADE_COMPARE(scene.fieldArraySize(2), 0); CORRADE_COMPARE_AS(scene.mappingAsArray(2), @@ -144,6 +155,7 @@ void SceneToolsTest::combine() { TestSuite::Compare::Container); CORRADE_COMPARE(scene.fieldName(3), sceneFieldCustom(15)); + CORRADE_COMPARE(scene.fieldFlags(3), SceneFieldFlag::OrderedMapping); CORRADE_COMPARE(scene.fieldType(3), SceneFieldType::Int); CORRADE_COMPARE(scene.fieldArraySize(3), 2); CORRADE_COMPARE_AS(scene.mappingAsArray(3), @@ -159,6 +171,7 @@ void SceneToolsTest::combine() { TestSuite::Compare::Container); CORRADE_COMPARE(scene.fieldName(4), SceneField::Camera); + CORRADE_COMPARE(scene.fieldFlags(4), SceneFieldFlags{}); CORRADE_COMPARE(scene.fieldType(4), SceneFieldType::UnsignedShort); CORRADE_COMPARE(scene.fieldSize(4), 0); CORRADE_COMPARE(scene.fieldArraySize(4), 0); @@ -340,12 +353,12 @@ void SceneToolsTest::convertToSingleFunctionObjects() { /* Haha now I can use sceneCombine() to conveniently prepare the initial state here, without having to mess with an ArrayTuple */ - const UnsignedShort parentMappingData[]{15, 21, 22, 23, 1}; - const Byte parentFieldData[]{-1, -1, 21, 22, -1}; + const UnsignedShort parentMappingData[]{2, 15, 21, 22, 23}; + const Byte parentFieldData[]{-1, -1, -1, 21, 22}; /* Two objects have two and three mesh assignments respectively, meaning we need three extra */ - const UnsignedShort meshMappingData[]{15, 23, 23, 23, 1, 15, 21}; + const UnsignedShort meshMappingData[]{15, 23, 23, 23, 2, 15, 21}; const Containers::Pair meshMaterialFieldData[]{ {6, 4}, {1, 0}, @@ -357,14 +370,39 @@ void SceneToolsTest::convertToSingleFunctionObjects() { }; /* One camera is attached to an object that already has a mesh, meaning we - need a third extra object */ - const UnsignedShort cameraMappingData[]{22, 1}; + need a fourth extra object */ + const UnsignedShort cameraMappingData[]{22, 2}; const UnsignedInt cameraFieldData[]{1, 5}; + + /* Lights don't conflict with anything so they *could* retain the + ImplicitMapping flag */ + const UnsignedShort lightMappingData[]{0, 1}; + const UnsignedByte lightFieldData[]{15, 23}; + + /* Object 0 and 1 has a light, 2 a mesh already, meaning we need a fifth, + sixth and seventh extra object and we lose the ImplicitMapping flag. */ + const UnsignedShort fooMappingData[]{0, 1, 2, 3}; + const Float fooFieldData[]{1.0f, 2.0f, 3.0f, 4.0f}; + + /* This field is not among the fields to convert so it should preserve the + ImplicitMapping flag */ + const UnsignedShort foo2MappingData[]{0, 1}; + const Byte foo2FieldData[]{-5, -7}; + + /* This field shares mapping with foo (and thus has the ImplicitMapping + flag), but it's not among the fields to convert. Since the mapping gets + changed, it should not retain the ImplicitMapping flag. */ + const Byte foo3FieldData[]{-1, -2, 7, 2}; + SceneData original = Implementation::sceneCombine(SceneMappingType::UnsignedShort, data.originalObjectCount, Containers::arrayView({ - SceneFieldData{SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentFieldData)}, + SceneFieldData{SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentFieldData), data.parentFieldFlagsInput}, SceneFieldData{SceneField::Mesh, Containers::arrayView(meshMappingData), Containers::StridedArrayView1D{meshMaterialFieldData, &meshMaterialFieldData[0].first(), Containers::arraySize(meshMaterialFieldData), sizeof(meshMaterialFieldData[0])}}, SceneFieldData{SceneField::MeshMaterial, Containers::arrayView(meshMappingData), Containers::StridedArrayView1D{meshMaterialFieldData, &meshMaterialFieldData[0].second(), Containers::arraySize(meshMaterialFieldData), sizeof(meshMaterialFieldData[0])}}, SceneFieldData{SceneField::Camera, Containers::arrayView(cameraMappingData), Containers::arrayView(cameraFieldData)}, + SceneFieldData{SceneField::Light, Containers::arrayView(lightMappingData), Containers::arrayView(lightFieldData), SceneFieldFlag::ImplicitMapping}, + SceneFieldData{sceneFieldCustom(15), Containers::arrayView(fooMappingData), Containers::arrayView(fooFieldData), SceneFieldFlag::ImplicitMapping}, + SceneFieldData{sceneFieldCustom(16), Containers::arrayView(foo2MappingData), Containers::arrayView(foo2FieldData), SceneFieldFlag::ImplicitMapping}, + SceneFieldData{sceneFieldCustom(17), Containers::arrayView(fooMappingData), Containers::arrayView(foo3FieldData), SceneFieldFlag::ImplicitMapping} })); SceneData scene = Implementation::sceneConvertToSingleFunctionObjects(original, Containers::arrayView({ @@ -373,6 +411,12 @@ void SceneToolsTest::convertToSingleFunctionObjects() { get automatically updated as they share the same object mapping. OTOH including them would break the output. */ SceneField::Camera, + /* A field with implicit mapping that doesn't conflict with anything so + it *could* retain the flag */ + SceneField::Light, + /* A field with implicit mapping, which loses the flag because entries + get reassigned */ + sceneFieldCustom(15), /* Include also a field that's not present -- it should get skipped */ SceneField::ImporterState }), 63); @@ -381,14 +425,31 @@ void SceneToolsTest::convertToSingleFunctionObjects() { it's large enough */ CORRADE_COMPARE(scene.mappingBound(), data.expectedObjectCount); - /* Object 1 should have a new child that has the camera, as it has a mesh */ + /* Object 0 should have new children with "foo", as it has a light */ + CORRADE_COMPARE_AS(scene.childrenFor(0), + Containers::arrayView({67}), + TestSuite::Compare::Container); + CORRADE_COMPARE_AS(scene.lightsFor(0), + Containers::arrayView({15}), + TestSuite::Compare::Container); + + /* Object 1 should have a new child with "foo", as it has a light */ CORRADE_COMPARE_AS(scene.childrenFor(1), - Containers::arrayView({66}), + Containers::arrayView({68}), + TestSuite::Compare::Container); + CORRADE_COMPARE_AS(scene.lightsFor(1), + Containers::arrayView({23}), + TestSuite::Compare::Container); + + /* Object 2 should have a new child with the camera and "foo", as it has a + mesh */ + CORRADE_COMPARE_AS(scene.childrenFor(2), + Containers::arrayView({66, 69}), TestSuite::Compare::Container); - CORRADE_COMPARE_AS(scene.meshesMaterialsFor(1), + CORRADE_COMPARE_AS(scene.meshesMaterialsFor(2), (Containers::arrayView>({{7, 2}})), TestSuite::Compare::Container); - CORRADE_COMPARE_AS(scene.camerasFor(1), + CORRADE_COMPARE_AS(scene.camerasFor(2), Containers::arrayView({}), TestSuite::Compare::Container); CORRADE_COMPARE_AS(scene.camerasFor(66), @@ -422,36 +483,95 @@ void SceneToolsTest::convertToSingleFunctionObjects() { TestSuite::Compare::Container); /* To be extra sure, verify the actual data. Parents have a few objects - added, the rest is the same */ + added, the rest is the same. Because new objects are added at the end, + the ordered flag is preserved if present. */ CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView>({ + {2, -1}, {15, -1}, {21, -1}, {22, 21}, {23, 22}, - {1, -1}, {63, 23}, {64, 23}, {65, 15}, - {66, 1} + {66, 2}, + {67, 0}, + {68, 1}, + {69, 2}, })), TestSuite::Compare::Container); + CORRADE_COMPARE(scene.fieldFlags(SceneField::Parent), data.parentFieldFlagsExpected); /* Meshes / materials have certain objects reassigned, field data stay the - same */ + same. There was no flag before so neither is after. */ CORRADE_COMPARE_AS(scene.meshesMaterialsAsArray(), (Containers::arrayView>>({ {15, {6, 4}}, {23, {1, 0}}, {63, {2, 3}}, {64, {4, 2}}, - {1, {7, 2}}, + {2, {7, 2}}, {65, {3, 1}}, {21, {5, -1}} })), TestSuite::Compare::Container); + CORRADE_COMPARE(scene.fieldFlags(SceneField::Mesh), SceneFieldFlags{}); + CORRADE_COMPARE(scene.fieldFlags(SceneField::MeshMaterial), SceneFieldFlags{}); - /* Cameras have certain objects reassigned, field data stay the same */ + /* Cameras have certain objects reassigned, field data stay the same. There + was no flag before so neither is after. */ CORRADE_COMPARE_AS(scene.camerasAsArray(), (Containers::arrayView>({ {22, 1}, {66, 5} })), TestSuite::Compare::Container); + CORRADE_COMPARE(scene.fieldFlags(SceneField::Camera), SceneFieldFlags{}); + + /* Lights stay the same, thus the implicit flag could be preserved. It's + not currently, though. */ + CORRADE_COMPARE_AS(scene.lightsAsArray(), (Containers::arrayView>({ + {0, 15}, + {1, 23} + })), TestSuite::Compare::Container); + { + CORRADE_EXPECT_FAIL("Logic for preserving flags of untouched fields is rather complex and thus not implemented yet."); + CORRADE_COMPARE(scene.fieldFlags(SceneField::Light), SceneFieldFlag::ImplicitMapping); + } { + CORRADE_COMPARE(scene.fieldFlags(SceneField::Light), SceneFieldFlags{}); + } + + /* A custom field gets the last object reassigned, field data stay the + same. The implicit flag gets turned to nothing after that. */ + CORRADE_COMPARE_AS(scene.mappingAsArray(sceneFieldCustom(15)), (Containers::arrayView({ + 67, 68, 69, 3 + })), TestSuite::Compare::Container); + CORRADE_COMPARE_AS(scene.field(sceneFieldCustom(15)), + Containers::arrayView(fooFieldData), + TestSuite::Compare::Container); + CORRADE_COMPARE(scene.fieldFlags(sceneFieldCustom(15)), SceneFieldFlags{}); + + /* A custom field that is not among fields to convert so it preserves the + flag */ + CORRADE_COMPARE_AS(scene.mappingAsArray(sceneFieldCustom(16)), (Containers::arrayView({ + 0, 1 + })), TestSuite::Compare::Container); + CORRADE_COMPARE_AS(scene.field(sceneFieldCustom(16)), + Containers::arrayView(foo2FieldData), + TestSuite::Compare::Container); + { + CORRADE_EXPECT_FAIL("Logic for preserving flags of untouched fields is rather complex and thus not implemented yet."); + CORRADE_COMPARE(scene.fieldFlags(sceneFieldCustom(16)), SceneFieldFlag::ImplicitMapping); + } { + CORRADE_COMPARE(scene.fieldFlags(sceneFieldCustom(16)), SceneFieldFlags{}); + } + + /* A custom field that is not among fields to convert but it shares the + mapping with a field that is and that gets changed. The implicit flag + should thus get removed here as well. */ + CORRADE_COMPARE_AS(scene.mappingAsArray(sceneFieldCustom(17)), (Containers::arrayView({ + 67, 68, 69, 3 + })), TestSuite::Compare::Container); + CORRADE_COMPARE_AS(scene.field(sceneFieldCustom(17)), + Containers::arrayView(foo3FieldData), + TestSuite::Compare::Container); + CORRADE_COMPARE(scene.fieldFlags(sceneFieldCustom(17)), SceneFieldFlags{}); + } }}}}