From 40c7232bb24807b706d1223507e27571eb4e249b Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Mon, 13 Jan 2020 16:55:40 -0800
Subject: [PATCH] revert part of a recent TransformManager change

setParent() is not supposed to reorder child/parents in the array
because it doesn't rely on them being orderer. This also has the
advantage of not invalidating Instances.

However, commitLocalTransformTransaction() does rely on child/parent
ordering and the previous change did fix that.
---
 filament/src/components/TransformManager.cpp | 16 ++++------------
 filament/test/filament_test.cpp              |  6 +++---
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/filament/src/components/TransformManager.cpp b/filament/src/components/TransformManager.cpp
index 8ee9381810..0ed6d741db 100644
--- a/filament/src/components/TransformManager.cpp
+++ b/filament/src/components/TransformManager.cpp
@@ -65,18 +65,10 @@ void FTransformManager::setParent(Instance i, Instance parent) noexcept {
             removeNode(i);
             insertNode(i, parent);
             updateNodeTransform(i);
-            // Ensure that instances are still in order
-            if (!mLocalTransformTransactionOpen && i < parent) {
-                Instance end = manager.end();
-                while (i != end) {
-                    Instance iParent = Instance(manager[i].parent);
-                    if (UTILS_UNLIKELY(iParent > i)) {
-                        swapNode(i, iParent);
-                    } else {
-                        ++i;
-                    }
-                }
-            }
+            // Note: setParent() doesn't reorder the child after the parent in the array,
+            // but that's not a problem because TransformManager doesn't rely on that.
+            // Also note that commitLocalTransformTransaction() does reorder all children after
+            // their parent, as an optimization to calculate the world transform.
         }
     }
 }
diff --git a/filament/test/filament_test.cpp b/filament/test/filament_test.cpp
index 3f62fbc572..648ebe5300 100644
--- a/filament/test/filament_test.cpp
+++ b/filament/test/filament_test.cpp
@@ -235,13 +235,13 @@ TEST(FilamentTest, TransformManager) {
     tcm.create(entities[2]);
     EXPECT_TRUE(tcm.hasComponent(entities[2]));
     TransformManager::Instance newParent = tcm.getInstance(entities[2]);
-    EXPECT_TRUE(bool(newParent));
+    ASSERT_LT(child, newParent);
 
     // test reparenting
     tcm.setParent(child, newParent);
 
-    // make sure child/parent are out of order
-    ASSERT_LT(child, newParent);
+    // make sure child/parent are out of order (i.e.: setParent() doesn't invalidate instances)
+    EXPECT_LT(tcm.getInstance(entities[1]), tcm.getInstance(entities[2]));
 
     // local transaction reorders parent/child
     tcm.openLocalTransformTransaction();