From d9e5b51f47f5d9fbfe619bbd738f69e1ab72bad4 Mon Sep 17 00:00:00 2001 From: Jeremiah Morgan Date: Thu, 26 Dec 2024 15:44:48 +0000 Subject: [PATCH] address comments --- libopenage/curve/array.h | 94 +++++++++++++++----------- libopenage/curve/base_curve.h | 4 +- libopenage/curve/tests/container.cpp | 84 ++++++++++++++++++++++- libopenage/curve/tests/curve_types.cpp | 81 ---------------------- 4 files changed, 139 insertions(+), 124 deletions(-) diff --git a/libopenage/curve/array.h b/libopenage/curve/array.h index 7076082459..34109365db 100644 --- a/libopenage/curve/array.h +++ b/libopenage/curve/array.h @@ -16,10 +16,10 @@ template class Array : event::EventEntity { public: Array(const std::shared_ptr &loop, - size_t id, - const std::string &idstr = "", - const EventEntity::single_change_notifier ¬ifier = nullptr) : - EventEntity(loop, notifier), _id{id}, _idstr{idstr}, loop{loop}{} + size_t id, + const std::string &idstr = "", + const EventEntity::single_change_notifier ¬ifier = nullptr) : + EventEntity(loop, notifier), _id{id}, _idstr{idstr}, loop{loop} {} Array(const Array &) = delete; @@ -28,16 +28,16 @@ class Array : event::EventEntity { * Get the last element with elem->time <= time from * the keyfram container at a given index */ - T get(const time::time_t &t, const size_t index) const; + T at(const time::time_t &t, const size_t index) const; /** * Get an array of the last elements with elem->time <= time from * all keyfram containers contained within this array curve */ - std::array get_all(const time::time_t &t) const; - + std::array get(const time::time_t &t) const; + // Get the amount of KeyframeContainers in array curve consteval size_t size() const; /** @@ -107,92 +107,107 @@ class Array : event::EventEntity { } - KeyframeContainer& operator[] (size_t index) - { - return this->container[index]; - } - - KeyframeContainer operator[] (size_t index) const - { - return this->container[index]; + // get a copy to the KeyframeContainer at index + KeyframeContainer operator[](size_t index) const { + return this->container.at(index); } + // Array::Iterator is used to iterate over KeyframeContainers contained in a curve at a given time. class Iterator { public: Iterator(Array *curve, const time::time_t &time = time::TIME_MAX, size_t offset = 0) : curve(curve), time(time), offset(offset) {}; + // returns a copy of the keyframe at the current offset and time const T operator*() { - return curve->frame(this->time, this->offset).second; + return this->curve->frame(this->time, this->offset).second; } + // increments the Iterator to point at the next KeyframeContainer void operator++() { this->offset++; } + // Compare two Iterators by their offset bool operator!=(const Array::Iterator &rhs) const { return this->offset != rhs.offset; } private: + // used to index the Curve::Array pointed to by this iterator size_t offset; + + // curve::Array that this iterator is iterating over Array *curve; + + // time at which this iterator is iterating over time::time_t time; }; - Iterator begin(const time::time_t &time = time::TIME_MAX); + // iterator pointing to a keyframe of the first KeyframeContainer in the curve at a given time + Iterator begin(const time::time_t &time = time::TIME_MIN); - Iterator end(const time::time_t &time = time::TIME_MAX); + // iterator pointing after the last KeyframeContainer in the curve at a given time + Iterator end(const time::time_t &time = time::TIME_MIN); private: std::array, Size> container; - //hint for KeyframeContainer operations + /** + * hints for KeyframeContainer operations, mutable as hints + * are updated by const read functions. + * This is used to speed up the search for next keyframe to be accessed + */ mutable std::array last_element = {}; /** - * Identifier for the container - */ + * Identifier for the container + */ const size_t _id; /** - * Human-readable identifier for the container - */ + * Human-readable identifier for the container + */ const std::string _idstr; /** - * The eventloop this curve was registered to - */ + * The eventloop this curve was registered to + */ const std::shared_ptr loop; }; template std::pair Array::frame(const time::time_t &t, const size_t index) const { - size_t frmae_index = container[index].last(t, this->last_element[index]); - this->last_element[index] = frmae_index; - return container[index].get(frmae_index).make_pair(); + size_t &hint = this->last_element[index]; + size_t frame_index = this->container.at(index).last(t, hint); + hint = frame_index; + return this->container.at(index).get(frame_index).make_pair(); } template std::pair Array::next_frame(const time::time_t &t, const size_t index) const { - size_t frmae_index = container[index].last(t, this->last_element[index]); - this->last_element[index] = frmae_index; - return container[index].get(frmae_index + 1).make_pair(); + size_t &hint = this->last_element[index]; + size_t frame_index = this->container.at(index).last(t, hint); + hint = frame_index; + return this->container.at(index).get(frame_index + 1).make_pair(); } template -T Array::get(const time::time_t &t, const size_t index) const { - return this->frame(t, index).second; +T Array::at(const time::time_t &t, const size_t index) const { + size_t &hint = this->last_element[index]; + size_t frame_index = this->container.at(index).last(t, hint); + hint = frame_index; + return this->container.at(index).get(frame_index).val(); } template -std::array Array::get_all(const time::time_t &t) const { +std::array Array::get(const time::time_t &t) const { return [&](std::index_sequence) { - return std::array{this->get(t, I)...}; + return std::array{this->at(t, I)...}; }(std::make_index_sequence{}); } @@ -204,22 +219,21 @@ consteval size_t Array::size() const { template void Array::set_insert(const time::time_t &t, const size_t index, T value) { - this->last_element[index] = this->container[index].insert_after(Keyframe(t, value), this->last_element[index]); + this->last_element[index] = this->container.at(index).insert_after(Keyframe{t, value}, this->last_element[index]); } template void Array::set_last(const time::time_t &t, const size_t index, T value) { - - size_t frame_index = this->container[index].insert_after(Keyframe(t, value), this->last_element[index]); + size_t frame_index = this->container.at(index).insert_after(Keyframe{t, value}, this->last_element[index]); this->last_element[index] = frame_index; - this->container[index].erase_after(frame_index); + this->container.at(index).erase_after(frame_index); } template void Array::set_replace(const time::time_t &t, const size_t index, T value) { - this->container[index].insert_overwrite(Keyframe(t, value), this->last_element[index]); + this->container.at(index).insert_overwrite(Keyframe{t, value}, this->last_element[index]); } template diff --git a/libopenage/curve/base_curve.h b/libopenage/curve/base_curve.h index dc58885b98..3d2ff20343 100644 --- a/libopenage/curve/base_curve.h +++ b/libopenage/curve/base_curve.h @@ -245,7 +245,7 @@ template std::pair BaseCurve::frame(const time::time_t &time) const { auto e = this->container.last(time, this->container.size()); auto elem = this->container.get(e); - return std::make_pair(elem.time(), elem.val()); + return elem.make_pair(); } @@ -254,7 +254,7 @@ std::pair BaseCurve::next_frame(const time::time_t &ti auto e = this->container.last(time, this->container.size()); e++; auto elem = this->container.get(e); - return std::make_pair(elem.time(), elem.val()); + return elem.make_pair(); } template diff --git a/libopenage/curve/tests/container.cpp b/libopenage/curve/tests/container.cpp index 020e2aad99..51a5e4bee2 100644 --- a/libopenage/curve/tests/container.cpp +++ b/libopenage/curve/tests/container.cpp @@ -9,6 +9,7 @@ #include #include +#include "curve/array.h" #include "curve/iterator.h" #include "curve/map.h" #include "curve/map_filter_iterator.h" @@ -56,7 +57,7 @@ void test_map() { // Basic tests test lookup in the middle of the range. { - auto t = map.at(2, 0); //At timestamp 2 element 0 + auto t = map.at(2, 0); // At timestamp 2 element 0 TESTEQUALS(t.has_value(), true); TESTEQUALS(t.value().value(), 0); t = map.at(20, 5); @@ -242,11 +243,92 @@ void test_queue() { TESTEQUALS(q.empty(100001), false); } +void test_array() { + auto f = std::make_shared(); + + Array a(f, 0); + a.set_insert(1, 0, 0); + a.set_insert(1, 1, 1); + a.set_insert(1, 2, 2); + a.set_insert(1, 3, 3); + // a = [[0:0, 1:0],[0:0, 1:1],[0:0, 1:2],[0:0, 1:3]] + + auto res = a.get(1); + TESTEQUALS(res.at(0), 0); + TESTEQUALS(res.at(1), 1); + TESTEQUALS(res.at(2), 2); + TESTEQUALS(res.at(3), 3); + + Array other(f, 0); + other.set_last(0, 0, 999); + other.set_last(0, 1, 999); + other.set_last(0, 2, 999); + other.set_last(0, 3, 999); + + other.set_insert(1, 0, 4); + other.set_insert(1, 1, 5); + other.set_insert(1, 2, 6); + other.set_insert(1, 3, 7); + // other = [[0:999, 1:4],[0:999, 1:5],[0:999, 1:6],[0:999, 1:7]] + + + a.sync(other, 1); + // a = [[0:0, 1:4],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] + + res = a.get(0); + TESTEQUALS(res.at(0), 0); + TESTEQUALS(res.at(1), 0); + TESTEQUALS(res.at(2), 0); + TESTEQUALS(res.at(3), 0); + res = a.get(1); + TESTEQUALS(res.at(0), 4); + TESTEQUALS(res.at(1), 5); + TESTEQUALS(res.at(2), 6); + TESTEQUALS(res.at(3), 7); + + // Additional tests + a.set_insert(2, 0, 15); + a.set_insert(2, 0, 20); + a.set_replace(2, 0, 25); + TESTEQUALS(a.at(2, 0), 25); + // a = [[0:0, 1:4, 2:25],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] + + a.set_insert(3, 0, 30); + a.set_insert(4, 0, 40); + a.set_last(3, 0, 35); + TESTEQUALS(a.at(4, 0), 35); + // a = [[0:0, 1:4, 2:25, 3:35],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] + + auto frame = a.frame(1, 2); + TESTEQUALS(frame.second, 6); + TESTEQUALS(frame.first, 1); + + a.set_insert(5, 3, 40); + auto next_frame = a.next_frame(1, 3); + TESTEQUALS(next_frame.second, 40); + TESTEQUALS(next_frame.first, 5); + + // Test operator[] + TESTEQUALS(a[0].get(a[0].last(2)).val(), 25); + TESTEQUALS(a[1].get(a[1].last(2)).val(), 5); + + // Test begin and end + auto it = a.begin(1); + TESTEQUALS(*it, 4); + ++it; + TESTEQUALS(*it, 5); + ++it; + TESTEQUALS(*it, 6); + ++it; + TESTEQUALS(*it, 7); +} + void container() { test_map(); test_list(); test_queue(); + test_array(); } diff --git a/libopenage/curve/tests/curve_types.cpp b/libopenage/curve/tests/curve_types.cpp index 6c754e5df9..935aa7141d 100644 --- a/libopenage/curve/tests/curve_types.cpp +++ b/libopenage/curve/tests/curve_types.cpp @@ -5,7 +5,6 @@ #include #include -#include "curve/array.h" #include "curve/continuous.h" #include "curve/discrete.h" #include "curve/discrete_mod.h" @@ -387,86 +386,6 @@ void curve_types() { TESTEQUALS(c.get(1), 0); TESTEQUALS(c.get(5), 0); } - - { // array - auto f = std::make_shared(); - - Array a(f,0); - a.set_insert(time::time_t(1), 0, 0); - a.set_insert(time::time_t(1), 1, 1); - a.set_insert(time::time_t(1), 2, 2); - a.set_insert(time::time_t(1), 3, 3); - //a = [[0:0, 1:0],[0:0, 1:1],[0:0, 1:2],[0:0, 1:3]] - - auto res = a.get_all(time::time_t(1)); - TESTEQUALS(res[0], 0); - TESTEQUALS(res[1], 1); - TESTEQUALS(res[2], 2); - TESTEQUALS(res[3], 3); - - Array other(f,0); - other[0].last(999); - other[1].last(999); - other[2].last(999); - other[3].last(999); - other.set_insert(time::time_t(1), 0, 4); - other.set_insert(time::time_t(1), 1, 5); - other.set_insert(time::time_t(1), 2, 6); - other.set_insert(time::time_t(1), 3, 7); - //other = [[0:999, 1:4],[0:999, 1:5],[0:999, 1:6],[0:999, 1:7]] - - - a.sync(other, time::time_t(1)); - //a = [[0:0, 1:4],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] - - res = a.get_all(time::time_t(0)); - TESTEQUALS(res[0], 0); - TESTEQUALS(res[1], 0); - TESTEQUALS(res[2], 0); - TESTEQUALS(res[3], 0); - res = a.get_all(time::time_t(1)); - TESTEQUALS(res[0], 4); - TESTEQUALS(res[1], 5); - TESTEQUALS(res[2], 6); - TESTEQUALS(res[3], 7); - - // Additional tests - a.set_insert(time::time_t(2), 0, 15); - a.set_insert(time::time_t(2), 0, 20); - a.set_replace(time::time_t(2), 0, 25); - TESTEQUALS(a.get(time::time_t(2), 0), 25); - // a = [[0:0, 1:4, 2:25],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] - - a.set_insert(time::time_t(3), 0, 30); - a.set_insert(time::time_t(4), 0, 40); - a.set_last(time::time_t(3), 0, 35); - TESTEQUALS(a.get(time::time_t(3), 0), 35); - // a = [[0:0, 1:4, 2:25, 3:35],[0:0, 1:5],[0:0, 1:6],[0:0, 1:7]] - - auto frame = a.frame(time::time_t(1), 2); - TESTEQUALS(frame.second, 6); - TESTEQUALS(frame.first, time::time_t(1)); - - a.set_insert(time::time_t(5), 3, 40); - auto next_frame = a.next_frame(time::time_t(1), 3); - TESTEQUALS(next_frame.second, 40); - TESTEQUALS(next_frame.first, time::time_t(5)); - - // Test operator[] - TESTEQUALS(a[0].get(a[0].last(time::time_t(2))).val(), 25); - TESTEQUALS(a[1].get(a[1].last(time::time_t(2))).val(), 5); - - // Test begin and end - auto it = a.begin(time::time_t(1)); - TESTEQUALS(*it, 4); - ++it; - TESTEQUALS(*it, 5); - ++it; - TESTEQUALS(*it, 6); - ++it; - TESTEQUALS(*it, 7); - ++it; - } } } // namespace openage::curve::tests