From a894c434b25cecb634056e1ec7b6a85de0e54a2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 12 May 2012 13:32:08 +0200 Subject: [PATCH] Revert "Caching light position relative to camera." * The light didn't catch camera transformation changes, so it was returning wrong position for most of the time. * The multiplication was in wrong order, it should be multiplied with camera matrix from the left. I need to find an solution for this, because now it is one redundant matrix*vector multiplication per object per frame again. This reverts commit 0443bbe28647a2a749bd8b3d445e9e59e9d8b882. Conflicts: src/Light.cpp src/Test/LightTest.cpp --- src/Light.cpp | 15 +--------- src/Light.h | 15 +++++----- src/Test/CMakeLists.txt | 1 - src/Test/LightTest.cpp | 63 ----------------------------------------- src/Test/LightTest.h | 32 --------------------- 5 files changed, 8 insertions(+), 118 deletions(-) delete mode 100644 src/Test/LightTest.cpp delete mode 100644 src/Test/LightTest.h diff --git a/src/Light.cpp b/src/Light.cpp index c1ade3c20..9c12bfff2 100644 --- a/src/Light.cpp +++ b/src/Light.cpp @@ -14,26 +14,13 @@ */ #include "Light.h" -#include "Camera.h" namespace Magnum { -Vector3 Light::position(Camera* camera) { - CORRADE_ASSERT(scene() && camera->scene() == scene(), "Light: camera and light aren't in the same scene!", Vector3()) - - if(camera != _camera) { - _camera = camera; - setDirty(); - } - - setClean(); - return _position; -} - void Light::clean(const Matrix4& absoluteTransformation) { Object::clean(absoluteTransformation); - _position = (absoluteTransformation*_camera->cameraMatrix())[3].xyz(); + _position = absoluteTransformation[3]; } } diff --git a/src/Light.h b/src/Light.h index efedb0aba..7caf4370e 100644 --- a/src/Light.h +++ b/src/Light.h @@ -34,15 +34,15 @@ class Light: public Object { * @brief Constructor * @param parent Parent object */ - inline Light(Object* parent = nullptr): Object(parent), _camera(nullptr) {} + inline Light(Object* parent = nullptr): Object(parent) {} /** - * @brief Light position relative to given camera - * - * The position is cached until the camera is changed to another or - * the light dirty bit is set. + * @brief Light position relative to root object (scene) */ - Vector3 position(Camera* camera); + inline Vector4 position() { + setClean(); + return _position; + } protected: /** @@ -51,8 +51,7 @@ class Light: public Object { void clean(const Matrix4& absoluteTransformation); private: - Camera* _camera; - Vector3 _position; + Vector4 _position; }; } diff --git a/src/Test/CMakeLists.txt b/src/Test/CMakeLists.txt index 639eb4c61..b97a6786b 100644 --- a/src/Test/CMakeLists.txt +++ b/src/Test/CMakeLists.txt @@ -1,4 +1,3 @@ corrade_add_test(ObjectTest ObjectTest.h ObjectTest.cpp MagnumTestLib) corrade_add_test(CameraTest CameraTest.h CameraTest.cpp Magnum) -corrade_add_test(LightTest LightTest.h LightTest.cpp MagnumTestLib) corrade_add_test(SceneTest SceneTest.h SceneTest.cpp Magnum) diff --git a/src/Test/LightTest.cpp b/src/Test/LightTest.cpp deleted file mode 100644 index ac7dc3a1e..000000000 --- a/src/Test/LightTest.cpp +++ /dev/null @@ -1,63 +0,0 @@ -/* - Copyright © 2010, 2011, 2012 Vladimír Vondruš - - This file is part of Magnum. - - Magnum is free software: you can redistribute it and/or modify - it under the terms of the GNU Lesser General Public License version 3 - only, as published by the Free Software Foundation. - - Magnum is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU Lesser General Public License version 3 for more details. -*/ - -#include "LightTest.h" - -#include -#include - -#include "Camera.h" -#include "Light.h" -#include "Scene.h" - -QTEST_APPLESS_MAIN(Magnum::Test::LightTest) - -using namespace std; -using namespace Corrade::Utility; - -namespace Magnum { namespace Test { - -void LightTest::positionWrongCamera() { - stringstream ss; - Error::setOutput(&ss); - - Camera c; - Light l; - QVERIFY(l.position(&c) == Vector3()); - QVERIFY(ss.str() == "Light: camera and light aren't in the same scene!\n"); -} - -void LightTest::position() { - Scene s; - - Object lightParent(&s); - lightParent.translate(Vector3::zAxis(3)); - Light light(&lightParent); - - Object cameraParent(&s); - cameraParent.rotate(deg(90.0f), Vector3::xAxis()); - Camera camera(&cameraParent); - - QVERIFY(light.position(&camera) == (Matrix4::translation(Vector3::zAxis(3))* - Matrix4::rotation(deg(90.0f), Vector3::xAxis()).inverted())[3].xyz()); - - /* Set another camera */ - Camera another(&cameraParent); - another.scale(Vector3(3.0f)); - QVERIFY(light.position(&another) == (Matrix4::translation(Vector3::zAxis(3))* - (Matrix4::scaling(Vector3(3.0f))*Matrix4::rotation(deg(90.0f), Vector3::xAxis())).inverted())[3].xyz()); -} - -}} diff --git a/src/Test/LightTest.h b/src/Test/LightTest.h deleted file mode 100644 index 601834df7..000000000 --- a/src/Test/LightTest.h +++ /dev/null @@ -1,32 +0,0 @@ -#ifndef Magnum_Test_LightTest_h -#define Magnum_Test_LightTest_h -/* - Copyright © 2010, 2011, 2012 Vladimír Vondruš - - This file is part of Magnum. - - Magnum is free software: you can redistribute it and/or modify - it under the terms of the GNU Lesser General Public License version 3 - only, as published by the Free Software Foundation. - - Magnum is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU Lesser General Public License version 3 for more details. -*/ - -#include - -namespace Magnum { namespace Test { - -class LightTest: public QObject { - Q_OBJECT - - private slots: - void positionWrongCamera(); - void position(); -}; - -}} - -#endif