From cd77b1a7e6a4963bebb19cdd8d4c209ce8745548 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Sun, 19 Jun 2022 19:17:52 -0400 Subject: [PATCH] use vector as overflow instead of secondary storage for winding --- common/debugger.natvis | 10 +- common/test.cc | 120 ++++++++++++++++++ include/common/polylib.hh | 257 ++++++++++++++------------------------ 3 files changed, 216 insertions(+), 171 deletions(-) diff --git a/common/debugger.natvis b/common/debugger.natvis index 0e57e8c1..3578155f 100644 --- a/common/debugger.natvis +++ b/common/debugger.natvis @@ -21,12 +21,12 @@ {count} points - vector - - 1 + count + count - &array[0] - + array[$i] + vector[$i - $T1] + diff --git a/common/test.cc b/common/test.cc index 4abb7669..24b0f164 100644 --- a/common/test.cc +++ b/common/test.cc @@ -370,3 +370,123 @@ TEST_CASE("resetContainer", "[settings]") CHECK(settings::source::DEFAULT == stringSetting1.getSource()); CHECK("abc" == stringSetting1.value()); } + +// this is insanely dumb, is there a better way of doing this? +#define private public +#include "common/polylib.hh" +#undef private + +TEST_CASE("winding iterators", "[winding_base_t]") +{ + polylib::winding_base_t<4> winding; + + CHECK(winding.begin() == winding.end()); + + winding.emplace_back(0, 0, 0); + + CHECK(winding.begin() != winding.end()); + + winding.emplace_back(1, 1, 1); + winding.emplace_back(2, 2, 2); + winding.emplace_back(3, 3, 3); + + CHECK(winding.size() == 4); + + CHECK(winding.vector.size() == 0); + + // check that iterators match up before expansion + { + auto it = winding.begin(); + + for (size_t i = 0; i < winding.size(); i++) { + CHECK((*it)[0] == i); + + CHECK(it == (winding.begin() + i)); + + it++; + } + + CHECK(it == winding.end()); + } + + winding.emplace_back(4, 4, 4); + winding.emplace_back(5, 5, 5); + + // check that iterators match up after expansion + { + auto it = winding.begin(); + + for (size_t i = 0; i < winding.size(); i++) { + CHECK((*it)[0] == i); + + auto composed_it = winding.begin() + i; + CHECK(it == composed_it); + + it++; + } + + CHECK(it == winding.end()); + } + + // check that constructors work + { + polylib::winding_base_t<4> winding_other(winding); + + { + auto it = winding_other.begin(); + + for (size_t i = 0; i < winding_other.size(); i++) { + CHECK((*it)[0] == i); + + auto composed_it = winding_other.begin() + i; + CHECK(it == composed_it); + + it++; + } + + CHECK(it == winding_other.end()); + } + } + + + { + polylib::winding_base_t<4> winding_other({ { 0, 0, 0 }, { 1, 1, 1 }, { 2, 2, 2 }, { 3, 3, 3 }, { 4, 4, 4 } }); + + { + auto it = winding_other.begin(); + + for (size_t i = 0; i < winding_other.size(); i++) { + CHECK((*it)[0] == i); + + auto composed_it = winding_other.begin() + i; + CHECK(it == composed_it); + + it++; + } + + CHECK(it == winding_other.end()); + } + } + + { + polylib::winding_base_t<4> winding_other(std::move(winding)); + + CHECK(winding.size() == 0); + CHECK(winding.begin() == winding.end()); + + { + auto it = winding_other.begin(); + + for (size_t i = 0; i < winding_other.size(); i++) { + CHECK((*it)[0] == i); + + auto composed_it = winding_other.begin() + i; + CHECK(it == composed_it); + + it++; + } + + CHECK(it == winding_other.end()); + } + } +} diff --git a/include/common/polylib.hh b/include/common/polylib.hh index fe43c13d..f1c7ac90 100644 --- a/include/common/polylib.hh +++ b/include/common/polylib.hh @@ -36,7 +36,7 @@ inline bool PointInWindingEdges(const winding_edges_t &wi, const qvec3d &point) } // Polygonal winding; uses stack allocation for the first N -// points, and moves to a dynamic array after that. +// points, and uses a dynamic vector for storage after that. template struct winding_base_t { @@ -45,83 +45,63 @@ private: using vector_type = std::vector; using variant_type = std::variant; size_t count = 0; - bool isVector = false; array_type array; vector_type vector; public: - template + template class iterator_base { - std::variant it; + friend struct winding_base_t; + + using container_type = typename std::conditional_t; + size_t index; + container_type w; + + iterator_base(size_t index_in, container_type w_in) : index(index_in), w(w_in) { } public: - using iterator_category = typename vector_iterator::iterator_category; - using value_type = typename vector_iterator::value_type; - using difference_type = typename vector_iterator::difference_type; - using pointer = typename vector_iterator::pointer; - using reference = typename vector_iterator::reference; + using iterator_category = std::random_access_iterator_tag; + using value_type = typename std::conditional_t; + using difference_type = ptrdiff_t; + using pointer = value_type*; + using reference = value_type&; - iterator_base(array_iterator it) : it(it) { } + iterator_base(const iterator_base &) = default; + iterator_base &operator=(const iterator_base &) noexcept = default; - iterator_base(vector_iterator it) : it(it) { } - - [[nodiscard]] constexpr reference operator*() const noexcept + [[nodiscard]] inline reference operator*() const noexcept { - if (std::holds_alternative(it)) - return *std::get(it); - return *std::get(it); + return (*w)[index]; } - constexpr iterator_base &operator=(const iterator_base &) noexcept = default; - constexpr iterator_base &operator++() noexcept { - if (std::holds_alternative(it)) - ++std::get(it); - else - ++std::get(it); - + index++; return *this; } constexpr iterator_base &operator++(int) noexcept { - if (std::holds_alternative(it)) - std::get(it)++; - else - std::get(it)++; - + index++; return *this; } constexpr iterator_base &operator--() noexcept { - if (std::holds_alternative(it)) - --std::get(it); - else - --std::get(it); - + index--; return *this; } constexpr iterator_base operator--(int) noexcept { - if (std::holds_alternative(it)) - std::get(it)--; - else - std::get(it)--; - + index--; return *this; } constexpr iterator_base &operator+=(const difference_type _Off) noexcept { - if (std::holds_alternative(it)) - std::get(it) += _Off; - else - std::get(it) += _Off; - + index += _Off; return *this; } @@ -134,63 +114,23 @@ public: constexpr iterator_base &operator-=(const difference_type _Off) noexcept { - if (std::holds_alternative(it)) - std::get(it) -= _Off; - else - std::get(it) -= _Off; - + index -= _Off; return *this; } [[nodiscard]] constexpr bool operator==(const iterator_base &_Off) const noexcept { - if (std::holds_alternative(it)) { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit == std::get(_Off.it); - } else { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit == std::get(_Off.it); - } + return w == _Off.w && index == _Off.index; } [[nodiscard]] constexpr bool operator!=(const iterator_base &_Off) const noexcept { - if (std::holds_alternative(it)) { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit != std::get(_Off.it); - } else { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit != std::get(_Off.it); - } + return !(*this == _Off); } [[nodiscard]] constexpr difference_type operator-(const iterator_base &_Off) const noexcept { - if (std::holds_alternative(it)) { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit - std::get(_Off.it); - } else { - auto sit = std::get(it); - - Q_assert(std::holds_alternative(_Off.it)); - - return sit - std::get(_Off.it); - } + return index - _Off.index; } [[nodiscard]] constexpr iterator_base operator-(const difference_type _Off) const noexcept @@ -200,7 +140,7 @@ public: return _Tmp; } - [[nodiscard]] constexpr reference operator[](const difference_type _Off) const noexcept + [[nodiscard]] inline reference operator[](const difference_type _Off) const noexcept { return *(*this + _Off); } @@ -212,35 +152,40 @@ public: // construct winding with initial size; may allocate // memory, and sets size, but does not initialize any // of them. - inline winding_base_t(const size_t &initial_size) : count(initial_size), isVector(count > N) + inline winding_base_t(const size_t &initial_size) : count(initial_size) { - if (isVector) { - vector.resize(count); + if (count > N) { + vector.resize(count - N); } } // construct winding from range. - // iterators must have operator-. + // iterators must have operator+ and operator-. template, int> = 0> - inline winding_base_t(Iter begin, Iter end) : count(end - begin), isVector(count > N) + inline winding_base_t(Iter begin, Iter end) : winding_base_t(end - begin) { - if (isVector) { - vector = std::move(vector_type(begin, end)); - } else { - std::copy(begin, end, array.begin()); + // copy the array range + std::copy_n(begin, min(count, N), array.begin()); + + // copy the vector range, if required + if (count > N) { + std::copy(begin + N, end, vector.begin()); } } // initializer list constructor inline winding_base_t(std::initializer_list l) : winding_base_t(l.begin(), l.end()) {} - // copy constructor + // copy constructor; uses optimized method of copying + // data over. inline winding_base_t(const winding_base_t ©) : winding_base_t(copy.size()) { - if (isVector) { - memcpy(&vector.front(), ©.vector.front(), count * sizeof(qvec3d)); - } else { - memcpy(&array.front(), ©.array.front(), count * sizeof(qvec3d)); + // copy array range + memcpy(&array.front(), ©.array.front(), min(count, N) * sizeof(qvec3d)); + + // copy vector range, if required + if (count > N) { + memcpy(&vector.front(), ©.vector.front(), (count - N) * sizeof(qvec3d)); } } @@ -248,14 +193,15 @@ public: inline winding_base_t(winding_base_t &&move) noexcept : count(move.count) { count = move.count; - isVector = move.isVector; - if (move.isVector) { + // blit over array data + memcpy(&array.front(), &move.array.front(), min(count, N) * sizeof(qvec3d)); + + // move vector data, if available + if (count > N) { vector = std::move(move.vector); - move.isVector = false; - } else { - memcpy(&array.front(), &move.array.front(), count * sizeof(qvec3d)); } + move.count = 0; } @@ -263,12 +209,14 @@ public: inline winding_base_t &operator=(const winding_base_t ©) { count = copy.count; - isVector = copy.isVector; - - if (isVector) { - memcpy(&vector.front(), ©.vector.front(), count * sizeof(qvec3d)); - } else { - memcpy(&array.front(), ©.array.front(), count * sizeof(qvec3d)); + + // copy array range + memcpy(&array.front(), ©.array.front(), min(count, N) * sizeof(qvec3d)); + + // copy vector range, if required + if (count > N) { + vector.resize(count - N); + memcpy(&vector.front(), ©.vector.front(), (count - N) * sizeof(qvec3d)); } return *this; @@ -278,14 +226,15 @@ public: inline winding_base_t &operator=(winding_base_t &&move) { count = move.count; - isVector = move.isVector; - if (move.isVector) { + // blit over array data + memcpy(&array.front(), &move.array.front(), min(count, N) * sizeof(qvec3d)); + + // move vector data, if available + if (count > N) { vector = std::move(move.vector); - move.isVector = false; - } else { - memcpy(&array.front(), &move.array.front(), count * sizeof(qvec3d)); } + move.count = 0; return *this; @@ -297,10 +246,6 @@ public: inline const size_t &size() const { return count; } - inline const qvec3d *data() const { return isVector ? vector.data() : array.data(); } - - inline qvec3d *data() { return isVector ? vector.data() : array.data(); } - inline qvec3d &at(const size_t &index) { #ifdef _DEBUG @@ -308,8 +253,10 @@ public: throw std::invalid_argument("index"); #endif - if (isVector) + if (index >= N) { return vector[index]; + } + return array[index]; } @@ -320,69 +267,61 @@ public: throw std::invalid_argument("index"); #endif - if (isVector) + if (index >= N) { return vector[index]; + } + return array[index]; } // un-bounds-checked inline qvec3d &operator[](const size_t &index) { - if (isVector) - return vector[index]; + if (index >= N) { + return vector[index - N]; + } + return array[index]; } // un-bounds-checked inline const qvec3d &operator[](const size_t &index) const { - if (isVector) - return vector[index]; + if (index >= N) { + return vector[index - N]; + } + return array[index]; } - using const_iterator = iterator_base; + using const_iterator = iterator_base; const const_iterator begin() const { - if (isVector) - return const_iterator(vector.begin()); - return const_iterator(array.begin()); + return const_iterator(0, this); } const const_iterator end() const { - if (isVector) - return const_iterator(vector.end()); - return const_iterator(array.begin() + count); + return const_iterator(count, this); } - using iterator = iterator_base; + using iterator = iterator_base; iterator begin() { - if (isVector) - return iterator(vector.begin()); - return iterator(array.begin()); + return iterator(0, this); } iterator end() { - if (isVector) - return iterator(vector.end()); - return iterator(array.begin() + count); + return iterator(count, this); } template qvec3d &emplace_back(Args &&...vec) { - // move us to dynamic - if (count == N) { - vector = std::move(vector_type(begin(), end())); - isVector = true; - } - count++; - if (isVector) { + if (count > N) { return vector.emplace_back(std::forward(vec)...); } @@ -395,30 +334,16 @@ public: void resize(const size_t &new_size) { - // move us to dynamic if we'll expand too big - if (new_size > N && !isVector) { - vector.resize(new_size); - std::copy(begin(), end(), vector.begin()); - } else if (isVector) { - if (new_size > N) { - vector.resize(new_size); - // move us to array if we're shrinking - } else { - std::copy_n(vector.begin(), new_size, array.begin()); - } + // resize vector if necessary + if (new_size > N) { + vector.resize(new_size - N); } count = new_size; - isVector = count > N; } void clear() { - if (isVector) { - vector.clear(); - isVector = false; - } - count = 0; }