From 482c69f314b1b22ba1e5c9edc700797151b56183 Mon Sep 17 00:00:00 2001 From: John Wellbelove Date: Thu, 27 Aug 2020 13:31:27 +0100 Subject: [PATCH] Small refactoring --- include/etl/bresenham_line.h | 209 +++++++++++++++++++++-------------- include/etl/utility.h | 21 ++++ test/test_bresenham_line.cpp | 99 +++++++++++++---- test/test_memory.cpp | 2 - 4 files changed, 224 insertions(+), 107 deletions(-) diff --git a/include/etl/bresenham_line.h b/include/etl/bresenham_line.h index 0fbe1eed..88e4e242 100644 --- a/include/etl/bresenham_line.h +++ b/include/etl/bresenham_line.h @@ -31,21 +31,23 @@ SOFTWARE. #ifndef ETL_BRESENHAM_LINE_INCLUDED #define ETL_BRESENHAM_LINE_INCLUDED -#include +#include #include "platform.h" #include "iterator.h" -#include "exception.h" +#include "static_assert.h" +#include "type_traits.h" +#include "utility.h" namespace etl { //*************************************************************************** /// A pseudo-container that generates points on a line, using Bresenham's /// line algorithm. - /// TCoordinate must support integral x & y members. - /// TWorking is the internal working variable type. Default is int. + /// T is the type for the etl::coordinate_2d value type. + /// TWork is the internal working variable type. Default is int16_t. //*************************************************************************** - template + template class bresenham_line { public: @@ -53,15 +55,18 @@ namespace etl //*************************************************** /// Standard container types. //*************************************************** - typedef TCoordinate value_type; - typedef size_t size_type; - typedef ptrdiff_t difference_type; - typedef value_type& reference; - typedef const value_type& const_reference; - typedef value_type* pointer; - typedef const value_type* const_pointer; + typedef etl::coordinate_2d value_type; + typedef size_t size_type; + typedef ptrdiff_t difference_type; + typedef value_type& reference; + typedef const value_type& const_reference; + typedef value_type* pointer; + typedef const value_type* const_pointer; - class iterator : public etl::iterator + //*************************************************** + /// Const Iterator + //*************************************************** + class const_iterator : public etl::iterator { public: @@ -70,7 +75,7 @@ namespace etl //*************************************************** /// Default constructor //*************************************************** - iterator() + const_iterator() : p_bresenham_line(ETL_NULLPTR) { } @@ -78,7 +83,7 @@ namespace etl //*************************************************** /// Copy constuctor //*************************************************** - iterator(const iterator& other) + const_iterator(const const_iterator& other) : p_bresenham_line(other.p_bresenham_line) { } @@ -86,7 +91,7 @@ namespace etl //*************************************************** /// Assignment operator //*************************************************** - iterator& operator =(const iterator& rhs) + const_iterator& operator =(const const_iterator& rhs) { p_bresenham_line = rhs.p_bresenham_line; @@ -96,10 +101,10 @@ namespace etl //*************************************************** /// Pre-increment operator //*************************************************** - iterator& operator ++() + const_iterator& operator ++() { // Has the end of the series has been reached? - if (p_bresenham_line->n_coordinates_remaining == TWorking(0)) + if (p_bresenham_line->n_coordinates_remaining == 0) { // Mark it as an end iterator. p_bresenham_line = ETL_NULLPTR; @@ -123,7 +128,7 @@ namespace etl //*************************************************** /// Equality operator //*************************************************** - friend bool operator ==(const iterator& lhs, const iterator& rhs) + friend bool operator ==(const const_iterator& lhs, const const_iterator& rhs) { return lhs.p_bresenham_line == rhs.p_bresenham_line; } @@ -131,7 +136,7 @@ namespace etl //*************************************************** /// Inequality operator //*************************************************** - friend bool operator !=(const iterator& lhs, const iterator& rhs) + friend bool operator !=(const const_iterator& lhs, const const_iterator& rhs) { return !(lhs == rhs); } @@ -141,27 +146,101 @@ namespace etl //*************************************************** /// Constructor for use by bresenham_line //*************************************************** - iterator(bresenham_line* pb) + const_iterator(bresenham_line* pb) : p_bresenham_line(pb) { } - bresenham_line* p_bresenham_line; + bresenham_line* p_bresenham_line; }; //*************************************************** /// Constructor. /// Supplied first and last coordinates //*************************************************** - bresenham_line(const_reference first_, const_reference last_) - : first(first_) - , coordinate(first_) - , x_increment((last_.x < first_.x) ? -1 : 1) - , y_increment((last_.y < first_.y) ? -1 : 1) - , dx((last_.x < first_.x) ? first_.x - last_.x : last_.x - first_.x) - , dy((last_.y < first_.y) ? first_.y - last_.y : last_.y - first_.y) - , do_minor_increment(false) + bresenham_line(etl::coordinate_2d first_, etl::coordinate_2d last_) { + initialise(first_.x, first_.y, last_.x, last_.y); + } + + //*************************************************** + /// Constructor. + /// Supplied first and last coordinates + //*************************************************** + bresenham_line(T first_x, T first_y, T last_x, T last_y) + { + initialise(first_x, first_y, last_x, last_y); + } + + //*************************************************** + /// Update the line. + /// Supplied first and last coordinates + //*************************************************** + void update_line(etl::coordinate_2d first_, etl::coordinate_2d last_) + { + initialise(first_.x, first_.y, last_.x, last_.y); + } + + //*************************************************** + /// Update the line. + /// Supplied first and last coordinates + //*************************************************** + void update_line(T first_x, T first_y, T last_x, T last_y) + { + initialise(first_x, first_y, last_x, last_y); + } + + //*************************************************** + /// Get a const_iterator to the first coordinate. + /// Resets the Bresenham line. + //*************************************************** + const_iterator begin() + { + n_coordinates_remaining = total_n_coordinates - 1; // We already have the first coordinate. + coordinate = first; + + return const_iterator(this); + } + + //*************************************************** + /// Get a const_iterator to one past the last coordinate. + //*************************************************** + const_iterator end() const + { + return const_iterator(); + } + + //*************************************************** + /// Get the size of the series. + //*************************************************** + size_t size() const + { + return size_t(total_n_coordinates); + } + + //*************************************************** + /// Get the current number of generated points. + //*************************************************** + size_t count() const + { + return size_t(total_n_coordinates - n_coordinates_remaining); + } + + private: + + //*************************************************** + /// Get the current number of generated points. + //*************************************************** + void initialise(T first_x, T first_y, T last_x, T last_y) + { + first = value_type(first_x, first_y); + coordinate = value_type(first_x, first_y); + x_increment = (last_x < first_x) ? -1 : 1; + y_increment = (last_y < first_y) ? -1 : 1; + dx = (last_x < first_x) ? first_x - last_x : last_x - first_x; + dy = (last_y < first_y) ? first_y - last_y : last_y - first_y; + do_minor_increment = false; + if (is_y_major_axis()) { total_n_coordinates = dy + 1; @@ -180,48 +259,10 @@ namespace etl n_coordinates_remaining = total_n_coordinates - 1; // We already have the first coordinate. } - //*************************************************** - /// Get an iterator to the first coordinate. - /// Resets the Bresenham line. - //*************************************************** - iterator begin() - { - n_coordinates_remaining = total_n_coordinates - 1; // We already have the first coordinate. - coordinate = first; - - return iterator(this); - } - - //*************************************************** - /// Get an iterator to one past the last coordinate. - //*************************************************** - iterator end() const - { - return iterator(); - } - - //*************************************************** - /// Get the size of the series. - //*************************************************** - size_t size() const - { - return total_n_coordinates; - } - - //*************************************************** - /// Get the current number of generated points. - //*************************************************** - size_t count() const - { - return total_n_coordinates - n_coordinates_remaining; - } - - private: - //*************************************************** /// Returns true if Y is the major axis. //*************************************************** - int is_y_major_axis() const + bool is_y_major_axis() const { return dx < dy; } @@ -236,11 +277,11 @@ namespace etl // Y major axis. if (do_minor_increment) { - coordinate.x += x_increment; + coordinate.x = T(coordinate.x + x_increment); balance -= dy; } - coordinate.y += y_increment; + coordinate.y = T(coordinate.y + y_increment); balance += dx; } else @@ -248,11 +289,11 @@ namespace etl // X major axis. if (do_minor_increment) { - coordinate.y += y_increment; + coordinate.y = T(coordinate.y + y_increment); balance -= dx; } - coordinate.x += x_increment; + coordinate.x = T(coordinate.x + x_increment); balance += dy; } @@ -269,18 +310,18 @@ namespace etl return coordinate; } - typedef TWorking working_t; + typedef TWork work_t; - const value_type first; - value_type coordinate; - const working_t x_increment; - const working_t y_increment; - working_t dx; - working_t dy; - working_t total_n_coordinates; - working_t n_coordinates_remaining; - working_t balance; - bool do_minor_increment; + value_type first; + value_type coordinate; + work_t x_increment; + work_t y_increment; + work_t dx; + work_t dy; + work_t total_n_coordinates; + work_t n_coordinates_remaining; + work_t balance; + bool do_minor_increment; }; } diff --git a/include/etl/utility.h b/include/etl/utility.h index e9901fb5..92960da9 100644 --- a/include/etl/utility.h +++ b/include/etl/utility.h @@ -328,6 +328,27 @@ namespace etl { return t; } + + //****************************************************************************** + /// 2D coordinate type. + template + struct coordinate_2d + { + coordinate_2d() + : x(T(0)) + , y(T(0)) + { + } + + coordinate_2d(T x_, T y_) + : x(x_) + , y(y_) + { + } + + T x; + T y; + }; } #endif diff --git a/test/test_bresenham_line.cpp b/test/test_bresenham_line.cpp index d3512658..faa86eed 100644 --- a/test/test_bresenham_line.cpp +++ b/test/test_bresenham_line.cpp @@ -26,6 +26,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ******************************************************************************/ + #include "UnitTest++/UnitTest++.h" #include "etl/bresenham_line.h" @@ -34,11 +35,9 @@ SOFTWARE. namespace { - struct Point - { - int x; - int y; - }; + using Value = char; + + using Point = etl::coordinate_2d; bool operator ==(Point lhs, Point rhs) { @@ -47,11 +46,11 @@ namespace std::ostream& operator << (std::ostream& os, Point point) { - os << "(" << point.x << "," << point.y << ")"; + os << "(" << point.x << "," << point.x << ")"; return os; } - using BresenhamLine = etl::bresenham_line; + using BresenhamLine = etl::bresenham_line; SUITE(test_bresenham_line) { @@ -61,15 +60,15 @@ namespace Point first = { -5, 5 }; Point last = { 5, 5 }; - BresenhamLine bl(first, last); + BresenhamLine bl(first.x, first.y, last.x, last.y); - std::vector expected{ Point{-5, 5 }, Point{-4, 5 }, Point{-3, 5 }, Point{-2, 5 }, Point{-1, 5 }, Point{ 0, 5 }, + std::vector expected{ Point{-5, 5 }, Point{-4, 5 }, Point{-3, 5 }, Point{-2, 5 }, Point{-1, 5 }, Point{ 0, 5 }, Point{ 1, 5 }, Point{ 2, 5 }, Point{ 3, 5 }, Point{ 4, 5 }, Point{ 5, 5} }; std::vector actual; CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -93,7 +92,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -117,7 +116,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -141,7 +140,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -165,7 +164,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -189,7 +188,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -213,7 +212,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -237,7 +236,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -261,7 +260,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -285,7 +284,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -309,7 +308,7 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } @@ -333,7 +332,65 @@ namespace CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. - for (BresenhamLine::iterator itr = bl.begin(); itr != bl.end(); ++itr) + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) + { + actual.push_back(*itr); + } + + CHECK_EQUAL(expected.size(), bl.count()); + CHECK_EQUAL(expected.size(), actual.size()); + CHECK_ARRAY_EQUAL(expected.data(), actual.data(), (std::max(expected.size(), actual.size()))); + } + + //************************************************************************* + TEST(update_the_line_from_coordinate_2d) + { + Point first = { 3, -5 }; + Point last = { -3, 5 }; + + BresenhamLine bl(first, last); + + Point new_first = { -3, 5 }; + Point new_last = { 3, -5 }; + + bl.update_line(new_first, new_last); + + std::vector expected{ Point{ -3, 5 }, Point{ -3, 4 }, Point{ -2, 3 }, Point{ -1, 2 }, Point{ -1, 1 }, Point{ 0, 0 }, + Point{ 1, -1 }, Point{ 1, -2 }, Point{ 2, -3 }, Point{ 2, -4 }, Point{ 3, -5 } }; + std::vector actual; + + CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. + + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) + { + actual.push_back(*itr); + } + + CHECK_EQUAL(expected.size(), bl.count()); + CHECK_EQUAL(expected.size(), actual.size()); + CHECK_ARRAY_EQUAL(expected.data(), actual.data(), (std::max(expected.size(), actual.size()))); + } + + //************************************************************************* + TEST(update_the_line_from_individual_coordinates) + { + Point first = { 3, -5 }; + Point last = { -3, 5 }; + + BresenhamLine bl(first, last); + + Point new_first = { -3, 5 }; + Point new_last = { 3, -5 }; + + bl.update_line(new_first.x, new_first.y, new_last.x, new_last.y); + + std::vector expected{ Point{ -3, 5 }, Point{ -3, 4 }, Point{ -2, 3 }, Point{ -1, 2 }, Point{ -1, 1 }, Point{ 0, 0 }, + Point{ 1, -1 }, Point{ 1, -2 }, Point{ 2, -3 }, Point{ 2, -4 }, Point{ 3, -5 } }; + std::vector actual; + + CHECK_EQUAL(1U, bl.count()); // We already have the first coordinate. + + for (BresenhamLine::const_iterator itr = bl.begin(); itr != bl.end(); ++itr) { actual.push_back(*itr); } diff --git a/test/test_memory.cpp b/test/test_memory.cpp index 8a01ecf8..68f86d43 100644 --- a/test/test_memory.cpp +++ b/test/test_memory.cpp @@ -887,7 +887,6 @@ namespace { etl::unique_ptr up(new int); - int* p = new int(1); up = nullptr; CHECK(up.get() == nullptr); @@ -1018,7 +1017,6 @@ namespace { etl::unique_ptr up(new int[4]); - int* p = new int[4]; up = nullptr; CHECK(up.get() == nullptr);