From 67900c79a4b6a5c5f2a9f24b7f3a3e8c5a6e683e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 4 Oct 2021 19:59:29 +0200 Subject: [PATCH] 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 | 8 ++++++ src/Magnum/Trade/SceneData.h | 10 +++++--- src/Magnum/Trade/Test/SceneDataTest.cpp | 33 ++++++++++++++++++++++--- 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/Magnum/Trade/SceneData.cpp b/src/Magnum/Trade/SceneData.cpp index d22c78a2b..8fa76a42d 100644 --- a/src/Magnum/Trade/SceneData.cpp +++ b/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&& data, const std::initializer_list fields, const void* const importerState): SceneData{objectType, objectCount, std::move(data), Implementation::initializerListToArrayWithDefaultDeleter(fields), importerState} {} diff --git a/src/Magnum/Trade/SceneData.h b/src/Magnum/Trade/SceneData.h index 8a9a5b9b1..0884eb174 100644 --- a/src/Magnum/Trade/SceneData.h +++ b/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, diff --git a/src/Magnum/Trade/Test/SceneDataTest.cpp b/src/Magnum/Trade/Test/SceneDataTest.cpp index 306bdfc0a..95d69eedc 100644 --- a/src/Magnum/Trade/Test/SceneDataTest.cpp +++ b/src/Magnum/Trade/Test/SceneDataTest.cpp @@ -98,6 +98,7 @@ struct SceneDataTest: TestSuite::Tester { void constructMismatchedTRSViews(); template void constructMismatchedTRSDimensionality(); void constructMismatchedMeshMaterialView(); + void constructAmbiguousSkinDimensions(); void constructCopy(); void constructMove(); @@ -253,6 +254,7 @@ SceneDataTest::SceneDataTest() { &SceneDataTest::constructMismatchedTRSDimensionality, &SceneDataTest::constructMismatchedTRSDimensionality, &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{}); CORRADE_VERIFY(!std::is_copy_assignable{}); @@ -3649,8 +3664,10 @@ template void SceneDataTest::skinsAsArray() { Containers::StridedArrayView1D 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 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 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 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)} }};