Improved Object transformation caching to reuse absolute transformation.

Object::setClean() now computes absolute transformation while traversing
through object parents and passes it as parameter to clean(), which is
now virtual a meant to be reimplemented instead of setClean().

Updated and greatly improved unit test.

src/Camera.cpp

@@ -69,10 +69,10 @@ void Camera::setViewport(const Math::Vector2<GLsizei>& size) {
     fixAspectRatio();
 }
 
-void Camera::setClean() {
+void Camera::clean(const Matrix4& absoluteTransformation) {
-    if(!isDirty()) return;
+    Object::clean(absoluteTransformation);
-    _cameraMatrix = absoluteTransformation().inverted();
+
-    Object::setClean();
+    _cameraMatrix = absoluteTransformation.inverted();
 }
 
 void Camera::fixAspectRatio() {

src/Camera.h

@@ -127,12 +127,12 @@ class MAGNUM_EXPORT Camera: public Object {
          */
         virtual void draw();
 
+    protected:
         /**
          * Recalculates camera matrix.
          */
-        void setClean();
+        void clean(const Matrix4& absoluteTransformation);
 
-    protected:
         /**
          * @brief Draw object children
          *

src/Light.cpp

@@ -30,10 +30,10 @@ Vector3 Light::position(Camera* camera) {
     return _position;
 }
 
-void Light::setClean() {
+void Light::clean(const Matrix4& absoluteTransformation) {
-    if(!isDirty()) return;
+    Object::clean(absoluteTransformation);
-    _position = (absoluteTransformation()*_camera->cameraMatrix())[3].xyz();
+
-    Object::setClean();
+    _position = (absoluteTransformation*_camera->cameraMatrix())[3].xyz();
 }
 
 }

src/Light.h

@@ -44,10 +44,11 @@ class Light: public Object {
          */
         Vector3 position(Camera* camera);
 
+    protected:
         /**
          * Recomputes light position.
          */
-        void setClean();
+        void clean(const Matrix4& absoluteTransformation);
 
     private:
         Camera* _camera;

src/Object.cpp

@@ -14,6 +14,8 @@
 */
 
 #include "Object.h"
+
+#include <stack>
 #include "Scene.h"
 #include "Camera.h"
 
@@ -118,13 +120,33 @@ void Object::setDirty() {
 }
 
 void Object::setClean() {
-    /* The object (and all its parents) is already clean, nothing to do */
+    /* The object (and all its parents) are already clean, nothing to do */
     if(!dirty) return;
 
-    dirty = false;
+    /* Collect all parents */
+    stack<Object*> objects;
+    Object* p = this;
+    for(;;) {
+        objects.push(p);
+
+        /* Stop on root object / scene / clean object */
+        if(p->parent() == nullptr || p->parent() == p || !p->parent()->isDirty())
+            break;
 
-    /* Make all parents clean */
+        p = p->parent();
-    if(_parent != nullptr && _parent != this) _parent->setClean();
+    }
+
+    /* Call setClean(const Matrix4&) for every parent and also this object */
+    Object* o = objects.top();
+    objects.pop();
+    Matrix4 absoluteTransformation = o->absoluteTransformation();
+    o->clean(absoluteTransformation);
+    while(!objects.empty()) {
+        o = objects.top();
+        objects.pop();
+        absoluteTransformation = absoluteTransformation*o->transformation();
+        o->clean(absoluteTransformation);
+    }
 }
 
 }

src/Object.h

@@ -159,25 +159,37 @@ class MAGNUM_EXPORT Object {
 
         /*@}*/
 
+        /**
+         * @brief Draw object
+         * @param transformationMatrix      %Matrix specifying object
+         *      transformation relative to the scene.
+         * @param camera                    Active camera (containing
+         *      projection matrix)
+         *
+         * Default implementation does nothing.
+         */
+        virtual void draw(const Matrix4& transformationMatrix, Camera* camera) {}
+
         /** @{ @name Caching helpers
          *
          * If the object (absolute) transformation or anything depending on it
          * is used many times when drawing (such as e.g. position of light
          * object), it's good to cache these values, so they don't have to be
-         * computed again on every request.
+         * recalculated again on every request.
+         *
+         * If setDirty() is called on an object (or the object is transformed),
+         * it and all its children are marked as dirty. If any object is
+         * already dirty, it and all its children are skipped, because they
+         * are already dirty too.
          *
-         * If the object or any parent is transformed, the transformed object
+         * If setClean() is called on an object, it and all its parents are
-         * and all its children are marked as dirty. If currently active camera
+         * cleaned. If any object is already clean, it and all its parents are
-         * is transformed, the scene goes through all children and calls
+         * skipped, because they are already clean too.
-         * setDirty() recursively on every clean object (if the object is
-         * already dirty, it and all its children are skipped, because they are
-         * dirty too).
          *
          * These functions are used to manage dirty status of the object. If
-         * the object doesn't cache anything, it's no need to bother about them,
+         * the object doesn't cache anything, it's no need to bother about
-         * but if does, setClean() should be reimplemented and used to
+         * them, but if does, clean() should be reimplemented and used to
-         * regenerate the cache. Thus the dirty status is managed only for
+         * regenerate the cache.
-         * these objects, which are calling setClean().
          */
 
         /**
@@ -190,36 +202,52 @@ class MAGNUM_EXPORT Object {
         /**
          * @brief Set object and all its children as dirty
          *
-         * Recursively calls setDirty() on every child. If the object is already
+         * Recursively calls setDirty() on every child. If the object is
-         * marked as dirty, the function does nothing.
+         * already marked as dirty, the function does nothing. It is usually
-         * @attention Reimplementations must call also this function!
+         * not needed to reimplement this function, only if you for example
+         * need to reset some state on object which is not child of this. All
+         * computations should be done in setClean().
+         *
+         * Reimplementations should call this function at the end, i.e.:
+         * @code
+         * void setDirty() {
+         *     // ...
          *
-         * @see setClean()
+         *     Object::setDirty();
+         * }
+         * @endcode
          */
         virtual void setDirty();
 
         /**
          * @brief Set object and all its parents as clean
          *
-         * Recursively calls setClean() on every parent. Default implementation
+         * Recursively calls clean() on every parent which is not already
-         * only marks the object as clean, but if the object does any caching,
+         * clean.
-         * this function should be reimplemented to regenerate the cache.
-         * @attention Reimplementations must call also this function!
          */
-        virtual void setClean();
+        void setClean();
-
-        /*@}*/
 
+    protected:
         /**
-         * @brief Draw object
+         * @brief Clean the object
-         * @param transformationMatrix      %Matrix specifying object
-         *      transformation relative to the scene.
-         * @param camera                    Active camera (containing
-         *      projection matrix)
          *
-         * Default implementation does nothing.
+         * When reimplementing, use absolute transformation passed as
+         * parameter instead of absoluteTransformation(), which is not
+         * efficient. The reimplementation should call this function at the
+         * beginning, i.e.:
+         * @code
+         * void clean(const Matrix4& absoluteTransformation) {
+         *     Object::clean(absoluteTransformation);
+         *
+         *     // ...
+         * }
+         * @endcode
          */
-        virtual void draw(const Matrix4& transformationMatrix, Camera* camera) {}
+        virtual inline void clean(const Matrix4& absoluteTransformation) {
+            dirty = false;
+        }
+
+        /*@}*/
 
     private:
         Object* _parent;

src/Test/ObjectTest.cpp

@@ -130,36 +130,60 @@ void ObjectTest::scene() {
 void ObjectTest::dirty() {
     Scene scene;
 
-    Object* childOne = new Object(&scene);
+    CleaningObject* childOne = new CleaningObject(&scene);
-    Object* childTwo = new Object(childOne);
+    childOne->scale(Vector3(2.0f));
-    Object* childThree = new Object(childTwo);
+    CleaningObject* childTwo = new CleaningObject(childOne);
+    childTwo->translate(Vector3::xAxis(1.0f));
+    CleaningObject* childThree = new CleaningObject(childTwo);
+    childThree->rotate(deg(90.0f), Vector3::yAxis());
 
     /* Object is dirty at the beginning */
+    QVERIFY(scene.isDirty());
     QVERIFY(childOne->isDirty());
 
-    /* Clean the object and all its parents */
+    /* Clean the object and all its dirty parents (but not children) */
-    childThree->setClean();
+    childOne->setClean();
-    QVERIFY(!childThree->isDirty());
+    QVERIFY(childOne->cleanedAbsoluteTransformation == childOne->absoluteTransformation());
-    QVERIFY(!childTwo->isDirty());
-    QVERIFY(!childOne->isDirty());
     QVERIFY(!scene.isDirty());
+    QVERIFY(!childOne->isDirty());
+    QVERIFY(childTwo->isDirty());
+    QVERIFY(childThree->isDirty());
+
+    /* If the object itself is already clean, it shouldn't clean it again */
+    childOne->cleanedAbsoluteTransformation = Matrix4(Matrix4::Zero);
+    childOne->setClean();
+    QVERIFY(childOne->cleanedAbsoluteTransformation == Matrix4(Matrix4::Zero));
+
+    /* If any object in the hierarchy is already clean, it shouldn't clean it again */
+    childTwo->setClean();
+    QVERIFY(childOne->cleanedAbsoluteTransformation == Matrix4(Matrix4::Zero));
+    QVERIFY(childTwo->cleanedAbsoluteTransformation == childTwo->absoluteTransformation());
+    QVERIFY(!childOne->isDirty());
+    QVERIFY(!childTwo->isDirty());
+    QVERIFY(childThree->isDirty());
 
-    /* Mark object and all its children as dirty */
+    /* Mark object and all its children as dirty (but not parents) */
     childTwo->setDirty();
+    QVERIFY(!scene.isDirty());
+    QVERIFY(!childOne->isDirty());
     QVERIFY(childTwo->isDirty());
     QVERIFY(childThree->isDirty());
 
-    /* Reparent object => make it dirty */
+    /* Reparent object => make it and its children dirty (but not parents) */
     childThree->setClean();
-    childOne->setParent(nullptr);
+    QVERIFY(childThree->cleanedAbsoluteTransformation == childThree->absoluteTransformation());
-    childOne->setParent(&scene);
+    childTwo->setParent(nullptr);
-    QVERIFY(childOne->isDirty());
+    QVERIFY(childTwo->isDirty());
+    QVERIFY(!childOne->isDirty());
+    childTwo->setParent(&scene);
+    QVERIFY(!scene.isDirty());
     QVERIFY(childTwo->isDirty());
     QVERIFY(childThree->isDirty());
 
-    /* Set object transformation => make it dirty */
+    /* Set object transformation => make it and its children dirty (but not parents) */
     childThree->setClean();
-    childTwo->setTransformation({});
+    childTwo->setTransformation(Matrix4::translation(Vector3::xAxis(1.0f)));
+    QVERIFY(!scene.isDirty());
     QVERIFY(childTwo->isDirty());
     QVERIFY(childThree->isDirty());
 }

src/Test/ObjectTest.h

@@ -31,6 +31,19 @@ class ObjectTest: public QObject {
         void absoluteTransformation();
         void scene();
         void dirty();
+
+    private:
+        class CleaningObject: public Object {
+            public:
+                CleaningObject(Object* parent = nullptr): Object(parent) {}
+
+                inline void clean(const Matrix4& absoluteTransformation) {
+                    Object::clean(absoluteTransformation);
+
+                    cleanedAbsoluteTransformation = absoluteTransformation;
+                }
+                Matrix4 cleanedAbsoluteTransformation;
+        };
 };
 
 }}