From 74f190ef7e2ab4a43fb4912a3c80c0f1cf25a0b6 Mon Sep 17 00:00:00 2001 From: Kalle Bladin Date: Fri, 20 May 2016 11:46:36 -0400 Subject: [PATCH] More clean up in camera class. --- include/openspace/util/camera.h | 87 +++++++++++++++++++---------- src/util/camera.cpp | 98 ++++++++++++++++++--------------- 2 files changed, 113 insertions(+), 72 deletions(-) diff --git a/include/openspace/util/camera.h b/include/openspace/util/camera.h index 8964ba72c9..34e05e5db6 100644 --- a/include/openspace/util/camera.h +++ b/include/openspace/util/camera.h @@ -39,16 +39,37 @@ #include namespace openspace { - - template - struct CachedDatum - { - CachedDatum() { isDirty = true; } - T datum; - bool isDirty; - }; - + /** + This class still needs some more love. Suggested improvements: + - Remove psc from the camera class interface. + - Accessors should return constant references to double precision class members. + - Remove the scaling variable (What is it used for?) + - Remove the maxFov and sinMaxfov variables. Redundant since the fov is embedded + within the perspective projection matrix. + - Remove focusposition, part of the integration with the scale graph. The + "focus position" should not be needed since we assume the camera is always + positioned relative to its origin. When orbiting another object (not in origin), + the focus position should probably be handled outside the camera class + (interaction handler) since it does not affect the state of the camera + (only how it interacts). + - The class might need some more reasonable accessors depending on use cases. + (up vector world space?) + - Make clear which function returns a combined view matrix (things that are + dependent on the separate sgct nodes). + */ class Camera { + /** + Used to explicitly show which variables within the Camera class that are used + for caching. + */ + template + struct CachedDatum + { + CachedDatum() { isDirty = true; } + T datum; + bool isDirty; + }; + // For testing double vs float precision typedef glm::dquat Quat; typedef glm::dmat4 Mat4; @@ -63,8 +84,8 @@ namespace openspace { ~Camera(); // Mutators - void setPosition(psc pos); - void setFocusPosition(psc pos); + void setPositionVec3(Vec3 pos); + void setFocusPositionVec3(Vec3 pos); void setRotation(Quat rotation); void setScaling(glm::vec2 scaling); void setMaxFov(float fov); @@ -73,9 +94,11 @@ namespace openspace { void rotate(Quat rotation); // Accessors - psc position() const; - psc unsynchedPosition() const; - psc focusPosition() const; + // Remove Vec3 from the name when psc is gone + const Vec3& positionVec3() const; + const Vec3& unsynchedPositionVec3() const; + const Vec3& focusPositionVec3() const; + // Should return const refs glm::vec3 viewDirectionWorldSpace() const; glm::vec3 lookUpVectorCameraSpace() const; const glm::vec2& scaling() const; @@ -84,21 +107,11 @@ namespace openspace { float maxFov() const; float sinMaxFov() const; - //@TODO this should simply be called viewMatrix! - //Rename after removing deprecated methods + // @TODO this should simply be called viewMatrix! + // Or it needs to be changed so that it actually is combined. Right now it is + // only the view matrix that is the same for all SGCT cameras. glm::mat4 combinedViewMatrix() const; - // DEPRECATED ACCESSORS (GETTERS) - // @TODO use Camera::SgctInternal interface instead - [[deprecated("Replaced by Camera::SgctInternal::viewMatrix()")]] - const glm::mat4& viewMatrix() const; - - [[deprecated("Replaced by Camera::SgctInternal::projectionMatrix()")]] - const glm::mat4& projectionMatrix() const; - - [[deprecated("Replaced by Camera::SgctInternal::viewProjectionMatrix()")]] - const glm::mat4& viewProjectionMatrix() const; - // Synchronization void postSynchronizationPreDraw(); void preSynchronization(); @@ -135,6 +148,25 @@ namespace openspace { mutable CachedDatum _cachedViewProjectionMatrix; mutable std::mutex _mutex; } sgctInternal; + + // Deprecated + [[deprecated("Replaced by Camera::setPositionVec3()")]] + void setPosition(psc pos); + [[deprecated("Replaced by Camera::setFocusPositionVec3()")]] + void setFocusPosition(psc pos); + [[deprecated("Replaced by Camera::positionVec3()")]] + psc position() const; + [[deprecated("Replaced by Camera::unsynchedPositionVec3()")]] + psc unsynchedPosition() const; + [[deprecated("Replaced by Camera::focusPositionVec3()")]] + psc focusPosition() const; + // @TODO use Camera::SgctInternal interface instead + [[deprecated("Replaced by Camera::SgctInternal::viewMatrix()")]] + const glm::mat4& viewMatrix() const; + [[deprecated("Replaced by Camera::SgctInternal::projectionMatrix()")]] + const glm::mat4& projectionMatrix() const; + [[deprecated("Replaced by Camera::SgctInternal::viewProjectionMatrix()")]] + const glm::mat4& viewProjectionMatrix() const; private: /** Class encapsulating data that needs to be synched between SGCT nodes. @@ -162,7 +194,6 @@ namespace openspace { SyncData _position; Vec3 _focusPosition; - float _maxFov; // Cached data diff --git a/src/util/camera.cpp b/src/util/camera.cpp index 9833760068..afd9c5a3df 100644 --- a/src/util/camera.cpp +++ b/src/util/camera.cpp @@ -31,6 +31,10 @@ namespace openspace { + ////////////////////////////////////////////////////////////////////////////////////// + // CAMERA // + ////////////////////////////////////////////////////////////////////////////////////// + const Camera::Vec3 Camera::_VIEW_DIRECTION_CAMERA_SPACE = Camera::Vec3(0, 0, -1); const Camera::Vec3 Camera::_LOOKUP_VECTOR_CAMERA_SPACE = Camera::Vec3(0, 1, 0); @@ -56,18 +60,15 @@ namespace openspace { Camera::~Camera() { } - ////////////////////////////////////////////////////////////////////////////////////// - // CAMERA MUTATORS (SETTERS) // - ////////////////////////////////////////////////////////////////////////////////////// - - void Camera::setPosition(psc pos) { + // Mutators + void Camera::setPositionVec3(Vec3 pos) { std::lock_guard _lock(_mutex); - _position.local = pos.dvec3(); + _position.local = pos; } - void Camera::setFocusPosition(psc pos) { + void Camera::setFocusPositionVec3(Vec3 pos) { std::lock_guard _lock(_mutex); - _focusPosition = pos.dvec3(); + _focusPosition = pos; } void Camera::setRotation(Quat rotation) { @@ -88,29 +89,23 @@ namespace openspace { _cachedSinMaxFov.isDirty = true; } - ////////////////////////////////////////////////////////////////////////////////////// - // CAMERA RELATICVE MUTATORS // - ////////////////////////////////////////////////////////////////////////////////////// - + // Relative mutators void Camera::rotate(Quat rotation) { std::lock_guard _lock(_mutex); _rotation.local = _rotation.local * rotation; } - ////////////////////////////////////////////////////////////////////////////////////// - // CAMERA ACCESSORS (GETTERS) // - ////////////////////////////////////////////////////////////////////////////////////// - - psc Camera::position() const { - return psc(_position.synced); + // Accessors + const Camera::Vec3& Camera::positionVec3() const{ + return _position.synced; } - psc Camera::unsynchedPosition() const { - return psc(_position.local); + const Camera::Vec3& Camera::unsynchedPositionVec3() const{ + return _position.local; } - psc Camera::focusPosition() const { - return psc(_focusPosition); + const Camera::Vec3& Camera::focusPositionVec3() const{ + return _focusPosition; } glm::vec3 Camera::viewDirectionWorldSpace() const { @@ -162,27 +157,8 @@ namespace openspace { } return _cachedCombinedViewMatrix.datum; } - - ////////////////////////////////////////////////////////////////////////////////////// - // DEPRECATED CAMERA ACCESSORS (GETTERS) // - ////////////////////////////////////////////////////////////////////////////////////// - - const glm::mat4& Camera::viewMatrix() const { - return sgctInternal.viewMatrix(); - } - - const glm::mat4& Camera::projectionMatrix() const { - return sgctInternal.projectionMatrix(); - } - - const glm::mat4& Camera::viewProjectionMatrix() const { - return sgctInternal.viewProjectionMatrix(); - } - - ////////////////////////////////////////////////////////////////////////////////////// - // CAMERA SYNCHRONIZATION // - ////////////////////////////////////////////////////////////////////////////////////// - + + // Synchronization void Camera::serialize(SyncBuffer* syncBuffer) { std::lock_guard _lock(_mutex); @@ -218,7 +194,7 @@ namespace openspace { } ////////////////////////////////////////////////////////////////////////////////////// - // SGCT NODE DEPENTENT // + // SGCT INTERNAL // ////////////////////////////////////////////////////////////////////////////////////// Camera::SgctInternal::SgctInternal() : _viewMatrix() @@ -256,4 +232,38 @@ namespace openspace { return _cachedViewProjectionMatrix.datum; } + // Deprecated + void Camera::setPosition(psc pos) { + std::lock_guard _lock(_mutex); + _position.local = pos.dvec3(); + } + + void Camera::setFocusPosition(psc pos) { + std::lock_guard _lock(_mutex); + _focusPosition = pos.dvec3(); + } + + psc Camera::position() const { + return psc(_position.synced); + } + + psc Camera::unsynchedPosition() const { + return psc(_position.local); + } + + psc Camera::focusPosition() const { + return psc(_focusPosition); + } + + const glm::mat4& Camera::viewMatrix() const { + return sgctInternal.viewMatrix(); + } + + const glm::mat4& Camera::projectionMatrix() const { + return sgctInternal.projectionMatrix(); + } + + const glm::mat4& Camera::viewProjectionMatrix() const { + return sgctInternal.viewProjectionMatrix(); + } } // namespace openspace \ No newline at end of file