From c19940cd771c9bdbd739ce90a2f53ead350e9825 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= <mosra@centrum.cz>
Date: Mon, 1 Apr 2024 11:47:03 +0200
Subject: [PATCH] SceneGraph: don't limit transformation calculation to 65k
 objects.

This was due to the internal counter in the object being just 2 bytes,
which was made in order to optimize the Object class size. In practice
however, not counting the vtables, the class has SEVEN pointers and then
a transformation part that's in most cases multiple of 8. Which means
that on 64 bit platforms there was always 5 bytes of padding next to
this 2-byte counter and 1-byte flags, thus no reason to have it 2-byte
anyway.

On 32-bit platforms this *might* cause the Object to get bigger, yes,
but as it's heap-allocated it's more likely that it's put on a
16-byte-aligned address and thus occupying a multiple of 16 bytes
anyway. In any case, this won't really hurt, because the SceneGraph
situation is bad enough already due to all the pointer chasings and
overhead from loose allocations.
---
 src/Magnum/SceneGraph/Object.h   | 10 +++++++++-
 src/Magnum/SceneGraph/Object.hpp | 18 +++++++++---------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/src/Magnum/SceneGraph/Object.h b/src/Magnum/SceneGraph/Object.h
index 2631b2db9..deb92bb0d 100644
--- a/src/Magnum/SceneGraph/Object.h
+++ b/src/Magnum/SceneGraph/Object.h
@@ -400,8 +400,16 @@ template<class Transformation> class Object: public AbstractObject<Transformatio
 
         typedef Implementation::ObjectFlag Flag;
         typedef Implementation::ObjectFlags Flags;
-        UnsignedShort counter;
+        UnsignedInt counter;
         Flags flags;
+        /* 3 byte padding after flags. Assuming the object is pointer-aligned
+           on 64bit and the Transformation data are a multiple of 8 bytes
+           (which is the case for all except MatrixTransformation2D), we don't
+           save anything by making the counter just 16-bit, and only cause
+           unnecessary suffering for users that have large scenes. Most of the
+           memory overhead is from all those loose allocations anyway (i.e.,
+           causing the Object instance to be put in a multiple of 16 bytes
+           even, instead of 8). */
 };
 
 }}
diff --git a/src/Magnum/SceneGraph/Object.hpp b/src/Magnum/SceneGraph/Object.hpp
index 69447819c..018410e95 100644
--- a/src/Magnum/SceneGraph/Object.hpp
+++ b/src/Magnum/SceneGraph/Object.hpp
@@ -43,7 +43,7 @@ template<UnsignedInt dimensions, class T> AbstractObject<dimensions, T>::~Abstra
 
 template<UnsignedInt dimensions, class T> AbstractTransformation<dimensions, T>::AbstractTransformation() {}
 
-template<class Transformation> Object<Transformation>::Object(Object<Transformation>* parent): counter(0xFFFFu), flags(Flag::Dirty) {
+template<class Transformation> Object<Transformation>::Object(Object<Transformation>* parent): counter{~UnsignedInt{}}, flags{Flag::Dirty} {
     setParent(parent);
 }
 
@@ -203,7 +203,7 @@ computed and recursively concatenated together. Resulting transformations for
 joints which were originally in `object` list is then returned.
 */
 template<class Transformation> std::vector<typename Transformation::DataType> Object<Transformation>::transformations(std::vector<std::reference_wrapper<Object<Transformation>>> objects, const typename Transformation::DataType& finalTransformation) const {
-    CORRADE_ASSERT(objects.size() < 0xFFFFu, "SceneGraph::Object::transformations(): too large scene", {});
+    CORRADE_ASSERT(objects.size() < ~UnsignedInt{}, "SceneGraph::Object::transformations(): too large scene", {});
 
     /* Remember object count for later */
     std::size_t objectCount = objects.size();
@@ -213,9 +213,9 @@ template<class Transformation> std::vector<typename Transformation::DataType> Ob
     for(std::size_t i = 0; i != objects.size(); ++i) {
         /* Multiple occurrences of one object in the array, don't overwrite it
            with different counter */
-        if(objects[i].get().counter != 0xFFFFu) continue;
+        if(objects[i].get().counter != ~UnsignedInt{}) continue;
 
-        objects[i].get().counter = UnsignedShort(i);
+        objects[i].get().counter = UnsignedInt(i);
         objects[i].get().flags |= Flag::Joint;
     }
     std::vector<std::reference_wrapper<Object<Transformation>>> jointObjects(objects);
@@ -254,10 +254,10 @@ template<class Transformation> std::vector<typename Transformation::DataType> Ob
             /* If not already marked as joint, mark it as such and add it to
                list of joint objects */
             if(!(parent->flags & Flag::Joint)) {
-                CORRADE_ASSERT(jointObjects.size() < 0xFFFFu,
+                CORRADE_ASSERT(jointObjects.size() < ~UnsignedInt{},
                                "SceneGraph::Object::transformations(): too large scene", {});
-                CORRADE_INTERNAL_ASSERT(parent->counter == 0xFFFFu);
-                parent->counter = UnsignedShort(jointObjects.size());
+                CORRADE_INTERNAL_ASSERT(parent->counter == ~UnsignedInt{});
+                parent->counter = UnsignedInt(jointObjects.size());
                 parent->flags |= Flag::Joint;
                 jointObjects.push_back(*parent);
             }
@@ -287,9 +287,9 @@ template<class Transformation> std::vector<typename Transformation::DataType> Ob
     for(auto i: jointObjects) {
         /* All not-already cleaned objects (...duplicate occurrences) should
            have joint mark */
-        CORRADE_INTERNAL_ASSERT(i.get().counter == 0xFFFFu || i.get().flags & Flag::Joint);
+        CORRADE_INTERNAL_ASSERT(i.get().counter == ~UnsignedInt{} || i.get().flags & Flag::Joint);
         i.get().flags &= ~Flag::Joint;
-        i.get().counter = 0xFFFFu;
+        i.get().counter = ~UnsignedInt{};
     }
 
     /* Shrink the array to contain only transformations of requested objects and return */