From d65d33912a16b4ff5531e65767e7f663a13c6c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 24 Apr 2012 21:44:45 +0200 Subject: [PATCH] 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 | 8 ++-- src/Camera.h | 4 +- src/Light.cpp | 8 ++-- src/Light.h | 3 +- src/Object.cpp | 30 ++++++++++++-- src/Object.h | 86 +++++++++++++++++++++++++++-------------- src/Test/ObjectTest.cpp | 54 +++++++++++++++++++------- src/Test/ObjectTest.h | 13 +++++++ 8 files changed, 147 insertions(+), 59 deletions(-) diff --git a/src/Camera.cpp b/src/Camera.cpp index 186556296..199d2cfc8 100644 --- a/src/Camera.cpp +++ b/src/Camera.cpp @@ -69,10 +69,10 @@ void Camera::setViewport(const Math::Vector2& size) { fixAspectRatio(); } -void Camera::setClean() { - if(!isDirty()) return; - _cameraMatrix = absoluteTransformation().inverted(); - Object::setClean(); +void Camera::clean(const Matrix4& absoluteTransformation) { + Object::clean(absoluteTransformation); + + _cameraMatrix = absoluteTransformation.inverted(); } void Camera::fixAspectRatio() { diff --git a/src/Camera.h b/src/Camera.h index aaa950c68..4a9452ac2 100644 --- a/src/Camera.h +++ b/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 * diff --git a/src/Light.cpp b/src/Light.cpp index 9ba5eeecf..c1ade3c20 100644 --- a/src/Light.cpp +++ b/src/Light.cpp @@ -30,10 +30,10 @@ Vector3 Light::position(Camera* camera) { return _position; } -void Light::setClean() { - if(!isDirty()) return; - _position = (absoluteTransformation()*_camera->cameraMatrix())[3].xyz(); - Object::setClean(); +void Light::clean(const Matrix4& absoluteTransformation) { + Object::clean(absoluteTransformation); + + _position = (absoluteTransformation*_camera->cameraMatrix())[3].xyz(); } } diff --git a/src/Light.h b/src/Light.h index f04e70385..efedb0aba 100644 --- a/src/Light.h +++ b/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; diff --git a/src/Object.cpp b/src/Object.cpp index 85f312894..861b03346 100644 --- a/src/Object.cpp +++ b/src/Object.cpp @@ -14,6 +14,8 @@ */ #include "Object.h" + +#include #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 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 */ - if(_parent != nullptr && _parent != this) _parent->setClean(); + p = p->parent(); + } + + /* 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); + } } } diff --git a/src/Object.h b/src/Object.h index 4fceb7f2b..1a3926ceb 100644 --- a/src/Object.h +++ b/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 - * and all its children are marked as dirty. If currently active camera - * is transformed, the scene goes through all children and calls - * setDirty() recursively on every clean object (if the object is - * already dirty, it and all its children are skipped, because they are - * dirty too). + * If setClean() is called on an object, it and all its parents are + * cleaned. If any object is already clean, it and all its parents are + * skipped, because they are already clean 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, - * but if does, setClean() should be reimplemented and used to - * regenerate the cache. Thus the dirty status is managed only for - * these objects, which are calling setClean(). + * the object doesn't cache anything, it's no need to bother about + * them, but if does, clean() should be reimplemented and used to + * regenerate the cache. */ /** @@ -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 - * marked as dirty, the function does nothing. - * @attention Reimplementations must call also this function! + * Recursively calls setDirty() on every child. If the object is + * already marked as dirty, the function does nothing. It is usually + * 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 - * only marks the object as clean, but if the object does any caching, - * this function should be reimplemented to regenerate the cache. - * @attention Reimplementations must call also this function! + * Recursively calls clean() on every parent which is not already + * clean. */ - virtual void setClean(); - - /*@}*/ + void setClean(); + protected: /** - * @brief Draw object - * @param transformationMatrix %Matrix specifying object - * transformation relative to the scene. - * @param camera Active camera (containing - * projection matrix) + * @brief Clean the object * - * 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; diff --git a/src/Test/ObjectTest.cpp b/src/Test/ObjectTest.cpp index 979923f5b..880d70e9d 100644 --- a/src/Test/ObjectTest.cpp +++ b/src/Test/ObjectTest.cpp @@ -130,36 +130,60 @@ void ObjectTest::scene() { void ObjectTest::dirty() { Scene scene; - Object* childOne = new Object(&scene); - Object* childTwo = new Object(childOne); - Object* childThree = new Object(childTwo); + CleaningObject* childOne = new CleaningObject(&scene); + childOne->scale(Vector3(2.0f)); + 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 */ - childThree->setClean(); - QVERIFY(!childThree->isDirty()); - QVERIFY(!childTwo->isDirty()); - QVERIFY(!childOne->isDirty()); + /* Clean the object and all its dirty parents (but not children) */ + childOne->setClean(); + QVERIFY(childOne->cleanedAbsoluteTransformation == childOne->absoluteTransformation()); 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); - childOne->setParent(&scene); - QVERIFY(childOne->isDirty()); + QVERIFY(childThree->cleanedAbsoluteTransformation == childThree->absoluteTransformation()); + childTwo->setParent(nullptr); + 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()); } diff --git a/src/Test/ObjectTest.h b/src/Test/ObjectTest.h index ef7a629e8..4071c6791 100644 --- a/src/Test/ObjectTest.h +++ b/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; + }; }; }}