Merge pull request #145 from TomHarte/RandomNoise

Eliminates occasional on-screen noise

Outputs/CRT/CRT.cpp

@@ -140,12 +140,15 @@ void CRT::advance_cycles(unsigned int number_of_cycles, bool hsync_requested, bo
 		bool is_output_segment = ((is_output_run && next_run_length) && !horizontal_flywheel_->is_in_retrace() && !vertical_flywheel_->is_in_retrace());
 		uint8_t *next_run = nullptr;
 		if(is_output_segment && !openGL_output_builder_.composite_output_buffer_is_full()) {
-			next_run = openGL_output_builder_.array_builder.get_input_storage(SourceVertexSize);
+			bool did_retain_source_data = openGL_output_builder_.texture_builder.retain_latest();
+			if(did_retain_source_data) {
+				next_run = openGL_output_builder_.array_builder.get_input_storage(SourceVertexSize);
+			}
 		}
 
 		if(next_run) {
-			// output_y and texture locations will be written later; we won't necessarily know what it is outside of the locked region
+			// output_y and texture locations will be written later; we won't necessarily know what they are
-			openGL_output_builder_.texture_builder.retain_latest();
+			// outside of the locked region
 			source_output_position_x1() = (uint16_t)horizontal_flywheel_->get_current_output_position();
 			source_phase() = colour_burst_phase_;
 			source_amplitude() = colour_burst_amplitude_;

Outputs/CRT/Internals/TextureBuilder.cpp

@@ -37,8 +37,8 @@ TextureBuilder::TextureBuilder(size_t bytes_per_pixel, GLenum texture_unit) :
 		bytes_per_pixel_(bytes_per_pixel),
 		write_areas_start_x_(0),
 		write_areas_start_y_(0),
+		first_unsubmitted_y_(0),
 		is_full_(false),
-		did_submit_(false),
 		number_of_write_areas_(0) {
 	image_.resize(bytes_per_pixel * InputBufferBuilderWidth * InputBufferBuilderHeight);
 	glGenTextures(1, &texture_name_);
@@ -61,55 +61,44 @@ inline uint8_t *TextureBuilder::pointer_to_location(uint16_t x, uint16_t y) {
 }
 
 uint8_t *TextureBuilder::allocate_write_area(size_t required_length) {
+	// Keep a flag to indicate whether the buffer was full at allocate_write_area; if it was then
+	// don't return anything now, and decline to act upon follow-up methods. is_full_ may be reset
+	// by asynchronous calls to submit. was_full_ will not be touched by it.
+	was_full_ = is_full_;
 	if(is_full_) return nullptr;
 
-	uint16_t starting_x, starting_y;
+	// If there's not enough space on this line, move to the next. If the next is where the current
+	// submission group started, trigger is/was_full_ and return nothing.
+	if(write_areas_start_x_ + required_length + 2 > InputBufferBuilderWidth) {
+		write_areas_start_x_ = 0;
+		write_areas_start_y_ = (write_areas_start_y_ + 1) % InputBufferBuilderHeight;
 
-	if(!number_of_write_areas_) {
+		if(write_areas_start_y_ == first_unsubmitted_y_) {
-		starting_x = write_areas_start_x_;
+			was_full_ = is_full_ = true;
-		starting_y = write_areas_start_y_;
-	} else {
-		starting_x = write_areas_[number_of_write_areas_ - 1].x + write_areas_[number_of_write_areas_ - 1].length + 1;
-		starting_y = write_areas_[number_of_write_areas_ - 1].y;
-	}
-
-	if(starting_x + required_length + 2 > InputBufferBuilderWidth) {
-		starting_x = 0;
-		starting_y++;
-
-		if(starting_y == InputBufferBuilderHeight) {
-			is_full_ = true;
 			return nullptr;
 		}
 	}
 
-	write_area_.x = starting_x + 1;
+	// Queue up the latest write area.
-	write_area_.y = starting_y;
+	write_area_.x = write_areas_start_x_ + 1;
+	write_area_.y = write_areas_start_y_;
 	write_area_.length = (uint16_t)required_length;
 
+	// Return a video pointer.
 	return pointer_to_location(write_area_.x, write_area_.y);
 }
 
-bool TextureBuilder::is_full() {
-	return is_full_;
-}
-
-void TextureBuilder::retain_latest() {
-	if(is_full_) return;
-	if(number_of_write_areas_ < write_areas_.size())
-		write_areas_[number_of_write_areas_] = write_area_;
-	else
-		write_areas_.push_back(write_area_);
-	number_of_write_areas_++;
-}
-
 void TextureBuilder::reduce_previous_allocation_to(size_t actual_length) {
-	if(is_full_) return;
+	// If the previous allocate_write_area declined to act, decline also.
+	if(was_full_) return;
 
+	// Update the length of the current write area.
 	write_area_.length = (uint16_t)actual_length;
 
-	// book end the allocation with duplicates of the first and last pixel, to protect
+	// Bookend the allocation with duplicates of the first and last pixel, to protect
-	// against rounding errors when this run is drawn
+	// against rounding errors when this run is drawn.
+	// TODO: allow somebody else to specify the rule for generating a left-padding value and
+	// a right-padding value.
 	uint8_t *start_pointer = pointer_to_location(write_area_.x, write_area_.y);
 	memcpy(	&start_pointer[-bytes_per_pixel_],
 			start_pointer,
@@ -120,56 +109,66 @@ void TextureBuilder::reduce_previous_allocation_to(size_t actual_length) {
 			bytes_per_pixel_);
 }
 
-void TextureBuilder::submit() {
+bool TextureBuilder::retain_latest() {
-	uint16_t height = write_areas_start_y_ + (write_areas_start_x_ ? 1 : 0);
+	// If the previous allocate_write_area declined to act, decline also.
-	did_submit_ = true;
+	if(was_full_) return false;
 
-	glTexSubImage2D(	GL_TEXTURE_2D, 0,
+	// Account for the most recently written area as taken.
-						0, 0,
+	write_areas_start_x_ += write_area_.length + 2;
-						InputBufferBuilderWidth, height,
+
-						formatForDepth(bytes_per_pixel_), GL_UNSIGNED_BYTE,
+	// Store into the vector directly if there's already room, otherwise grow the vector.
-						image_.data());
+	// Probably I don't need to mess about with this myself; it's unnecessary second-guessing.
+	// TODO: profile and prove.
+	if(number_of_write_areas_ < write_areas_.size())
+		write_areas_[number_of_write_areas_] = write_area_;
+	else
+		write_areas_.push_back(write_area_);
+	number_of_write_areas_++;
+
+	return true;
+}
+
+bool TextureBuilder::is_full() {
+	return is_full_;
+}
+
+void TextureBuilder::submit() {
+	if(write_areas_start_y_ < first_unsubmitted_y_) {
+		// A write area start y less than the first line on which submissions began implies it must have wrapped
+		// around. So the submission set is everything back to zero before the current write area plus everything
+		// from the first unsubmitted y downward.
+		uint16_t height = write_areas_start_y_ + (write_areas_start_x_ ? 1 : 0);
+		glTexSubImage2D(	GL_TEXTURE_2D, 0,
+							0, 0,
+							InputBufferBuilderWidth, height,
+							formatForDepth(bytes_per_pixel_), GL_UNSIGNED_BYTE,
+							image_.data());
+
+		glTexSubImage2D(	GL_TEXTURE_2D, 0,
+							0, first_unsubmitted_y_,
+							InputBufferBuilderWidth, InputBufferBuilderHeight - first_unsubmitted_y_,
+							formatForDepth(bytes_per_pixel_), GL_UNSIGNED_BYTE,
+							image_.data() + first_unsubmitted_y_ * bytes_per_pixel_ * InputBufferBuilderWidth);
+	} else {
+		// If the current write area start y is after the first unsubmitted line, just submit the region in between.
+		uint16_t height = write_areas_start_y_ + (write_areas_start_x_ ? 1 : 0) - first_unsubmitted_y_;
+		glTexSubImage2D(	GL_TEXTURE_2D, 0,
+							0, first_unsubmitted_y_,
+							InputBufferBuilderWidth, height,
+							formatForDepth(bytes_per_pixel_), GL_UNSIGNED_BYTE,
+							image_.data() + first_unsubmitted_y_ * bytes_per_pixel_ * InputBufferBuilderWidth);
+	}
+
+	// Update the starting location for the next submission, and mark definitively that the buffer is once again not full.
+	first_unsubmitted_y_ = write_areas_start_y_;
+	is_full_ = false;
 }
 
 void TextureBuilder::flush(const std::function<void(const std::vector<WriteArea> &write_areas, size_t count)> &function) {
-	bool was_full = is_full_;
+	// Just throw everything currently in the flush queue to the provided function, and note that
-	if(did_submit_) {
+	// the queue is now empty.
-		write_areas_start_y_ = write_areas_start_x_ = 0;
+	if(number_of_write_areas_) {
-		is_full_ = false;
+		function(write_areas_, number_of_write_areas_);
 	}
-
-	if(number_of_write_areas_ && !was_full) {
-		if(write_areas_[0].x != write_areas_start_x_+1 || write_areas_[0].y != write_areas_start_y_) {
-			for(size_t area = 0; area < number_of_write_areas_; area++) {
-				WriteArea &write_area = write_areas_[area];
-
-				if(write_areas_start_x_ + write_area.length + 2 > InputBufferBuilderWidth) {
-					write_areas_start_x_ = 0;
-					write_areas_start_y_ ++;
-
-					if(write_areas_start_y_ == InputBufferBuilderHeight) {
-						is_full_ = true;
-						break;
-					}
-				}
-
-				memmove(
-					pointer_to_location(write_areas_start_x_, write_areas_start_y_),
-					pointer_to_location(write_area.x - 1, write_area.y),
-					(write_area.length + 2) * bytes_per_pixel_);
-				write_area.x = write_areas_start_x_ + 1;
-				write_area.y = write_areas_start_y_;
-			}
-		}
-
-		if(!is_full_) {
-			function(write_areas_, number_of_write_areas_);
-
-			write_areas_start_x_ = write_areas_[number_of_write_areas_-1].x + write_areas_[number_of_write_areas_-1].length + 1;
-			write_areas_start_y_ = write_areas_[number_of_write_areas_-1].y;
-		}
-	}
-
-	did_submit_ = false;
 	number_of_write_areas_ = 0;
 }

Outputs/CRT/Internals/TextureBuilder.hpp

@@ -25,7 +25,11 @@ namespace CRT {
 	with runs of data, ensuring each run is neighboured immediately to the left and right by copies of its
 	first and last pixels.
 
-	Intended usage:
+	Although this class is not itself inherently thread safe, it is built to permit one serialised stream
+	of calls to provide source data, with an interceding (but also serialised) submission to the GPU at any time.
+
+
+	Intended usage by the data generator:
 
 		(i)		allocate a write area with allocate_write_area, supplying a maximum size.
 		(ii)	call reduce_previous_allocation_to to announce the actual size written.
@@ -46,9 +50,13 @@ namespace CRT {
 	an opportunity to correlate the data with whatever else it is being tied to. It will continue to sit in
 	the CPU's memory space but has now passed beyond any further modification or reporting.
 
-		(v)		call submit to move data to the GPU and free up its CPU-side resources.
 
-	The data is now on the GPU, for whatever use the caller desires.
+	Intended usage by the GPU owner:
+
+		(i)		call submit to move data to the GPU and free up its CPU-side resources.
+
+	The latest data is now on the GPU, regardless of where the data provider may be in its process — only data
+	that has entered the submission queue is uploaded.
 
 */
 class TextureBuilder {
@@ -68,7 +76,8 @@ class TextureBuilder {
 		void reduce_previous_allocation_to(size_t actual_length);
 
 		/// Allocated runs are provisional; they will not appear in the next flush queue unless retained.
-		void retain_latest();
+		/// @returns @c true if a retain succeeded; @c false otherwise.
+		bool retain_latest();
 
 		/// @returns @c true if all future calls to @c allocate_write_area will fail on account of the input texture
 		/// being full; @c false if calls may succeed.
@@ -99,8 +108,8 @@ class TextureBuilder {
 		// the list of write areas that have ascended to the flush queue
 		std::vector<WriteArea> write_areas_;
 		size_t number_of_write_areas_;
-		bool is_full_;
+		bool is_full_, was_full_;
-		bool did_submit_;
+		uint16_t first_unsubmitted_y_;
 		inline uint8_t *pointer_to_location(uint16_t x, uint16_t y);
 
 		// Usually: the start position for the current batch of write areas.