Trade: allow omitting implicit SceneFieldData object mapping.

doc/snippets/MagnumTrade.cpp

@@ -915,6 +915,17 @@ Trade::SceneData{Trade::SceneMappingType::UnsignedInt, 120, std::move(data),
 /* [SceneFieldData-usage-offset-only] */
 }
 
+{
+/* [SceneFieldData-usage-implicit-mapping] */
+Containers::ArrayView<Matrix4> transformations = DOXYGEN_ELLIPSIS({});
+
+Trade::SceneFieldData field{Trade::SceneField::Transformation,
+    Containers::ArrayView<UnsignedInt>{nullptr, transformations.size()},
+    transformations,
+    Trade::SceneFieldFlag::ImplicitMapping};
+/* [SceneFieldData-usage-implicit-mapping] */
+}
+
 {
 typedef SceneGraph::Scene<SceneGraph::MatrixTransformation3D> Scene3D;
 typedef SceneGraph::Object<SceneGraph::MatrixTransformation3D> Object3D;

src/Magnum/Trade/SceneData.cpp

@@ -594,19 +594,30 @@ SceneData::SceneData(const SceneMappingType mappingType, const UnsignedLong mapp
             const UnsignedInt fieldTypeSize = sceneFieldTypeSize(field._fieldType)*
                 (field._fieldArraySize ? field._fieldArraySize : 1);
             if(field._flags & SceneFieldFlag::OffsetOnly) {
+                /* If an offset-only field has an implicit mapping, we ignore
+                   the offset / size completely */
+                if(!(field._flags >= SceneFieldFlag::ImplicitMapping)) {
                     const std::size_t mappingSize = field._mappingData.offset + (field._size - 1)*field._mappingStride + mappingTypeSize;
-                const std::size_t fieldSize = field._fieldData.offset + (field._size - 1)*field._fieldStride + fieldTypeSize;
                     CORRADE_ASSERT(mappingSize <= _data.size(),
                         "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;
                 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
+                   nullptr */
+                if(!(field._flags >= SceneFieldFlag::ImplicitMapping && !field._mappingData.pointer)) {
                     const void* const mappingBegin = field._mappingData.pointer;
-                const void* const fieldBegin = field._fieldData.pointer;
                     const void* const mappingEnd = static_cast<const char*>(field._mappingData.pointer) + (field._size - 1)*field._mappingStride + mappingTypeSize;
-                const void* const fieldEnd = static_cast<const char*>(field._fieldData.pointer) + (field._size - 1)*field._fieldStride + fieldTypeSize;
                     CORRADE_ASSERT(mappingBegin >= _data.begin() && mappingEnd <= _data.end(),
                         "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;
+                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 << "]", );
             }
@@ -806,6 +817,12 @@ Containers::ArrayView<char> SceneData::mutableData() & {
 
 Containers::StridedArrayView1D<const void> SceneData::fieldDataMappingViewInternal(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 with implicit mapping, ignore the
+       offset/stride and always assume it's not present */
+    if(field._flags >= (SceneFieldFlag::OffsetOnly|SceneFieldFlag::ImplicitMapping))
+        return {{nullptr, ~std::size_t{}}, size, field._mappingStride};
+
     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 ?
@@ -1140,6 +1157,18 @@ void SceneData::mappingIntoInternal(const UnsignedInt fieldId, const std::size_t
        checked by the callers */
 
     const SceneFieldData& field = _fields[fieldId];
+
+    /* If we don't have any data for an implicit mapping or the implicit
+       mapping is offset-only (where we always assume there's no data),
+       generate the sequence */
+    if((field._flags >= SceneFieldFlag::ImplicitMapping && !field._mappingData.pointer) ||
+       (field._flags >= (SceneFieldFlag::ImplicitMapping|SceneFieldFlag::OffsetOnly)))
+    {
+        for(std::size_t i = 0; i != destination.size(); ++i)
+            destination[i] = offset + i;
+        return;
+    }
+
     const Containers::StridedArrayView1D<const void> mappingData = fieldDataMappingViewInternal(field, offset, destination.size());
     const auto destination1ui = Containers::arrayCast<2, UnsignedInt>(destination);
 

src/Magnum/Trade/SceneData.h

@@ -566,7 +566,9 @@ enum class SceneFieldFlag: UnsignedByte {
      * from 0 up to size of the field. A superset of
      * @ref SceneFieldFlag::OrderedMapping. Object IDs in fields marked with
      * this flag can be looked up with an @f$ \mathcal{O}(1) @f$ complexity,
-     * but the field is restricted to exactly one value for each object.
+     * but the field is restricted to exactly one value for each object. If
+     * this flag is set, the object mapping view is allowed to be
+     * @cpp nullptr @ce.
      *
      * Note that validity of the object mapping data isn't checked in any way
      * and if the data doesn't correspond to rules of the flag, queries such
@@ -647,7 +649,18 @@ In some cases the object mapping is even implicit, i.e. the first entry of the
 field specifying data for object @cpp 0 @ce, second entry for object
 @cpp 1 @ce, third for object @cpp 2 @ce and so on. You can annotate such fields
 with @ref SceneFieldFlag::ImplicitMapping, which is a superset of
-@relativeref{SceneFieldFlag,OrderedMapping}.
+@relativeref{SceneFieldFlag,OrderedMapping}. Furthermore, to avoid having to
+generate such mapping data, the mapping view can be @cpp nullptr @ce if this
+flag is present. The view however still needs to have a size matching the field
+data size and the same @ref SceneMappingType as other fields passed to the
+@link SceneData @endlink:
+
+@snippet MagnumTrade.cpp SceneFieldData-usage-implicit-mapping
+
+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.
 */
 class MAGNUM_TRADE_EXPORT SceneFieldData {
     public:
@@ -675,6 +688,14 @@ class MAGNUM_TRADE_EXPORT SceneFieldData {
          * Expects that @p mappingData and @p fieldData have the same size,
          * @p fieldType corresponds to @p name and @p fieldArraySize is zero
          * for builtin fields.
+         *
+         * If @p flags contain @ref SceneFieldFlag::ImplicitMapping, the
+         * @p mappingData can be a @cpp nullptr @ce view (although it still has
+         * to follow other constraints regarding size and type). While
+         * @ref SceneData::mapping() will return it as-is,
+         * @relativeref{SceneData,mappingAsArray()} and
+         * @relativeref{SceneData,mappingInto()} functions will generate its
+         * contents on-the-fly.
          */
         constexpr explicit SceneFieldData(SceneField name, SceneMappingType mappingType, const Containers::StridedArrayView1D<const void>& mappingData, SceneFieldType fieldType, const Containers::StridedArrayView1D<const void>& fieldData, UnsignedShort fieldArraySize = 0, SceneFieldFlags flags = {}) noexcept;
 
@@ -699,6 +720,14 @@ class MAGNUM_TRADE_EXPORT SceneFieldData {
          * @p fieldData is contiguous and its size matches @p fieldType and
          * @p fieldArraySize and that @p fieldType corresponds to @p name and
          * @p fieldArraySize is zero for builtin attributes.
+         *
+         * If @p flags contain @ref SceneFieldFlag::ImplicitMapping, the
+         * @p mappingData can be a @cpp nullptr @ce view (although it still has
+         * to follow other constraints regarding size and type). While
+         * @ref SceneData::mapping() will return it as-is,
+         * @relativeref{SceneData,mappingAsArray()} and
+         * @relativeref{SceneData,mappingInto()} functions will generate its
+         * contents on-the-fly.
          */
         explicit SceneFieldData(SceneField name, const Containers::StridedArrayView2D<const char>& mappingData, SceneFieldType fieldType, const Containers::StridedArrayView2D<const char>& fieldData, UnsignedShort fieldArraySize = 0, SceneFieldFlags flags = {}) noexcept;
 
@@ -779,6 +808,15 @@ class MAGNUM_TRADE_EXPORT SceneFieldData {
          * @p fieldType / @p fieldArraySize checks against @p fieldStride can
          * be done. You're encouraged to use the @ref SceneFieldData(SceneField, SceneMappingType, const Containers::StridedArrayView1D<const void>&, SceneFieldType, const Containers::StridedArrayView1D<const void>&, UnsignedShort, SceneFieldFlags)
          * constructor if you want additional safeguards.
+         *
+         * If @p flags contain @ref SceneFieldFlag::ImplicitMapping, the
+         * @p mappingOffset and @p mappingStride fields are ignored and the
+         * object mapping is assumed to not be present (however you still have
+         * to follow constraints regarding its type). The
+         * @ref SceneData::mapping() will then return a @cpp nullptr @ce view,
+         * and the @relativeref{SceneData,mappingAsArray()} and
+         * @relativeref{SceneData,mappingInto()} functions will generate its
+         * contents on-the-fly.
          * @see @ref flags(), @ref fieldArraySize(),
          *      @ref mappingData(Containers::ArrayView<const void>) const,
          *      @ref fieldData(Containers::ArrayView<const void>) const
@@ -1621,6 +1659,10 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * to @ref SceneMappingType size) and is guaranteed to be contiguous.
          * Use the templated overload below to get the mapping in a concrete
          * type.
+         *
+         * If the field has @ref SceneFieldFlag::ImplicitMapping set and no
+         * data was supplied for it or it's @ref SceneFieldFlag::OffsetOnly,
+         * the returned view will be correctly sized but @cpp nullptr @ce.
          * @see @ref mutableMapping(UnsignedInt),
          *      @ref Corrade::Containers::StridedArrayView::isContiguous(),
          *      @ref sceneMappingTypeSize()
@@ -1644,6 +1686,10 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * The @p fieldId is expected to be smaller than @ref fieldCount() and
          * @p T is expected to correspond to @ref mappingType().
          *
+         * If the field has @ref SceneFieldFlag::ImplicitMapping set and either
+         * no data was supplied for it or it's @ref SceneFieldFlag::OffsetOnly,
+         * the returned view will be correctly sized but @cpp nullptr @ce.
+         *
          * You can also use the non-templated @ref mappingAsArray() accessor
          * (or the combined @ref parentsAsArray(),
          * @ref transformations2DAsArray(), @ref transformations3DAsArray(),
@@ -1677,6 +1723,10 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * @ref SceneMappingType size) and is guaranteed to be contiguous. Use
          * the templated overload below to get the object mapping in a concrete
          * type.
+         *
+         * If the field has @ref SceneFieldFlag::ImplicitMapping set and either
+         * no data was supplied for it or it's @ref SceneFieldFlag::OffsetOnly,
+         * the returned view will be correctly sized but @cpp nullptr @ce.
          * @see @ref hasField(), @ref mapping(UnsignedInt) const,
          *      @ref mutableMapping(SceneField),
          *      @ref Corrade::Containers::StridedArrayView::isContiguous()
@@ -1700,6 +1750,10 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * The @p fieldName is expected to exist and @p T is expected to
          * correspond to @ref mappingType().
          *
+         * If the field has @ref SceneFieldFlag::ImplicitMapping set and either
+         * no data was supplied for it or it's @ref SceneFieldFlag::OffsetOnly,
+         * the returned view will be correctly sized but @cpp nullptr @ce.
+         *
          * You can also use the non-templated @ref mappingAsArray() accessor
          * (or the combined @ref parentsAsArray(),
          * @ref transformations2DAsArray(), @ref transformations3DAsArray(),
@@ -1896,6 +1950,10 @@ class MAGNUM_TRADE_EXPORT SceneData {
          * arbitrary underlying type and returns it in a newly-allocated array.
          * The @p fieldId is expected to be smaller than @ref fieldCount().
          *
+         * If the field has @ref SceneFieldFlag::ImplicitMapping set and either
+         * no data was supplied for it or it's @ref SceneFieldFlag::OffsetOnly,
+         * the data will be generated on-the-fly.
+         *
          * Note that, for common fields, you can also use the
          * @ref parentsAsArray(), @ref transformations2DAsArray(),
          * @ref transformations3DAsArray(),

src/Magnum/Trade/Test/SceneDataTest.cpp

@@ -196,6 +196,8 @@ struct SceneDataTest: TestSuite::Tester {
     void fieldForFieldMissing();
     void findFieldObjectOffsetInvalidObject();
 
+    template<class T> void implicitNullMapping();
+
     void releaseFieldData();
     void releaseData();
 };
@@ -562,6 +564,11 @@ SceneDataTest::SceneDataTest() {
     addTests({&SceneDataTest::fieldForFieldMissing,
               &SceneDataTest::findFieldObjectOffsetInvalidObject,
 
+              &SceneDataTest::implicitNullMapping<UnsignedByte>,
+              &SceneDataTest::implicitNullMapping<UnsignedShort>,
+              &SceneDataTest::implicitNullMapping<UnsignedInt>,
+              &SceneDataTest::implicitNullMapping<UnsignedLong>,
+
               &SceneDataTest::releaseFieldData,
               &SceneDataTest::releaseData});
 }
@@ -5552,6 +5559,149 @@ void SceneDataTest::findFieldObjectOffsetInvalidObject() {
         "Trade::SceneData::skinsFor(): object 7 out of bounds for 7 objects\n");
 }
 
+template<class T> void SceneDataTest::implicitNullMapping() {
+    setTestCaseTemplateName(NameTraits<T>::name());
+
+    struct Field {
+        UnsignedByte id;
+        /* Mapping second so it isn't at offset 0, implying something weird */
+        T mapping;
+    } data[]{
+        {1, 0},
+        {7, 1},
+        {22, 2},
+        {15, 3},
+        {3, 5}, /* this is to know whether we got our or generated data */
+    };
+
+    Containers::StridedArrayView1D<Field> view = data;
+
+    SceneData scene{Implementation::sceneMappingTypeFor<T>(), 6, {}, data, {
+        /* Implicit mapping, with data supplied */
+        SceneFieldData{SceneField::Mesh, view.slice(&Field::mapping), view.slice(&Field::id), SceneFieldFlag::ImplicitMapping},
+        /* Implicit mapping, with no data */
+        SceneFieldData{SceneField::Camera, Containers::ArrayView<T>{nullptr, Containers::arraySize(data)}, view.slice(&Field::id), SceneFieldFlag::ImplicitMapping},
+        /* Implicit mapping offset-only, pointing to the data. This gets
+           ignored because there's no non-shitty non-magic-constants way to
+           know if the offset is valid. */
+        SceneFieldData{SceneField::Light, Containers::arraySize(data), Implementation::sceneMappingTypeFor<T>(), offsetof(Field, mapping), sizeof(Field), SceneFieldType::UnsignedByte, offsetof(Field, id), sizeof(Field), SceneFieldFlag::ImplicitMapping},
+        /* Implicit mapping offset-only, pointing to wherever. Abusing a Parent
+           field and faking it to be signed because Skin needs some
+           transformation field as well */
+        SceneFieldData{SceneField::Parent, Containers::arraySize(data), Implementation::sceneMappingTypeFor<T>(), 666666666, 0, SceneFieldType::Byte, offsetof(Field, id), sizeof(Field), SceneFieldFlag::ImplicitMapping}
+    }};
+
+    /* Only the non-null view will be non-null, the offset-only will all be
+       null also */
+    CORRADE_COMPARE(scene.mapping(0).data(), &data[0].mapping);
+    CORRADE_COMPARE(scene.mapping(1).data(), nullptr);
+    CORRADE_COMPARE(scene.mapping(2).data(), nullptr);
+    CORRADE_COMPARE(scene.mapping(3).data(), nullptr);
+
+    /* If the view is not nullptr, it'll use the data and won't generate */
+    {
+        CORRADE_COMPARE_AS(scene.meshesMaterialsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Containers::Pair<UnsignedInt, Int>>>({
+            {0, {1, -1}},
+            {1, {7, -1}},
+            {2, {22, -1}},
+            {3, {15, -1}},
+            {5, {3, -1}}, /* this would be 4 if generated */
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE_AS(scene.meshesMaterialsFor(3), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {15, -1}
+        })), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[5];
+        scene.meshesMaterialsInto(mapping, nullptr, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 5 /* this would be 4 if generated */
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[3];
+        scene.meshesMaterialsInto(2, mapping, nullptr, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            2, 3, 5 /* this would be 4 if generated */
+        }), TestSuite::Compare::Container);
+    }
+
+    /* If the view is nullptr, it'll generate the data */
+    {
+        CORRADE_COMPARE_AS(scene.camerasAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, UnsignedInt>>({
+            {0, 1},
+            {1, 7},
+            {2, 22},
+            {3, 15},
+            {4, 3},
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE_AS(scene.camerasFor(3), Containers::arrayView<UnsignedInt>({
+            15
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[5];
+        scene.camerasInto(mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 4
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[3];
+        scene.camerasInto(2, mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            2, 3, 4
+        }), TestSuite::Compare::Container);
+    }
+
+    /* For an offset-only implicit mapping it'll generate the data always, even
+       if the mapping is seemingly there */
+    {
+        CORRADE_COMPARE_AS(scene.lightsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, UnsignedInt>>({
+            {0, 1},
+            {1, 7},
+            {2, 22},
+            {3, 15},
+            {4, 3},
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE_AS(scene.lightsFor(3), Containers::arrayView<UnsignedInt>({
+            15
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[5];
+        scene.lightsInto(mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 4
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[3];
+        scene.lightsInto(2, mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            2, 3, 4
+        }), TestSuite::Compare::Container);
+    }
+
+    /* And if the offset is weirdly off it won't blow up on that */
+    {
+        CORRADE_COMPARE_AS(scene.parentsAsArray(), (Containers::arrayView<Containers::Pair<UnsignedInt, Int>>({
+            {0, 1},
+            {1, 7},
+            {2, 22},
+            {3, 15},
+            {4, 3},
+        })), TestSuite::Compare::Container);
+        CORRADE_COMPARE(scene.parentFor(3), 15);
+    } {
+        UnsignedInt mapping[5];
+        scene.parentsInto(mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            0, 1, 2, 3, 4
+        }), TestSuite::Compare::Container);
+    } {
+        UnsignedInt mapping[3];
+        scene.parentsInto(2, mapping, nullptr);
+        CORRADE_COMPARE_AS(Containers::arrayView(mapping), Containers::arrayView<UnsignedInt>({
+            2, 3, 4
+        }), TestSuite::Compare::Container);
+    }
+}
+
 void SceneDataTest::releaseFieldData() {
     struct Field {
         UnsignedByte object;