diff --git a/src/MagnumPlugins/GltfImporter/GltfImporter.conf b/src/MagnumPlugins/GltfImporter/GltfImporter.conf index b56a9ff29..371e5ad77 100644 --- a/src/MagnumPlugins/GltfImporter/GltfImporter.conf +++ b/src/MagnumPlugins/GltfImporter/GltfImporter.conf @@ -48,6 +48,12 @@ textureCoordinateYFlipInMaterial=false # this name. Change if your file uses a different identifier. objectIdAttribute=_OBJECT_ID +# Expose MeshAttribute::JointIds and Weights under custom "JOINTS" and +# "WEIGHTS" aliases for backwards compatibility with code that relies on +# those being present. If Magnum is built with MAGNUM_BUILD_DEPRECATED +# disabled, no aliases are provided and this option is ignored. +compatibilitySkinningAttributes=true + # Provide basic Phong material attributes even for PBR materials in order to # be compatible with PhongMaterialData workflows from version 2020.06 and # before. This option will eventually become disabled by default. diff --git a/src/MagnumPlugins/GltfImporter/GltfImporter.cpp b/src/MagnumPlugins/GltfImporter/GltfImporter.cpp index 4d4a11ff9..77433d7f7 100644 --- a/src/MagnumPlugins/GltfImporter/GltfImporter.cpp +++ b/src/MagnumPlugins/GltfImporter/GltfImporter.cpp @@ -231,19 +231,24 @@ struct GltfImporter::Document { we need them in three different places and on-demand construction would be too annoying to test. */ std::unordered_map sceneFieldsForName; - std::unordered_map - meshAttributesForName{ - /* Not a builtin MeshAttribute yet, but expected to be used by - people until builtin support is added. Wouldn't strictly need to be - present if the file has no skinning meshes but having them present - in the map always makes the implementation simpler. */ + std::unordered_map meshAttributesForName{ + #ifdef MAGNUM_BUILD_DEPRECATED + /* Added as aliases of MeshAttribute::JointIds and + MeshAttribute::Weights for backwards compatibility with code that + used skinning even before there was builtin support for it. + Wouldn't strictly need to be present if the file has no skinning + meshes but having them present in the map always makes the + implementation simpler. */ {"JOINTS"_s, meshAttributeCustom(0)}, {"WEIGHTS"_s, meshAttributeCustom(1)} + #endif }; Containers::Array> sceneFieldNamesTypes; Containers::Array meshAttributeNames{InPlaceInit, { + #ifdef MAGNUM_BUILD_DEPRECATED "JOINTS"_s, "WEIGHTS"_s + #endif }}; /* Mapping for multi-primitive meshes: @@ -2998,6 +3003,8 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign UnsignedInt bufferId = 0; UnsignedInt vertexCount = 0; std::size_t attributeId = 0; + UnsignedInt jointIdAttributeCount = 0; + UnsignedInt weightAttributeCount = 0; Containers::Pair lastNumberedAttribute; Math::Range1D bufferRange; Containers::Array attributeData{uniqueAttributeCount}; @@ -3045,6 +3052,7 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign allowed, name stays empty, which produces an error in a single place below. */ MeshAttribute name{}; + UnsignedShort arraySize = 0; if(baseAttributeName == "POSITION"_s) { if(accessor->second() == VertexFormat::Vector3 || accessor->second() == VertexFormat::Vector3b || @@ -3085,22 +3093,31 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign accessor->second() == VertexFormat::Vector3usNormalized || accessor->second() == VertexFormat::Vector4usNormalized) name = MeshAttribute::Color; - /* Not a builtin MeshAttribute yet, but expected to be used by people - until builtin support is added */ + /* Joints and weights are represented as an array attribute, so the + vertex format gets changed to a component format and the component + count becomes the array size. */ + /** @todo consider merging JOINTS_0, JOINTS_1 etc if they follow each + other and have the same type */ } else if(baseAttributeName == "JOINTS"_s) { if(accessor->second() == VertexFormat::Vector4ub || - accessor->second() == VertexFormat::Vector4us) - /** @todo update once these are builtin, but provide an opt-out - compatibility alias */ - name = _d->meshAttributesForName.at(baseAttributeName); + accessor->second() == VertexFormat::Vector4us) { + ++jointIdAttributeCount; + name = MeshAttribute::JointIds; + arraySize = vertexFormatComponentCount(accessor->second()); + accessor->second() = vertexFormatComponentFormat(accessor->second()); + } } else if(baseAttributeName == "WEIGHTS"_s) { if(accessor->second() == VertexFormat::Vector4 || accessor->second() == VertexFormat::Vector4ubNormalized || - accessor->second() == VertexFormat::Vector4usNormalized) - /** @todo update once these are builtin, but provide an opt-out - compatibility alias */ - name = _d->meshAttributesForName.at(baseAttributeName); - + accessor->second() == VertexFormat::Vector4usNormalized) { + ++weightAttributeCount; + name = MeshAttribute::Weights; + arraySize = vertexFormatComponentCount(accessor->second()); + /* vertexFormatComponentFormat() strips the normalized bit from + the format, need to go through the full vertexFormat() + composer instead */ + accessor->second() = vertexFormat(accessor->second(), 1, isVertexFormatNormalized(accessor->second())); + } /* Object ID, name custom. To avoid confusion, print the error together with saying it's an object ID attribute */ } else if(attribute.first() == configuration().value("objectIdAttribute")) { @@ -3170,12 +3187,29 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign /* Fill in an attribute. Points to the input data, will be patched to the output data once we know where it's allocated. */ - attributeData[attributeId++] = MeshAttributeData{name, accessor->second(), accessor->first()}; + attributeData[attributeId++] = MeshAttributeData{name, accessor->second(), accessor->first(), arraySize}; + + /* For backwards compatibility insert also a custom "JOINTS" / + "WEIGHTS" attribute which is a Vector4 instead of T[4] */ + #ifdef MAGNUM_BUILD_DEPRECATED + if((name == MeshAttribute::JointIds || name == MeshAttribute::Weights) && configuration().value("compatibilitySkinningAttributes")) { + arrayInsert(attributeData, attributeId++, InPlaceInit, _d->meshAttributesForName.at(baseAttributeName), vertexFormat(accessor->second(), arraySize, isVertexFormatNormalized(accessor->second())), accessor->first()); + } + #endif } /* Verify we really filled all attributes */ CORRADE_INTERNAL_ASSERT(attributeId == attributeData.size()); + /* If the deprecated "JOINTS" and "WEIGHTS" attributes were added, the + array was turned into a growable one (for simplicity). Convert it back + to a regular one to avoid assertions in AbstractImporter due to + potentially-dangling deleter pointers */ + #ifdef MAGNUM_BUILD_DEPRECATED + if(configuration().value("compatibilitySkinningAttributes")) + arrayShrink(attributeData, DefaultInit); + #endif + /* 3.7.2.1 (Geometry § Meshes § Overview) says "[count] MUST be non-zero", but we allow also none unless the strict option is enabled. Not printing a warning if the strict option is disabled as Magnum can handle the @@ -3185,6 +3219,15 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign return {}; } + /* 3.7.3.3 (Geometry § Skins § Skinned mesh attributes) says "For a given + primitive, the number of JOINTS_n attribute sets MUST be equal to the + number of WEIGHTS_n attribute sets". Which aligns well with the + assertion that's in MeshData itself. */ + if(jointIdAttributeCount != weightAttributeCount) { + Error{} << "Trade::GltfImporter::mesh(): the mesh has" << jointIdAttributeCount << "JOINTS_n attributes but" << weightAttributeCount << "WEIGHTS_n attributes"; + return {}; + } + /* Allocate & copy vertex data, if any */ Containers::ArrayView inputVertexData{reinterpret_cast(bufferRange.min()), bufferRange.size()}; Containers::Array vertexData{NoInit, bufferRange.size()}; @@ -3209,7 +3252,7 @@ Containers::Optional GltfImporter::doMesh(const UnsignedInt id, Unsign vertexCount, attributeData[i].stride()}; attributeData[i] = MeshAttributeData{attributeData[i].name(), - attributeData[i].format(), data}; + attributeData[i].format(), data, attributeData[i].arraySize()}; /* Flip Y axis of texture coordinates, unless it's done in the material instead */ diff --git a/src/MagnumPlugins/GltfImporter/GltfImporter.h b/src/MagnumPlugins/GltfImporter/GltfImporter.h index a67ba0eec..be9e47720 100644 --- a/src/MagnumPlugins/GltfImporter/GltfImporter.h +++ b/src/MagnumPlugins/GltfImporter/GltfImporter.h @@ -255,13 +255,24 @@ Import of morph data is not supported at the moment. @ref VertexFormat::Vector4ubNormalized, @ref VertexFormat::Vector3usNormalized or @ref VertexFormat::Vector4usNormalized -- Joint IDs and weights for skinning are imported as custom vertex attributes - named "JOINTS" and "WEIGHTS". Their mapping to/from a string can be queried - using @ref meshAttributeName() and @ref meshAttributeForName(). - Joint IDs are imported as @ref VertexFormat::Vector4ub or - @ref VertexFormat::Vector4us. Joint weights are imported as - @ref VertexFormat::Vector4, @ref VertexFormat::Vector4ubNormalized or - @ref VertexFormat::Vector4usNormalized. +- Skin joint IDs are imported as *arrays* of @ref VertexFormat::UnsignedByte + or @ref VertexFormat::UnsignedShort, skin weights then as arrays of + @ref VertexFormat::Float, @ref VertexFormat::UnsignedByteNormalized or + @ref VertexFormat::UnsignedShortNormalized. The + @ref MeshData::attributeArraySize() is currently always @cpp 4 @ce, but is + reserved to change (for example representing two consecutive sets as a + single array of 8 items). For backwards compatibility, unless the + @cb{.ini} compatibilitySkinningAttributes @ce + @ref Trade-GltfImporter-configuration "configuration option" or + @ref MAGNUM_BUILD_DEPRECATED is disabled, these are also exposed as custom + @cpp "JOINTS" @ce and @cpp "WEIGHTS" @ce attributes with + @ref VertexFormat::Vector4ub / + @ref VertexFormat::Vector4us and @ref VertexFormat::Vector4 / + @ref VertexFormat::Vector4ubNormalized / + @ref VertexFormat::Vector4usNormalized (non-array) formats, respectively, + with as many instances of these as needed to cover all array items. The + compatibility attributes *alias* the builtin ones, i.e. point to the same + memory, so their presence causes no extra overhead. - Per-vertex object ID attribute is imported as either @ref VertexFormat::UnsignedInt, @ref VertexFormat::UnsignedShort or @ref VertexFormat::UnsignedByte. By default `_OBJECT_ID` is the recognized diff --git a/src/MagnumPlugins/GltfImporter/Test/GltfImporterTest.cpp b/src/MagnumPlugins/GltfImporter/Test/GltfImporterTest.cpp index 8c39973f8..bc5217457 100644 --- a/src/MagnumPlugins/GltfImporter/Test/GltfImporterTest.cpp +++ b/src/MagnumPlugins/GltfImporter/Test/GltfImporterTest.cpp @@ -744,6 +744,23 @@ const struct { {"strict, binary", ".glb", true, "Trade::GltfImporter::mesh(): strict mode enabled, disallowing a mesh with no vertices\n"} }; +/** @todo remove once the compatibilitySkinningAttributes option is gone */ +const struct { + const char* name; + #ifdef MAGNUM_BUILD_DEPRECATED + Containers::Optional compatibilitySkinningAttributes; + #endif +} MeshSkinAttributeData[]{ + {"", + #ifdef MAGNUM_BUILD_DEPRECATED + {} + #endif + }, + #ifdef MAGNUM_BUILD_DEPRECATED + {"no compatibility attributes", false}, + #endif +}; + const struct { const char* name; Containers::Optional strict; @@ -878,14 +895,16 @@ const struct { "Trade::GltfImporter::openData(): accessor 1 has invalid normalized property\n"}, }; -constexpr struct { - const char* name; +const struct { + TestSuite::TestCaseDescriptionSourceLocation name; const char* message; } MeshInvalidData[]{ {"unrecognized primitive", "unrecognized primitive 666"}, {"different vertex count for each accessor", "mismatched vertex count for attribute TEXCOORD_0, expected 3 but got 4"}, + {"different number of WEIGHTS and JOINTS attributes", + "the mesh has 2 JOINTS_n attributes but 1 WEIGHTS_n attributes"}, {"unexpected index type", "unsupported index type Vector2ui"}, {"unsupported index component type", @@ -921,72 +940,72 @@ constexpr struct { {"buffer view index out of bounds", "buffer view index 16 out of range for 16 buffer views"}, {"accessor index out of bounds", - "accessor index 40 out of range for 40 accessors"}, + "accessor index 42 out of range for 42 accessors"}, {"mesh index accessor out of bounds", - "accessor index 40 out of range for 40 accessors"}, + "accessor index 42 out of range for 42 accessors"}, {"buffer with missing uri property", "buffer 2 has missing uri property"}, {"buffer with invalid uri property", - "Utility::Json::parseString(): expected a string, got Utility::JsonToken::Type::Array at {}:849:14\n" + "Utility::Json::parseString(): expected a string, got Utility::JsonToken::Type::Array at {}:875:14\n" "Trade::GltfImporter::mesh(): buffer 3 has invalid uri property\n"}, {"buffer with invalid uri", "invalid URI escape sequence %%"}, {"buffer with missing byteLength property", "buffer 5 has missing or invalid byteLength property"}, {"buffer with invalid byteLength property", - "Utility::Json::parseSize(): too large integer literal -3 at {}:863:21\n" + "Utility::Json::parseSize(): too large integer literal -3 at {}:889:21\n" "Trade::GltfImporter::mesh(): buffer 6 has missing or invalid byteLength property\n"}, {"buffer view with missing buffer property", "buffer view 9 has missing or invalid buffer property"}, {"buffer view with invalid buffer property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:799:17\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:825:17\n" "Trade::GltfImporter::mesh(): buffer view 10 has missing or invalid buffer property\n"}, {"buffer view with invalid byteOffset property", - "Utility::Json::parseSize(): too large integer literal -1 at {}:805:21\n" + "Utility::Json::parseSize(): too large integer literal -1 at {}:831:21\n" "Trade::GltfImporter::mesh(): buffer view 11 has invalid byteOffset property\n"}, {"buffer view with missing byteLength property", "buffer view 12 has missing or invalid byteLength property"}, {"buffer view with invalid byteLength property", - "Utility::Json::parseSize(): too large integer literal -12 at {}:815:21\n" + "Utility::Json::parseSize(): too large integer literal -12 at {}:841:21\n" "Trade::GltfImporter::mesh(): buffer view 13 has missing or invalid byteLength property\n"}, {"buffer view with invalid byteStride property", - "Utility::Json::parseUnsignedInt(): too large integer literal -4 at {}:821:21\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -4 at {}:847:21\n" "Trade::GltfImporter::mesh(): buffer view 14 has invalid byteStride property\n"}, {"accessor with missing bufferView property", "accessor 11 has missing or invalid bufferView property"}, {"accessor with invalid bufferView property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:675:21\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:687:21\n" "Trade::GltfImporter::mesh(): accessor 30 has missing or invalid bufferView property\n"}, {"accessor with invalid byteOffset property", - "Utility::Json::parseSize(): too large integer literal -1 at {}:683:21\n" + "Utility::Json::parseSize(): too large integer literal -1 at {}:695:21\n" "Trade::GltfImporter::mesh(): accessor 31 has invalid byteOffset property\n"}, {"accessor with missing componentType property", "accessor 32 has missing or invalid componentType property"}, {"accessor with invalid componentType property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:697:24\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:709:24\n" "Trade::GltfImporter::mesh(): accessor 33 has missing or invalid componentType property\n"}, {"accessor with missing count property", "accessor 34 has missing or invalid count property"}, {"accessor with invalid count property", - "Utility::Json::parseSize(): too large integer literal -1 at {}:711:16\n" + "Utility::Json::parseSize(): too large integer literal -1 at {}:723:16\n" "Trade::GltfImporter::mesh(): accessor 35 has missing or invalid count property\n"}, {"accessor with missing type property", "accessor 36 has missing or invalid type property"}, {"accessor with invalid type property", - "Utility::Json::parseString(): expected a string, got Utility::JsonToken::Type::Number at {}:725:15\n" + "Utility::Json::parseString(): expected a string, got Utility::JsonToken::Type::Number at {}:737:15\n" "Trade::GltfImporter::mesh(): accessor 37 has missing or invalid type property\n"}, {"accessor with invalid normalized property", - "Utility::Json::parseBool(): expected a bool, got Utility::JsonToken::Type::Null at {}:733:21\n" + "Utility::Json::parseBool(): expected a bool, got Utility::JsonToken::Type::Null at {}:745:21\n" "Trade::GltfImporter::mesh(): accessor 38 has invalid normalized property\n"}, {"invalid primitive property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:424:19\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:436:19\n" "Trade::GltfImporter::mesh(): invalid primitive mode property\n"}, {"invalid attribute property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:434:26\n" + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:446:26\n" "Trade::GltfImporter::mesh(): invalid attribute _WEIRD_EH\n"}, {"invalid indices property", - "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:444:22\n" - "Trade::GltfImporter::mesh(): invalid indices property\n"} + "Utility::Json::parseUnsignedInt(): too large integer literal -1 at {}:456:22\n" + "Trade::GltfImporter::mesh(): invalid indices property\n"}, }; constexpr struct { @@ -1605,9 +1624,12 @@ GltfImporterTest::GltfImporterTest() { addInstancedTests({&GltfImporterTest::meshNoIndicesNoVerticesNoBufferUri}, Containers::arraySize(MeshNoVerticesData)); - addTests({&GltfImporterTest::meshColors, - &GltfImporterTest::meshSkinAttributes, - &GltfImporterTest::meshCustomAttributes, + addTests({&GltfImporterTest::meshColors}); + + addInstancedTests({&GltfImporterTest::meshSkinAttributes}, + Containers::arraySize(MeshSkinAttributeData)); + + addTests({&GltfImporterTest::meshCustomAttributes, &GltfImporterTest::meshCustomAttributesNoFileOpened, &GltfImporterTest::meshDuplicateAttributes, &GltfImporterTest::meshUnorderedAttributes, @@ -3512,26 +3534,31 @@ void GltfImporterTest::mesh() { CORRADE_COMPARE(importer->meshForName("Indexed mesh"), 0); CORRADE_COMPARE(importer->meshForName("Nonexistent"), -1); - /* These are present always; see the meshSkinAttributes() test case for - details */ + /* These are present always on a deprecated build; see the + meshSkinAttributes() test case for details */ + #ifdef MAGNUM_BUILD_DEPRECATED + const UnsignedInt customAttributeOffset = 2; CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(0)), "JOINTS"); CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(1)), "WEIGHTS"); + #else + const UnsignedInt customAttributeOffset = 0; + #endif /* All attributes including the builtin ones are exposed via meshAttributeForName() to prepare for fallback cases where they get may get imported as custom. See the meshUnsupportedVertexFormats() test case for more information. */ - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(2)), "NORMAL"); - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(3)), "POSITION"); - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(4)), "TANGENT"); - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(5)), "_OBJECT_ID"); - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(6)), "TEXCOORD_0"); - CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(7)), ""); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 0)), "NORMAL"); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 1)), "POSITION"); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 2)), "TANGENT"); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 3)), "_OBJECT_ID"); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 4)), "TEXCOORD_0"); + CORRADE_COMPARE(importer->meshAttributeName(meshAttributeCustom(customAttributeOffset + 5)), ""); /* Check inverse mapping as well */ - CORRADE_COMPARE(importer->meshAttributeForName("POSITION"), meshAttributeCustom(3)); - CORRADE_COMPARE(importer->meshAttributeForName("TEXCOORD_0"), meshAttributeCustom(6)); - CORRADE_COMPARE(importer->meshAttributeForName("_OBJECT_ID"), meshAttributeCustom(5)); + CORRADE_COMPARE(importer->meshAttributeForName("POSITION"), meshAttributeCustom(customAttributeOffset + 1)); + CORRADE_COMPARE(importer->meshAttributeForName("TEXCOORD_0"), meshAttributeCustom(customAttributeOffset + 4)); + CORRADE_COMPARE(importer->meshAttributeForName("_OBJECT_ID"), meshAttributeCustom(customAttributeOffset + 3)); Containers::Optional mesh = importer->mesh(0); CORRADE_VERIFY(mesh); @@ -3742,23 +3769,44 @@ void GltfImporterTest::meshColors() { } void GltfImporterTest::meshSkinAttributes() { + auto&& data = MeshSkinAttributeData[testCaseInstanceId()]; + setTestCaseDescription(data.name); + Containers::Pointer importer = _manager.instantiate("GltfImporter"); + + #ifdef MAGNUM_BUILD_DEPRECATED + if(data.compatibilitySkinningAttributes) + importer->configuration().setValue("compatibilitySkinningAttributes", *data.compatibilitySkinningAttributes); + #endif + CORRADE_VERIFY(importer->openFile(Utility::Path::join(GLTFIMPORTER_TEST_DIR, "mesh-skin-attributes.gltf"))); - /* The mapping should be available even before the mesh is imported */ + /* The backwards compatibility mapping should be available even before the + mesh is imported, and for robustness always regardless of whether + compatibilitySkinningAttributes is set. Otherwise it could happen that + it won't get added during file opening, but then + compatibilitySkinningAttributes gets flipped back on and mesh import + asserts because there's no entry for JOINTS / WEIGHTS in the map. */ const MeshAttribute jointsAttribute = importer->meshAttributeForName("JOINTS"); - CORRADE_COMPARE(jointsAttribute, meshAttributeCustom(0)); const MeshAttribute weightsAttribute = importer->meshAttributeForName("WEIGHTS"); + #ifdef MAGNUM_BUILD_DEPRECATED + const UnsignedInt customAttributeOffset = 2; + CORRADE_COMPARE(jointsAttribute, meshAttributeCustom(0)); CORRADE_COMPARE(weightsAttribute, meshAttributeCustom(1)); + #else + const UnsignedInt customAttributeOffset = 0; + CORRADE_COMPARE(jointsAttribute, MeshAttribute{}); + CORRADE_COMPARE(weightsAttribute, MeshAttribute{}); + #endif /* However, the actual numbered names are exposed as well -- which is for the case when they would have an invalid vertex format, and thus would get imported as custom. See the meshUnsupportedVertexFormats() test case for more information. */ - CORRADE_COMPARE(importer->meshAttributeForName("JOINTS_0"), meshAttributeCustom(2)); - CORRADE_COMPARE(importer->meshAttributeForName("JOINTS_1"), meshAttributeCustom(3)); - CORRADE_COMPARE(importer->meshAttributeForName("WEIGHTS_0"), meshAttributeCustom(4)); - CORRADE_COMPARE(importer->meshAttributeForName("WEIGHTS_1"), meshAttributeCustom(5)); + CORRADE_COMPARE(importer->meshAttributeForName("JOINTS_0"), meshAttributeCustom(customAttributeOffset + 0)); + CORRADE_COMPARE(importer->meshAttributeForName("JOINTS_1"), meshAttributeCustom(customAttributeOffset + 1)); + CORRADE_COMPARE(importer->meshAttributeForName("WEIGHTS_0"), meshAttributeCustom(customAttributeOffset + 2)); + CORRADE_COMPARE(importer->meshAttributeForName("WEIGHTS_1"), meshAttributeCustom(customAttributeOffset + 3)); CORRADE_COMPARE(importer->meshCount(), 1); @@ -3766,7 +3814,15 @@ void GltfImporterTest::meshSkinAttributes() { CORRADE_VERIFY(mesh); CORRADE_VERIFY(!mesh->isIndexed()); - CORRADE_COMPARE(mesh->attributeCount(), 5); + /* Position + two pairs of joints & weights */ + #ifdef MAGNUM_BUILD_DEPRECATED + if(!data.compatibilitySkinningAttributes || *data.compatibilitySkinningAttributes) { + CORRADE_COMPARE(mesh->attributeCount(), 5 + 4); + } else + #endif + { + CORRADE_COMPARE(mesh->attributeCount(), 5); + } CORRADE_COMPARE(mesh->attributeFormat(MeshAttribute::Position), VertexFormat::Vector3); CORRADE_COMPARE_AS(mesh->attribute(MeshAttribute::Position), Containers::arrayView({ @@ -3775,37 +3831,85 @@ void GltfImporterTest::meshSkinAttributes() { {-2.0f, 1.0f, 0.3f} }), TestSuite::Compare::Container); - /* Custom attributes with multiple sets */ - CORRADE_COMPARE(mesh->attributeCount(jointsAttribute), 2); - CORRADE_COMPARE(mesh->attributeFormat(jointsAttribute, 0), VertexFormat::Vector4ub); - CORRADE_COMPARE_AS(mesh->attribute(jointsAttribute), + /* Attributes. All of them are currently four-component so casting to a + Vector4 for easier comparison */ + /** @todo implement multi-dimensional support in Compare::Container instead + and drop the workaround */ + CORRADE_COMPARE(mesh->attributeCount(MeshAttribute::JointIds), 2); + CORRADE_COMPARE(mesh->attributeFormat(MeshAttribute::JointIds, 0), VertexFormat::UnsignedByte); + CORRADE_COMPARE(mesh->attributeArraySize(MeshAttribute::JointIds, 0), 4); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector4ub>(mesh->attribute(MeshAttribute::JointIds, 0))), Containers::arrayView({ {1, 2, 3, 4}, {5, 6, 7, 8}, {9, 10, 11, 12} }), TestSuite::Compare::Container); - CORRADE_COMPARE(mesh->attributeFormat(jointsAttribute, 1), VertexFormat::Vector4us); - CORRADE_COMPARE_AS(mesh->attribute(jointsAttribute, 1), + CORRADE_COMPARE(mesh->attributeFormat(MeshAttribute::JointIds, 1), VertexFormat::UnsignedShort); + CORRADE_COMPARE(mesh->attributeArraySize(MeshAttribute::JointIds, 1), 4); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector4us>(mesh->attribute(MeshAttribute::JointIds, 1))), Containers::arrayView({ {13, 14, 15, 16}, {17, 18, 19, 20}, {21, 22, 23, 24} }), TestSuite::Compare::Container); - CORRADE_COMPARE(mesh->attributeCount(weightsAttribute), 2); - CORRADE_COMPARE(mesh->attributeFormat(weightsAttribute, 0), VertexFormat::Vector4); - CORRADE_COMPARE_AS(mesh->attribute(weightsAttribute), + CORRADE_COMPARE(mesh->attributeCount(MeshAttribute::Weights), 2); + CORRADE_COMPARE(mesh->attributeFormat(MeshAttribute::Weights, 0), VertexFormat::Float); + CORRADE_COMPARE(mesh->attributeArraySize(MeshAttribute::Weights, 0), 4); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector4>(mesh->attribute(MeshAttribute::Weights, 0))), Containers::arrayView({ {0.125f, 0.25f, 0.375f, 0.0f}, {0.1f, 0.05f, 0.05f, 0.05f}, {0.2f, 0.0f, 0.3f, 0.0f} }), TestSuite::Compare::Container); - CORRADE_COMPARE(mesh->attributeFormat(weightsAttribute, 1), VertexFormat::Vector4usNormalized); - CORRADE_COMPARE_AS(mesh->attribute(weightsAttribute, 1), + CORRADE_COMPARE(mesh->attributeFormat(MeshAttribute::Weights, 1), VertexFormat::UnsignedShortNormalized); + CORRADE_COMPARE(mesh->attributeArraySize(MeshAttribute::Weights, 1), 4); + CORRADE_COMPARE_AS((Containers::arrayCast<1, const Vector4us>(mesh->attribute(MeshAttribute::Weights, 1))), Containers::arrayView({ { 0, 0xffff/8, 0, 0xffff/8}, {0xffff/2, 0xffff/8, 0xffff/16, 0xffff/16}, { 0, 0xffff/4, 0xffff/4, 0} }), TestSuite::Compare::Container); + + /* Backwards compatibility custom attributes */ + #ifdef MAGNUM_BUILD_DEPRECATED + if(!data.compatibilitySkinningAttributes || *data.compatibilitySkinningAttributes) { + CORRADE_COMPARE(mesh->attributeCount(jointsAttribute), 2); + CORRADE_COMPARE(mesh->attributeFormat(jointsAttribute, 0), VertexFormat::Vector4ub); + CORRADE_COMPARE_AS(mesh->attribute(jointsAttribute), + Containers::arrayView({ + {1, 2, 3, 4}, + {5, 6, 7, 8}, + {9, 10, 11, 12} + }), TestSuite::Compare::Container); + CORRADE_COMPARE(mesh->attributeFormat(jointsAttribute, 1), VertexFormat::Vector4us); + CORRADE_COMPARE_AS(mesh->attribute(jointsAttribute, 1), + Containers::arrayView({ + {13, 14, 15, 16}, + {17, 18, 19, 20}, + {21, 22, 23, 24} + }), TestSuite::Compare::Container); + CORRADE_COMPARE(mesh->attributeCount(weightsAttribute), 2); + CORRADE_COMPARE(mesh->attributeFormat(weightsAttribute, 0), VertexFormat::Vector4); + CORRADE_COMPARE_AS(mesh->attribute(weightsAttribute), + Containers::arrayView({ + {0.125f, 0.25f, 0.375f, 0.0f}, + {0.1f, 0.05f, 0.05f, 0.05f}, + {0.2f, 0.0f, 0.3f, 0.0f} + }), TestSuite::Compare::Container); + CORRADE_COMPARE(mesh->attributeFormat(weightsAttribute, 1), VertexFormat::Vector4usNormalized); + CORRADE_COMPARE_AS(mesh->attribute(weightsAttribute, 1), + Containers::arrayView({ + { 0, 0xffff/8, 0, 0xffff/8}, + {0xffff/2, 0xffff/8, 0xffff/16, 0xffff/16}, + { 0, 0xffff/4, 0xffff/4, 0} + }), TestSuite::Compare::Container); + + /* The compat attributes should alias the builtin ones, not have the + data duplicated */ + CORRADE_COMPARE(mesh->attributeOffset(jointsAttribute), mesh->attributeOffset(MeshAttribute::JointIds)); + CORRADE_COMPARE(mesh->attributeOffset(weightsAttribute), mesh->attributeOffset(MeshAttribute::Weights)); + } + #endif } void GltfImporterTest::meshCustomAttributes() { @@ -3822,25 +3926,34 @@ void GltfImporterTest::meshCustomAttributes() { "Trade::GltfImporter::openData(): unknown attribute NOT_AN_IDENTITY, importing as custom attribute\n"); } + /* On a deprecated build the first two attributes are hardcoded JOINTS and + WEIGHTS */ + const UnsignedInt customAttributeOffset = + #ifdef MAGNUM_BUILD_DEPRECATED + 2 + #else + 0 + #endif + ; + /* The mapping should be available even before the mesh is imported. - Attributes are sorted in declaration order; the first two attributes are - hardcoded JOINTS and WEIGHTS. */ + Attributes are sorted in declaration order. */ const MeshAttribute tbnAttribute = importer->meshAttributeForName("_TBN"); - CORRADE_COMPARE(tbnAttribute, meshAttributeCustom(2)); + CORRADE_COMPARE(tbnAttribute, meshAttributeCustom(customAttributeOffset + 0)); CORRADE_COMPARE(importer->meshAttributeName(tbnAttribute), "_TBN"); CORRADE_COMPARE(importer->meshAttributeForName("Nonexistent"), MeshAttribute{}); const MeshAttribute uvRotation = importer->meshAttributeForName("_UV_ROTATION"); - CORRADE_COMPARE(uvRotation, meshAttributeCustom(3)); + CORRADE_COMPARE(uvRotation, meshAttributeCustom(customAttributeOffset + 1)); CORRADE_COMPARE(importer->meshAttributeName(uvRotation), "_UV_ROTATION"); const MeshAttribute tbnPreciserAttribute = importer->meshAttributeForName("_TBN_PRECISER"); const MeshAttribute objectIdAttribute = importer->meshAttributeForName("OBJECT_ID3"); const MeshAttribute doubleShotAttribute = importer->meshAttributeForName("_DOUBLE_SHOT"); - CORRADE_COMPARE(doubleShotAttribute, meshAttributeCustom(8)); + CORRADE_COMPARE(doubleShotAttribute, meshAttributeCustom(customAttributeOffset + 6)); const MeshAttribute negativePaddingAttribute = importer->meshAttributeForName("_NEGATIVE_PADDING"); - CORRADE_COMPARE(negativePaddingAttribute, meshAttributeCustom(6)); + CORRADE_COMPARE(negativePaddingAttribute, meshAttributeCustom(customAttributeOffset + 4)); const MeshAttribute notAnIdentityAttribute = importer->meshAttributeForName("NOT_AN_IDENTITY"); CORRADE_VERIFY(notAnIdentityAttribute != MeshAttribute{}); diff --git a/src/MagnumPlugins/GltfImporter/Test/mesh-invalid.gltf b/src/MagnumPlugins/GltfImporter/Test/mesh-invalid.gltf index e4594916f..78781246a 100644 --- a/src/MagnumPlugins/GltfImporter/Test/mesh-invalid.gltf +++ b/src/MagnumPlugins/GltfImporter/Test/mesh-invalid.gltf @@ -23,6 +23,18 @@ } ] }, + { + "name": "different number of WEIGHTS and JOINTS attributes", + "primitives": [ + { + "attributes": { + "JOINTS_0": 40, + "WEIGHTS_0": 41, + "JOINTS_1": 40 + } + } + ] + }, { "name": "unexpected index type", "primitives": [ @@ -193,7 +205,7 @@ "primitives": [ { "attributes": { - "NORMAL": 40 + "NORMAL": 42 } } ] @@ -203,7 +215,7 @@ "primitives": [ { "attributes": {}, - "indices": 40 + "indices": 42 } ] }, @@ -738,6 +750,20 @@ "componentType": 5126, "count": 0, "type": "VEC3" + }, + { + "name": "40", + "bufferView": 1, + "componentType": 5123, + "count": 2, + "type": "VEC4" + }, + { + "name": "41", + "bufferView": 1, + "componentType": 5126, + "count": 2, + "type": "VEC4" } ], "bufferViews": [ diff --git a/src/MagnumPlugins/GltfSceneConverter/Test/GltfSceneConverterTest.cpp b/src/MagnumPlugins/GltfSceneConverter/Test/GltfSceneConverterTest.cpp index b5665944b..a83aeabb1 100644 --- a/src/MagnumPlugins/GltfSceneConverter/Test/GltfSceneConverterTest.cpp +++ b/src/MagnumPlugins/GltfSceneConverter/Test/GltfSceneConverterTest.cpp @@ -2132,17 +2132,16 @@ void GltfSceneConverterTest::addMeshDuplicateAttribute() { CORRADE_SKIP("GltfImporter plugin not found, cannot test a roundtrip"); Containers::Pointer importer = _importerManager.instantiate("GltfImporter"); + + importer->configuration().setValue("compatibilitySkinningAttributes", false); + CORRADE_VERIFY(importer->openFile(filename)); CORRADE_COMPARE(importer->meshCount(), 1); - const MeshAttribute importedJointsAttribute = importer->meshAttributeForName("JOINTS"); - const MeshAttribute importedWeightsAttribute = importer->meshAttributeForName("WEIGHTS"); const MeshAttribute importedSecondaryPositionAttribute = importer->meshAttributeForName("_POSITION_1"); const MeshAttribute importedSecondaryObjectIdAttribute = importer->meshAttributeForName("_OBJECT_ID_1"); const MeshAttribute importedCustomAttribute = importer->meshAttributeForName("_YOLO"); const MeshAttribute importedSecondaryCustomAttribute = importer->meshAttributeForName("_YOLO_1"); - CORRADE_VERIFY(importedJointsAttribute != MeshAttribute{}); - CORRADE_VERIFY(importedWeightsAttribute != MeshAttribute{}); CORRADE_VERIFY(importedSecondaryPositionAttribute != MeshAttribute{}); CORRADE_VERIFY(importedSecondaryObjectIdAttribute != MeshAttribute{}); CORRADE_VERIFY(importedCustomAttribute != MeshAttribute{}); @@ -2154,7 +2153,7 @@ void GltfSceneConverterTest::addMeshDuplicateAttribute() { CORRADE_VERIFY(!imported->isIndexed()); CORRADE_COMPARE(imported->attributeCount(), 15); - /* CgltfImporter (stable-)sorts the attributes first to figure out the + /* GltfImporter (stable-)sorts the attributes first to figure out the numbering. Check that the numbers match by comparing types. */ CORRADE_COMPARE(imported->attributeName(0), MeshAttribute::Color); @@ -2162,10 +2161,12 @@ void GltfSceneConverterTest::addMeshDuplicateAttribute() { CORRADE_COMPARE(imported->attributeName(1), MeshAttribute::Color); CORRADE_COMPARE(imported->attributeFormat(1), VertexFormat::Vector3ubNormalized); - CORRADE_COMPARE(imported->attributeName(2), importedJointsAttribute); - CORRADE_COMPARE(imported->attributeFormat(2), VertexFormat::Vector4ub); - CORRADE_COMPARE(imported->attributeName(3), importedJointsAttribute); - CORRADE_COMPARE(imported->attributeFormat(3), VertexFormat::Vector4us); + CORRADE_COMPARE(imported->attributeName(2), MeshAttribute::JointIds); + CORRADE_COMPARE(imported->attributeFormat(2), VertexFormat::UnsignedByte); + CORRADE_COMPARE(imported->attributeArraySize(2), 4); + CORRADE_COMPARE(imported->attributeName(3), MeshAttribute::JointIds); + CORRADE_COMPARE(imported->attributeFormat(3), VertexFormat::UnsignedShort); + CORRADE_COMPARE(imported->attributeArraySize(3), 4); CORRADE_COMPARE(imported->attributeName(4), MeshAttribute::Position); CORRADE_COMPARE(imported->attributeFormat(4), VertexFormat::Vector3); @@ -2177,10 +2178,12 @@ void GltfSceneConverterTest::addMeshDuplicateAttribute() { CORRADE_COMPARE(imported->attributeName(7), MeshAttribute::TextureCoordinates); CORRADE_COMPARE(imported->attributeFormat(7), VertexFormat::Vector2ubNormalized); - CORRADE_COMPARE(imported->attributeName(8), importedWeightsAttribute); - CORRADE_COMPARE(imported->attributeFormat(8), VertexFormat::Vector4ubNormalized); - CORRADE_COMPARE(imported->attributeName(9), importedWeightsAttribute); - CORRADE_COMPARE(imported->attributeFormat(9), VertexFormat::Vector4); + CORRADE_COMPARE(imported->attributeName(8), MeshAttribute::Weights); + CORRADE_COMPARE(imported->attributeFormat(8), VertexFormat::UnsignedByteNormalized); + CORRADE_COMPARE(imported->attributeArraySize(8), 4); + CORRADE_COMPARE(imported->attributeName(9), MeshAttribute::Weights); + CORRADE_COMPARE(imported->attributeFormat(9), VertexFormat::Float); + CORRADE_COMPARE(imported->attributeArraySize(9), 4); CORRADE_COMPARE(imported->attributeName(10), MeshAttribute::ObjectId); CORRADE_COMPARE(imported->attributeFormat(10), VertexFormat::UnsignedShort);