Trade: allow omitting trivial SceneField::Parent data.

doc/snippets/MagnumTrade.cpp

@@ -926,6 +926,16 @@ Trade::SceneFieldData field{Trade::SceneField::Transformation,
 /* [SceneFieldData-usage-implicit-mapping] */
 }
 
+{
+std::size_t objectCount{};
+/* [SceneFieldData-usage-trivial-parent] */
+Trade::SceneFieldData field{Trade::SceneField::Parent,
+    Containers::ArrayView<UnsignedInt>{nullptr, objectCount},
+    Containers::ArrayView<Int>{nullptr, objectCount},
+    Trade::SceneFieldFlag::ImplicitMapping|Trade::SceneFieldFlag::TrivialField};
+/* [SceneFieldData-usage-trivial-parent] */
+}
+
 {
 typedef SceneGraph::Scene<SceneGraph::MatrixTransformation3D> Scene3D;
 typedef SceneGraph::Object<SceneGraph::MatrixTransformation3D> Object3D;

src/Magnum/Trade/SceneData.cpp

@@ -485,6 +485,7 @@ Debug& operator<<(Debug& debug, const SceneFieldFlag value) {
         _c(OffsetOnly)
         _c(ImplicitMapping)
         _c(OrderedMapping)
+        _c(TrivialField)
         #undef _c
         /* LCOV_EXCL_STOP */
     }
@@ -497,7 +498,8 @@ Debug& operator<<(Debug& debug, const SceneFieldFlags value) {
         SceneFieldFlag::OffsetOnly,
         SceneFieldFlag::ImplicitMapping,
         /* This one is implied by ImplicitMapping, so has to be after */
-        SceneFieldFlag::OrderedMapping
+        SceneFieldFlag::OrderedMapping,
+        SceneFieldFlag::TrivialField
     });
 }
 
@@ -602,9 +604,14 @@ SceneData::SceneData(const SceneMappingType mappingType, const UnsignedLong mapp
                         "Trade::SceneData: offset-only mapping data of field" << i << "span" << mappingSize << "bytes but passed data array has only" << _data.size(), );
                 }
 
-                const std::size_t fieldSize = field._fieldData.offset + (field._size - 1)*field._fieldStride + fieldTypeSize;
+                /* If an offset-only field is trivial, we ignore the offset /
-                CORRADE_ASSERT(fieldSize <= _data.size(),
+                   size completely. Trivial fields are whitelisted, which was
-                    "Trade::SceneData: offset-only field data of field" << i << "span" << fieldSize << "bytes but passed data array has only" << _data.size(), );
+                   already checked in SceneFieldData constructor. */
+                if(!(field._flags >= SceneFieldFlag::TrivialField)) {
+                    const std::size_t fieldSize = field._fieldData.offset + (field._size - 1)*field._fieldStride + fieldTypeSize;
+                    CORRADE_ASSERT(fieldSize <= _data.size(),
+                        "Trade::SceneData: offset-only field data of field" << i << "span" << fieldSize << "bytes but passed data array has only" << _data.size(), );
+                }
 
             } else {
                 /* If a field has an implicit mapping, we allow it to be
@@ -616,10 +623,15 @@ SceneData::SceneData(const SceneMappingType mappingType, const UnsignedLong mapp
                         "Trade::SceneData: mapping data [" << Debug::nospace << mappingBegin << Debug::nospace << ":" << Debug::nospace << mappingEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
                 }
 
-                const void* const fieldBegin = field._fieldData.pointer;
+                /* Trivial fields are allowed to be nullptr. Trivial fields are
-                const void* const fieldEnd = static_cast<const char*>(field._fieldData.pointer) + (field._size - 1)*field._fieldStride + fieldTypeSize;
+                   whitelisted, which was already checked in SceneFieldData
-                CORRADE_ASSERT(fieldBegin >= _data.begin() && fieldEnd <= _data.end(),
+                   constructor. */
-                    "Trade::SceneData: field data [" << Debug::nospace << fieldBegin << Debug::nospace << ":" << Debug::nospace << fieldEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
+                if(!(field._flags >= SceneFieldFlag::TrivialField && !field._fieldData.pointer)) {
+                    const void* const fieldBegin = field._fieldData.pointer;
+                    const void* const fieldEnd = static_cast<const char*>(field._fieldData.pointer) + (field._size - 1)*field._fieldStride + fieldTypeSize;
+                    CORRADE_ASSERT(fieldBegin >= _data.begin() && fieldEnd <= _data.end(),
+                        "Trade::SceneData: field data [" << Debug::nospace << fieldBegin << Debug::nospace << ":" << Debug::nospace << fieldEnd << Debug::nospace << "] of field" << i << "are not contained in passed data array [" << Debug::nospace << static_cast<const void*>(_data.begin()) << Debug::nospace << ":" << Debug::nospace << static_cast<const void*>(_data.end()) << Debug::nospace << "]", );
+                }
             }
         }
         #endif
@@ -838,6 +850,12 @@ Containers::StridedArrayView1D<const void> SceneData::fieldDataMappingViewIntern
 
 Containers::StridedArrayView1D<const void> SceneData::fieldDataFieldViewInternal(const SceneFieldData& field, const std::size_t offset, const std::size_t size) const {
     CORRADE_INTERNAL_ASSERT(offset + size <= field._size);
+
+    /* If this is a offset-only field that's trivial, ignore the offset/stride
+       and always assume it's not present */
+    if(field._flags >= (SceneFieldFlag::OffsetOnly|SceneFieldFlag::TrivialField))
+        return {{nullptr, ~std::size_t{}}, size, field._fieldStride};
+
     return Containers::StridedArrayView1D<const void>{
         /* We're *sure* the view is correct, so faking the view size */
         {static_cast<const char*>(field._flags & SceneFieldFlag::OffsetOnly ?
@@ -1239,6 +1257,17 @@ void SceneData::parentsIntoInternal(const UnsignedInt fieldId, const std::size_t
        checked by the callers */
 
     const SceneFieldData& field = _fields[fieldId];
+
+    /* If we don't have any data for a trivial parent or the trivial parent is
+       offset-only (where we always assume there's no data), generate the data */
+    if((field._flags >= SceneFieldFlag::TrivialField && !field._fieldData.pointer) ||
+       (field._flags >= (SceneFieldFlag::TrivialField|SceneFieldFlag::OffsetOnly)))
+    {
+        for(std::size_t i = 0; i != destination.size(); ++i)
+            destination[i] = -1;
+        return;
+    }
+
     const Containers::StridedArrayView1D<const void> fieldData = fieldDataFieldViewInternal(field, offset, destination.size());
     const auto destination1i = Containers::arrayCast<2, Int>(destination);
 

src/Magnum/Trade/SceneData.h

@@ -105,6 +105,9 @@ enum class SceneField: UnsignedInt {
      * no parent. An object should have only one parent, altough this isn't
      * enforced in any way, and which of the duplicate fields gets used is not
      * defined.
+     *
+     * This field is allowed to have @ref SceneFieldFlag::TrivialField set,
+     * which implies it has @cpp -1 @ce for all values.
      * @see @ref SceneData::parentsAsArray(), @ref SceneData::parentFor(),
      *      @ref SceneData::childrenFor()
      */
@@ -579,6 +582,13 @@ enum class SceneFieldFlag: UnsignedByte {
      * @f$ \mathcal{O}(n) @f$ lookup complexity.
      */
     ImplicitMapping = (1 << 2)|OrderedMapping,
+
+    /**
+     * The field has a trivial content. Currently allowed only for
+     * @ref SceneField::Parent, indicating all entries are @cpp -1 @ce. If this
+     * flag is set, the field view is allowed to be @cpp nullptr @ce.
+     */
+    TrivialField = 1 << 3
 };
 
 /**
@@ -661,6 +671,18 @@ Fields that are both @ref SceneFieldFlag::OffsetOnly and
 @ref SceneFieldFlag::ImplicitMapping have their object mapping data always
 ignored as it's not possible to know whether the offset points to actual data
 or not.
+
+@subsection Trade-SceneFieldData-usage-trivial-field Trivial fields
+
+The @ref SceneField::Parent can be annotated with
+@ref SceneFieldFlag::TrivialField, which implies that all nodes are in scene
+root. While similar effect could be achieved by repeating a @cpp -1 @ce using
+zero stride, the main purpose of this flag is in combination with
+@ref SceneFieldFlag::ImplicitMapping --- that way you can indicate that all
+objects in the scene are top-level without having to explicitly supply any
+field data:
+
+@snippet MagnumTrade.cpp SceneFieldData-usage-trivial-parent
 */
 class MAGNUM_TRADE_EXPORT SceneFieldData {
     public:
@@ -3113,6 +3135,10 @@ namespace Implementation {
     constexpr bool isSceneFieldArrayAllowed(SceneField name) {
         return isSceneFieldCustom(name);
     }
+
+    constexpr bool isSceneFieldAllowedTrivial(SceneField name) {
+        return name == SceneField::Parent;
+    }
 }
 
 constexpr SceneFieldData::SceneFieldData(const SceneField name, const SceneMappingType mappingType, const Containers::StridedArrayView1D<const void>& mappingData, const SceneFieldType fieldType, const Containers::StridedArrayView1D<const void>& fieldData, const UnsignedShort fieldArraySize, const SceneFieldFlags flags) noexcept:
@@ -3121,7 +3147,10 @@ constexpr SceneFieldData::SceneFieldData(const SceneField name, const SceneMappi
     _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isSceneFieldTypeCompatibleWithField(name, fieldType),
         "Trade::SceneFieldData:" << fieldType << "is not a valid type for" << name), name)},
     _flags{(CORRADE_CONSTEXPR_ASSERT(!(flags & SceneFieldFlag::OffsetOnly),
-        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::OffsetOnly for a view"), flags)},
+        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::OffsetOnly for a view"),
+        CORRADE_CONSTEXPR_ASSERT(!(flags & SceneFieldFlag::TrivialField) || Implementation::isSceneFieldAllowedTrivial(name),
+        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::TrivialField for" << name),
+        flags)},
     _mappingType{mappingType},
     _mappingStride{(CORRADE_CONSTEXPR_ASSERT(mappingData.stride() >= -32768 && mappingData.stride() <= 32767,
         "Trade::SceneFieldData: expected mapping view stride to fit into 16 bits, but got" << mappingData.stride()), Short(mappingData.stride()))},
@@ -3150,7 +3179,10 @@ constexpr SceneFieldData::SceneFieldData(const SceneField name, const std::size_
     _size{size},
     _name{(CORRADE_CONSTEXPR_ASSERT(Implementation::isSceneFieldTypeCompatibleWithField(name, fieldType),
         "Trade::SceneFieldData:" << fieldType << "is not a valid type for" << name), name)},
-    _flags{flags|SceneFieldFlag::OffsetOnly},
+    _flags{(
+        CORRADE_CONSTEXPR_ASSERT(!(flags & SceneFieldFlag::TrivialField) || Implementation::isSceneFieldAllowedTrivial(name),
+        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::TrivialField for" << name),
+        flags|SceneFieldFlag::OffsetOnly)},
     _mappingType{mappingType},
     _mappingStride{(CORRADE_CONSTEXPR_ASSERT(mappingStride >= -32768 && mappingStride <= 32767,
         "Trade::SceneFieldData: expected mapping view stride to fit into 16 bits, but got" << mappingStride), Short(mappingStride))},

src/Magnum/Trade/Test/SceneDataTest.cpp

@@ -84,6 +84,7 @@ struct SceneDataTest: TestSuite::Tester {
     void constructFieldTooLargeMappingStride();
     void constructFieldTooLargeFieldStride();
     void constructFieldOffsetOnlyNotAllowed();
+    void constructFieldTrivialNotAllowed();
     void constructFieldWrongDataAccess();
     void constructField2DWrongSize();
     void constructField2DNonContiguous();
@@ -197,6 +198,7 @@ struct SceneDataTest: TestSuite::Tester {
     void findFieldObjectOffsetInvalidObject();
 
     template<class T> void implicitNullMapping();
+    template<class T> void trivialNullParent();
 
     void releaseFieldData();
     void releaseData();
@@ -377,6 +379,7 @@ SceneDataTest::SceneDataTest() {
               &SceneDataTest::constructFieldTooLargeMappingStride,
               &SceneDataTest::constructFieldTooLargeFieldStride,
               &SceneDataTest::constructFieldOffsetOnlyNotAllowed,
+              &SceneDataTest::constructFieldTrivialNotAllowed,
               &SceneDataTest::constructFieldWrongDataAccess,
               &SceneDataTest::constructField2DWrongSize,
               &SceneDataTest::constructField2DNonContiguous,
@@ -569,6 +572,11 @@ SceneDataTest::SceneDataTest() {
               &SceneDataTest::implicitNullMapping<UnsignedInt>,
               &SceneDataTest::implicitNullMapping<UnsignedLong>,
 
+              &SceneDataTest::trivialNullParent<Byte>,
+              &SceneDataTest::trivialNullParent<Short>,
+              &SceneDataTest::trivialNullParent<Int>,
+              &SceneDataTest::trivialNullParent<Long>,
+
               &SceneDataTest::releaseFieldData,
               &SceneDataTest::releaseData});
 }
@@ -1111,6 +1119,23 @@ void SceneDataTest::constructFieldOffsetOnlyNotAllowed() {
         "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::OffsetOnly for a view\n");
 }
 
+void SceneDataTest::constructFieldTrivialNotAllowed() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    const UnsignedShort rotationMappingData[3]{};
+    const Quaternion rotationFieldData[3];
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    SceneFieldData{SceneField::Rotation, 3, SceneMappingType::UnsignedShort, 0, sizeof(UnsignedShort), SceneFieldType::Quaternion, 0, sizeof(Quaternion), SceneFieldFlag::TrivialField};
+    SceneFieldData{sceneFieldCustom(666), Containers::arrayView(rotationMappingData), Containers::arrayView(rotationFieldData), SceneFieldFlag::TrivialField};
+    CORRADE_COMPARE(out.str(),
+        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::TrivialField for Trade::SceneField::Rotation\n"
+        "Trade::SceneFieldData: can't pass Trade::SceneFieldFlag::TrivialField for Trade::SceneField::Custom(666)\n");
+}
+
 void SceneDataTest::constructFieldWrongDataAccess() {
     #ifdef CORRADE_NO_ASSERT
     CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
@@ -5702,6 +5727,141 @@ template<class T> void SceneDataTest::implicitNullMapping() {
     }
 }
 
+template<class T> void SceneDataTest::trivialNullParent() {
+    setTestCaseTemplateName(NameTraits<T>::name());
+
+    struct Field {
+        UnsignedInt mapping;
+        T parent;
+    } data[]{
+        {0, -1},
+        {1, -1},
+        {2, -1},
+        {3, -2}, /* this is to know whether we got our or generated data */
+        {5, -1}, /* this is to know whether we got our or generated data */
+    };
+
+    Containers::StridedArrayView1D<Field> view = data;
+
+    /* If the view is not nullptr, it'll use the data and won't generate */
+    {
+        SceneData scene{SceneMappingType::UnsignedInt, 6, {}, data, {
+            SceneFieldData{SceneField::Parent, view.slice(&Field::mapping), view.slice(&Field::parent), SceneFieldFlag::TrivialField},
+        }};
+        CORRADE_COMPARE(scene.field(0).data(), &data[0].parent);
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, -1},
+            {1, -1},
+            {2, -1},
+            {3, -2}, /* this would be -1 if generated */
+            {5, -1}, /* this would be 4 if generated */
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), -2);
+
+        Int parents[5];
+        scene.parentsInto(nullptr, parents);
+        CORRADE_COMPARE_AS(Containers::arrayView(parents), Containers::arrayView<Int>({
+            -1, -1, -1, -2, /* this would be 4 if generated */ -1
+        }), TestSuite::Compare::Container);
+    }
+
+    /* If the view is nullptr, it'll generate the data */
+    {
+        SceneData scene{SceneMappingType::UnsignedInt, 6, {}, data, {
+            SceneFieldData{SceneField::Parent, view.slice(&Field::mapping), Containers::ArrayView<T>{nullptr, view.size()}, SceneFieldFlag::TrivialField},
+        }};
+        CORRADE_COMPARE(scene.field(0).data(), nullptr);
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, -1},
+            {1, -1},
+            {2, -1},
+            {3, -1},
+            {5, -1}, /* this would be 4 if generated */
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), -1);
+
+        Int parents[5];
+        scene.parentsInto(nullptr, parents);
+        CORRADE_COMPARE_AS(Containers::arrayView(parents), Containers::arrayView<Int>({
+            -1, -1, -1, -1, -1
+        }), TestSuite::Compare::Container);
+    }
+
+    /* For an offset-only trivial field it'll generate the data always, even
+       if the field is seemingly there */
+    {
+        SceneData scene{SceneMappingType::UnsignedInt, 6, {}, data, {
+            SceneFieldData{SceneField::Parent, view.size(), SceneMappingType::UnsignedInt, offsetof(Field, mapping), sizeof(Field), Implementation::SceneFieldTypeFor<T>::type(), offsetof(Field, parent), sizeof(Field), SceneFieldFlag::TrivialField},
+        }};
+        CORRADE_COMPARE(scene.field(0).data(), nullptr);
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, -1},
+            {1, -1},
+            {2, -1},
+            {3, -1},
+            {5, -1}, /* this would be 4 if generated */
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), -1);
+
+        Int parents[5];
+        scene.parentsInto(nullptr, parents);
+        CORRADE_COMPARE_AS(Containers::arrayView(parents), Containers::arrayView<Int>({
+            -1, -1, -1, -1, -1
+        }), TestSuite::Compare::Container);
+    }
+
+    /* And if the offset is weirdly off it won't blow up on that */
+    {
+        SceneData scene{SceneMappingType::UnsignedInt, 6, {}, data, {
+            SceneFieldData{SceneField::Parent, view.size(), SceneMappingType::UnsignedInt, offsetof(Field, mapping), sizeof(Field), Implementation::SceneFieldTypeFor<T>::type(), 666666666, 0, SceneFieldFlag::TrivialField},
+        }};
+        CORRADE_COMPARE(scene.field(0).data(), nullptr);
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, -1},
+            {1, -1},
+            {2, -1},
+            {3, -1},
+            {5, -1}, /* this would be 4 if generated */
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), -1);
+
+        Int parents[5];
+        scene.parentsInto(nullptr, parents);
+        CORRADE_COMPARE_AS(Containers::arrayView(parents), Containers::arrayView<Int>({
+            -1, -1, -1, -1, -1
+        }), TestSuite::Compare::Container);
+    }
+
+    /* Finally, neither the mapping nor the field is specified */
+    {
+        SceneData scene{SceneMappingType::UnsignedInt, 6, {}, data, {
+            SceneFieldData{SceneField::Parent,
+                Containers::ArrayView<UnsignedInt>{nullptr, view.size()},
+                Containers::ArrayView<T>{nullptr, view.size()},
+                SceneFieldFlag::ImplicitMapping|SceneFieldFlag::TrivialField},
+        }};
+        CORRADE_COMPARE(scene.field(0).data(), nullptr);
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, -1},
+            {1, -1},
+            {2, -1},
+            {3, -1},
+            {4, -1},
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), -1);
+
+        UnsignedInt mapping[5];
+        Int parents[5];
+        scene.parentsInto(mapping, parents);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 4
+        }), TestSuite::Compare::Container);
+        CORRADE_COMPARE_AS(Containers::arrayView(parents), Containers::arrayView<Int>({
+            -1, -1, -1, -1, -1
+        }), TestSuite::Compare::Container);
+    }
+}
+
 void SceneDataTest::releaseFieldData() {
     struct Field {
         UnsignedByte object;