From f669dd7c23db7ad360bbce543b191a650e3aa8f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 1 Aug 2013 17:54:08 +0200 Subject: [PATCH] Reducing pointer chasings, part 3d: less pointer passing in Shapes. Mostly just adapted to SceneGraph changes, functions not expecting nullptr are now taking/returning reference. --- src/Shapes/AbstractShape.cpp | 8 ++++---- src/Shapes/AbstractShape.h | 12 ++++++------ src/Shapes/Composition.cpp | 6 +++--- src/Shapes/Composition.h | 19 +++++++++---------- .../Implementation/CollisionDispatch.cpp | 16 ++++++++-------- src/Shapes/Implementation/CollisionDispatch.h | 2 +- src/Shapes/Shape.h | 16 ++++++++-------- src/Shapes/ShapeGroup.cpp | 8 ++++---- src/Shapes/ShapeGroup.h | 2 +- src/Shapes/Test/ShapeTest.cpp | 16 ++++++++-------- 10 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/Shapes/AbstractShape.cpp b/src/Shapes/AbstractShape.cpp index c552ab15d..f6cee4d27 100644 --- a/src/Shapes/AbstractShape.cpp +++ b/src/Shapes/AbstractShape.cpp @@ -31,7 +31,7 @@ namespace Magnum { namespace Shapes { -template AbstractShape::AbstractShape(SceneGraph::AbstractObject* object, ShapeGroup* group): SceneGraph::AbstractGroupedFeature, Float>(object, group) { +template AbstractShape::AbstractShape(SceneGraph::AbstractObject& object, ShapeGroup* group): SceneGraph::AbstractGroupedFeature, Float>(object, group) { SceneGraph::AbstractFeature::setCachedTransformations(SceneGraph::CachedTransformation::Absolute); } @@ -44,11 +44,11 @@ template const ShapeGroup* AbstractShape auto AbstractShape::type() const -> Type { - return abstractTransformedShape()->type(); + return abstractTransformedShape().type(); } -template bool AbstractShape::collides(const AbstractShape* other) const { - return Implementation::collides(abstractTransformedShape(), other->abstractTransformedShape()); +template bool AbstractShape::collides(const AbstractShape& other) const { + return Implementation::collides(abstractTransformedShape(), other.abstractTransformedShape()); } template void AbstractShape::markDirty() { diff --git a/src/Shapes/AbstractShape.h b/src/Shapes/AbstractShape.h index b4e30e8e6..cd888807e 100644 --- a/src/Shapes/AbstractShape.h +++ b/src/Shapes/AbstractShape.h @@ -37,8 +37,8 @@ namespace Magnum { namespace Shapes { namespace Implementation { - template inline const AbstractShape* getAbstractShape(const Shapes::AbstractShape* shape) { - return shape->abstractTransformedShape(); + template inline const AbstractShape& getAbstractShape(const Shapes::AbstractShape& shape) { + return shape.abstractTransformedShape(); } } @@ -50,7 +50,7 @@ brief introduction. @see AbstractShape2D, AbstractShape3D */ template class MAGNUM_SHAPES_EXPORT AbstractShape: public SceneGraph::AbstractGroupedFeature, Float> { - friend const Implementation::AbstractShape* Implementation::getAbstractShape<>(const AbstractShape*); + friend const Implementation::AbstractShape& Implementation::getAbstractShape<>(const AbstractShape&); public: enum: UnsignedInt { @@ -79,7 +79,7 @@ template class MAGNUM_SHAPES_EXPORT AbstractShape: publi * @param object Object holding this feature * @param group Group this shape belongs to */ - explicit AbstractShape(SceneGraph::AbstractObject* object, ShapeGroup* group = nullptr); + explicit AbstractShape(SceneGraph::AbstractObject& object, ShapeGroup* group = nullptr); /** * @brief Shape group containing this shape @@ -99,14 +99,14 @@ template class MAGNUM_SHAPES_EXPORT AbstractShape: publi * * Default implementation returns false. */ - bool collides(const AbstractShape* other) const; + bool collides(const AbstractShape& other) const; protected: /** Marks also the group as dirty */ void markDirty() override; private: - virtual const Implementation::AbstractShape MAGNUM_SHAPES_LOCAL * abstractTransformedShape() const = 0; + virtual const Implementation::AbstractShape MAGNUM_SHAPES_LOCAL & abstractTransformedShape() const = 0; }; /** @brief Base class for two-dimensional object shapes */ diff --git a/src/Shapes/Composition.cpp b/src/Shapes/Composition.cpp index 9f4fc5707..7a9fe3857 100644 --- a/src/Shapes/Composition.cpp +++ b/src/Shapes/Composition.cpp @@ -132,7 +132,7 @@ template Composition Composition return out; } -template bool Composition::collides(const Implementation::AbstractShape* const a, const std::size_t node, const std::size_t shapeBegin, const std::size_t shapeEnd) const { +template bool Composition::collides(const Implementation::AbstractShape& a, const std::size_t node, const std::size_t shapeBegin, const std::size_t shapeEnd) const { /* Empty group */ if(shapeBegin == shapeEnd) return false; @@ -141,7 +141,7 @@ template bool Composition::collides(const Im /* Collision on the left child. If the node is leaf one (no left child exists), do it directly, recurse instead. */ const bool collidesLeft = (_nodes[node].rightNode == 0 || _nodes[node].rightNode == 2) ? - Implementation::collides(a, _shapes[shapeBegin]) : + Implementation::collides(a, *_shapes[shapeBegin]) : collides(a, node+1, shapeBegin, shapeBegin+_nodes[node].rightShape); /* NOT operation */ @@ -155,7 +155,7 @@ template bool Composition::collides(const Im /* Now the collision result depends only on the right child. Similar to collision on the left child. */ return (_nodes[node].rightNode < 2) ? - Implementation::collides(a, _shapes[shapeBegin+_nodes[node].rightShape]) : + Implementation::collides(a, *_shapes[shapeBegin+_nodes[node].rightShape]) : collides(a, node+_nodes[node].rightNode-1, shapeBegin+_nodes[node].rightShape, shapeEnd); } diff --git a/src/Shapes/Composition.h b/src/Shapes/Composition.h index c2530c1fc..5552352cc 100644 --- a/src/Shapes/Composition.h +++ b/src/Shapes/Composition.h @@ -42,11 +42,11 @@ namespace Magnum { namespace Shapes { namespace Implementation { template struct ShapeHelper; - template inline AbstractShape* getAbstractShape(Composition& group, std::size_t i) { - return group._shapes[i]; + template inline AbstractShape& getAbstractShape(Composition& group, std::size_t i) { + return *group._shapes[i]; } - template inline const AbstractShape* getAbstractShape(const Composition& group, std::size_t i) { - return group._shapes[i]; + template inline const AbstractShape& getAbstractShape(const Composition& group, std::size_t i) { + return *group._shapes[i]; } } @@ -63,8 +63,8 @@ enum class CompositionOperation: UnsignedByte { Result of logical operations on shapes. See @ref shapes for brief introduction. */ template class MAGNUM_SHAPES_EXPORT Composition { - friend Implementation::AbstractShape* Implementation::getAbstractShape<>(Composition&, std::size_t); - friend const Implementation::AbstractShape* Implementation::getAbstractShape<>(const Composition&, std::size_t); + friend Implementation::AbstractShape& Implementation::getAbstractShape<>(Composition&, std::size_t); + friend const Implementation::AbstractShape& Implementation::getAbstractShape<>(const Composition&, std::size_t); friend struct Implementation::ShapeHelper>; public: @@ -142,8 +142,7 @@ template class MAGNUM_SHAPES_EXPORT Composition { #else template auto operator%(const T& other) const -> typename std::enable_if::type()), typename Implementation::ShapeDimensionTraits::Type>::value, bool>::type { #endif - Implementation::Shape a(other); - return collides(&a); + return collides(Implementation::Shape(other)); } private: @@ -152,11 +151,11 @@ template class MAGNUM_SHAPES_EXPORT Composition { CompositionOperation operation; }; - bool collides(const Implementation::AbstractShape* a) const { + bool collides(const Implementation::AbstractShape& a) const { return collides(a, 0, 0, _shapeCount); } - bool collides(const Implementation::AbstractShape* a, std::size_t node, std::size_t shapeBegin, std::size_t shapeEnd) const; + bool collides(const Implementation::AbstractShape& a, std::size_t node, std::size_t shapeBegin, std::size_t shapeEnd) const; template constexpr static std::size_t shapeCount(const T&) { return 1; diff --git a/src/Shapes/Implementation/CollisionDispatch.cpp b/src/Shapes/Implementation/CollisionDispatch.cpp index f359d059f..d4c0e8602 100644 --- a/src/Shapes/Implementation/CollisionDispatch.cpp +++ b/src/Shapes/Implementation/CollisionDispatch.cpp @@ -35,13 +35,13 @@ namespace Magnum { namespace Shapes { namespace Implementation { -template<> bool collides(const AbstractShape<2>* const a, const AbstractShape<2>* const b) { - if(a->type() < b->type()) return collides(b, a); +template<> bool collides(const AbstractShape<2>& a, const AbstractShape<2>& b) { + if(a.type() < b.type()) return collides(b, a); - switch(UnsignedInt(a->type())*UnsignedInt(b->type())) { + switch(UnsignedInt(a.type())*UnsignedInt(b.type())) { #define _c(aType, aClass, bType, bClass) \ case UnsignedInt(ShapeDimensionTraits<2>::Type::aType)*UnsignedInt(ShapeDimensionTraits<2>::Type::bType): \ - return static_cast*>(a)->shape % static_cast*>(b)->shape; + return static_cast&>(a).shape % static_cast&>(b).shape; _c(Sphere, Sphere2D, Point, Point2D) _c(Sphere, Sphere2D, Line, Line2D) _c(Sphere, Sphere2D, LineSegment, LineSegment2D) @@ -57,13 +57,13 @@ template<> bool collides(const AbstractShape<2>* const a, const AbstractShape<2> return false; } -template<> bool collides(const AbstractShape<3>* const a, const AbstractShape<3>* const b) { - if(a->type() < b->type()) return collides(b, a); +template<> bool collides(const AbstractShape<3>& a, const AbstractShape<3>& b) { + if(a.type() < b.type()) return collides(b, a); - switch(UnsignedInt(a->type())*UnsignedInt(b->type())) { + switch(UnsignedInt(a.type())*UnsignedInt(b.type())) { #define _c(aType, aClass, bType, bClass) \ case UnsignedInt(ShapeDimensionTraits<3>::Type::aType)*UnsignedInt(ShapeDimensionTraits<3>::Type::bType): \ - return static_cast*>(a)->shape % static_cast*>(b)->shape; + return static_cast&>(a).shape % static_cast&>(b).shape; _c(Sphere, Sphere3D, Point, Point3D) _c(Sphere, Sphere3D, Line, Line3D) _c(Sphere, Sphere3D, LineSegment, LineSegment3D) diff --git a/src/Shapes/Implementation/CollisionDispatch.h b/src/Shapes/Implementation/CollisionDispatch.h index 99517c970..a226e822c 100644 --- a/src/Shapes/Implementation/CollisionDispatch.h +++ b/src/Shapes/Implementation/CollisionDispatch.h @@ -39,7 +39,7 @@ multiply the two numbers together and switch() on the result. Because of multiplying two prime numbers, there is no ambiguity (the result is unique for each combination). */ -template bool collides(const AbstractShape* a, const AbstractShape* b); +template bool collides(const AbstractShape& a, const AbstractShape& b); }}} diff --git a/src/Shapes/Shape.h b/src/Shapes/Shape.h index 71854bd5c..e4ac03523 100644 --- a/src/Shapes/Shape.h +++ b/src/Shapes/Shape.h @@ -56,7 +56,7 @@ islands. @code Shapes::ShapeGroup3D shapes; -Object3D* object; +Object3D object; auto shape = new Shapes::Shape(object, {{}, 0.75f}, &shapes); Shapes::AbstractShape3D* firstCollision = shapes.firstCollision(shape); @@ -75,17 +75,17 @@ template class Shape: public AbstractShape { * @param shape Shape * @param group Group this shape belongs to */ - explicit Shape(SceneGraph::AbstractObject* object, const T& shape, ShapeGroup* group = nullptr): AbstractShape(object, group) { + explicit Shape(SceneGraph::AbstractObject& object, const T& shape, ShapeGroup* group = nullptr): AbstractShape(object, group) { Implementation::ShapeHelper::set(*this, shape); } /** @overload */ - explicit Shape(SceneGraph::AbstractObject* object, T&& shape, ShapeGroup* group = nullptr): AbstractShape(object, group) { + explicit Shape(SceneGraph::AbstractObject& object, T&& shape, ShapeGroup* group = nullptr): AbstractShape(object, group) { Implementation::ShapeHelper::set(*this, std::move(shape)); } /** @overload */ - explicit Shape(SceneGraph::AbstractObject* object, ShapeGroup* group = nullptr): AbstractShape(object, group) {} + explicit Shape(SceneGraph::AbstractObject& object, ShapeGroup* group = nullptr): AbstractShape(object, group) {} /** @brief Shape */ const T& shape() const { return _shape.shape; } @@ -110,8 +110,8 @@ template class Shape: public AbstractShape { void clean(const typename DimensionTraits::MatrixType& absoluteTransformationMatrix) override; private: - const Implementation::AbstractShape* abstractTransformedShape() const override { - return &_transformedShape; + const Implementation::AbstractShape& abstractTransformedShape() const override { + return _transformedShape; } Implementation::Shape _shape, _transformedShape; @@ -119,12 +119,12 @@ template class Shape: public AbstractShape { template inline Shape& Shape::setShape(const T& shape) { Implementation::ShapeHelper::set(*this, shape); - this->object()->setDirty(); + this->object().setDirty(); return *this; } template inline const T& Shape::transformedShape() { - this->object()->setClean(); + this->object().setClean(); return _transformedShape.shape; } diff --git a/src/Shapes/ShapeGroup.cpp b/src/Shapes/ShapeGroup.cpp index 7c28e30dc..a44d06480 100644 --- a/src/Shapes/ShapeGroup.cpp +++ b/src/Shapes/ShapeGroup.cpp @@ -33,7 +33,7 @@ template void ShapeGroup::setClean() { if(!this->isEmpty()) { std::vector*> objects(this->size()); for(std::size_t i = 0; i != this->size(); ++i) - objects[i] = (*this)[i]->object(); + objects[i] = &(*this)[i].object(); SceneGraph::AbstractObject::setClean(objects); } @@ -41,11 +41,11 @@ template void ShapeGroup::setClean() { dirty = false; } -template AbstractShape* ShapeGroup::firstCollision(const AbstractShape* shape) { +template AbstractShape* ShapeGroup::firstCollision(const AbstractShape& shape) { setClean(); for(std::size_t i = 0; i != this->size(); ++i) - if((*this)[i] != shape && (*this)[i]->collides(shape)) - return (*this)[i]; + if(&(*this)[i] != &shape && (*this)[i].collides(shape)) + return &(*this)[i]; return nullptr; } diff --git a/src/Shapes/ShapeGroup.h b/src/Shapes/ShapeGroup.h index bd181def2..fb683650c 100644 --- a/src/Shapes/ShapeGroup.h +++ b/src/Shapes/ShapeGroup.h @@ -86,7 +86,7 @@ template class MAGNUM_SHAPES_EXPORT ShapeGroup: public S * collisions, returns `nullptr`. Calls setClean() before the * operation. */ - AbstractShape* firstCollision(const AbstractShape* shape); + AbstractShape* firstCollision(const AbstractShape& shape); private: bool dirty; diff --git a/src/Shapes/Test/ShapeTest.cpp b/src/Shapes/Test/ShapeTest.cpp index 7c8e2d8b0..432c55a9d 100644 --- a/src/Shapes/Test/ShapeTest.cpp +++ b/src/Shapes/Test/ShapeTest.cpp @@ -60,11 +60,11 @@ void ShapeTest::clean() { ShapeGroup3D shapes; Object3D a(&scene); - auto shape = new Shapes::Shape(&a, {{1.0f, -2.0f, 3.0f}}, &shapes); + auto shape = new Shapes::Shape(a, {{1.0f, -2.0f, 3.0f}}, &shapes); a.scale(Vector3(-2.0f)); Object3D b(&scene); - new Shapes::Shape(&b, &shapes); + new Shapes::Shape(b, &shapes); /* Everything is dirty at the beginning */ CORRADE_VERIFY(shapes.isDirty()); @@ -100,13 +100,13 @@ void ShapeTest::firstCollision() { ShapeGroup3D shapes; Object3D a(&scene); - auto aShape = new Shape(&a, {{1.0f, -2.0f, 3.0f}, 1.5f}, &shapes); + Shape aShape(a, {{1.0f, -2.0f, 3.0f}, 1.5f}, &shapes); Object3D b(&scene); - auto bShape = new Shape(&b, {{3.0f, -2.0f, 3.0f}}, &shapes); + Shape bShape(b, {{3.0f, -2.0f, 3.0f}}, &shapes); Object3D c(&scene); - new Shape(&c, &shapes); + Shape cShape(c, &shapes); /* No collisions initially */ CORRADE_VERIFY(!shapes.firstCollision(aShape)); @@ -118,8 +118,8 @@ void ShapeTest::firstCollision() { /* Collision */ CORRADE_VERIFY(shapes.isDirty()); - CORRADE_VERIFY(shapes.firstCollision(aShape) == bShape); - CORRADE_VERIFY(shapes.firstCollision(bShape) == aShape); + CORRADE_VERIFY(shapes.firstCollision(aShape) == &bShape); + CORRADE_VERIFY(shapes.firstCollision(bShape) == &aShape); CORRADE_VERIFY(!shapes.isDirty()); } @@ -129,7 +129,7 @@ void ShapeTest::shapeGroup() { /* Verify construction */ Object2D a(&scene); - auto shape = new Shape(&a, Shapes::Sphere2D({}, 0.5f) || Shapes::Point2D({0.25f, -1.0f})); + auto shape = new Shape(a, Shapes::Sphere2D({}, 0.5f) || Shapes::Point2D({0.25f, -1.0f})); CORRADE_COMPARE(shape->transformedShape().size(), 2); /* Verify the original shape is updated */