From ba892c0ca7161dc95ae7b9b20d7b66daf43eb24e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 18 Apr 2023 11:47:04 +0200 Subject: [PATCH] SceneTools: tighten up & publish combineFields(). It'll be soon needed by other tools and library users as well. The main difference compared to the original internal API is that the detection for shared mapping views now takes also sizes and strides into account, without relying on just the data pointer. Some additional robustness checks are needed regarding fields that have mapping sharing enforced (as otherwise it'll fail in the SceneData constructor which isn't nice), that'll be done in subsequent commits. --- src/Magnum/SceneTools/CMakeLists.txt | 2 + src/Magnum/SceneTools/Combine.cpp | 51 +++++ src/Magnum/SceneTools/Combine.h | 84 ++++++++ .../SceneTools/Implementation/combine.h | 96 ++++----- .../convertToSingleFunctionObjects.h | 2 +- src/Magnum/SceneTools/Test/CMakeLists.txt | 4 +- src/Magnum/SceneTools/Test/CombineTest.cpp | 188 +++++++++++++++--- .../ConvertToSingleFunctionObjectsTest.cpp | 11 +- src/Magnum/Trade/SceneData.h | 2 + 9 files changed, 346 insertions(+), 94 deletions(-) create mode 100644 src/Magnum/SceneTools/Combine.cpp create mode 100644 src/Magnum/SceneTools/Combine.h diff --git a/src/Magnum/SceneTools/CMakeLists.txt b/src/Magnum/SceneTools/CMakeLists.txt index fc158c426..3ef8edfa1 100644 --- a/src/Magnum/SceneTools/CMakeLists.txt +++ b/src/Magnum/SceneTools/CMakeLists.txt @@ -43,10 +43,12 @@ set(MagnumSceneTools_SRCS ) # Files compiled with different flags for main library and unit test library set(MagnumSceneTools_GracefulAssert_SRCS + Combine.cpp FlattenTransformationHierarchy.cpp OrderClusterParents.cpp) set(MagnumSceneTools_HEADERS + Combine.h FlattenTransformationHierarchy.h OrderClusterParents.h diff --git a/src/Magnum/SceneTools/Combine.cpp b/src/Magnum/SceneTools/Combine.cpp new file mode 100644 index 000000000..bfc10edc8 --- /dev/null +++ b/src/Magnum/SceneTools/Combine.cpp @@ -0,0 +1,51 @@ +/* + 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 "Combine.h" + +#include "Magnum/SceneTools/Implementation/combine.h" + +namespace Magnum { namespace SceneTools { + +Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, const UnsignedLong mappingBound, const Containers::ArrayView fields) { + /* See the function documentation for details why is it inlined in a helper + header */ + /** @todo move everything here once that's dropped */ + return Implementation::combineFields(mappingType, mappingBound, fields); +} + +Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, const UnsignedLong mappingBound, const std::initializer_list fields) { + return combineFields(mappingType, mappingBound, Containers::arrayView(fields)); +} + +Trade::SceneData combineFields(const Trade::SceneData& scene) { + /* Can't just pass scene.fieldData() directly as those can be offset-only */ + Containers::Array fields{NoInit, scene.fieldCount()}; + for(std::size_t i = 0; i != fields.size(); ++i) + fields[i] = scene.fieldData(i); + return combineFields(scene.mappingType(), scene.mappingBound(), fields); +} + +}} diff --git a/src/Magnum/SceneTools/Combine.h b/src/Magnum/SceneTools/Combine.h new file mode 100644 index 000000000..390c8dc24 --- /dev/null +++ b/src/Magnum/SceneTools/Combine.h @@ -0,0 +1,84 @@ +#ifndef Magnum_SceneTools_Combine_h +#define Magnum_SceneTools_Combine_h +/* + 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. +*/ + +/** @file + * @brief Function @ref Magnum::SceneTools::combineFields() + * @m_since_latest + */ + +#include + +#include "Magnum/SceneTools/visibility.h" +#include "Magnum/Trade/Trade.h" + +namespace Magnum { namespace SceneTools { + +/** +@brief Combine scene fields together +@m_since_latest + +Combines fields of varying @ref Trade::SceneMappingType together into a +@ref Trade::SceneData of a single @p mappingType. If any fields fully share +their mapping views (such as @ref Trade::SceneField::Mesh and +@relativeref{Trade::SceneField,MeshMaterial}, including fields for which this +isn't enforced), the sharing gets preserved. Partial sharing or sharing of data +views (as opposed to mapping views) isn't recognized and the data will get +duplicated. + +Fields pointing to existing memory are copied to the output, fields with +(sized) @cpp nullptr @ce mapping or data views are treated as placeholders for +copying the data later, with memory left uninitialized. If you however want to +have placeholder mapping data shared among multiple fields you have to allocate +them upfront. Note that offset-only @ref Trade::SceneFieldData instances are +not supported in the @p fields array. + +The resulting fields are always tightly packed (not interleaved). Returned data +flags have both @ref Trade::DataFlag::Mutable and @ref Trade::DataFlag::Owned, +so mutable attribute access is guaranteed. +*/ +MAGNUM_SCENETOOLS_EXPORT Trade::SceneData combineFields(Trade::SceneMappingType mappingType, UnsignedLong mappingBound, Containers::ArrayView fields); + +/** +@overload +@m_since_latest +*/ +MAGNUM_SCENETOOLS_EXPORT Trade::SceneData combineFields(Trade::SceneMappingType mappingType, UnsignedLong mappingBound, std::initializer_list fields); + +/** +@brief Combine scene fields from scratch +@m_since_latest + +Calls @ref combineFields(Trade::SceneMappingType, UnsignedLong, Containers::ArrayView) +with mapping type, bound and fields coming from @p scene. Useful for +conveniently repacking an existing scene and throwing away data not referenced +by any field. +*/ +MAGNUM_SCENETOOLS_EXPORT Trade::SceneData combineFields(const Trade::SceneData& scene); + +}} + +#endif diff --git a/src/Magnum/SceneTools/Implementation/combine.h b/src/Magnum/SceneTools/Implementation/combine.h index c695a0405..575a8e0e2 100644 --- a/src/Magnum/SceneTools/Implementation/combine.h +++ b/src/Magnum/SceneTools/Implementation/combine.h @@ -25,7 +25,7 @@ DEALINGS IN THE SOFTWARE. */ -#include +#include #include #include #include @@ -38,12 +38,20 @@ namespace Magnum { namespace SceneTools { namespace Implementation { +/* The combineFields() is currently transitively used also by Trade for + (deprecated) backwards compatibility in SceneData, in particular by + convertToSingleFunctionObjects(). Making Trade depend on SceneTools in a + deprecated build would be a nasty complication, so the functions are inlined + in a header that gets included in both */ +/** @todo move everything to Combine.cpp and anonymous namespace once that + compatibility is dropped */ + template void combineCopyMappings(const Containers::ArrayView fields, const Containers::ArrayView> itemViews, const Containers::ArrayView> itemViewMappings) { std::size_t latestMapping = 0; for(std::size_t i = 0; i != fields.size(); ++i) { - /* If there are no aliased object mappings, itemViewMappings should be + /* If there are no shared object mappings, itemViewMappings should be monotonically increasing. If it's not, it means the mapping is - shared with something earlier and it got already copied -- skip. */ + shared with something earlier which got already copied -- skip. */ const std::size_t mapping = itemViewMappings[i].first(); if(i && mapping <= latestMapping) continue; latestMapping = mapping; @@ -69,28 +77,16 @@ template void combineCopyMappings(const Containers::ArrayView fields) { +inline Trade::SceneData combineFields(const Trade::SceneMappingType mappingType, const UnsignedLong mappingBound, const Containers::ArrayView fields) { const std::size_t mappingTypeSize = sceneMappingTypeSize(mappingType); const std::size_t mappingTypeAlignment = sceneMappingTypeAlignment(mappingType); - /* Go through all fields and collect ArrayTuple allocations for these */ - std::unordered_map uniqueMappings; + /* Track unique mapping views (pointer, size, stride) so fields that shared + a mapping before stay shared after as well. A map is used because + it has conveniently implemented ordering, an unordered_map couldn't be + used without manually implementing a std::tuple hash because STL DOES + NOT HAVE IT, UGH. */ + std::map, UnsignedInt> uniqueMappings; Containers::Array items; Containers::Array> itemViewMappings{NoInit, fields.size()}; @@ -101,58 +97,45 @@ inline Trade::SceneData combine(const Trade::SceneMappingType mappingType, const Containers::Array> itemViews{fields.size()*2}; std::size_t itemViewOffset = 0; + /* Go through all fields and collect ArrayTuple allocations for these */ for(std::size_t i = 0; i != fields.size(); ++i) { const Trade::SceneFieldData& field = fields[i]; - CORRADE_INTERNAL_ASSERT(!(field.flags() & Trade::SceneFieldFlag::OffsetOnly)); + CORRADE_ASSERT(!(field.flags() & Trade::SceneFieldFlag::OffsetOnly), + "SceneTools::combineFields(): field" << i << "is offset-only", (Trade::SceneData{Trade::SceneMappingType::UnsignedInt, 0, nullptr, {}})); - /* Mapping data. Allocate if the view is a placeholder of if it wasn't - used by other fields yet. std::pair is used due to this being - returned from a std::unordered_map. */ - std::pair::iterator, bool> inserted; + /* Mapping data. If the view isn't a placeholder, check if it is + shared with an existing view already, and insert it if not. */ + std::pair, UnsignedInt>::iterator, bool> inserted; if(field.mappingData().data()) - inserted = uniqueMappings.emplace(field.mappingData().data(), itemViewOffset); + inserted = uniqueMappings.emplace(std::make_tuple(field.mappingData().data(), field.mappingData().size(), field.mappingData().stride()), itemViewOffset); + + /* If it's shared (inserting failed), remember the field ID it's shared + with. We don't need the original size or stride for anything after + -- it was just used to find matching views, and if a match was + found, it already has a correct size, and the stride is implicit. */ if(field.mappingData().data() && !inserted.second) { itemViewMappings[i].first() = inserted.first->second; - /* Expect that fields sharing the same object mapping view have the - exact same length (the length gets stored in the output view - during the ArrayTuple::Item construction). - - We could just ignore the sharing in that case, but that'd only - lead to misery down the line -- imagine a field that shares the - first two items with a mesh and a material object mapping. If it - would be the last, it gets duplicated and everything is great, - however if it's the first then both mesh and the material get - duplicated, and that then asserts inside the SceneData - constructor, as those are always expected to share. - - One option that would solve this would be to store pointer+size - in the objectMappings map (and then only mappings that share - also the same size would be shared), another would be to use the - longest used view (and then the shorter prefixes would share - with it). The ultimate option would be to have some range map - where it'd be possible to locate also arbitrary subranges, not - just prefixes. A whole other topic altogether is checking for - the same stride, which is not done at all. - - This might theoretically lead to assertions also when two - compile-time arrays share a common prefix and get deduplicated - by the compiler. But that's unlikely, at least for the internal - use case we have right now. */ - CORRADE_INTERNAL_ASSERT(itemViews[inserted.first->second].size()[0] == field.size()); + + /* If it's not shared or it's a placeholder, allocate a new mapping + view of given size by adding a new item to the list of views to + allocate by an ArrayTuple. */ } else { itemViewMappings[i].first() = itemViewOffset; arrayAppend(items, InPlaceInit, NoInit, std::size_t(field.size()), mappingTypeSize, mappingTypeAlignment, itemViews[itemViewOffset]); ++itemViewOffset; } - /* Field data. No aliasing here right now, no sharing between mapping - and field data either. */ + /* Field data, just allocate space for it. No extra logic needed -- no + aliasing here right now, no sharing between mapping and field data + either. */ /** @todo field aliasing might be useful at some point */ itemViewMappings[i].second() = itemViewOffset; arrayAppend(items, InPlaceInit, NoInit, std::size_t(field.size()), sceneFieldTypeSize(field.fieldType())*(field.fieldArraySize() ? field.fieldArraySize() : 1), sceneFieldTypeAlignment(field.fieldType()), itemViews[itemViewOffset]); ++itemViewOffset; } + CORRADE_INTERNAL_ASSERT(itemViewOffset <= itemViews.size()); + /* Allocate the data */ Containers::Array outData = Containers::ArrayTuple{items}; CORRADE_INTERNAL_ASSERT(!outData.deleter()); @@ -166,6 +149,7 @@ inline Trade::SceneData combine(const Trade::SceneMappingType mappingType, const combineCopyMappings(fields, itemViews, itemViewMappings); else if(mappingType == Trade::SceneMappingType::UnsignedLong) combineCopyMappings(fields, itemViews, itemViewMappings); + else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ /* Copy the field data over. No special handling needed here. */ for(std::size_t i = 0; i != fields.size(); ++i) { diff --git a/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h b/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h index 9aaabeaaf..c73934338 100644 --- a/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h +++ b/src/Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h @@ -158,7 +158,7 @@ inline Trade::SceneData convertToSingleFunctionObjects(const Trade::SceneData& s } /* Combine the fields into a new SceneData */ - Trade::SceneData out = combine(Trade::SceneMappingType::UnsignedInt, Math::max(scene.mappingBound(), UnsignedLong(newObjectOffset) + objectsToAdd), fields); + Trade::SceneData out = combineFields(Trade::SceneMappingType::UnsignedInt, Math::max(scene.mappingBound(), UnsignedLong(newObjectOffset) + objectsToAdd), fields); /* Copy existing parent object/field data to a prefix of the output */ const Containers::StridedArrayView1D outParentMapping = out.mutableMapping(parentFieldId); diff --git a/src/Magnum/SceneTools/Test/CMakeLists.txt b/src/Magnum/SceneTools/Test/CMakeLists.txt index 83fd2ad83..328ae256d 100644 --- a/src/Magnum/SceneTools/Test/CMakeLists.txt +++ b/src/Magnum/SceneTools/Test/CMakeLists.txt @@ -50,8 +50,8 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/configure.h.cmake file(GENERATE OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/$/configure.h INPUT ${CMAKE_CURRENT_BINARY_DIR}/configure.h.in) -corrade_add_test(SceneToolsCombineTest CombineTest.cpp LIBRARIES MagnumTrade) -corrade_add_test(SceneToolsConvertToSingleFunc___Test ConvertToSingleFunctionObjectsTest.cpp LIBRARIES MagnumTrade) +corrade_add_test(SceneToolsCombineTest CombineTest.cpp LIBRARIES MagnumSceneToolsTestLib) +corrade_add_test(SceneToolsConvertToSingleFunc___Test ConvertToSingleFunctionObjectsTest.cpp LIBRARIES MagnumSceneToolsTestLib) corrade_add_test(SceneToolsFlattenTra___HierarchyTest FlattenTransformationHierarchyTest.cpp LIBRARIES MagnumSceneToolsTestLib) corrade_add_test(SceneToolsOrderClusterParentsTest OrderClusterParentsTest.cpp LIBRARIES MagnumSceneToolsTestLib) diff --git a/src/Magnum/SceneTools/Test/CombineTest.cpp b/src/Magnum/SceneTools/Test/CombineTest.cpp index e00a5000b..10c01667c 100644 --- a/src/Magnum/SceneTools/Test/CombineTest.cpp +++ b/src/Magnum/SceneTools/Test/CombineTest.cpp @@ -23,30 +23,37 @@ DEALINGS IN THE SOFTWARE. */ +#include #include #include #include +#include #include "Magnum/Math/Complex.h" #include "Magnum/Math/Vector2.h" -#include "Magnum/SceneTools/Implementation/combine.h" +#include "Magnum/SceneTools/Combine.h" +#include "Magnum/Trade/SceneData.h" namespace Magnum { namespace SceneTools { namespace Test { namespace { struct CombineTest: TestSuite::Tester { explicit CombineTest(); - void test(); - void alignment(); - void mappingShared(); - void mappingPlaceholderFieldPlaceholder(); - void mappingSharedFieldPlaceholder(); + void fields(); + void fieldsAlignment(); + void fieldsMappingShared(); + void fieldsMappingSharedPartial(); + void fieldsMappingPlaceholderFieldPlaceholder(); + void fieldsMappingSharedFieldPlaceholder(); + + void fieldsOffsetOnly(); + void fieldsFromDataOffsetOnly(); }; -struct { +const struct { const char* name; Trade::SceneMappingType objectType; -} TestData[]{ +} FieldsData[]{ {"UnsignedByte output", Trade::SceneMappingType::UnsignedByte}, {"UnsignedShort output", Trade::SceneMappingType::UnsignedShort}, {"UnsignedInt output", Trade::SceneMappingType::UnsignedInt}, @@ -54,19 +61,23 @@ struct { }; CombineTest::CombineTest() { - addInstancedTests({&CombineTest::test}, - Containers::arraySize(TestData)); + addInstancedTests({&CombineTest::fields}, + Containers::arraySize(FieldsData)); + + addTests({&CombineTest::fieldsAlignment, + &CombineTest::fieldsMappingShared, + &CombineTest::fieldsMappingSharedPartial, + &CombineTest::fieldsMappingPlaceholderFieldPlaceholder, + &CombineTest::fieldsMappingSharedFieldPlaceholder, - addTests({&CombineTest::alignment, - &CombineTest::mappingShared, - &CombineTest::mappingPlaceholderFieldPlaceholder, - &CombineTest::mappingSharedFieldPlaceholder}); + &CombineTest::fieldsOffsetOnly, + &CombineTest::fieldsFromDataOffsetOnly}); } using namespace Math::Literals; -void CombineTest::test() { - auto&& data = TestData[testCaseInstanceId()]; +void CombineTest::fields() { + auto&& data = FieldsData[testCaseInstanceId()]; setTestCaseDescription(data.name); /* Testing the four possible object types, it should be possible to combine @@ -109,7 +120,7 @@ void CombineTest::test() { }; auto foos = Containers::stridedArrayView(fooData); - Trade::SceneData scene = Implementation::combine(data.objectType, 167, Containers::arrayView({ + Trade::SceneData scene = combineFields(data.objectType, 167, { Trade::SceneFieldData{Trade::SceneField::Mesh, meshes.slice(&Mesh::mapping), meshes.slice(&Mesh::mesh)}, @@ -127,7 +138,7 @@ void CombineTest::test() { Trade::SceneFieldFlag::OrderedMapping}, /* Empty field */ Trade::SceneFieldData{Trade::SceneField::Camera, Containers::ArrayView{}, Containers::ArrayView{}} - })); + }); CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(scene.mappingType(), data.objectType); @@ -190,20 +201,20 @@ void CombineTest::test() { CORRADE_COMPARE(scene.fieldArraySize(4), 0); } -void CombineTest::alignment() { +void CombineTest::fieldsAlignment() { const UnsignedShort meshMappingData[]{15, 23, 47}; const UnsignedByte meshFieldData[]{0, 1, 2}; const UnsignedShort translationMappingData[]{5}; /* 1 byte padding before */ const Vector2d translationFieldData[]{{1.5, 3.0}}; /* 4 byte padding before */ - Trade::SceneData scene = Implementation::combine(Trade::SceneMappingType::UnsignedShort, 167, Containers::arrayView({ + Trade::SceneData scene = combineFields(Trade::SceneMappingType::UnsignedShort, 167, { Trade::SceneFieldData{Trade::SceneField::Mesh, Containers::arrayView(meshMappingData), Containers::arrayView(meshFieldData)}, Trade::SceneFieldData{Trade::SceneField::Translation, Containers::arrayView(translationMappingData), Containers::arrayView(translationFieldData)} - })); + }); CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(scene.mappingType(), Trade::SceneMappingType::UnsignedShort); @@ -243,7 +254,7 @@ void CombineTest::alignment() { CORRADE_COMPARE(scene.field(1).stride()[0], 16); } -void CombineTest::mappingShared() { +void CombineTest::fieldsMappingShared() { const UnsignedShort meshMappingData[3]{}; const UnsignedByte meshFieldData[3]{}; const Int meshMaterialFieldData[3]{}; @@ -252,7 +263,7 @@ void CombineTest::mappingShared() { const Vector2 translationFieldData[2]{}; const Complex rotationFieldData[2]{}; - Trade::SceneData scene = Implementation::combine(Trade::SceneMappingType::UnsignedInt, 173, Containers::arrayView({ + Trade::SceneData scene = combineFields(Trade::SceneMappingType::UnsignedInt, 173, { /* Deliberately in an arbitrary order to avoid false assumptions like fields sharing the same object mapping always being after each other */ @@ -268,7 +279,7 @@ void CombineTest::mappingShared() { Trade::SceneFieldData{Trade::SceneField::Rotation, Containers::arrayView(translationRotationMappingData), Containers::arrayView(rotationFieldData)} - })); + }); CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(scene.mappingType(), Trade::SceneMappingType::UnsignedInt); @@ -284,11 +295,56 @@ void CombineTest::mappingShared() { CORRADE_COMPARE(scene.mapping(Trade::SceneField::Translation).data(), scene.mapping(Trade::SceneField::Rotation).data()); } -void CombineTest::mappingPlaceholderFieldPlaceholder() { +void CombineTest::fieldsMappingSharedPartial() { + const UnsignedShort mappingData[]{15, 23, 47, 26, 3}; + + /* Field data don't have any special treatment so their values aren't + tested */ + const UnsignedByte meshData[3]{}; + const UnsignedShort lightData[2]{}; + const Int parentData[3]{}; + + Trade::SceneData scene = combineFields(Trade::SceneMappingType::UnsignedInt, 173, { + Trade::SceneFieldData{Trade::SceneField::Mesh, + Containers::arrayView(mappingData).prefix(3), + Containers::arrayView(meshData)}, + Trade::SceneFieldData{Trade::SceneField::Light, + Containers::arrayView(mappingData).prefix(2), + Containers::arrayView(lightData)}, + Trade::SceneFieldData{Trade::SceneField::Parent, + Containers::stridedArrayView(mappingData).every(2), + Containers::arrayView(parentData)}, + }); + + CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); + CORRADE_COMPARE(scene.mappingType(), Trade::SceneMappingType::UnsignedInt); + CORRADE_COMPARE(scene.mappingBound(), 173); + CORRADE_COMPARE(scene.fieldCount(), 3); + + CORRADE_COMPARE_AS(scene.mapping(Trade::SceneField::Mesh), + Containers::arrayView({15u, 23u, 47u}), + TestSuite::Compare::Container); + + CORRADE_COMPARE_AS(scene.mapping(Trade::SceneField::Light), + Containers::arrayView({15u, 23u}), + TestSuite::Compare::Container); + + CORRADE_COMPARE_AS(scene.mapping(Trade::SceneField::Parent), + Containers::arrayView({15u, 47u, 3u}), + TestSuite::Compare::Container); + + /* All mappings should be deinterleaved */ + for(UnsignedInt i = 0; i != scene.fieldCount(); ++i) { + CORRADE_ITERATION(scene.fieldName(i)); + CORRADE_COMPARE(scene.mapping(i).stride(), sizeof(UnsignedInt)); + } +} + +void CombineTest::fieldsMappingPlaceholderFieldPlaceholder() { const UnsignedShort meshMappingData[]{15, 23, 47}; const UnsignedByte meshFieldData[]{0, 1, 2}; - Trade::SceneData scene = Implementation::combine(Trade::SceneMappingType::UnsignedShort, 173, Containers::arrayView({ + Trade::SceneData scene = combineFields(Trade::SceneMappingType::UnsignedShort, 173, { Trade::SceneFieldData{Trade::SceneField::Camera, Containers::ArrayView{nullptr, 1}, Containers::ArrayView{nullptr, 1}}, @@ -304,7 +360,7 @@ void CombineTest::mappingPlaceholderFieldPlaceholder() { Trade::SceneFieldData{Trade::sceneFieldCustom(15), Containers::ArrayView{nullptr, 2}, Containers::StridedArrayView2D{{nullptr, 16}, {2, 4}}}, - })); + }); CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(scene.mappingType(), Trade::SceneMappingType::UnsignedShort); @@ -345,18 +401,18 @@ void CombineTest::mappingPlaceholderFieldPlaceholder() { CORRADE_COMPARE(scene.field(Trade::sceneFieldCustom(15)).stride()[0], 4*2); } -void CombineTest::mappingSharedFieldPlaceholder() { +void CombineTest::fieldsMappingSharedFieldPlaceholder() { const UnsignedInt meshMappingData[]{15, 23, 47}; const UnsignedByte meshFieldData[]{0, 1, 2}; - Trade::SceneData scene = Implementation::combine(Trade::SceneMappingType::UnsignedInt, 173, Containers::arrayView({ + Trade::SceneData scene = combineFields(Trade::SceneMappingType::UnsignedInt, 173, { Trade::SceneFieldData{Trade::SceneField::Mesh, Containers::arrayView(meshMappingData), Containers::arrayView(meshFieldData)}, Trade::SceneFieldData{Trade::SceneField::MeshMaterial, Containers::arrayView(meshMappingData), Containers::ArrayView{nullptr, 3}}, - })); + }); CORRADE_COMPARE(scene.dataFlags(), Trade::DataFlag::Owned|Trade::DataFlag::Mutable); CORRADE_COMPARE(scene.mappingType(), Trade::SceneMappingType::UnsignedInt); @@ -383,6 +439,78 @@ void CombineTest::mappingSharedFieldPlaceholder() { CORRADE_COMPARE(scene.field(Trade::SceneField::MeshMaterial).stride()[0], 4); } +void CombineTest::fieldsOffsetOnly() { + CORRADE_SKIP_IF_NO_ASSERT(); + + const struct Field { + UnsignedInt mapping; + UnsignedByte mesh; + UnsignedShort light; + } data[]{ + {1, 3, 5}, + {4, 6, 8}, + }; + auto view = Containers::stridedArrayView(data); + + std::ostringstream out; + Error redirectError{&out}; + combineFields(Trade::SceneMappingType::UnsignedInt, 173, { + Trade::SceneFieldData{Trade::SceneField::Mesh, + view.slice(&Field::mapping), + view.slice(&Field::mesh)}, + Trade::SceneFieldData{Trade::SceneField::Light, 2, + Trade::SceneMappingType::UnsignedInt, offsetof(Field, mapping), sizeof(Field), + Trade::SceneFieldType::UnsignedShort, offsetof(Field, light), sizeof(Field)} + }); + CORRADE_COMPARE(out.str(), "SceneTools::combineFields(): field 1 is offset-only\n"); +} + +void CombineTest::fieldsFromDataOffsetOnly() { + /* Same as fieldFromData(), but wrapped in a Scene first, which makes it + work */ + + const struct Field { + UnsignedInt mapping; + UnsignedByte mesh; + UnsignedShort light; + } data[]{ + {1, 3, 5}, + {4, 6, 8}, + }; + auto view = Containers::stridedArrayView(data); + + Trade::SceneData scene{Trade::SceneMappingType::UnsignedInt, 22, {}, data, { + Trade::SceneFieldData{Trade::SceneField::Mesh, + view.slice(&Field::mapping), + view.slice(&Field::mesh)}, + Trade::SceneFieldData{Trade::SceneField::Light, 2, + Trade::SceneMappingType::UnsignedInt, offsetof(Field, mapping), sizeof(Field), + Trade::SceneFieldType::UnsignedShort, offsetof(Field, light), sizeof(Field)} + }}; + + Trade::SceneData combined = combineFields(scene); + /* Since it's tightly packed, it's less data now */ + CORRADE_COMPARE(combined.data().size(), 2*4 + 2*1 + 2*2); + CORRADE_COMPARE_AS(combined.data().size(), sizeof(data), + TestSuite::Compare::Less); + + /* The two mappings are shared */ + CORRADE_COMPARE_AS(combined.mapping(Trade::SceneField::Mesh), + Containers::arrayView({1u, 4u}), + TestSuite::Compare::Container); + CORRADE_COMPARE_AS(combined.mapping(Trade::SceneField::Light), + Containers::arrayView({1u, 4u}), + TestSuite::Compare::Container); + CORRADE_COMPARE(combined.mapping(Trade::SceneField::Light).data(), combined.mapping(Trade::SceneField::Mesh).data()); + + CORRADE_COMPARE_AS(combined.field(Trade::SceneField::Mesh), + Containers::arrayView({3, 6}), + TestSuite::Compare::Container); + CORRADE_COMPARE_AS(combined.field(Trade::SceneField::Light), + Containers::arrayView({5, 8}), + TestSuite::Compare::Container); +} + }}}} CORRADE_TEST_MAIN(Magnum::SceneTools::Test::CombineTest) diff --git a/src/Magnum/SceneTools/Test/ConvertToSingleFunctionObjectsTest.cpp b/src/Magnum/SceneTools/Test/ConvertToSingleFunctionObjectsTest.cpp index c0028069b..4f78e297f 100644 --- a/src/Magnum/SceneTools/Test/ConvertToSingleFunctionObjectsTest.cpp +++ b/src/Magnum/SceneTools/Test/ConvertToSingleFunctionObjectsTest.cpp @@ -29,6 +29,7 @@ #include "Magnum/Math/Complex.h" #include "Magnum/Math/Vector2.h" +#include "Magnum/SceneTools/Combine.h" #include "Magnum/SceneTools/Implementation/convertToSingleFunctionObjects.h" namespace Magnum { namespace SceneTools { namespace Test { namespace { @@ -70,7 +71,7 @@ void ConvertToSingleFunctionObjectsTest::test() { auto&& data = TestData[testCaseInstanceId()]; setTestCaseDescription(data.name); - /* Haha now I can use sceneCombine() to conveniently prepare the initial + /* Haha now I can use combineFields() to conveniently prepare the initial state here, without having to mess with an ArrayTuple */ const UnsignedShort parentMappingData[]{2, 15, 21, 22, 23}; @@ -114,7 +115,7 @@ void ConvertToSingleFunctionObjectsTest::test() { changed, it should not retain the ImplicitMapping flag. */ const Byte foo3FieldData[]{-1, -2, 7, 2}; - Trade::SceneData original = Implementation::combine(Trade::SceneMappingType::UnsignedShort, data.originalObjectCount, Containers::arrayView({ + Trade::SceneData original = combineFields(Trade::SceneMappingType::UnsignedShort, data.originalObjectCount, { Trade::SceneFieldData{Trade::SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentFieldData), @@ -144,7 +145,7 @@ void ConvertToSingleFunctionObjectsTest::test() { Containers::arrayView(fooMappingData), Containers::arrayView(foo3FieldData), Trade::SceneFieldFlag::ImplicitMapping} - })); + }); Trade::SceneData scene = Implementation::convertToSingleFunctionObjects(original, Containers::arrayView({ Trade::SceneField::Mesh, @@ -327,7 +328,7 @@ void ConvertToSingleFunctionObjectsTest::fieldsToCopy() { const UnsignedLong fooMappingData[]{15, 23, 15, 21}; const Int fooFieldData[]{0, 1, 2, 3, 4, 5, 6, 7}; - Trade::SceneData original = Implementation::combine(Trade::SceneMappingType::UnsignedShort, 50, Containers::arrayView({ + Trade::SceneData original = combineFields(Trade::SceneMappingType::UnsignedShort, 50, { Trade::SceneFieldData{Trade::SceneField::Parent, Containers::arrayView(parentMappingData), Containers::arrayView(parentFieldData)}, @@ -345,7 +346,7 @@ void ConvertToSingleFunctionObjectsTest::fieldsToCopy() { Trade::SceneFieldData{Trade::SceneField::Transformation, Trade::SceneMappingType::UnsignedShort, nullptr, Trade::SceneFieldType::Matrix4x4, nullptr} - })); + }); Trade::SceneData scene = Implementation::convertToSingleFunctionObjects(original, Containers::arrayView({ diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index 34939e27f..390054ee3 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/src/Magnum/Trade/SceneData.h @@ -803,6 +803,8 @@ MAGNUM_TRADE_EXPORT Debug& operator<<(Debug& debug, SceneFieldFlags value); Convenience type for populating @ref SceneData, see @ref Trade-SceneData-populating "its documentation" for an introduction. +Additionally usable in various @ref SceneTools algorithms such as +@ref SceneTools::combineFields(Trade::SceneMappingType, UnsignedLong, Containers::ArrayView). @section Trade-SceneFieldData-usage Usage