From 0d3bfd404463bf0946c8c46616f788aa169d48e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 1 Aug 2013 17:40:55 +0200 Subject: [PATCH] Reducing pointer chasings, part 3c: less pointer passing in SceneGraph. Main change is that features take reference to containing object instead of pointer, as the feature must always belong to some object. Feature groups now return references to features and features return reference to containing object, as these cannot be null. Passing `*this` to AbstractFeature (and Camera[23]D) constructor might now clash with deleted copy constructor, added templated constructor to catch and resolve these ambiguous cases. --- doc/scenegraph.dox | 18 +++---- src/SceneGraph/AbstractCamera.h | 4 +- src/SceneGraph/AbstractCamera.hpp | 10 ++-- src/SceneGraph/AbstractFeature.h | 25 ++++++---- src/SceneGraph/AbstractFeature.hpp | 4 +- src/SceneGraph/AbstractGroupedFeature.h | 6 +-- src/SceneGraph/Animable.h | 4 +- src/SceneGraph/Animable.hpp | 64 ++++++++++++------------- src/SceneGraph/Camera2D.h | 9 +++- src/SceneGraph/Camera2D.hpp | 2 +- src/SceneGraph/Camera3D.h | 9 +++- src/SceneGraph/Camera3D.hpp | 2 +- src/SceneGraph/Drawable.h | 8 ++-- src/SceneGraph/FeatureGroup.h | 30 ++++++------ src/SceneGraph/FeatureGroup.hpp | 8 ++-- src/SceneGraph/Test/AnimableTest.cpp | 26 +++++----- src/SceneGraph/Test/CameraTest.cpp | 24 +++++----- src/SceneGraph/Test/ObjectTest.cpp | 14 +++--- 18 files changed, 144 insertions(+), 123 deletions(-) diff --git a/doc/scenegraph.dox b/doc/scenegraph.dox index 9cd506d36..83cc9fe9e 100644 --- a/doc/scenegraph.dox +++ b/doc/scenegraph.dox @@ -82,8 +82,8 @@ Parent object can be either passed in constructor or using Object::setParent(). @code Scene3D scene; -Object3D* first = new Object3D(&scene); -Object3D* second = new Object3D(first); +auto first = new Object3D(&scene); +auto second = new Object3D(first); @endcode %Object children can be accessed using Object::firstChild() and @@ -107,7 +107,7 @@ The object is derived from the transformation you specified earlier in the transformation implementation. %Scene, as a root object, cannot have any transformation. For convenience you can use method chaining: @code -Object3D* next = new Object3D; +auto next = new Object3D; next->setParent(another) .translate(Vector3::yAxis(3.0f)) .rotateY(35.0_degf); @@ -123,7 +123,7 @@ Each feature takes pointer to holder object in constructor, so adding a feature to an object might look like this: @code Object3D* o; -MyFeature* feature = new MyFeature(o); +auto feature = new MyFeature(o); @endcode Features of an object can be accessed using Object::firstFeature() and @@ -149,7 +149,7 @@ Simplified example: @code class Bomb: public Object3D, SceneGraph::Drawable3D, SceneGraph:.Animable3D { public: - Bomb(Object3D* parent): Object3D(parent), SceneGraph::Drawable3D(this), SceneGraph::Animable3D(this) {} + Bomb(Object3D* parent): Object3D(parent), SceneGraph::Drawable3D(*this), SceneGraph::Animable3D(*this) {} protected: // drawing implementation for Drawable feature @@ -195,8 +195,8 @@ cleaning function(s): @code class CachingObject: public Object3D, SceneGraph::AbstractFeature3D { public: - CachingObject(Object3D* parent): SceneGraph::AbstractFeature3D(this) { - setCachedTransformations(CachedTransformation::Absolute); + CachingObject(Object3D* parent): SceneGraph::AbstractFeature3D(*this) { + setCachedTransformations(SceneGraph::CachedTransformation::Absolute); } protected: @@ -258,7 +258,7 @@ inherited after the %Object class: @code class MyObject: public Object3D, MyFeature { public: - MyObject(Object3D* parent): Object3D(parent), MyFeature(this) {} + MyObject(Object3D* parent): Object3D(parent), MyFeature(*this) {} }; @endcode When constructing MyObject, Object3D constructor is called first and then @@ -271,7 +271,7 @@ However, if we would inherit MyFeature first, it will cause problems: @code class MyObject: MyFeature, public Object3D { public: - MyObject(Object3D* parent): MyFeature(this), Object3D(parent) {} // crash! + MyObject(Object3D* parent): MyFeature(*this), Object3D(parent) {} // crash! }; @endcode MyFeature tries to add itself to feature list in not-yet-constructed Object3D, diff --git a/src/SceneGraph/AbstractCamera.h b/src/SceneGraph/AbstractCamera.h index 1d5d80bd0..f1a03ada6 100644 --- a/src/SceneGraph/AbstractCamera.h +++ b/src/SceneGraph/AbstractCamera.h @@ -92,7 +92,7 @@ template class MAGNUM_SCENEGRAPH_EXPORT Abstrac * applied as first. */ typename DimensionTraits::MatrixType cameraMatrix() { - AbstractFeature::object()->setClean(); + AbstractFeature::object().setClean(); return _cameraMatrix; } @@ -139,7 +139,7 @@ template class MAGNUM_SCENEGRAPH_EXPORT Abstrac * @brief Constructor * @param object Object holding the camera */ - explicit AbstractCamera(AbstractObject* object); + explicit AbstractCamera(AbstractObject& object); ~AbstractCamera(); diff --git a/src/SceneGraph/AbstractCamera.hpp b/src/SceneGraph/AbstractCamera.hpp index ba4c48a15..cc1ef8e2e 100644 --- a/src/SceneGraph/AbstractCamera.hpp +++ b/src/SceneGraph/AbstractCamera.hpp @@ -68,7 +68,7 @@ template typename DimensionTraits AbstractCamera::AbstractCamera(AbstractObject* object): AbstractFeature(object), _aspectRatioPolicy(AspectRatioPolicy::NotPreserved) { +template AbstractCamera::AbstractCamera(AbstractObject& object): AbstractFeature(object), _aspectRatioPolicy(AspectRatioPolicy::NotPreserved) { AbstractFeature::setCachedTransformations(CachedTransformation::InvertedAbsolute); } @@ -86,22 +86,22 @@ template void AbstractCamera::se } template void AbstractCamera::draw(DrawableGroup& group) { - AbstractObject* scene = AbstractFeature::object()->scene(); + AbstractObject* scene = AbstractFeature::object().scene(); CORRADE_ASSERT(scene, "Camera::draw(): cannot draw when camera is not part of any scene", ); /* Compute camera matrix */ - AbstractFeature::object()->setClean(); + AbstractFeature::object().setClean(); /* Compute transformations of all objects in the group relative to the camera */ std::vector*> objects(group.size()); for(std::size_t i = 0; i != group.size(); ++i) - objects[i] = group[i]->object(); + objects[i] = &group[i].object(); std::vector::MatrixType> transformations = scene->transformationMatrices(objects, _cameraMatrix); /* Perform the drawing */ for(std::size_t i = 0; i != transformations.size(); ++i) - group[i]->draw(transformations[i], this); + group[i].draw(transformations[i], *this); } }} diff --git a/src/SceneGraph/AbstractFeature.h b/src/SceneGraph/AbstractFeature.h index de9c71892..decaf76b7 100644 --- a/src/SceneGraph/AbstractFeature.h +++ b/src/SceneGraph/AbstractFeature.h @@ -100,7 +100,7 @@ cleanInverted() or both. Example: @code class CachingFeature: public SceneGraph::AbstractFeature3D { public: - CachingFeature(SceneGraph::AbstractObject3D* object): SceneGraph::AbstractFeature3D(object) { + CachingFeature(SceneGraph::AbstractObject3D& object): SceneGraph::AbstractFeature3D(object) { setCachedTransformations(CachedTransformation::Absolute); } @@ -133,11 +133,11 @@ parameter: @code class TransformingFeature: public SceneGraph::AbstractFeature3D { public: - template TransformingFeature(SceneGraph::Object* object): + template TransformingFeature(SceneGraph::Object& object): SceneGraph::AbstractFeature3D(object), transformation(object) {} private: - SceneGraph::AbstractTranslationRotation3D* transformation; + SceneGraph::AbstractTranslationRotation3D& transformation; }; @endcode If we take for example @ref Object "Object", it is @@ -145,7 +145,7 @@ derived from @ref AbstractObject "AbstractObject3D" and @ref BasicMatrixTransformation3D "MatrixTransformation3D", which is derived from @ref AbstractBasicTranslationRotationScaling3D "AbstractTranslationRotationScaling3D", which is derived from @ref AbstractBasicTranslationRotation3D "AbstractTranslationRotation3D", -which is automatically extracted from the pointer in our constructor. +which is automatically extracted from the reference in our constructor. @section AbstractFeature-explicit-specializations Explicit template specializations @@ -174,18 +174,25 @@ template class MAGNUM_SCENEGRAPH_EXPORT Abstrac * @brief Constructor * @param object %Object holding this feature */ - explicit AbstractFeature(AbstractObject* object); + explicit AbstractFeature(AbstractObject& object); + + #ifndef DOXYGEN_GENERATING_OUTPUT + /* This is here to avoid ambiguity with deleted copy constructor when + passing `*this` from class subclassing both AbstractFeature and + AbstractObject */ + template, U>::value>::type> AbstractFeature(U& object): AbstractFeature(static_cast&>(object)) {} + #endif virtual ~AbstractFeature() = 0; /** @brief %Object holding this feature */ - AbstractObject* object() { - return Containers::LinkedListItem, AbstractObject>::list(); + AbstractObject& object() { + return *Containers::LinkedListItem, AbstractObject>::list(); } /** @overload */ - const AbstractObject* object() const { - return Containers::LinkedListItem, AbstractObject>::list(); + const AbstractObject& object() const { + return *Containers::LinkedListItem, AbstractObject>::list(); } /** @brief Previous feature or `nullptr`, if this is first feature */ diff --git a/src/SceneGraph/AbstractFeature.hpp b/src/SceneGraph/AbstractFeature.hpp index f0da6915e..0684c4670 100644 --- a/src/SceneGraph/AbstractFeature.hpp +++ b/src/SceneGraph/AbstractFeature.hpp @@ -32,8 +32,8 @@ namespace Magnum { namespace SceneGraph { -template AbstractFeature::AbstractFeature(AbstractObject* object) { - object->Containers::template LinkedList>::insert(this); +template AbstractFeature::AbstractFeature(AbstractObject& object) { + object.Containers::template LinkedList>::insert(this); } template AbstractFeature::~AbstractFeature() = default; diff --git a/src/SceneGraph/AbstractGroupedFeature.h b/src/SceneGraph/AbstractGroupedFeature.h index dd589c8b2..2a280a5e9 100644 --- a/src/SceneGraph/AbstractGroupedFeature.h +++ b/src/SceneGraph/AbstractGroupedFeature.h @@ -78,8 +78,8 @@ template class AbstractGroupedFe * Adds the feature to the object and to group, if specified. * @see FeatureGroup::add() */ - explicit AbstractGroupedFeature(AbstractObject* object, FeatureGroup* group = nullptr): AbstractFeature(object), _group(nullptr) { - if(group) group->add(static_cast(this)); + explicit AbstractGroupedFeature(AbstractObject& object, FeatureGroup* group = nullptr): AbstractFeature(object), _group(nullptr) { + if(group) group->add(static_cast(*this)); } /** @@ -89,7 +89,7 @@ template class AbstractGroupedFe * any. */ ~AbstractGroupedFeature() { - if(_group) _group->remove(static_cast(this)); + if(_group) _group->remove(static_cast(*this)); } /** @brief Group this feature belongs to */ diff --git a/src/SceneGraph/Animable.h b/src/SceneGraph/Animable.h index 3221d64de..c8d81cef8 100644 --- a/src/SceneGraph/Animable.h +++ b/src/SceneGraph/Animable.h @@ -78,7 +78,7 @@ typedef SceneGraph::Scene Scene3D; class AnimableObject: public Object3D, SceneGraph::Animable3D { public: - AnimableObject(Object* parent = nullptr, SceneGraph::DrawableGroup3D* group = nullptr): Object3D(parent), SceneGraph::Animable3D(this, group) { + AnimableObject(Object* parent = nullptr, SceneGraph::DrawableGroup3D* group = nullptr): Object3D(parent), SceneGraph::Animable3D(*this, group) { setDuration(10.0f); // ... } @@ -153,7 +153,7 @@ template class MAGNUM_SCENEGRAPH_EXPORT Animabl * adds the feature to the object and also to group, if specified. * @see setDuration(), setState(), setRepeated(), AnimableGroup::add() */ - explicit Animable(AbstractObject* object, AnimableGroup* group = nullptr); + explicit Animable(AbstractObject& object, AnimableGroup* group = nullptr); ~Animable(); diff --git a/src/SceneGraph/Animable.hpp b/src/SceneGraph/Animable.hpp index b9a35e596..5ba80c88f 100644 --- a/src/SceneGraph/Animable.hpp +++ b/src/SceneGraph/Animable.hpp @@ -35,7 +35,7 @@ namespace Magnum { namespace SceneGraph { -template Animable::Animable(AbstractObject* object, AnimableGroup* group): AbstractGroupedFeature, T>(object, group), _duration(0.0f), startTime(std::numeric_limits::infinity()), pauseTime(-std::numeric_limits::infinity()), previousState(AnimationState::Stopped), currentState(AnimationState::Stopped), _repeated(false), _repeatCount(0), repeats(0) {} +template Animable::Animable(AbstractObject& object, AnimableGroup* group): AbstractGroupedFeature, T>(object, group), _duration(0.0f), startTime(std::numeric_limits::infinity()), pauseTime(-std::numeric_limits::infinity()), previousState(AnimationState::Stopped), currentState(AnimationState::Stopped), _repeated(false), _repeatCount(0), repeats(0) {} template Animable::~Animable() {} @@ -65,74 +65,74 @@ template void AnimableGroup::ste wakeUp = false; for(std::size_t i = 0; i != AnimableGroup::size(); ++i) { - Animable* animable = (*this)[i]; + Animable& animable = (*this)[i]; /* The animation was stopped recently, just decrease count of running animations if the animation was running before */ - if(animable->previousState != AnimationState::Stopped && animable->currentState == AnimationState::Stopped) { - if(animable->previousState == AnimationState::Running) + if(animable.previousState != AnimationState::Stopped && animable.currentState == AnimationState::Stopped) { + if(animable.previousState == AnimationState::Running) --_runningCount; - animable->previousState = AnimationState::Stopped; - animable->animationStopped(); + animable.previousState = AnimationState::Stopped; + animable.animationStopped(); continue; /* The animation was paused recently, set pause time to previous frame time */ - } else if(animable->previousState == AnimationState::Running && animable->currentState == AnimationState::Paused) { - animable->previousState = AnimationState::Paused; - animable->pauseTime = time; + } else if(animable.previousState == AnimationState::Running && animable.currentState == AnimationState::Paused) { + animable.previousState = AnimationState::Paused; + animable.pauseTime = time; --_runningCount; - animable->animationPaused(); + animable.animationPaused(); continue; /* Skip the rest for not running animations */ - } else if(animable->currentState != AnimationState::Running) { - CORRADE_INTERNAL_ASSERT(animable->previousState == animable->currentState); + } else if(animable.currentState != AnimationState::Running) { + CORRADE_INTERNAL_ASSERT(animable.previousState == animable.currentState); continue; /* The animation was started recently, set start time to previous frame time, reset repeat count */ - } else if(animable->previousState == AnimationState::Stopped) { - animable->previousState = AnimationState::Running; - animable->startTime = time; - animable->repeats = 0; + } else if(animable.previousState == AnimationState::Stopped) { + animable.previousState = AnimationState::Running; + animable.startTime = time; + animable.repeats = 0; ++_runningCount; - animable->animationStarted(); + animable.animationStarted(); /* The animation was resumed recently, add pause duration to start time */ - } else if(animable->previousState == AnimationState::Paused) { - animable->previousState = AnimationState::Running; - animable->startTime += time - animable->pauseTime; + } else if(animable.previousState == AnimationState::Paused) { + animable.previousState = AnimationState::Running; + animable.startTime += time - animable.pauseTime; ++_runningCount; - animable->animationResumed(); + animable.animationResumed(); } - CORRADE_INTERNAL_ASSERT(animable->previousState == AnimationState::Running); + CORRADE_INTERNAL_ASSERT(animable.previousState == AnimationState::Running); /* Animation time exceeded duration */ - if(animable->_duration != 0.0f && time-animable->startTime > animable->_duration) { + if(animable._duration != 0.0f && time-animable.startTime > animable._duration) { /* Not repeated or repeat count exceeded, stop */ - if(!animable->_repeated || animable->repeats+1 == animable->_repeatCount) { - animable->previousState = AnimationState::Stopped; - animable->currentState = AnimationState::Stopped; + if(!animable._repeated || animable.repeats+1 == animable._repeatCount) { + animable.previousState = AnimationState::Stopped; + animable.currentState = AnimationState::Stopped; --_runningCount; - animable->animationStopped(); + animable.animationStopped(); continue; } /* Increase repeat count and add duration to startTime */ - ++animable->repeats; - animable->startTime += animable->_duration; + ++animable.repeats; + animable.startTime += animable._duration; } /* Animation is still running, perform animation step */ - CORRADE_ASSERT(time-animable->startTime >= 0.0f, + CORRADE_ASSERT(time-animable.startTime >= 0.0f, "SceneGraph::AnimableGroup::step(): animation was started in future - probably wrong time passed", ); CORRADE_ASSERT(delta >= 0.0f, "SceneGraph::AnimableGroup::step(): negative delta passed", ); - animable->animationStep(time - animable->startTime, delta); + animable.animationStep(time - animable.startTime, delta); } - CORRADE_INTERNAL_ASSERT(_runningCount <= AnimableGroup::size()); + CORRADE_INTERNAL_ASSERT((_runningCount <= AnimableGroup::size())); } }} diff --git a/src/SceneGraph/Camera2D.h b/src/SceneGraph/Camera2D.h index 3016a405e..d1be4b0db 100644 --- a/src/SceneGraph/Camera2D.h +++ b/src/SceneGraph/Camera2D.h @@ -65,7 +65,14 @@ template class MAGNUM_SCENEGRAPH_EXPORT BasicCamera2D: public AbstractC * Sets orthographic projection to the default OpenGL cube (range @f$ [-1; 1] @f$ in all directions). * @see setProjection() */ - explicit BasicCamera2D(AbstractObject<2, T>* object); + explicit BasicCamera2D(AbstractObject<2, T>& object); + + #ifndef DOXYGEN_GENERATING_OUTPUT + /* This is here to avoid ambiguity with deleted copy constructor when + passing `*this` from class subclassing both BasicCamera2D and + AbstractObject */ + template, U>::value>::type> BasicCamera2D(U& object): BasicCamera2D(static_cast&>(object)) {} + #endif /** * @brief Set projection diff --git a/src/SceneGraph/Camera2D.hpp b/src/SceneGraph/Camera2D.hpp index 447c0aee1..302d74e9b 100644 --- a/src/SceneGraph/Camera2D.hpp +++ b/src/SceneGraph/Camera2D.hpp @@ -35,7 +35,7 @@ namespace Magnum { namespace SceneGraph { -template BasicCamera2D::BasicCamera2D(AbstractObject<2, T>* object): AbstractCamera<2, T>(object) {} +template BasicCamera2D::BasicCamera2D(AbstractObject<2, T>& object): AbstractCamera<2, T>(object) {} template BasicCamera2D& BasicCamera2D::setProjection(const Math::Vector2& size) { AbstractCamera<2, T>::rawProjectionMatrix = Math::Matrix3::projection(size); diff --git a/src/SceneGraph/Camera3D.h b/src/SceneGraph/Camera3D.h index 234a2ad15..6d6c665f4 100644 --- a/src/SceneGraph/Camera3D.h +++ b/src/SceneGraph/Camera3D.h @@ -67,7 +67,14 @@ template class MAGNUM_SCENEGRAPH_EXPORT BasicCamera3D: public AbstractC * @brief Constructor * @param object %Object holding this feature */ - explicit BasicCamera3D(AbstractObject<3, T>* object); + explicit BasicCamera3D(AbstractObject<3, T>& object); + + #ifndef DOXYGEN_GENERATING_OUTPUT + /* This is here to avoid ambiguity with deleted copy constructor when + passing `*this` from class subclassing both BasicCamera3D and + AbstractObject */ + template, U>::value>::type> BasicCamera3D(U& object): BasicCamera3D(static_cast&>(object)) {} + #endif /** * @brief Set orthographic projection diff --git a/src/SceneGraph/Camera3D.hpp b/src/SceneGraph/Camera3D.hpp index c1e5bda74..bef3b77e9 100644 --- a/src/SceneGraph/Camera3D.hpp +++ b/src/SceneGraph/Camera3D.hpp @@ -35,7 +35,7 @@ namespace Magnum { namespace SceneGraph { -template BasicCamera3D::BasicCamera3D(AbstractObject<3, T>* object): AbstractCamera<3, T>(object), _near(T(0)), _far(T(0)) {} +template BasicCamera3D::BasicCamera3D(AbstractObject<3, T>& object): AbstractCamera<3, T>(object), _near(T(0)), _far(T(0)) {} template BasicCamera3D& BasicCamera3D::setOrthographic(const Math::Vector2& size, T near, T far) { /** @todo Get near/far from the matrix */ diff --git a/src/SceneGraph/Drawable.h b/src/SceneGraph/Drawable.h index a224f4296..b8e388a2f 100644 --- a/src/SceneGraph/Drawable.h +++ b/src/SceneGraph/Drawable.h @@ -49,11 +49,11 @@ typedef SceneGraph::Scene Scene3D; class DrawableObject: public Object3D, SceneGraph::Drawable3D { public: - DrawableObject(Object* parent = nullptr, SceneGraph::DrawableGroup3D* group = nullptr): Object3D(parent), SceneGraph::Drawable3D(this, group) { + DrawableObject(Object* parent = nullptr, SceneGraph::DrawableGroup3D* group = nullptr): Object3D(parent), SceneGraph::Drawable3D(*this, group) { // ... } - void draw(const Matrix4& transformationMatrix, AbstractCamera3D* camera) override { + void draw(const Matrix4& transformationMatrix, AbstractCamera3D& camera) override { // ... } } @@ -127,7 +127,7 @@ template class Drawable: public AbstractGrouped * Adds the feature to the object and also to the group, if specified. * Otherwise you can use DrawableGroup::add(). */ - explicit Drawable(AbstractObject* object, DrawableGroup* drawables = nullptr): AbstractGroupedFeature, T>(object, drawables) {} + explicit Drawable(AbstractObject& object, DrawableGroup* drawables = nullptr): AbstractGroupedFeature, T>(object, drawables) {} /** * @brief Draw the object using given camera @@ -137,7 +137,7 @@ template class Drawable: public AbstractGrouped * * Projection matrix can be retrieved from AbstractCamera::projectionMatrix(). */ - virtual void draw(const typename DimensionTraits::MatrixType& transformationMatrix, AbstractCamera* camera) = 0; + virtual void draw(const typename DimensionTraits::MatrixType& transformationMatrix, AbstractCamera& camera) = 0; }; #ifndef CORRADE_GCC46_COMPATIBILITY diff --git a/src/SceneGraph/FeatureGroup.h b/src/SceneGraph/FeatureGroup.h index e632e8ec5..742c4b2c2 100644 --- a/src/SceneGraph/FeatureGroup.h +++ b/src/SceneGraph/FeatureGroup.h @@ -47,8 +47,8 @@ template class MAGNUM_SCENEGRAPH_EXPORT Abstrac explicit AbstractFeatureGroup(); virtual ~AbstractFeatureGroup(); - void add(AbstractFeature* feature); - void remove(AbstractFeature* feature); + void add(AbstractFeature& feature); + void remove(AbstractFeature& feature); std::vector*> features; }; @@ -84,13 +84,13 @@ template class FeatureGroup: pub } /** @brief Feature at given index */ - Feature* operator[](std::size_t index) { - return static_cast(AbstractFeatureGroup::features[index]); + Feature& operator[](std::size_t index) { + return *static_cast(AbstractFeatureGroup::features[index]); } /** @overload */ - const Feature* operator[](std::size_t index) const { - return static_cast(AbstractFeatureGroup::features[index]); + const Feature& operator[](std::size_t index) const { + return *static_cast(AbstractFeatureGroup::features[index]); } /** @@ -100,7 +100,7 @@ template class FeatureGroup: pub * If the features is part of another group, it is removed from it. * @see remove(), AbstractGroupedFeature::AbstractGroupedFeature() */ - FeatureGroup& add(Feature* feature); + FeatureGroup& add(Feature& feature); /** * @brief Remove feature from the group @@ -109,7 +109,7 @@ template class FeatureGroup: pub * The feature must be part of the group. * @see add() */ - FeatureGroup& remove(Feature* feature); + FeatureGroup& remove(Feature& feature); }; #ifndef CORRADE_GCC46_COMPATIBILITY @@ -162,23 +162,23 @@ template FeatureGroup::features) static_cast(i)->_group = nullptr; } -template FeatureGroup& FeatureGroup::add(Feature* feature) { +template FeatureGroup& FeatureGroup::add(Feature& feature) { /* Remove from previous group */ - if(feature->_group) - feature->_group->remove(feature); + if(feature._group) + feature._group->remove(feature); /* Crossreference the feature and group together */ AbstractFeatureGroup::add(feature); - feature->_group = this; + feature._group = this; return *this; } -template FeatureGroup& FeatureGroup::remove(Feature* feature) { - CORRADE_ASSERT(feature->_group == this, +template FeatureGroup& FeatureGroup::remove(Feature& feature) { + CORRADE_ASSERT(feature._group == this, "SceneGraph::AbstractFeatureGroup::remove(): feature is not part of this group", *this); AbstractFeatureGroup::remove(feature); - feature->_group = nullptr; + feature._group = nullptr; return *this; } diff --git a/src/SceneGraph/FeatureGroup.hpp b/src/SceneGraph/FeatureGroup.hpp index 0f2cd1f1f..4caf9855e 100644 --- a/src/SceneGraph/FeatureGroup.hpp +++ b/src/SceneGraph/FeatureGroup.hpp @@ -37,12 +37,12 @@ namespace Magnum { namespace SceneGraph { template AbstractFeatureGroup::AbstractFeatureGroup() = default; template AbstractFeatureGroup::~AbstractFeatureGroup() = default; -template void AbstractFeatureGroup::add(AbstractFeature* feature) { - features.push_back(feature); +template void AbstractFeatureGroup::add(AbstractFeature& feature) { + features.push_back(&feature); } -template void AbstractFeatureGroup::remove(AbstractFeature* feature) { - features.erase(std::find(features.begin(), features.end(), feature)); +template void AbstractFeatureGroup::remove(AbstractFeature& feature) { + features.erase(std::find(features.begin(), features.end(), &feature)); } }} diff --git a/src/SceneGraph/Test/AnimableTest.cpp b/src/SceneGraph/Test/AnimableTest.cpp index 3eba97476..eb9c9eb7d 100644 --- a/src/SceneGraph/Test/AnimableTest.cpp +++ b/src/SceneGraph/Test/AnimableTest.cpp @@ -61,7 +61,7 @@ AnimableTest::AnimableTest() { void AnimableTest::state() { class StateTrackingAnimable: public SceneGraph::Animable3D { public: - StateTrackingAnimable(AbstractObject3D* object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group) { + StateTrackingAnimable(AbstractObject3D& object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group) { setDuration(1.0f); } @@ -81,7 +81,7 @@ void AnimableTest::state() { CORRADE_COMPARE(group.runningCount(), 0); /* Verify initial state */ - StateTrackingAnimable animable(&object, &group); + StateTrackingAnimable animable(object, &group); CORRADE_COMPARE(animable.state(), AnimationState::Stopped); CORRADE_VERIFY(animable.trackedState.empty()); group.step(1.0f, 1.0f); @@ -143,18 +143,18 @@ void AnimableTest::state() { CORRADE_COMPARE(group.runningCount(), 0); /* Verify running count can go past 0/1 */ - auto a = new StateTrackingAnimable(&object, &group); - auto b = new StateTrackingAnimable(&object, &group); + auto a = new StateTrackingAnimable(object, &group); + auto b = new StateTrackingAnimable(object, &group); a->setState(AnimationState::Running); b->setState(AnimationState::Running); - group.add(a).add(b); + group.add(*a).add(*b); group.step(1.0f, 1.0f); CORRADE_COMPARE(group.runningCount(), 2); } class OneShotAnimable: public SceneGraph::Animable3D { public: - OneShotAnimable(AbstractObject3D* object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f) { + OneShotAnimable(AbstractObject3D& object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f) { setDuration(10.0f); setState(AnimationState::Running); } @@ -179,7 +179,7 @@ class OneShotAnimable: public SceneGraph::Animable3D { void AnimableTest::step() { class InifiniteAnimable: public SceneGraph::Animable3D { public: - InifiniteAnimable(AbstractObject3D* object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f), delta(0.0f) {} + InifiniteAnimable(AbstractObject3D& object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f), delta(0.0f) {} Float time, delta; @@ -192,7 +192,7 @@ void AnimableTest::step() { Object3D object; AnimableGroup3D group; - InifiniteAnimable animable(&object, &group); + InifiniteAnimable animable(object, &group); /* Calling step() if no object is running should do nothing */ group.step(5.0f, 0.5f); @@ -217,7 +217,7 @@ void AnimableTest::step() { void AnimableTest::duration() { Object3D object; AnimableGroup3D group; - OneShotAnimable animable(&object, &group); + OneShotAnimable animable(object, &group); CORRADE_VERIFY(!animable.isRepeated()); /* First animation step is in duration, verify that animation is still @@ -238,7 +238,7 @@ void AnimableTest::duration() { void AnimableTest::repeat() { class RepeatingAnimable: public SceneGraph::Animable3D { public: - RepeatingAnimable(AbstractObject3D* object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f) { + RepeatingAnimable(AbstractObject3D& object, AnimableGroup3D* group = nullptr): SceneGraph::Animable3D(object, group), time(-1.0f) { setDuration(10.0f); setState(AnimationState::Running); setRepeated(true); @@ -254,7 +254,7 @@ void AnimableTest::repeat() { Object3D object; AnimableGroup3D group; - RepeatingAnimable animable(&object, &group); + RepeatingAnimable animable(object, &group); CORRADE_COMPARE(animable.repeatCount(), 0); /* First animation steps is in first loop iteration */ @@ -299,7 +299,7 @@ void AnimableTest::repeat() { void AnimableTest::stop() { Object3D object; AnimableGroup3D group; - OneShotAnimable animable(&object, &group); + OneShotAnimable animable(object, &group); CORRADE_COMPARE(animable.repeatCount(), 0); /* Eat up some absolute time */ @@ -324,7 +324,7 @@ void AnimableTest::stop() { void AnimableTest::pause() { Object3D object; AnimableGroup3D group; - OneShotAnimable animable(&object, &group); + OneShotAnimable animable(object, &group); /* First two steps, animation is running */ group.step(1.0f, 0.5f); diff --git a/src/SceneGraph/Test/CameraTest.cpp b/src/SceneGraph/Test/CameraTest.cpp index 54402cdc2..14ed3c555 100644 --- a/src/SceneGraph/Test/CameraTest.cpp +++ b/src/SceneGraph/Test/CameraTest.cpp @@ -107,14 +107,14 @@ void CameraTest::fixAspectRatio() { void CameraTest::defaultProjection2D() { Object2D o; - Camera2D camera(&o); + Camera2D camera(o); CORRADE_COMPARE(camera.projectionMatrix(), Matrix3()); CORRADE_COMPARE(camera.projectionSize(), Vector2(2.0f)); } void CameraTest::defaultProjection3D() { Object3D o; - Camera3D camera(&o); + Camera3D camera(o); CORRADE_COMPARE(camera.projectionMatrix(), Matrix4()); CORRADE_COMPARE(camera.projectionSize(), Vector2(2.0f)); } @@ -122,7 +122,7 @@ void CameraTest::defaultProjection3D() { void CameraTest::projectionSize2D() { Vector2 projectionSize(4.0f, 3.0f); Object2D o; - Camera2D camera(&o); + Camera2D camera(o); camera.setProjection(projectionSize); CORRADE_COMPARE(camera.projectionSize(), projectionSize); } @@ -130,21 +130,21 @@ void CameraTest::projectionSize2D() { void CameraTest::projectionSizeOrthographic() { Vector2 projectionSizeRectangle(5.0f, 4.0f); Object3D o; - Camera3D camera(&o); + Camera3D camera(o); camera.setOrthographic(projectionSizeRectangle, 1, 9); CORRADE_COMPARE(camera.projectionSize(), projectionSizeRectangle); } void CameraTest::projectionSizePerspective() { Object3D o; - Camera3D camera(&o); + Camera3D camera(o); camera.setPerspective(Deg(27.0f), 2.35f, 32.0f, 100); CORRADE_COMPARE(camera.projectionSize(), Vector2(0.48015756f, 0.204322f)); } void CameraTest::projectionSizeViewport() { Object3D o; - Camera3D camera(&o); + Camera3D camera(o); camera.setViewport({200, 300}); CORRADE_COMPARE(camera.projectionSize(), Vector2(2.0f, 2.0f)); @@ -158,10 +158,10 @@ void CameraTest::projectionSizeViewport() { void CameraTest::draw() { class Drawable: public SceneGraph::Drawable3D { public: - Drawable(AbstractObject3D* object, DrawableGroup3D* group, Matrix4& result): SceneGraph::Drawable3D(object, group), result(result) {} + Drawable(AbstractObject3D& object, DrawableGroup3D* group, Matrix4& result): SceneGraph::Drawable3D(object, group), result(result) {} protected: - void draw(const Matrix4& transformationMatrix, AbstractCamera3D*) { + void draw(const Matrix4& transformationMatrix, AbstractCamera3D&) override { result = transformationMatrix; } @@ -175,19 +175,19 @@ void CameraTest::draw() { Object3D first(&scene); Matrix4 firstTransformation; first.scale(Vector3(5.0f)); - new Drawable(&first, &group, firstTransformation); + new Drawable(first, &group, firstTransformation); Object3D second(&scene); Matrix4 secondTransformation; second.translate(Vector3::yAxis(3.0f)); - new Drawable(&second, &group, secondTransformation); + new Drawable(second, &group, secondTransformation); Object3D third(&second); Matrix4 thirdTransformation; third.translate(Vector3::zAxis(-1.5f)); - new Drawable(&third, &group, thirdTransformation); + new Drawable(third, &group, thirdTransformation); - Camera3D camera(&third); + Camera3D camera(third); camera.draw(group); CORRADE_COMPARE(firstTransformation, Matrix4::translation({0.0f, -3.0f, 1.5f})*Matrix4::scaling(Vector3(5.0f))); diff --git a/src/SceneGraph/Test/ObjectTest.cpp b/src/SceneGraph/Test/ObjectTest.cpp index 272b915d7..0dbbefe7a 100644 --- a/src/SceneGraph/Test/ObjectTest.cpp +++ b/src/SceneGraph/Test/ObjectTest.cpp @@ -52,7 +52,7 @@ typedef SceneGraph::Scene Scene3D; class CachingObject: public Object3D, AbstractFeature3D { public: - CachingObject(Object3D* parent = nullptr): Object3D(parent), AbstractFeature3D(this) { + CachingObject(Object3D* parent = nullptr): Object3D(parent), AbstractFeature3D(*this) { setCachedTransformations(CachedTransformation::Absolute); } @@ -271,7 +271,7 @@ void ObjectTest::setClean() { class CachingFeature: public AbstractFeature3D { public: - CachingFeature(AbstractObject3D* object): AbstractFeature3D(object) { + CachingFeature(AbstractObject3D& object): AbstractFeature3D(object) { setCachedTransformations(CachedTransformation::Absolute); } @@ -284,7 +284,7 @@ void ObjectTest::setClean() { class CachingInvertedFeature: public AbstractFeature3D { public: - CachingInvertedFeature(AbstractObject3D* object): AbstractFeature3D(object) { + CachingInvertedFeature(AbstractObject3D& object): AbstractFeature3D(object) { setCachedTransformations(CachedTransformation::InvertedAbsolute); } @@ -300,8 +300,8 @@ void ObjectTest::setClean() { CachingObject* childTwo = new CachingObject(childOne); childTwo->translate(Vector3::xAxis(1.0f)); - CachingFeature* childTwoFeature = new CachingFeature(childTwo); - CachingInvertedFeature* childTwoFeature2 = new CachingInvertedFeature(childTwo); + CachingFeature* childTwoFeature = new CachingFeature(*childTwo); + CachingInvertedFeature* childTwoFeature2 = new CachingInvertedFeature(*childTwo); CachingObject* childThree = new CachingObject(childTwo); childThree->rotate(Deg(90.0f), Vector3::yAxis()); @@ -369,7 +369,7 @@ void ObjectTest::setCleanListHierarchy() { class CachingFeature: public AbstractFeature3D { public: - CachingFeature(AbstractObject3D* object): AbstractFeature3D(object) { + CachingFeature(AbstractObject3D& object): AbstractFeature3D(object) { setCachedTransformations(CachedTransformation::Absolute); } @@ -385,7 +385,7 @@ void ObjectTest::setCleanListHierarchy() { CachingObject* childTwo = new CachingObject(childOne); childTwo->translate(Vector3::xAxis(1.0f)); - CachingFeature* childTwoFeature = new CachingFeature(childTwo); + CachingFeature* childTwoFeature = new CachingFeature(*childTwo); CachingObject* childThree = new CachingObject(childTwo); childThree->rotate(Deg(90.0f), Vector3::yAxis());