From 8ef35d42bb03569d8d8880123f84b752b6c3d460 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Thu, 14 Oct 2021 15:33:07 +0200
Subject: [PATCH] Trade: the compatibility default for transformless scenes
 should be TRS.

That's what plugins implemented with the legacy API did, as it's more
flexible than a matrix.
---
 src/Magnum/Trade/AbstractImporter.cpp         | 16 +++++++++------
 .../Trade/Test/AbstractImporterTest.cpp       | 20 +++++++++++--------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp
index b3a436042..2146519d2 100644
--- a/src/Magnum/Trade/AbstractImporter.cpp
+++ b/src/Magnum/Trade/AbstractImporter.cpp
@@ -653,11 +653,13 @@ Containers::Pointer<ObjectData2D> AbstractImporter::doObject2D(const UnsignedInt
     ObjectFlags2D flags;
     Containers::Optional<Matrix3> transformation = scene.transformation2DFor(id);
     Containers::Optional<Containers::Triple<Vector2, Complex, Vector2>> trs = scene.translationRotationScaling2DFor(id);
+    /* If the object has neither a TRS nor a transformation field, assign an
+       empty TRS transform. Not a matrix, because a TRS is more flexible and
+       thus more desired. */
+    if(!transformation && !trs)
+        trs.emplace(Vector2{}, Complex{}, Vector2{1.0f});
     if(trs)
         flags |= ObjectFlag2D::HasTranslationRotationScaling;
-    /* If the object has neither a TRS nor a transformation field, assign an
-       identity transform to it */
-    else if(!transformation) transformation = Matrix3{};
 
     std::vector<UnsignedInt> children; /* not const so we can move it */
     {
@@ -825,11 +827,13 @@ Containers::Pointer<ObjectData3D> AbstractImporter::doObject3D(const UnsignedInt
     ObjectFlags3D flags;
     Containers::Optional<Matrix4> transformation = scene.transformation3DFor(id);
     Containers::Optional<Containers::Triple<Vector3, Quaternion, Vector3>> trs = scene.translationRotationScaling3DFor(id);
+    /* If the object has neither a TRS nor a transformation field, assign an
+       empty TRS transform. Not a matrix, because a TRS is more flexible and
+       thus more desired. */
+    if(!transformation && !trs)
+        trs.emplace(Vector3{}, Quaternion{}, Vector3{1.0f});
     if(trs)
         flags |= ObjectFlag3D::HasTranslationRotationScaling;
-    /* If the object has neither a TRS nor a transformation field, assign an
-       identity transform to it */
-    else if(!transformation) transformation = Matrix4{};
 
     std::vector<UnsignedInt> children; /* not const so we can move it */
     {
diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp
index ffd4aabc1..d7d44c859 100644
--- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp
+++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp
@@ -2530,13 +2530,15 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless2D() {
         std::vector<UnsignedInt>{},
         TestSuite::Compare::Container);
 
+    /* If we have neither a matrix nor a TRS, having an identity TRS is better
+       as it's more flexible compared to a matrix */
     {
         Containers::Pointer<ObjectData2D> o = importer.object2D(5);
         CORRADE_VERIFY(o);
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType2D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags2D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag2D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix3{});
         CORRADE_COMPARE_AS(o->children(),
             (std::vector<UnsignedInt>{2, 3}),
@@ -2547,7 +2549,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless2D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType2D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags2D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag2D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix3{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},
@@ -2558,7 +2560,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless2D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType2D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags2D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag2D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix3{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},
@@ -2569,7 +2571,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless2D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType2D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags2D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag2D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix3{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},
@@ -2634,13 +2636,15 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless3D() {
         (std::vector<UnsignedInt>{5, 1}),
         TestSuite::Compare::Container);
 
+    /* If we have neither a matrix nor a TRS, having an identity TRS is better
+       as it's more flexible compared to a matrix */
     {
         Containers::Pointer<ObjectData3D> o = importer.object3D(5);
         CORRADE_VERIFY(o);
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType3D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags3D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag3D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix4{});
         CORRADE_COMPARE_AS(o->children(),
             (std::vector<UnsignedInt>{2, 3}),
@@ -2651,7 +2655,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless3D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType3D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags3D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag3D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix4{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},
@@ -2662,7 +2666,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless3D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType3D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags3D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag3D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix4{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},
@@ -2673,7 +2677,7 @@ void AbstractImporterTest::sceneDeprecatedFallbackTransformless3D() {
         CORRADE_COMPARE(o->importerState(), nullptr);
         CORRADE_COMPARE(o->instanceType(), ObjectInstanceType3D::Empty);
         CORRADE_COMPARE(o->instance(), -1);
-        CORRADE_COMPARE(o->flags(), ObjectFlags3D{});
+        CORRADE_COMPARE(o->flags(), ObjectFlag3D::HasTranslationRotationScaling);
         CORRADE_COMPARE(o->transformation(), Matrix4{});
         CORRADE_COMPARE_AS(o->children(),
             std::vector<UnsignedInt>{},