From 9a106a6906d6a9718541633525de540e667b7625 Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Fri, 2 Feb 2024 02:02:19 +0000 Subject: Don't keep a span, create as needed Removes the error prone issue where data/size get out of sync. Fixes overflow issues leading to memory corruption of GPU data. --- lib/glContainer.h | 106 ++++++++++++++++++++++++++-------------------- test/test-glContainer.cpp | 19 ++++----- test/test-instancing.cpp | 6 +-- 3 files changed, 72 insertions(+), 59 deletions(-) diff --git a/lib/glContainer.h b/lib/glContainer.h index b4238d5..1cdeee0 100644 --- a/lib/glContainer.h +++ b/lib/glContainer.h @@ -50,84 +50,84 @@ public: begin() { map(); - return data_.begin(); + return mkspan().begin(); } [[nodiscard]] iterator end() { map(); - return data_.end(); + return mkspan().end(); } [[nodiscard]] const_iterator begin() const { map(); - return const_span {data_}.begin(); + return mkcspan().begin(); } [[nodiscard]] const_iterator end() const { map(); - return const_span {data_}.end(); + return mkcspan().end(); } [[nodiscard]] const_iterator cbegin() const { map(); - return const_span {data_}.begin(); + return mkcspan().begin(); } [[nodiscard]] const_iterator cend() const { map(); - return const_span {data_}.end(); + return mkcspan().end(); } [[nodiscard]] reverse_iterator rbegin() { map(); - return data_.rbegin(); + return mkspan().rbegin(); } [[nodiscard]] reverse_iterator rend() { map(); - return data_.rend(); + return mkspan().rend(); } [[nodiscard]] const_reverse_iterator rbegin() const { map(); - return const_span {data_}.rbegin(); + return mkcspan().rbegin(); } [[nodiscard]] const_reverse_iterator rend() const { map(); - return const_span {data_}.rend(); + return mkcspan().rend(); } [[nodiscard]] const_reverse_iterator crbegin() const { map(); - return const_span {data_}.rbegin(); + return mkcspan().rbegin(); } [[nodiscard]] const_reverse_iterator crend() const { map(); - return const_span {data_}.rend(); + return mkcspan().rend(); } [[nodiscard]] const auto & @@ -148,8 +148,8 @@ public: if (pos >= size()) { throw std::out_of_range {__FUNCTION__}; } - if (data_.data()) { - data_[pos] = value; + if (data_) { + mkspan()[pos] = value; } else { glBindBuffer(GL_ARRAY_BUFFER, buffer_); @@ -165,7 +165,7 @@ public: throw std::out_of_range {__FUNCTION__}; } map(); - return data_[pos]; + return mkspan()[pos]; } [[nodiscard]] const_reference_type @@ -175,63 +175,63 @@ public: throw std::out_of_range {__FUNCTION__}; } map(); - return data_[pos]; + return mkcspan()[pos]; } [[nodiscard]] reference_type operator[](size_type pos) { map(); - return data_[pos]; + return mkspan()[pos]; } [[nodiscard]] const_reference_type operator[](size_type pos) const { map(); - return data_[pos]; + return mkcspan()[pos]; } [[nodiscard]] pointer_type data() { map(); - return data_.data(); + return data_; } [[nodiscard]] const_pointer_type data() const { map(); - return data_.data(); + return data_; } [[nodiscard]] reference_type front() { map(); - return data_.front(); + return mkspan().front(); } [[nodiscard]] reference_type back() { map(); - return data_.back(); + return mkspan().back(); } [[nodiscard]] const_reference_type front() const { map(); - return data_.front(); + return mkcspan().front(); } [[nodiscard]] const_reference_type back() const { map(); - return data_.back(); + return mkcspan().back(); } [[nodiscard]] bool @@ -249,7 +249,7 @@ public: void unmap() const { - if (data_.data()) { + if (data_) { glUnmapNamedBuffer(buffer_); data_ = {}; } @@ -291,8 +291,10 @@ public: allocBuffer(newSize); mapForAdd(); } - for (auto uninitialised = data_.data() + size_; uninitialised < data_.data() + newSize; ++uninitialised) { - new (uninitialised) T {}; + if (newSize > size_) { + for (auto & uninitialised : mkspan().subspan(size_, newSize - size_)) { + new (&uninitialised) T {}; + } } setSize(newSize); } @@ -333,7 +335,7 @@ public: auto newSize = size_ + 1; reserve(newSize); mapForAdd(); - new (&data_[size_]) T {std::forward

(ps)...}; + new (&*end()) T {std::forward

(ps)...}; setSize(newSize); return back(); } @@ -347,10 +349,10 @@ public: const auto idx = pos - begin(); reserve(newSize); mapForAdd(); - auto newT = begin() + idx; - std::move_backward(newT, end(), end() + 1); - newT->~T(); - new (&*newT) T {std::forward

(ps)...}; + pos = begin() + idx; + std::move_backward(pos, end(), end() + 1); + pos->~T(); + new (&*pos) T {std::forward

(ps)...}; setSize(newSize); return pos; } @@ -362,7 +364,7 @@ public: auto newSize = size_ + 1; reserve(newSize); mapForAdd(); - new (&data_[size_]) T {std::move(p)}; + new (&*end()) T {std::move(p)}; setSize(newSize); return back(); } @@ -375,10 +377,10 @@ public: const auto idx = pos - begin(); reserve(newSize); mapForAdd(); - auto newT = begin() + idx; - std::move_backward(newT, end(), end() + 1); - newT->~T(); - new (&*newT) T {std::move(p)}; + pos = begin() + idx; + std::move_backward(pos, end(), end() + 1); + pos->~T(); + new (&*pos) T {std::move(p)}; setSize(newSize); return pos; } @@ -388,11 +390,9 @@ public: { if constexpr (!is_trivial_dest) { map(); - data_[--size_].~T(); - } - else { - --size_; + back().~T(); } + --size_; } void @@ -419,7 +419,7 @@ protected: void setSize(size_type s) { - data_ = {data_.data(), size_ = s}; + size_ = s; } void @@ -446,14 +446,28 @@ protected: void mapForAdd() const { - if (!data_.data()) { - data_ = {static_cast(glMapNamedBuffer(buffer_, GL_READ_WRITE)), size_}; - assert(data_.data()); + if (!data_) { + data_ = static_cast(glMapNamedBuffer(buffer_, GL_READ_WRITE)); + assert(data_); } } + span + mkspan() const + { + assert(!size_ || data_); + return span {data_, size_}; + } + + const_span + mkcspan() const + { + assert(!size_ || data_); + return const_span {data_, size_}; + } + glBuffer buffer_; std::size_t capacity_ {}; std::size_t size_ {}; - mutable span data_; + mutable T * data_; }; diff --git a/test/test-glContainer.cpp b/test/test-glContainer.cpp index f19ca74..382bd7a 100644 --- a/test/test-glContainer.cpp +++ b/test/test-glContainer.cpp @@ -23,14 +23,14 @@ BOOST_FIXTURE_TEST_SUITE(i, glContainer) BOOST_AUTO_TEST_CASE(createDestroy, *boost::unit_test::timeout(1)) { - BOOST_CHECK(!data_.data()); + BOOST_CHECK(!data_); BOOST_CHECK_NO_THROW(map()); - BOOST_REQUIRE(!data_.data()); + BOOST_REQUIRE(!data_); BOOST_CHECK_NO_THROW(emplace_back(0)); BOOST_CHECK_NO_THROW(map()); - BOOST_REQUIRE(data_.data()); + BOOST_REQUIRE(data_); BOOST_CHECK_NO_THROW(unmap()); - BOOST_CHECK(!data_.data()); + BOOST_CHECK(!data_); } BOOST_AUTO_TEST_CASE(push_back_test, *boost::unit_test::timeout(1)) @@ -44,7 +44,6 @@ BOOST_AUTO_TEST_CASE(push_back_test, *boost::unit_test::timeout(1)) BOOST_CHECK_NO_THROW(push_back(5)); BOOST_CHECK_EQUAL(capacity_, 8); BOOST_CHECK_EQUAL(size_, 5); - BOOST_CHECK_EQUAL(data_.size(), 5); { std::array expected1 {1, 2, 3, 4, 5}; BOOST_CHECK_EQUAL_COLLECTIONS(begin(), end(), expected1.begin(), expected1.end()); @@ -166,7 +165,7 @@ BOOST_AUTO_TEST_CASE(getters) BOOST_CHECK_EQUAL(2, at(1)); BOOST_CHECK_EQUAL(1, (*this)[0]); BOOST_CHECK_EQUAL(2, (*this)[1]); - BOOST_CHECK_EQUAL(data_.data(), data()); + BOOST_CHECK_EQUAL(data_, data()); BOOST_CHECK_THROW(std::ignore = at(2), std::out_of_range); const auto & constCont {*this}; @@ -183,7 +182,7 @@ BOOST_AUTO_TEST_CASE(getters) BOOST_CHECK_EQUAL(2, constCont.at(1)); BOOST_CHECK_EQUAL(1, constCont[0]); BOOST_CHECK_EQUAL(2, constCont[1]); - BOOST_CHECK_EQUAL(data_.data(), constCont.data()); + BOOST_CHECK_EQUAL(data_, constCont.data()); BOOST_CHECK_THROW(std::ignore = constCont.at(2), std::out_of_range); } @@ -209,16 +208,16 @@ BOOST_AUTO_TEST_CASE(random_write) BOOST_CHECK_NO_THROW(resize(3)); BOOST_CHECK_EQUAL(size(), 3); BOOST_CHECK_NO_THROW(unmap()); - BOOST_REQUIRE(!data_.data()); + BOOST_REQUIRE(!data_); BOOST_CHECK_NO_THROW(at(0, 10)); BOOST_CHECK_NO_THROW(at(1, 20)); BOOST_CHECK_NO_THROW(at(2, 30)); - BOOST_CHECK(!data_.data()); + BOOST_CHECK(!data_); { std::array expected1 {10, 20, 30}; BOOST_CHECK_EQUAL_COLLECTIONS(begin(), end(), expected1.begin(), expected1.end()); } - BOOST_CHECK(data_.data()); + BOOST_CHECK(data_); BOOST_CHECK_NO_THROW(at(1, 40)); { std::array expected1 {10, 40, 30}; diff --git a/test/test-instancing.cpp b/test/test-instancing.cpp index 98133f3..c1860a4 100644 --- a/test/test-instancing.cpp +++ b/test/test-instancing.cpp @@ -56,11 +56,11 @@ BOOST_AUTO_TEST_CASE(autoMapUnmap) { { auto proxy = acquire(); - BOOST_CHECK(data_.data()); + BOOST_CHECK(data_); std::ignore = bufferName(); - BOOST_CHECK(data_.data()); + BOOST_CHECK(data_); BOOST_CHECK_EQUAL(1, size()); - BOOST_CHECK(!data_.data()); + BOOST_CHECK(!data_); } BOOST_CHECK_EQUAL(0, size()); } -- cgit v1.2.3