Browse Source

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.
pull/638/head
Vladimír Vondruš 2 years ago
parent
commit
c19940cd77
  1. 10
      src/Magnum/SceneGraph/Object.h
  2. 18
      src/Magnum/SceneGraph/Object.hpp

10
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). */
};
}}

18
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 */

Loading…
Cancel
Save