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.

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<P>(ps)...};
+		new (&*end()) T {std::forward<P>(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<P>(ps)...};
+		pos = begin() + idx;
+		std::move_backward(pos, end(), end() + 1);
+		pos->~T();
+		new (&*pos) T {std::forward<P>(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<T *>(glMapNamedBuffer(buffer_, GL_READ_WRITE)), size_};
-			assert(data_.data());
+		if (!data_) {
+			data_ = static_cast<T *>(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<int>)
 
 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());
 }