From f6e51c2f771726eec73b07824ebde2cf8bd522f0 Mon Sep 17 00:00:00 2001
From: Mikko Rasa <tdb@tdb.fi>
Date: Sun, 26 Aug 2012 10:53:56 +0300
Subject: [PATCH] Alter the working logic of Bufferable to avoid some problems

It was possible for the buffer data to be updated without going through
the dirty check in UniformBlock::apply, leaving the UniformBlock with a
null buf_range and resulting in a segfault.
---
 source/bufferable.cpp   | 24 ++++++++++++++++--------
 source/bufferable.h     |  6 ++++--
 source/uniformblock.cpp | 22 ++++++++++++++++++----
 source/uniformblock.h   |  4 +++-
 4 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/source/bufferable.cpp b/source/bufferable.cpp
index 5c7cb9e0..4660e9c3 100644
--- a/source/bufferable.cpp
+++ b/source/bufferable.cpp
@@ -36,7 +36,7 @@ void Bufferable::use_buffer(Buffer *buf, Bufferable *prev)
 			prev_in_buffer->next_in_buffer = this;
 	}
 
-	update_buffer_offsets();
+	update_buffer_offset();
 }
 
 void Bufferable::unlink_from_buffer()
@@ -46,13 +46,13 @@ void Bufferable::unlink_from_buffer()
 	if(next_in_buffer)
 	{
 		next_in_buffer->prev_in_buffer = prev_in_buffer;
-		next_in_buffer->update_buffer_offsets();
+		next_in_buffer->update_buffer_offset();
 	}
 	prev_in_buffer = 0;
 	next_in_buffer = 0;
 }
 
-void Bufferable::update_buffer_offsets()
+void Bufferable::update_buffer_offset()
 {
 	unsigned offset = 0;
 	if(prev_in_buffer)
@@ -62,10 +62,14 @@ void Bufferable::update_buffer_offsets()
 	{
 		buffer_offset = offset;
 		dirty = true;
+		offset_changed();
 	}
 
 	if(next_in_buffer)
-		next_in_buffer->update_buffer_offsets();
+		next_in_buffer->update_buffer_offset();
+
+	/* Do not resize the buffer here, as the offsets may change multiple times
+	before the buffer is actually used */
 }
 
 void Bufferable::update_buffer_data() const
@@ -77,12 +81,16 @@ void Bufferable::update_buffer_data() const
 	for(const Bufferable *b=first; b; b=b->next_in_buffer)
 		total_size += b->get_data_size();
 
-	buffer->data(total_size, 0);
-	for(const Bufferable *b=first; b; b=b->next_in_buffer)
+	if(total_size!=buffer->get_size())
 	{
-		buffer->sub_data(b->buffer_offset, b->get_data_size(), b->get_data());
-		b->dirty = false;
+		buffer->data(total_size, 0);
+		// Resizing the buffer invalidates its contents.
+		for(const Bufferable *b=first; b; b=b->next_in_buffer)
+			b->dirty = true;
 	}
+
+	upload_data();
+	dirty = false;
 }
 
 } // namespace GL
diff --git a/source/bufferable.h b/source/bufferable.h
index dd7cff8e..fa81554d 100644
--- a/source/bufferable.h
+++ b/source/bufferable.h
@@ -25,10 +25,12 @@ private:
 
 protected:
 	virtual unsigned get_data_size() const = 0;
-	virtual const void *get_data() const = 0;
 
-	void update_buffer_offsets();
+	void update_buffer_offset();
+	virtual void offset_changed() { }
+
 	void update_buffer_data() const;
+	virtual void upload_data() const = 0;
 };
 
 } // namespace GL
diff --git a/source/uniformblock.cpp b/source/uniformblock.cpp
index 350f57ab..6ea0d320 100644
--- a/source/uniformblock.cpp
+++ b/source/uniformblock.cpp
@@ -22,6 +22,24 @@ UniformBlock::UniformBlock(unsigned s):
 	buf_range(0)
 { }
 
+UniformBlock::~UniformBlock()
+{
+	delete buf_range;
+}
+
+void UniformBlock::offset_changed()
+{
+	delete buf_range;
+	buf_range = 0;
+}
+
+void UniformBlock::upload_data() const
+{
+	if(!buf_range)
+		buf_range = new BufferRange(*buffer, buffer_offset, size);
+	buf_range->data(&data[0]);
+}
+
 void UniformBlock::attach(int index, const Uniform &uni)
 {
 	uniforms[index] = &uni;
@@ -45,11 +63,7 @@ void UniformBlock::apply(int index) const
 	if(buffer)
 	{
 		if(dirty)
-		{
 			update_buffer_data();
-			if(!buf_range)
-				buf_range = new BufferRange(*buffer, buffer_offset, size);
-		}
 		buf_range->bind_to(UNIFORM_BUFFER, index);
 	}
 	else
diff --git a/source/uniformblock.h b/source/uniformblock.h
index fbc9c704..66b3c7e0 100644
--- a/source/uniformblock.h
+++ b/source/uniformblock.h
@@ -29,10 +29,12 @@ private:
 public:
 	UniformBlock();
 	UniformBlock(unsigned);
+	~UniformBlock();
 
 private:
-	virtual const void *get_data() const { return &data[0]; }
 	virtual unsigned get_data_size() const { return size; }
+	virtual void offset_changed();
+	virtual void upload_data() const;
 
 public:
 	void attach(int, const Uniform &);
-- 
2.45.2