From d9e7096a9bb6296a075054eb7b275a16e8f54890 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 17 Nov 2013 19:16:53 +0100 Subject: [PATCH] Shapes: use Containers::Array instead of naked array in Composition. Makes the copying code simpler. Also added assertion that we copy into available memory and not somewhere out of bounds. The test now fails the assertion. --- src/Shapes/Composition.cpp | 49 ++++++++++++++------------------------ src/Shapes/Composition.h | 20 ++++++++-------- 2 files changed, 28 insertions(+), 41 deletions(-) diff --git a/src/Shapes/Composition.cpp b/src/Shapes/Composition.cpp index 7a9fe3857..08c424180 100644 --- a/src/Shapes/Composition.cpp +++ b/src/Shapes/Composition.cpp @@ -59,42 +59,30 @@ Because these values are relative to parent, they don't need to be modified when concatenating. */ -template Composition::Composition(const Composition& other): _shapeCount(other._shapeCount), _nodeCount(other._nodeCount) { +template Composition::Composition(const Composition& other) { copyShapes(0, other); copyNodes(0, other); } -template Composition::Composition(Composition&& other): _shapeCount(other._shapeCount), _nodeCount(other._nodeCount), _shapes(other._shapes), _nodes(other._nodes) { +template Composition::Composition(Composition&& other): _shapes(std::move(other._shapes)), _nodes(std::move(other._nodes)) { other._shapes = nullptr; - other._shapeCount = 0; - other._nodes = nullptr; - other._nodeCount = 0; } template Composition::~Composition() { - for(std::size_t i = 0; i != _shapeCount; ++i) + for(std::size_t i = 0; i != _shapes.size(); ++i) delete _shapes[i]; - - delete[] _shapes; - delete[] _nodes; } template Composition& Composition::operator=(const Composition& other) { - for(std::size_t i = 0; i != _shapeCount; ++i) + for(std::size_t i = 0; i != _shapes.size(); ++i) delete _shapes[i]; - if(_shapeCount != other._shapeCount) { - delete[] _shapes; - _shapeCount = other._shapeCount; - _shapes = new Implementation::AbstractShape*[_shapeCount]; - } + if(_shapes.size() != other._shapes.size()) + _shapes = Containers::Array*>(other._shapes.size()); - if(_nodeCount != other._nodeCount) { - delete[] _nodes; - _nodeCount = other._nodeCount; - _nodes = new Node[_nodeCount]; - } + if(_nodes.size() != other._nodes.size()) + _nodes = Containers::Array(other._nodes.size()); copyShapes(0, other); copyNodes(0, other); @@ -102,33 +90,32 @@ template Composition& Composition Composition& Composition::operator=(Composition&& other) { - std::swap(other._shapeCount, _shapeCount); - std::swap(other._nodeCount, _nodeCount); std::swap(other._shapes, _shapes); std::swap(other._nodes, _nodes); return *this; } template void Composition::copyShapes(const std::size_t offset, Composition&& other) { - std::move(other._shapes, other._shapes+other._shapeCount, _shapes+offset); - delete[] other._shapes; + CORRADE_INTERNAL_ASSERT(_shapes.size() >= other._shapes.size()+offset); + std::move(other._shapes.begin(), other._shapes.end(), _shapes.begin()+offset); other._shapes = nullptr; - other._shapeCount = 0; } template void Composition::copyShapes(const std::size_t offset, const Composition& other) { - for(std::size_t i = 0; i != other._shapeCount; ++i) - _shapes[i+offset] = other._shapes[i]->clone(); + CORRADE_INTERNAL_ASSERT(_shapes.size() >= other._shapes.size()+offset); + for(Implementation::AbstractShape * const* i = other._shapes.begin(), ** o = _shapes.begin()+offset; i != other._shapes.end(); ++i, ++o) + *o = (*i)->clone(); } template void Composition::copyNodes(std::size_t offset, const Composition& other) { - std::copy(other._nodes, other._nodes+other._nodeCount, _nodes+offset); + CORRADE_INTERNAL_ASSERT(_nodes.size() >= other._nodes.size()+offset); + std::copy(other._nodes.begin(), other._nodes.end(), _nodes.begin()+offset); } template Composition Composition::transformed(const typename DimensionTraits::MatrixType& matrix) const { Composition out(*this); - for(std::size_t i = 0; i != _shapeCount; ++i) - _shapes[i]->transform(matrix, out._shapes[i]); + for(Implementation::AbstractShape * const* i = _shapes.begin(), * const* o = out._shapes.begin(); i != _shapes.end(); ++i, ++o) + (*i)->transform(matrix, *o); return out; } @@ -136,7 +123,7 @@ template bool Composition::collides(const Im /* Empty group */ if(shapeBegin == shapeEnd) return false; - CORRADE_INTERNAL_ASSERT(node < _nodeCount && shapeBegin < shapeEnd); + CORRADE_INTERNAL_ASSERT(node < _nodes.size() && shapeBegin < shapeEnd); /* Collision on the left child. If the node is leaf one (no left child exists), do it directly, recurse instead. */ diff --git a/src/Shapes/Composition.h b/src/Shapes/Composition.h index 55431878b..c07768397 100644 --- a/src/Shapes/Composition.h +++ b/src/Shapes/Composition.h @@ -30,6 +30,7 @@ #include #include +#include #include #include "DimensionTraits.h" @@ -95,7 +96,7 @@ template class MAGNUM_SHAPES_EXPORT Composition { * * Creates empty hierarchy. */ - explicit Composition(): _shapeCount(0), _nodeCount(0), _shapes(nullptr), _nodes(nullptr) {} + explicit Composition() {} /** * @brief Unary operation constructor @@ -130,7 +131,7 @@ template class MAGNUM_SHAPES_EXPORT Composition { Composition transformed(const typename DimensionTraits::MatrixType& matrix) const; /** @brief Count of shapes in the hierarchy */ - std::size_t size() const { return _shapeCount; } + std::size_t size() const { return _shapes.size(); } /** @brief Type of shape at given position */ Type type(std::size_t i) const { return _shapes[i]->type(); } @@ -154,7 +155,7 @@ template class MAGNUM_SHAPES_EXPORT Composition { }; bool collides(const Implementation::AbstractShape& a) const { - return collides(a, 0, 0, _shapeCount); + return collides(a, 0, 0, _shapes.size()); } bool collides(const Implementation::AbstractShape& a, std::size_t node, std::size_t shapeBegin, std::size_t shapeEnd) const; @@ -163,13 +164,13 @@ template class MAGNUM_SHAPES_EXPORT Composition { return 1; } constexpr static std::size_t shapeCount(const Composition& hierarchy) { - return hierarchy._shapeCount; + return hierarchy._shapes.size(); } template constexpr static std::size_t nodeCount(const T&) { return 0; } constexpr static std::size_t nodeCount(const Composition& hierarchy) { - return hierarchy._nodeCount; + return hierarchy._nodes.size(); } template void copyShapes(std::size_t offset, const T& shape) { @@ -181,9 +182,8 @@ template class MAGNUM_SHAPES_EXPORT Composition { template void copyNodes(std::size_t, const T&) {} void copyNodes(std::size_t offset, const Composition& other); - std::size_t _shapeCount, _nodeCount; - Implementation::AbstractShape** _shapes; - Node* _nodes; + Containers::Array*> _shapes; + Containers::Array _nodes; }; /** @brief Two-dimensional shape hierarchy */ @@ -255,7 +255,7 @@ template inline auto operator||(T&& a, U&& b) -> enableIfAreSh #undef enableIfAreShapeType #endif -template template Composition::Composition(CompositionOperation operation, T&& a): _shapeCount(shapeCount(a)), _nodeCount(nodeCount(a)+1), _shapes(new Implementation::AbstractShape*[_shapeCount]), _nodes(new Node[_nodeCount]) { +template template Composition::Composition(CompositionOperation operation, T&& a): _shapes(shapeCount(a)), _nodes(nodeCount(a)+1) { CORRADE_ASSERT(operation == CompositionOperation::Not, "Shapes::Composition::Composition(): unary operation expected", ); _nodes[0].operation = operation; @@ -267,7 +267,7 @@ template template Composition::Comp copyShapes(0, std::forward(a)); } -template template Composition::Composition(CompositionOperation operation, T&& a, U&& b): _shapeCount(shapeCount(a) + shapeCount(b)), _nodeCount(nodeCount(a) + nodeCount(b) + 1), _shapes(new Implementation::AbstractShape*[_shapeCount]), _nodes(new Node[_nodeCount]) { +template template Composition::Composition(CompositionOperation operation, T&& a, U&& b): _shapes(shapeCount(a) + shapeCount(b)), _nodes(nodeCount(a) + nodeCount(b) + 1) { CORRADE_ASSERT(operation != CompositionOperation::Not, "Shapes::Composition::Composition(): binary operation expected", ); _nodes[0].operation = operation;