Merge pull request #6 from TomHarte/GarbageLines

Fixes the two most obvious potential causes of garbage lines

OSBindings/Mac/Clock Signal/Views/CSOpenGLView.m

@@ -60,6 +60,7 @@ static CVReturn DisplayLinkCallback(CVDisplayLinkRef displayLink, const CVTimeSt
 		BOOL didSkip = _hasSkipped;
 		dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
 			[self.delegate openGLView:self didUpdateToTime:time didSkipPreviousUpdate:didSkip frequency:frequency];
+			[self drawViewOnlyIfDirty:YES];
 			OSAtomicTestAndClear(processingMask, &_updateIsOngoing);
 		});
 		_hasSkipped = NO;
@@ -68,7 +69,7 @@ static CVReturn DisplayLinkCallback(CVDisplayLinkRef displayLink, const CVTimeSt
 	// Draw the display only if a previous draw is not still ongoing. -drawViewOnlyIfDirty: is guaranteed
 	// to be safe to call concurrently with -openGLView:updateToTime: so there's no need to worry about
 	// the above interrupting the below or vice versa.
-	if(!OSAtomicTestAndSet(drawingMask, &_updateIsOngoing))
+	if(!OSAtomicTestAndSet(drawingMask, &_updateIsOngoing) && _hasSkipped)
 	{
 		dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_HIGH, 0), ^{
 			[self drawViewOnlyIfDirty:YES];

Outputs/CRT/CRT.cpp

@@ -279,35 +279,53 @@ void CRT::output_scan(const Scan *const scan)
 */
 void CRT::output_sync(unsigned int number_of_cycles)
 {
+	_openGL_output_builder->lock_output();
 	Scan scan{
 		.type = Scan::Type::Sync,
 		.number_of_cycles = number_of_cycles
 	};
 	output_scan(&scan);
+	_openGL_output_builder->unlock_output();
 }
 
 void CRT::output_blank(unsigned int number_of_cycles)
 {
+	_openGL_output_builder->lock_output();
 	Scan scan {
 		.type = Scan::Type::Blank,
 		.number_of_cycles = number_of_cycles
 	};
 	output_scan(&scan);
+	_openGL_output_builder->unlock_output();
 }
 
 void CRT::output_level(unsigned int number_of_cycles)
 {
-	Scan scan {
+	_openGL_output_builder->lock_output();
-		.type = Scan::Type::Level,
+	if(!_openGL_output_builder->input_buffer_is_full())
-		.number_of_cycles = number_of_cycles,
+	{
-		.tex_x = _openGL_output_builder->get_last_write_x_posititon(),
+		Scan scan {
-		.tex_y = _openGL_output_builder->get_last_write_y_posititon()
+			.type = Scan::Type::Level,
-	};
+			.number_of_cycles = number_of_cycles,
-	output_scan(&scan);
+			.tex_x = _openGL_output_builder->get_last_write_x_posititon(),
+			.tex_y = _openGL_output_builder->get_last_write_y_posititon()
+		};
+		output_scan(&scan);
+	}
+	else
+	{
+		Scan scan {
+			.type = Scan::Type::Blank,
+			.number_of_cycles = number_of_cycles
+		};
+		output_scan(&scan);
+	}
+	_openGL_output_builder->unlock_output();
 }
 
 void CRT::output_colour_burst(unsigned int number_of_cycles, uint8_t phase, uint8_t amplitude)
 {
+	_openGL_output_builder->lock_output();
 	Scan scan {
 		.type = Scan::Type::ColourBurst,
 		.number_of_cycles = number_of_cycles,
@@ -315,12 +333,15 @@ void CRT::output_colour_burst(unsigned int number_of_cycles, uint8_t phase, uint
 		.amplitude = amplitude
 	};
 	output_scan(&scan);
+	_openGL_output_builder->unlock_output();
 }
 
 void CRT::output_data(unsigned int number_of_cycles, unsigned int source_divider)
 {
-	if(_openGL_output_builder->reduce_previous_allocation_to(number_of_cycles / source_divider))
+	_openGL_output_builder->lock_output();
+	if(!_openGL_output_builder->input_buffer_is_full())
 	{
+		_openGL_output_builder->reduce_previous_allocation_to(number_of_cycles / source_divider);
 		Scan scan {
 			.type = Scan::Type::Data,
 			.number_of_cycles = number_of_cycles,
@@ -332,8 +353,13 @@ void CRT::output_data(unsigned int number_of_cycles, unsigned int source_divider
 	}
 	else
 	{
-		output_blank(number_of_cycles);
+		Scan scan {
+			.type = Scan::Type::Blank,
+			.number_of_cycles = number_of_cycles
+		};
+		output_scan(&scan);
 	}
+	_openGL_output_builder->unlock_output();
 }
 
 Outputs::CRT::Rect CRT::get_rect_for_area(int first_line_after_sync, int number_of_lines, int first_cycle_after_sync, int number_of_cycles, float aspect_ratio)

Outputs/CRT/Internals/CRTInputBufferBuilder.cpp

@@ -41,19 +41,26 @@ void CRTInputBufferBuilder::allocate_write_area(size_t required_length)
 	}
 }
 
-bool CRTInputBufferBuilder::reduce_previous_allocation_to(size_t actual_length)
+bool CRTInputBufferBuilder::is_full()
 {
-	if(_next_write_y_position == InputBufferBuilderHeight) return false;
+	return (_next_write_y_position == InputBufferBuilderHeight);
+}
+
+void CRTInputBufferBuilder::reduce_previous_allocation_to(size_t actual_length)
+{
+	if(_next_write_y_position == InputBufferBuilderHeight) return;
 
 	uint8_t *const image_pointer = _image.get();
 
 	// correct if the writing cursor was reset while a client was writing
-	if(_next_write_x_position == 0 && _next_write_y_position == 0 && _write_target_pointer != 1)
+	if(_next_write_x_position == 0 && _next_write_y_position == 0)
 	{
-		memmove(&image_pointer[1], &image_pointer[_write_target_pointer], actual_length);
+		memmove(&image_pointer[1], &image_pointer[_write_target_pointer], actual_length * _bytes_per_pixel);
 		_write_target_pointer = 1;
 		_last_allocation_amount = actual_length;
 		_next_write_x_position = (uint16_t)(actual_length + 2);
+		_write_x_position = 1;
+		_write_y_position = 0;
 	}
 
 	// book end the allocation with duplicates of the first and last pixel, to protect
@@ -68,8 +75,6 @@ bool CRTInputBufferBuilder::reduce_previous_allocation_to(size_t actual_length)
 
 	// return any allocated length that wasn't actually used to the available pool
 	_next_write_x_position -= (_last_allocation_amount - actual_length);
-
-	return true;
 }
 
 uint8_t *CRTInputBufferBuilder::get_image_pointer()

Outputs/CRT/Internals/CRTInputBufferBuilder.hpp

@@ -23,7 +23,7 @@ struct CRTInputBufferBuilder {
 	CRTInputBufferBuilder(size_t bytes_per_pixel);
 
 	void allocate_write_area(size_t required_length);
-	bool reduce_previous_allocation_to(size_t actual_length);
+	void reduce_previous_allocation_to(size_t actual_length);
 
 	uint16_t get_and_finalise_current_line();
 	uint8_t *get_image_pointer();
@@ -36,6 +36,8 @@ struct CRTInputBufferBuilder {
 
 	size_t get_bytes_per_pixel();
 
+	bool is_full();
+
 	private:
 		// where pixel data will be put to the next time a write is requested
 		uint16_t _next_write_x_position, _next_write_y_position;

Outputs/CRT/Internals/CRTOpenGL.cpp

@@ -41,8 +41,14 @@ struct Range {
 	GLsizei location, length;
 };
 
-static int getCircularRanges(GLsizei start, GLsizei end, GLsizei buffer_length, GLsizei granularity, Range *ranges)
+static int getCircularRanges(GLsizei *start_pointer, GLsizei *end_pointer, GLsizei buffer_length, GLsizei granularity, Range *ranges)
 {
+	GLsizei start = *start_pointer;
+	GLsizei end = *end_pointer;
+
+	*end_pointer %= buffer_length;
+	*start_pointer = *end_pointer;
+
 	start -= start%granularity;
 	end -= end%granularity;
 
@@ -160,8 +166,6 @@ OpenGLOutputBuilder::OpenGLOutputBuilder(unsigned int buffer_depth) :
 
 OpenGLOutputBuilder::~OpenGLOutputBuilder()
 {
-//	glUnmapBuffer(GL_ARRAY_BUFFER);
-//	glUnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
 	glDeleteTextures(1, &textureName);
 	glDeleteBuffers(1, &output_array_buffer);
 	glDeleteBuffers(1, &source_array_buffer);
@@ -193,19 +197,11 @@ void OpenGLOutputBuilder::draw_frame(unsigned int output_width, unsigned int out
 	// determine how many lines are newly reclaimed; they'll need to be cleared
 	Range clearing_zones[2], source_drawing_zones[2];
 	Range output_drawing_zones[2];
-	int number_of_clearing_zones		= getCircularRanges(_cleared_composite_output_y, _composite_src_output_y, IntermediateBufferHeight, 1, clearing_zones);
+	int number_of_clearing_zones		= getCircularRanges(&_cleared_composite_output_y,		&_composite_src_output_y,		IntermediateBufferHeight,	1,					clearing_zones);
-	int number_of_source_drawing_zones	= getCircularRanges(_drawn_source_buffer_data_pointer, _source_buffer_data_pointer, SourceVertexBufferDataSize, 2*SourceVertexSize, source_drawing_zones);
+	int number_of_source_drawing_zones	= getCircularRanges(&_drawn_source_buffer_data_pointer,	&_source_buffer_data_pointer,	SourceVertexBufferDataSize,	2*SourceVertexSize,	source_drawing_zones);
-	int number_of_output_drawing_zones	= getCircularRanges(_drawn_output_buffer_data_pointer, _output_buffer_data_pointer, OutputVertexBufferDataSize, 6*OutputVertexSize, output_drawing_zones);
+	int number_of_output_drawing_zones	= getCircularRanges(&_drawn_output_buffer_data_pointer,	&_output_buffer_data_pointer,	OutputVertexBufferDataSize,	6*OutputVertexSize,	output_drawing_zones);
 	uint16_t completed_texture_y		= _buffer_builder->get_and_finalise_current_line();
 
-	_composite_src_output_y %= IntermediateBufferHeight;
-	_source_buffer_data_pointer %= SourceVertexBufferDataSize;
-	_output_buffer_data_pointer %= OutputVertexBufferDataSize;
-
-	_cleared_composite_output_y = _composite_src_output_y;
-	_drawn_source_buffer_data_pointer = _source_buffer_data_pointer;
-	_drawn_output_buffer_data_pointer = _output_buffer_data_pointer;
-
 	if(_fence != nullptr)
 	{
 		glClientWaitSync(_fence, GL_SYNC_FLUSH_COMMANDS_BIT, GL_TIMEOUT_IGNORED);

Outputs/CRT/Internals/CRTOpenGL.hpp

@@ -60,7 +60,7 @@ class OpenGLOutputBuilder {
 		std::unique_ptr<std::mutex> _output_mutex;
 
 		// transient buffers indicating composite data not yet decoded
-		uint16_t _composite_src_output_y, _cleared_composite_output_y;
+		GLsizei _composite_src_output_y, _cleared_composite_output_y;
 
 		std::unique_ptr<OpenGL::OutputShader> output_shader_program;
 		std::unique_ptr<OpenGL::IntermediateShader> composite_input_shader_program, composite_separation_filter_program, composite_y_filter_shader_program, composite_chrominance_filter_shader_program;
@@ -107,14 +107,12 @@ class OpenGLOutputBuilder {
 		inline uint8_t *get_next_source_run()
 		{
 			if(_source_buffer_data_pointer == _drawn_source_buffer_data_pointer + SourceVertexBufferDataSize) return nullptr;
-			_output_mutex->lock();
 			return &_source_buffer_data.get()[_source_buffer_data_pointer % SourceVertexBufferDataSize];
 		}
 
 		inline void complete_source_run()
 		{
 			_source_buffer_data_pointer += 2 * SourceVertexSize;
-			_output_mutex->unlock();
 		}
 
 		inline bool composite_output_run_has_room_for_vertices(GLsizei vertices_to_write)
@@ -125,13 +123,21 @@ class OpenGLOutputBuilder {
 		inline uint8_t *get_next_output_run()
 		{
 			if(_output_buffer_data_pointer == _drawn_output_buffer_data_pointer + OutputVertexBufferDataSize) return nullptr;
-			_output_mutex->lock();
 			return &_output_buffer_data.get()[_output_buffer_data_pointer % OutputVertexBufferDataSize];
 		}
 
 		inline void complete_output_run(GLsizei vertices_written)
 		{
 			_output_buffer_data_pointer += vertices_written * OutputVertexSize;
+		}
+
+		inline void lock_output()
+		{
+			_output_mutex->lock();
+		}
+
+		inline void unlock_output()
+		{
 			_output_mutex->unlock();
 		}
 
@@ -167,9 +173,14 @@ class OpenGLOutputBuilder {
 			return _buffer_builder->get_write_target();
 		}
 
-		inline bool reduce_previous_allocation_to(size_t actual_length)
+		inline void reduce_previous_allocation_to(size_t actual_length)
 		{
-			return _buffer_builder->reduce_previous_allocation_to(actual_length);
+			_buffer_builder->reduce_previous_allocation_to(actual_length);
+		}
+
+		inline bool input_buffer_is_full()
+		{
+			return _buffer_builder->is_full();
 		}
 
 		inline uint16_t get_last_write_x_posititon()