From 75ba9226ada7fb31327aeeef3169af36a56e1853 Mon Sep 17 00:00:00 2001
From: Max Schwarz <max.schwarz@online.de>
Date: Mon, 16 Mar 2020 11:02:02 +0100
Subject: [PATCH 1/2] AssimpImporter: ignore up direction for COLLADA

Otherwise we get ugly inconsistent behavior with the PreTransformVertices
flag.
---
 .../AssimpImporter/AssimpImporter.cpp         | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp b/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp
index e3b0cb425..754915b71 100644
--- a/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp
+++ b/src/MagnumPlugins/AssimpImporter/AssimpImporter.cpp
@@ -50,6 +50,7 @@
 #include <Magnum/Trade/TextureData.h>
 #include <MagnumPlugins/AnyImageImporter/AnyImageImporter.h>
 
+#include <assimp/config.h>
 #include <assimp/postprocess.h>
 #include <assimp/Importer.hpp>
 #include <assimp/IOStream.hpp>
@@ -94,19 +95,35 @@ void fillDefaultConfiguration(Utility::ConfigurationGroup& conf) {
     postprocess.setValue("SortByPType", true);
 }
 
+/* Setup an Assimp::Importer with sane default configuration */
+Corrade::Containers::Pointer<Assimp::Importer> createImporter()
+{
+    Corrade::Containers::Pointer<Assimp::Importer> importer{new Assimp::Importer};
+
+    /* If this is set to false (default), Assimp will return a transformation
+       in the root aiNode to rotate the scene to Y-Up. With the default settings
+       this transform is ignored, since we only look at the children of the
+       root node. But if the user sets the PreTransformVertices flag, Assimp
+       will apply this transformation. Since nobody needs that, switch it
+       off. */
+    importer->SetPropertyBool(AI_CONFIG_IMPORT_COLLADA_IGNORE_UP_DIRECTION, true);
+
+    return importer;
+}
+
 }
 
-AssimpImporter::AssimpImporter(): _importer{new Assimp::Importer} {
+AssimpImporter::AssimpImporter(): _importer{createImporter()} {
     /** @todo horrible workaround, fix this properly */
     fillDefaultConfiguration(configuration());
 }
 
-AssimpImporter::AssimpImporter(PluginManager::Manager<AbstractImporter>& manager): AbstractImporter(manager), _importer{new Assimp::Importer} {
+AssimpImporter::AssimpImporter(PluginManager::Manager<AbstractImporter>& manager): AbstractImporter(manager), _importer{createImporter()} {
     /** @todo horrible workaround, fix this properly */
     fillDefaultConfiguration(configuration());
 }
 
-AssimpImporter::AssimpImporter(PluginManager::AbstractManager& manager, const std::string& plugin): AbstractImporter(manager, plugin), _importer{new Assimp::Importer}  {}
+AssimpImporter::AssimpImporter(PluginManager::AbstractManager& manager, const std::string& plugin): AbstractImporter(manager, plugin), _importer{createImporter()}  {}
 
 AssimpImporter::~AssimpImporter() = default;
 

From 7b2412828bd913db6a11bb16d4f14c61f594161c Mon Sep 17 00:00:00 2001
From: Max Schwarz <max.schwarz@online.de>
Date: Wed, 1 Apr 2020 09:34:47 +0200
Subject: [PATCH 2/2] AssimpImporter: Test: add test case for
 COLLADA_IGNORE_UP_DIRECTION

---
 .../Test/AssimpImporterTest.cpp               | 32 +++++++++++++++++++
 .../AssimpImporter/Test/line.dae              |  4 +++
 2 files changed, 36 insertions(+)

diff --git a/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp b/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp
index 38d894432..dd955edde 100644
--- a/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp
+++ b/src/MagnumPlugins/AssimpImporter/Test/AssimpImporterTest.cpp
@@ -104,6 +104,7 @@ struct AssimpImporterTest: TestSuite::Tester {
     void emptyGltf();
     void scene();
     void sceneCollapsedNode();
+    void pretransformedMesh();
 
     void imageEmbedded();
     void imageExternal();
@@ -170,6 +171,7 @@ AssimpImporterTest::AssimpImporterTest() {
               &AssimpImporterTest::emptyGltf,
               &AssimpImporterTest::scene,
               &AssimpImporterTest::sceneCollapsedNode,
+              &AssimpImporterTest::pretransformedMesh,
 
               &AssimpImporterTest::imageEmbedded,
               &AssimpImporterTest::imageExternal,
@@ -714,6 +716,36 @@ void AssimpImporterTest::sceneCollapsedNode() {
     }
 }
 
+void AssimpImporterTest::pretransformedMesh() {
+    Containers::Pointer<AbstractImporter> importer = _manager.instantiate("AssimpImporter");
+
+    importer->configuration().group("postprocess")->setValue("PreTransformVertices", true);
+
+    CORRADE_VERIFY(importer->openFile(Utility::Directory::join(ASSIMPIMPORTER_TEST_DIR, "line.dae")));
+
+    CORRADE_COMPARE(importer->meshCount(), 1);
+    CORRADE_COMPARE(importer->object3DCount(), 1);
+
+    Containers::Optional<Trade::MeshData> mesh = importer->mesh(0);
+    CORRADE_VERIFY(mesh);
+    CORRADE_COMPARE(mesh->primitive(), MeshPrimitive::Lines);
+
+    CORRADE_VERIFY(mesh->isIndexed());
+    CORRADE_COMPARE_AS(mesh->indices<UnsignedInt>(),
+        Containers::arrayView<UnsignedInt>({0, 1}),
+        TestSuite::Compare::Container);
+
+    CORRADE_COMPARE(mesh->attributeCount(), 1);
+    CORRADE_COMPARE_AS(mesh->attribute<Vector3>(MeshAttribute::Position),
+        Containers::arrayView<Vector3>({
+            {-1.0f, 1.0f, 1.0f}, {-1.0f, -1.0f, 1.0f}
+        }), TestSuite::Compare::Container);
+
+    Containers::Pointer<Trade::ObjectData3D> meshObject = importer->object3D(0);
+    CORRADE_COMPARE(meshObject->instanceType(), ObjectInstanceType3D::Mesh);
+    CORRADE_COMPARE(meshObject->instance(), 0);
+}
+
 void AssimpImporterTest::imageEmbedded() {
     if(_manager.loadState("PngImporter") == PluginManager::LoadState::NotFound)
         CORRADE_SKIP("PngImporter plugin not found, cannot test");
diff --git a/src/MagnumPlugins/AssimpImporter/Test/line.dae b/src/MagnumPlugins/AssimpImporter/Test/line.dae
index 138c68847..2fccb98d3 100644
--- a/src/MagnumPlugins/AssimpImporter/Test/line.dae
+++ b/src/MagnumPlugins/AssimpImporter/Test/line.dae
@@ -1,5 +1,9 @@
 <?xml version="1.0" encoding="utf-8"?>
 <COLLADA xmlns="http://www.collada.org/2005/11/COLLADASchema" version="1.4.1">
+  <asset>
+    <unit name="meter" meter="1"/>
+    <up_axis>Z_UP</up_axis>
+  </asset>
   <library_geometries>
     <geometry id="Cube-mesh" name="Cube">
       <mesh>