Merge pull request #262 from TomHarte/BookEnds

Adds a facility for 'bookending' data runs, eliminating an occasional Electron rendering error

Machines/Electron/Video.cpp

@@ -32,6 +32,13 @@ namespace {
 	static const int real_time_clock_interrupt_2 = 56704;
 	static const int display_end_interrupt_1 = (first_graphics_line + display_end_interrupt_line)*cycles_per_line;
 	static const int display_end_interrupt_2 = (first_graphics_line + field_divider_line + display_end_interrupt_line)*cycles_per_line;
+
+	struct FourBPPBookender: public Outputs::CRT::TextureBuilder::Bookender {
+		void add_bookends(uint8_t *const left_value, uint8_t *const right_value, uint8_t *left_bookend, uint8_t *right_bookend) {
+			*left_bookend = static_cast<uint8_t>(((*left_value) & 0x0f) | (((*left_value) & 0x0f) << 4));
+			*right_bookend = static_cast<uint8_t>(((*right_value) & 0xf0) | (((*right_value) & 0xf0) >> 4));
+		}
+	};
 }
 
 #pragma mark - Lifecycle
@@ -49,6 +56,8 @@ VideoOutput::VideoOutput(uint8_t *memory) : ram_(memory) {
 			"texValue >>= 4 - (int(icoordinate.x * 8) & 4);"
 			"return vec3( uvec3(texValue) & uvec3(4u, 2u, 1u));"
 		"}");
+	std::unique_ptr<Outputs::CRT::TextureBuilder::Bookender> bookender(new FourBPPBookender);
+	crt_->set_bookender(std::move(bookender));
 	// TODO: as implied below, I've introduced a clock's latency into the graphics pipeline somehow. Investigate.
 	crt_->set_visible_area(crt_->get_rect_for_area(first_graphics_line - 3, 256, (first_graphics_cycle+1) * crt_cycles_multiplier, 80 * crt_cycles_multiplier, 4.0f / 3.0f));
 }

Outputs/CRT/CRT.hpp

@@ -312,6 +312,10 @@ class CRT {
 			});
 		}
 
+		inline void set_bookender(std::unique_ptr<TextureBuilder::Bookender> bookender) {
+			openGL_output_builder_.texture_builder.set_bookender(std::move(bookender));
+		}
+
 		inline void set_output_device(OutputDevice output_device) {
 			enqueue_openGL_function([output_device, this] {
 				openGL_output_builder_.set_output_device(output_device);

Outputs/CRT/Internals/TextureBuilder.cpp

@@ -13,7 +13,9 @@
 
 using namespace Outputs::CRT;
 
-static const GLint internalFormatForDepth(size_t depth) {
+namespace {
+
+const GLint internalFormatForDepth(size_t depth) {
 	switch(depth) {
 		default: return GL_FALSE;
 		case 1: return GL_R8UI;
@@ -23,7 +25,7 @@ static const GLint internalFormatForDepth(size_t depth) {
 	}
 }
 
-static const GLenum formatForDepth(size_t depth) {
+const GLenum formatForDepth(size_t depth) {
 	switch(depth) {
 		default: return GL_FALSE;
 		case 1: return GL_RED_INTEGER;
@@ -33,6 +35,21 @@ static const GLenum formatForDepth(size_t depth) {
 	}
 }
 
+struct DefaultBookender: public TextureBuilder::Bookender {
+	public:
+		DefaultBookender(size_t bytes_per_pixel) : bytes_per_pixel_(bytes_per_pixel) {}
+
+		void add_bookends(uint8_t *const left_value, uint8_t *const right_value, uint8_t *left_bookend, uint8_t *right_bookend) {
+			memcpy(left_bookend, left_value, bytes_per_pixel_);
+			memcpy(right_bookend, right_value, bytes_per_pixel_);
+		}
+
+	private:
+		size_t bytes_per_pixel_;
+};
+
+}
+
 TextureBuilder::TextureBuilder(size_t bytes_per_pixel, GLenum texture_unit) :
 		bytes_per_pixel_(bytes_per_pixel),
 		write_areas_start_x_(0),
@@ -50,6 +67,8 @@ TextureBuilder::TextureBuilder(size_t bytes_per_pixel, GLenum texture_unit) :
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
 	glTexImage2D(GL_TEXTURE_2D, 0, internalFormatForDepth(bytes_per_pixel), InputBufferBuilderWidth, InputBufferBuilderHeight, 0, formatForDepth(bytes_per_pixel), GL_UNSIGNED_BYTE, nullptr);
+
+	set_bookender(nullptr);
 }
 
 TextureBuilder::~TextureBuilder() {
@@ -103,13 +122,14 @@ void TextureBuilder::reduce_previous_allocation_to(size_t actual_length) {
 	// 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) - bytes_per_pixel_;
-	memcpy(	start_pointer,
-			&start_pointer[bytes_per_pixel_],
-			bytes_per_pixel_);
+	bookender_->add_bookends(&start_pointer[bytes_per_pixel_], &start_pointer[actual_length * bytes_per_pixel_], start_pointer, &start_pointer[(actual_length + 1) * bytes_per_pixel_]);
+}
 
-	memcpy(	&start_pointer[(actual_length + 1) * bytes_per_pixel_],
-			&start_pointer[actual_length * bytes_per_pixel_],
-			bytes_per_pixel_);
+void TextureBuilder::set_bookender(std::unique_ptr<TextureBuilder::Bookender> bookender) {
+	bookender_ = std::move(bookender);
+	if(!bookender_) {
+		bookender_.reset(new DefaultBookender(bytes_per_pixel_));
+	}
 }
 
 bool TextureBuilder::retain_latest() {

Outputs/CRT/Internals/TextureBuilder.hpp

@@ -98,6 +98,22 @@ class TextureBuilder {
 		/// allocated, indicating their final resting locations and their lengths.
 		void flush(const std::function<void(const std::vector<WriteArea> &write_areas, size_t count)> &);
 
+		/// A Bookender helps to paper over precision errors when rendering; its job is to provide single-sample
+		/// extensions that duplicate the left and right edges of a written area. By default the texture builder will
+		/// simply copy the appropriate number of bytes per pixel, but if the client is using a packed pixel format
+		/// then that may be incorrect, e.g. if each sample is a byte but contains two pixels, each in a single nibble,
+		/// then the correct duplication might be a byte composed of copies of the two top nibbles as the left bookend,
+		/// and one composed of copies of the two bottom nibbles on the right.
+		struct Bookender {
+			/// Writes to left_bookend the sample that should appear as a continuation before the left_value;
+			/// writes to right_bookend the sample that should appear as a continuation after right_value.
+			virtual void add_bookends(uint8_t *const left_value, uint8_t *const right_value, uint8_t *left_bookend, uint8_t *right_bookend) = 0;
+		};
+
+		/// Sets the current bookender. The bookender be called synchronously within the builder-writing thread.
+		/// Supply nullptr to engage the default bookender.
+		void set_bookender(std::unique_ptr<Bookender> bookender);
+
 	private:
 		// the buffer size
 		size_t bytes_per_pixel_;
@@ -120,6 +136,8 @@ class TextureBuilder {
 		// Caveat: reset to the origin upon a submit. So used in comparison by flush to
 		// determine whether the current batch of write areas needs to be relocated.
 		uint16_t write_areas_start_x_, write_areas_start_y_;
+
+		std::unique_ptr<Bookender> bookender_;
 };
 
 }