Trade: SceneField::Skin now requires a transformation to be present.

Even an empty field, it's there just to disambiguate whether the skin is
2D or 3D.

src/Magnum/Trade/SceneData.cpp

@@ -384,6 +384,7 @@ SceneData::SceneData(const SceneObjectType objectType, const UnsignedLong object
     #ifndef CORRADE_NO_ASSERT
     UnsignedInt meshField = ~UnsignedInt{};
     UnsignedInt meshMaterialField = ~UnsignedInt{};
+    UnsignedInt skinField = ~UnsignedInt{};
     #endif
     for(std::size_t i = 0; i != _fields.size(); ++i) {
         const SceneFieldData& field = _fields[i];
@@ -465,6 +466,8 @@ SceneData::SceneData(const SceneObjectType objectType, const UnsignedLong object
             meshField = i;
         } else if(_fields[i]._name == SceneField::MeshMaterial) {
             meshMaterialField = i;
+        } else if(_fields[i]._name == SceneField::Skin) {
+            skinField = i;
         }
         #endif
     }
@@ -560,6 +563,11 @@ SceneData::SceneData(const SceneObjectType objectType, const UnsignedLong object
             _dimensions = 3;
         } else CORRADE_INTERNAL_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */
     }
+
+    /* A skin field requires some transformation field to exist in order to
+       disambiguate between 2D and 3D */
+    CORRADE_ASSERT(skinField == ~UnsignedInt{} || _dimensions,
+        "Trade::SceneData: a skin field requires some transformation field to be present in order to disambiguate between 2D and 3D", );
 }
 
 SceneData::SceneData(const SceneObjectType objectType, const UnsignedLong objectCount, Containers::Array<char>&& data, const std::initializer_list<SceneFieldData> fields, const void* const importerState): SceneData{objectType, objectCount, std::move(data), Implementation::initializerListToArrayWithDefaultDeleter(fields), importerState} {}

src/Magnum/Trade/SceneData.h

@@ -273,12 +273,14 @@ enum class SceneField: UnsignedInt {
 
     /**
      * ID of a skin associated with this object, corresponding to the ID
-     * passed to @ref AbstractImporter::skin(). Type is usually
-     * @ref SceneFieldType::UnsignedInt, but can be also any of
-     * @relativeref{SceneFieldType,UnsignedByte} or
+     * passed to @ref AbstractImporter::skin2D() or
+     * @ref AbstractImporter::skin3D(), depending on whether the scene has a 2D
+     * or 3D transformation. Type is usually @ref SceneFieldType::UnsignedInt,
+     * but can be also any of @relativeref{SceneFieldType,UnsignedByte} or
      * @relativeref{SceneFieldType,UnsignedShort}. An object can have multiple
      * skins associated.
-     * @see @ref SceneData::skinsAsArray(), @ref SceneData::skinsFor()
+     * @see @ref SceneData::is2D(), @ref SceneData::is3D(),
+     *      @ref SceneData::skinsAsArray(), @ref SceneData::skinsFor()
      */
     Skin,
 

src/Magnum/Trade/Test/SceneDataTest.cpp

@@ -98,6 +98,7 @@ struct SceneDataTest: TestSuite::Tester {
     void constructMismatchedTRSViews();
     template<class T> void constructMismatchedTRSDimensionality();
     void constructMismatchedMeshMaterialView();
+    void constructAmbiguousSkinDimensions();
 
     void constructCopy();
     void constructMove();
@@ -253,6 +254,7 @@ SceneDataTest::SceneDataTest() {
               &SceneDataTest::constructMismatchedTRSDimensionality<Float>,
               &SceneDataTest::constructMismatchedTRSDimensionality<Double>,
               &SceneDataTest::constructMismatchedMeshMaterialView,
+              &SceneDataTest::constructAmbiguousSkinDimensions,
 
               &SceneDataTest::constructCopy,
               &SceneDataTest::constructMove,
@@ -1778,6 +1780,19 @@ void SceneDataTest::constructMismatchedMeshMaterialView() {
         "Trade::SceneData: Trade::SceneField::MeshMaterial object data [0xcafe0000:0xcafe0008] is different from Trade::SceneField::Mesh object data [0xcafe0000:0xcafe000c]\n");
 }
 
+void SceneDataTest::constructAmbiguousSkinDimensions() {
+    #ifdef CORRADE_NO_ASSERT
+    CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");
+    #endif
+
+    std::ostringstream out;
+    Error redirectError{&out};
+    SceneData{SceneObjectType::UnsignedInt, 0, nullptr, {
+        SceneFieldData{SceneField::Skin, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::UnsignedInt, nullptr}
+    }};
+    CORRADE_COMPARE(out.str(), "Trade::SceneData: a skin field requires some transformation field to be present in order to disambiguate between 2D and 3D\n");
+}
+
 void SceneDataTest::constructCopy() {
     CORRADE_VERIFY(!std::is_copy_constructible<SceneData>{});
     CORRADE_VERIFY(!std::is_copy_assignable<SceneData>{});
@@ -3649,8 +3664,10 @@ template<class T> void SceneDataTest::skinsAsArray() {
     Containers::StridedArrayView1D<Field> view = fields;
 
     SceneData scene{SceneObjectType::UnsignedByte, 50, {}, fields, {
-        /* To verify it isn't just picking the first ever field */
-        SceneFieldData{SceneField::Parent, SceneObjectType::UnsignedByte, nullptr, SceneFieldType::Int, nullptr},
+        /* To verify it isn't just picking the first ever field; also to
+           satisfy the requirement of having a transformation field to
+           disambiguate the dimensionality */
+        SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedByte, nullptr, SceneFieldType::Vector3, nullptr},
         SceneFieldData{SceneField::Skin, view.slice(&Field::object), view.slice(&Field::skin)}
     }};
 
@@ -3679,8 +3696,10 @@ void SceneDataTest::skinsIntoArray() {
     Containers::StridedArrayView1D<Field> view = fields;
 
     SceneData scene{SceneObjectType::UnsignedInt, 5, {}, fields, {
-        /* To verify it isn't just picking the first ever field */
-        SceneFieldData{SceneField::Parent, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Int, nullptr},
+        /* To verify it isn't just picking the first ever field; also to
+           satisfy the requirement of having a transformation field to
+           disambiguate the dimensionality */
+        SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Vector3, nullptr},
         SceneFieldData{SceneField::Skin,
             view.slice(&Field::object),
             view.slice(&Field::skin)},
@@ -3718,6 +3737,9 @@ void SceneDataTest::skinsIntoArrayInvalidSizeOrOffset() {
     Containers::StridedArrayView1D<Field> view = fields;
 
     SceneData scene{SceneObjectType::UnsignedInt, 5, {}, fields, {
+        /* To satisfy the requirement of having a transformation field to
+           disambiguate the dimensionality */
+        SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Vector3, nullptr},
         SceneFieldData{SceneField::Skin, view.slice(&Field::object), view.slice(&Field::skin)}
     }};
 
@@ -4617,6 +4639,9 @@ void SceneDataTest::skinsFor() {
     Containers::StridedArrayView1D<Field> view = fields;
 
     SceneData scene{SceneObjectType::UnsignedInt, 7, {}, fields, {
+        /* To satisfy the requirement of having a transformation field to
+           disambiguate the dimensionality */
+        SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Vector3, nullptr},
         SceneFieldData{SceneField::Skin, view.slice(&Field::object), view.slice(&Field::skin)}
     }};