From cb3c837e308f3bd4f425f80b83d0abd91a97eb39 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 3 Dec 2016 18:19:12 -0500 Subject: [PATCH 1/8] Simplified interface by baking in last-minute-only updates. --- Outputs/CRT/CRT.cpp | 18 ++++++++-------- Outputs/CRT/Internals/ArrayBuilder.cpp | 29 +++++++++----------------- Outputs/CRT/Internals/ArrayBuilder.hpp | 15 +++---------- 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/Outputs/CRT/CRT.cpp b/Outputs/CRT/CRT.cpp index 3f68cddfe..e4791c740 100644 --- a/Outputs/CRT/CRT.cpp +++ b/Outputs/CRT/CRT.cpp @@ -140,7 +140,6 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi source_input_position_y() = tex_y; source_output_position_x1() = (uint16_t)horizontal_flywheel_->get_current_output_position(); // Don't write output_y now, write it later; we won't necessarily know what it is outside of the locked region -// source_output_position_y() = openGL_output_builder_->get_composite_output_y(); source_phase() = colour_burst_phase_; source_amplitude() = colour_burst_amplitude_; source_phase_time() = (uint8_t)colour_burst_time_; // assumption: burst was within the first 1/16 of the line @@ -193,13 +192,7 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi openGL_output_builder_.lock_output(); // Get and write all those previously unwritten output ys - uint16_t output_y = openGL_output_builder_.get_composite_output_y(); - size_t size; - uint8_t *buffered_lines = openGL_output_builder_.array_builder.get_unflushed_input(size); - for(size_t position = 0; position < size; position += SourceVertexSize) - { - (*(uint16_t *)&buffered_lines[position + SourceVertexOffsetOfOutputStart + 2]) = output_y; - } + const uint16_t output_y = openGL_output_builder_.get_composite_output_y(); // Construct the output run uint8_t *next_run = openGL_output_builder_.array_builder.get_output_storage(OutputVertexSize); @@ -210,7 +203,14 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi output_tex_y() = output_y; output_x2() = (uint16_t)horizontal_flywheel_->get_current_output_position(); } - openGL_output_builder_.array_builder.flush(); + openGL_output_builder_.array_builder.flush( + [output_y] (uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size) + { + for(size_t position = 0; position < input_size; position += SourceVertexSize) + { + (*(uint16_t *)&input_buffer[position + SourceVertexOffsetOfOutputStart + 2]) = output_y; + } + }); openGL_output_builder_.unlock_output(); } diff --git a/Outputs/CRT/Internals/ArrayBuilder.cpp b/Outputs/CRT/Internals/ArrayBuilder.cpp index c934b3a5e..53a97e1b1 100644 --- a/Outputs/CRT/Internals/ArrayBuilder.cpp +++ b/Outputs/CRT/Internals/ArrayBuilder.cpp @@ -23,9 +23,8 @@ ArrayBuilder::ArrayBuilder(size_t input_size, size_t output_size, std::function< bool ArrayBuilder::is_full() { bool was_full; - buffer_mutex_.lock(); + std::lock_guard guard(buffer_mutex_); was_full = is_full_; - buffer_mutex_.unlock(); return was_full; } @@ -34,30 +33,24 @@ uint8_t *ArrayBuilder::get_input_storage(size_t size) return get_storage(size, input_); } -uint8_t *ArrayBuilder::get_unflushed_input(size_t &size) -{ - return input_.get_unflushed(size); -} - uint8_t *ArrayBuilder::get_output_storage(size_t size) { return get_storage(size, output_); } -uint8_t *ArrayBuilder::get_unflushed_output(size_t &size) +void ArrayBuilder::flush(const std::function &function) { - return output_.get_unflushed(size); -} - -void ArrayBuilder::flush() -{ - buffer_mutex_.lock(); + std::lock_guard guard(buffer_mutex_); if(!is_full_) { + size_t input_size, output_size; + uint8_t *input = input_.get_unflushed(input_size); + uint8_t *output = output_.get_unflushed(output_size); + function(input, input_size, output, output_size); + input_.flush(); output_.flush(); } - buffer_mutex_.unlock(); } void ArrayBuilder::bind_input() @@ -74,7 +67,7 @@ ArrayBuilder::Submission ArrayBuilder::submit() { ArrayBuilder::Submission submission; - buffer_mutex_.lock(); + std::lock_guard guard(buffer_mutex_); submission.input_size = input_.submit(true); submission.output_size = output_.submit(false); if(is_full_) @@ -83,7 +76,6 @@ ArrayBuilder::Submission ArrayBuilder::submit() input_.reset(); output_.reset(); } - buffer_mutex_.unlock(); return submission; } @@ -110,10 +102,9 @@ ArrayBuilder::Buffer::~Buffer() uint8_t *ArrayBuilder::get_storage(size_t size, Buffer &buffer) { - buffer_mutex_.lock(); + std::lock_guard guard(buffer_mutex_); uint8_t *pointer = buffer.get_storage(size); if(!pointer) is_full_ = true; - buffer_mutex_.unlock(); return pointer; } diff --git a/Outputs/CRT/Internals/ArrayBuilder.hpp b/Outputs/CRT/Internals/ArrayBuilder.hpp index ed72b98af..0be0644c6 100644 --- a/Outputs/CRT/Internals/ArrayBuilder.hpp +++ b/Outputs/CRT/Internals/ArrayBuilder.hpp @@ -41,26 +41,17 @@ class ArrayBuilder { /// @returns a pointer to the allocated area if allocation was possible; @c nullptr otherwise. uint8_t *get_input_storage(size_t size); - /// Gets the size of and a pointer to all data so far added to the input set but not yet flushed. - /// @returns a pointer from which it is safe to access @c size elements, which contains all regions returned via - /// @c get_input_storage in FIFO order. - uint8_t *get_unflushed_input(size_t &size); - /// Attempts to add @c size bytes to the output set. /// @returns a pointer to the allocated area if allocation was possible; @c nullptr otherwise. uint8_t *get_output_storage(size_t size); - /// Gets the size of and a pointer to all data so far added to the output set but not yet flushed. - /// @returns a pointer from which it is safe to access @c size elements, which contains all regions returned via - /// @c get_input_storage in FIFO order. - uint8_t *get_unflushed_output(size_t &size); - /// @returns @c true if either of the input or output storage areas is currently exhausted; @c false otherwise. bool is_full(); /// If neither input nor output was exhausted since the last flush, atomically commits both input and output - /// up to the currently allocated size for use upon the next @c submit. Otherwise acts as a no-op. - void flush(); + /// up to the currently allocated size for use upon the next @c submit, giving the supplied function a + /// chance to perform last-minute processing. Otherwise acts as a no-op. + void flush(const std::function &); /// Binds the input array to GL_ARRAY_BUFFER. void bind_input(); From 0edc043378777e049d30acfe3b1f7902d081336c Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Sat, 3 Dec 2016 20:47:19 -0500 Subject: [PATCH 2/8] Started introducing an extra layer of indirection so as to be able to bind the texture builder to the same flush and submit patern as the array builder. --- Outputs/CRT/CRT.cpp | 8 ++++---- Outputs/CRT/Internals/TextureBuilder.hpp | 22 ++++++++++------------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/Outputs/CRT/CRT.cpp b/Outputs/CRT/CRT.cpp index e4791c740..ec09e64db 100644 --- a/Outputs/CRT/CRT.cpp +++ b/Outputs/CRT/CRT.cpp @@ -315,8 +315,8 @@ void CRT::output_level(unsigned int number_of_cycles) Scan scan { .type = Scan::Type::Level, .number_of_cycles = number_of_cycles, - .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), - .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position() +// .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), +// .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position() }; output_scan(&scan); } @@ -349,8 +349,8 @@ void CRT::output_data(unsigned int number_of_cycles, unsigned int source_divider Scan scan { .type = Scan::Type::Data, .number_of_cycles = number_of_cycles, - .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), - .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position(), +// .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), +// .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position(), .source_divider = source_divider }; output_scan(&scan); diff --git a/Outputs/CRT/Internals/TextureBuilder.hpp b/Outputs/CRT/Internals/TextureBuilder.hpp index b346ab66c..211caedde 100644 --- a/Outputs/CRT/Internals/TextureBuilder.hpp +++ b/Outputs/CRT/Internals/TextureBuilder.hpp @@ -40,12 +40,6 @@ class TextureBuilder { /// and indicates that its actual final size was @c actual_length. void reduce_previous_allocation_to(size_t actual_length); - /// @returns the start column for the most recent allocated write area. - uint16_t get_last_write_x_position(); - - /// @returns the row of the most recent allocated write area. - uint16_t get_last_write_y_position(); - /// @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. bool is_full(); @@ -53,13 +47,13 @@ class TextureBuilder { /// Updates the currently-bound texture with all new data provided since the last @c submit. void submit(); + struct WriteArea { + uint16_t x, y; + size_t length; + }; + void flush(std::function &write_areas, size_t count)>); + 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_; - - // the most recent position returned for pixel data writing - uint16_t write_x_position_, write_y_position_; - // details of the most recent allocation size_t write_target_pointer_; size_t last_allocation_amount_; @@ -70,6 +64,10 @@ class TextureBuilder { // the buffer std::vector image_; GLuint texture_name_; + + std::vector write_areas_; + size_t number_of_write_areas_; + uint16_t write_areas_start_x_, write_areas_start_y_; }; } From 0fee8096c1af53c23f19edcd1f3b8a56873648d8 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 07:24:07 -0500 Subject: [PATCH 3/8] Made an attempt to shuffle the texture builder to a similar flush/submit pattern as the input builder. Don't care about thread safety yet, as it's obvious I'm going to need to move that back up to the CRT. --- Outputs/CRT/CRT.cpp | 16 ++- Outputs/CRT/Internals/TextureBuilder.cpp | 152 +++++++++++++++-------- Outputs/CRT/Internals/TextureBuilder.hpp | 18 +-- 3 files changed, 123 insertions(+), 63 deletions(-) diff --git a/Outputs/CRT/CRT.cpp b/Outputs/CRT/CRT.cpp index ec09e64db..e1a75b3f5 100644 --- a/Outputs/CRT/CRT.cpp +++ b/Outputs/CRT/CRT.cpp @@ -136,8 +136,8 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi if(next_run) { - source_input_position_x1() = tex_x; - source_input_position_y() = tex_y; +// source_input_position_x1() = tex_x; +// source_input_position_y() = tex_y; source_output_position_x1() = (uint16_t)horizontal_flywheel_->get_current_output_position(); // Don't write output_y now, write it later; we won't necessarily know what it is outside of the locked region source_phase() = colour_burst_phase_; @@ -204,8 +204,18 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi output_x2() = (uint16_t)horizontal_flywheel_->get_current_output_position(); } openGL_output_builder_.array_builder.flush( - [output_y] (uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size) + [output_y, this] (uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size) { + openGL_output_builder_.texture_builder.flush( + [output_y, input_buffer] (const std::vector &write_areas, size_t number_of_write_areas) + { + for(size_t run = 0; run < number_of_write_areas; run++) + { + *(uint16_t *)&input_buffer[run * SourceVertexSize + SourceVertexOffsetOfInputStart + 0] = write_areas[run].x; + *(uint16_t *)&input_buffer[run * SourceVertexSize + SourceVertexOffsetOfInputStart + 2] = write_areas[run].y; + *(uint16_t *)&input_buffer[run * SourceVertexSize + SourceVertexOffsetOfEnds + 0] = write_areas[run].x + write_areas[run].length; + } + }); for(size_t position = 0; position < input_size; position += SourceVertexSize) { (*(uint16_t *)&input_buffer[position + SourceVertexOffsetOfOutputStart + 2]) = output_y; diff --git a/Outputs/CRT/Internals/TextureBuilder.cpp b/Outputs/CRT/Internals/TextureBuilder.cpp index 1c7bf8f1f..28ed44857 100644 --- a/Outputs/CRT/Internals/TextureBuilder.cpp +++ b/Outputs/CRT/Internals/TextureBuilder.cpp @@ -39,8 +39,10 @@ static const GLenum formatForDepth(size_t depth) TextureBuilder::TextureBuilder(size_t bytes_per_pixel, GLenum texture_unit) : bytes_per_pixel_(bytes_per_pixel), - next_write_x_position_(0), - next_write_y_position_(0) + write_areas_start_x_(0), + write_areas_start_y_(0), + is_full_(false), + did_submit_(false) { image_.resize(bytes_per_pixel * InputBufferBuilderWidth * InputBufferBuilderHeight); glGenTextures(1, &texture_name_); @@ -59,81 +61,81 @@ TextureBuilder::~TextureBuilder() glDeleteTextures(1, &texture_name_); } +inline uint8_t *TextureBuilder::pointer_to_location(uint16_t x, uint16_t y) +{ + return &image_[((y * InputBufferBuilderWidth) + x) * bytes_per_pixel_]; +} + uint8_t *TextureBuilder::allocate_write_area(size_t required_length) { - if(next_write_y_position_ != InputBufferBuilderHeight) + if(is_full_) return nullptr; + + uint16_t starting_x, starting_y; + + if(!number_of_write_areas_) { - last_allocation_amount_ = required_length; - - if(next_write_x_position_ + required_length + 2 > InputBufferBuilderWidth) - { - next_write_x_position_ = 0; - next_write_y_position_++; - - if(next_write_y_position_ == InputBufferBuilderHeight) - return nullptr; - } - - write_x_position_ = next_write_x_position_ + 1; - write_y_position_ = next_write_y_position_; - write_target_pointer_ = (write_y_position_ * InputBufferBuilderWidth) + write_x_position_; - next_write_x_position_ += required_length + 2; + starting_x = write_areas_start_x_; + 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; } - else return nullptr; - return &image_[write_target_pointer_ * bytes_per_pixel_]; + WriteArea next_write_area; + if(starting_x + required_length + 2 > InputBufferBuilderWidth) + { + starting_x = 0; + starting_y++; + + if(starting_y == InputBufferBuilderHeight) + { + is_full_ = true; + return nullptr; + } + } + + next_write_area.x = starting_x + 1; + next_write_area.y = starting_y; + next_write_area.length = (uint16_t)required_length; + if(number_of_write_areas_ < write_areas_.size()) + write_areas_[number_of_write_areas_] = next_write_area; + else + write_areas_.push_back(next_write_area); + number_of_write_areas_++; + + return pointer_to_location(next_write_area.x, next_write_area.y); } bool TextureBuilder::is_full() { - return (next_write_y_position_ == InputBufferBuilderHeight); + return is_full_; } void TextureBuilder::reduce_previous_allocation_to(size_t actual_length) { - if(next_write_y_position_ == InputBufferBuilderHeight) return; + if(is_full_) return; - uint8_t *const image_pointer = image_.data(); - - // correct if the writing cursor was reset while a client was writing - if(next_write_x_position_ == 0 && next_write_y_position_ == 0) - { - memmove(&image_pointer[bytes_per_pixel_], &image_pointer[write_target_pointer_ * bytes_per_pixel_], 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; - } + WriteArea &write_area = write_areas_[number_of_write_areas_-1]; + write_area.length = (uint16_t)actual_length; // book end the allocation with duplicates of the first and last pixel, to protect // against rounding errors when this run is drawn - memcpy( &image_pointer[(write_target_pointer_ - 1) * bytes_per_pixel_], - &image_pointer[write_target_pointer_ * bytes_per_pixel_], + uint8_t *start_pointer = pointer_to_location(write_area.x, write_area.y); + memcpy( &start_pointer[-bytes_per_pixel_], + start_pointer, bytes_per_pixel_); - memcpy( &image_pointer[(write_target_pointer_ + actual_length) * bytes_per_pixel_], - &image_pointer[(write_target_pointer_ + actual_length - 1) * bytes_per_pixel_], + memcpy( &start_pointer[actual_length * bytes_per_pixel_], + &start_pointer[(actual_length - 1) * bytes_per_pixel_], bytes_per_pixel_); - - // return any allocated length that wasn't actually used to the available pool - next_write_x_position_ -= (last_allocation_amount_ - actual_length); -} - -uint16_t TextureBuilder::get_last_write_x_position() -{ - return write_x_position_; -} - -uint16_t TextureBuilder::get_last_write_y_position() -{ - return write_y_position_; } void TextureBuilder::submit() { - uint16_t height = write_y_position_ + (next_write_x_position_ ? 1 : 0); - next_write_x_position_ = next_write_y_position_ = 0; + uint16_t height = write_areas_start_y_ + (write_areas_start_x_ ? 1 : 0); + did_submit_ = true; glTexSubImage2D( GL_TEXTURE_2D, 0, 0, 0, @@ -141,3 +143,47 @@ void TextureBuilder::submit() formatForDepth(bytes_per_pixel_), GL_UNSIGNED_BYTE, image_.data()); } + +void TextureBuilder::flush(const std::function &write_areas, size_t count)> &function) +{ + bool was_full = is_full_; + if(did_submit_) + { + write_areas_start_y_ = write_areas_start_x_ = 0; + is_full_ = false; + } + + if(number_of_write_areas_ && !was_full) + { + if(write_areas_[0].x != write_areas_start_x_ || write_areas_[0].y != write_areas_start_y_) + { + for(auto write_area : write_areas_) + { + 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_); + } + + did_submit_ = false; + number_of_write_areas_ = 0; +} diff --git a/Outputs/CRT/Internals/TextureBuilder.hpp b/Outputs/CRT/Internals/TextureBuilder.hpp index 211caedde..43b4a7b3a 100644 --- a/Outputs/CRT/Internals/TextureBuilder.hpp +++ b/Outputs/CRT/Internals/TextureBuilder.hpp @@ -10,6 +10,7 @@ #define Outputs_CRT_Internals_TextureBuilder_hpp #include +#include #include #include @@ -48,16 +49,11 @@ class TextureBuilder { void submit(); struct WriteArea { - uint16_t x, y; - size_t length; + uint16_t x, y, length; }; - void flush(std::function &write_areas, size_t count)>); + void flush(const std::function &write_areas, size_t count)> &); private: - // details of the most recent allocation - size_t write_target_pointer_; - size_t last_allocation_amount_; - // the buffer size size_t bytes_per_pixel_; @@ -65,8 +61,16 @@ class TextureBuilder { std::vector image_; GLuint texture_name_; + // the current list of write areas std::vector write_areas_; size_t number_of_write_areas_; + bool is_full_; + bool did_submit_; + inline uint8_t *pointer_to_location(uint16_t x, uint16_t y); + + // Usually: the start position for the current batch of write areas. + // 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_; }; From f388ba11ccb1ad8298291b435c49391130bcbb46 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 07:26:23 -0500 Subject: [PATCH 4/8] Missed an initialisation. Fixed! --- Outputs/CRT/Internals/TextureBuilder.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Outputs/CRT/Internals/TextureBuilder.cpp b/Outputs/CRT/Internals/TextureBuilder.cpp index 28ed44857..dbb54da1b 100644 --- a/Outputs/CRT/Internals/TextureBuilder.cpp +++ b/Outputs/CRT/Internals/TextureBuilder.cpp @@ -42,7 +42,8 @@ TextureBuilder::TextureBuilder(size_t bytes_per_pixel, GLenum texture_unit) : write_areas_start_x_(0), write_areas_start_y_(0), is_full_(false), - did_submit_(false) + did_submit_(false), + number_of_write_areas_(0) { image_.resize(bytes_per_pixel * InputBufferBuilderWidth * InputBufferBuilderHeight); glGenTextures(1, &texture_name_); From 60f9ddfde81c011704e2621f7a3584c8f089a011 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 08:08:57 -0500 Subject: [PATCH 5/8] Fixed start test and added incrementation of start locations. --- Outputs/CRT/Internals/TextureBuilder.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Outputs/CRT/Internals/TextureBuilder.cpp b/Outputs/CRT/Internals/TextureBuilder.cpp index dbb54da1b..01d89f03f 100644 --- a/Outputs/CRT/Internals/TextureBuilder.cpp +++ b/Outputs/CRT/Internals/TextureBuilder.cpp @@ -156,7 +156,7 @@ void TextureBuilder::flush(const std::function if(number_of_write_areas_ && !was_full) { - if(write_areas_[0].x != write_areas_start_x_ || write_areas_[0].y != write_areas_start_y_) + if(write_areas_[0].x != write_areas_start_x_+1 || write_areas_[0].y != write_areas_start_y_) { for(auto write_area : write_areas_) { @@ -182,7 +182,12 @@ void TextureBuilder::flush(const std::function } 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; From 4ff33254e110bd421cd303d6ebc266093609c5c2 Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 18:48:30 -0500 Subject: [PATCH 6/8] Sought to shift locking back up to the CRT. And to be a bit more RAII-ish. --- Outputs/CRT/CRT.cpp | 55 ++++++-------------------- Outputs/CRT/CRT.hpp | 1 + Outputs/CRT/Internals/ArrayBuilder.cpp | 4 -- Outputs/CRT/Internals/ArrayBuilder.hpp | 5 +-- Outputs/CRT/Internals/CRTOpenGL.hpp | 13 ++---- 5 files changed, 20 insertions(+), 58 deletions(-) diff --git a/Outputs/CRT/CRT.cpp b/Outputs/CRT/CRT.cpp index e1a75b3f5..8f656e4c5 100644 --- a/Outputs/CRT/CRT.cpp +++ b/Outputs/CRT/CRT.cpp @@ -110,6 +110,7 @@ Flywheel::SyncEvent CRT::get_next_horizontal_sync_event(bool hsync_is_requested, void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divider, bool hsync_requested, bool vsync_requested, const bool vsync_charging, const Scan::Type type, uint16_t tex_x, uint16_t tex_y) { + std::unique_lock output_lock = openGL_output_builder_.get_output_lock(); number_of_cycles *= time_multiplier_; bool is_output_run = ((type == Scan::Type::Level) || (type == Scan::Type::Data)); @@ -189,8 +190,6 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi } else { - openGL_output_builder_.lock_output(); - // Get and write all those previously unwritten output ys const uint16_t output_y = openGL_output_builder_.get_composite_output_y(); @@ -221,8 +220,6 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi (*(uint16_t *)&input_buffer[position + SourceVertexOffsetOfOutputStart + 2]) = output_y; } }); - - openGL_output_builder_.unlock_output(); } is_writing_composite_run_ ^= true; } @@ -320,24 +317,11 @@ void CRT::output_blank(unsigned int number_of_cycles) void CRT::output_level(unsigned int number_of_cycles) { - if(!openGL_output_builder_.array_builder.is_full()) - { - Scan scan { - .type = Scan::Type::Level, - .number_of_cycles = number_of_cycles, -// .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), -// .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position() - }; - output_scan(&scan); - } - else - { - Scan scan { - .type = Scan::Type::Blank, - .number_of_cycles = number_of_cycles - }; - output_scan(&scan); - } + Scan scan { + .type = Scan::Type::Level, + .number_of_cycles = number_of_cycles, + }; + output_scan(&scan); } void CRT::output_colour_burst(unsigned int number_of_cycles, uint8_t phase, uint8_t amplitude) @@ -353,26 +337,13 @@ void CRT::output_colour_burst(unsigned int number_of_cycles, uint8_t phase, uint void CRT::output_data(unsigned int number_of_cycles, unsigned int source_divider) { - if(!openGL_output_builder_.array_builder.is_full()) - { - openGL_output_builder_.texture_builder.reduce_previous_allocation_to(number_of_cycles / source_divider); - Scan scan { - .type = Scan::Type::Data, - .number_of_cycles = number_of_cycles, -// .tex_x = openGL_output_builder_.texture_builder.get_last_write_x_position(), -// .tex_y = openGL_output_builder_.texture_builder.get_last_write_y_position(), - .source_divider = source_divider - }; - output_scan(&scan); - } - else - { - Scan scan { - .type = Scan::Type::Blank, - .number_of_cycles = number_of_cycles - }; - output_scan(&scan); - } + openGL_output_builder_.texture_builder.reduce_previous_allocation_to(number_of_cycles / source_divider); + Scan scan { + .type = Scan::Type::Data, + .number_of_cycles = number_of_cycles, + .source_divider = source_divider + }; + output_scan(&scan); } 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) diff --git a/Outputs/CRT/CRT.hpp b/Outputs/CRT/CRT.hpp index 4baa2a84f..85e495c95 100644 --- a/Outputs/CRT/CRT.hpp +++ b/Outputs/CRT/CRT.hpp @@ -192,6 +192,7 @@ class CRT { */ inline uint8_t *allocate_write_area(size_t required_length) { + std::unique_lock output_lock = openGL_output_builder_.get_output_lock(); return openGL_output_builder_.texture_builder.allocate_write_area(required_length); } diff --git a/Outputs/CRT/Internals/ArrayBuilder.cpp b/Outputs/CRT/Internals/ArrayBuilder.cpp index 53a97e1b1..61a9be2fb 100644 --- a/Outputs/CRT/Internals/ArrayBuilder.cpp +++ b/Outputs/CRT/Internals/ArrayBuilder.cpp @@ -23,7 +23,6 @@ ArrayBuilder::ArrayBuilder(size_t input_size, size_t output_size, std::function< bool ArrayBuilder::is_full() { bool was_full; - std::lock_guard guard(buffer_mutex_); was_full = is_full_; return was_full; } @@ -40,7 +39,6 @@ uint8_t *ArrayBuilder::get_output_storage(size_t size) void ArrayBuilder::flush(const std::function &function) { - std::lock_guard guard(buffer_mutex_); if(!is_full_) { size_t input_size, output_size; @@ -67,7 +65,6 @@ ArrayBuilder::Submission ArrayBuilder::submit() { ArrayBuilder::Submission submission; - std::lock_guard guard(buffer_mutex_); submission.input_size = input_.submit(true); submission.output_size = output_.submit(false); if(is_full_) @@ -102,7 +99,6 @@ ArrayBuilder::Buffer::~Buffer() uint8_t *ArrayBuilder::get_storage(size_t size, Buffer &buffer) { - std::lock_guard guard(buffer_mutex_); uint8_t *pointer = buffer.get_storage(size); if(!pointer) is_full_ = true; return pointer; diff --git a/Outputs/CRT/Internals/ArrayBuilder.hpp b/Outputs/CRT/Internals/ArrayBuilder.hpp index 0be0644c6..135a84c24 100644 --- a/Outputs/CRT/Internals/ArrayBuilder.hpp +++ b/Outputs/CRT/Internals/ArrayBuilder.hpp @@ -9,9 +9,9 @@ #ifndef ArrayBuilder_hpp #define ArrayBuilder_hpp -#include -#include +#include #include +#include #include "OpenGL.hpp" @@ -92,7 +92,6 @@ class ArrayBuilder { } output_, input_; uint8_t *get_storage(size_t size, Buffer &buffer); - std::mutex buffer_mutex_; bool is_full_; }; diff --git a/Outputs/CRT/Internals/CRTOpenGL.hpp b/Outputs/CRT/Internals/CRTOpenGL.hpp index d8a921b92..2c8718eac 100644 --- a/Outputs/CRT/Internals/CRTOpenGL.hpp +++ b/Outputs/CRT/Internals/CRTOpenGL.hpp @@ -90,6 +90,7 @@ class OpenGLOutputBuilder { GLsync fence_; public: + // These two are protected by output_mutex_. TextureBuilder texture_builder; ArrayBuilder array_builder; @@ -98,12 +99,11 @@ class OpenGLOutputBuilder { inline void set_colour_format(ColourSpace colour_space, unsigned int colour_cycle_numerator, unsigned int colour_cycle_denominator) { - output_mutex_.lock(); + std::lock_guard output_guard(output_mutex_); colour_space_ = colour_space; colour_cycle_numerator_ = colour_cycle_numerator; colour_cycle_denominator_ = colour_cycle_denominator; set_colour_space_uniforms(); - output_mutex_.unlock(); } inline void set_visible_area(Rect visible_area) @@ -111,14 +111,9 @@ class OpenGLOutputBuilder { visible_area_ = visible_area; } - inline void lock_output() + inline std::unique_lock get_output_lock() { - output_mutex_.lock(); - } - - inline void unlock_output() - { - output_mutex_.unlock(); + return std::unique_lock(output_mutex_); } inline OutputDevice get_output_device() From 33d52bb573343f1ffdc0d4712548d249c51d3dda Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 19:02:18 -0500 Subject: [PATCH 7/8] Ensured no over-moving. --- Outputs/CRT/Internals/TextureBuilder.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Outputs/CRT/Internals/TextureBuilder.cpp b/Outputs/CRT/Internals/TextureBuilder.cpp index 01d89f03f..29c0f87b1 100644 --- a/Outputs/CRT/Internals/TextureBuilder.cpp +++ b/Outputs/CRT/Internals/TextureBuilder.cpp @@ -158,8 +158,10 @@ void TextureBuilder::flush(const std::function { if(write_areas_[0].x != write_areas_start_x_+1 || write_areas_[0].y != write_areas_start_y_) { - for(auto write_area : write_areas_) + 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; From 5216dda675554bd0a623ccc22fea1a1d7b6e375f Mon Sep 17 00:00:00 2001 From: Thomas Harte Date: Tue, 6 Dec 2016 19:08:55 -0500 Subject: [PATCH 8/8] Added some brief extra exposition to the texture builder, cut all internal tex_x/y and source_divider stuff from the CRT. --- Outputs/CRT/CRT.cpp | 13 +++---------- Outputs/CRT/CRT.hpp | 6 +----- Outputs/CRT/Internals/TextureBuilder.hpp | 3 +++ 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/Outputs/CRT/CRT.cpp b/Outputs/CRT/CRT.cpp index 8f656e4c5..0096fc0c9 100644 --- a/Outputs/CRT/CRT.cpp +++ b/Outputs/CRT/CRT.cpp @@ -108,7 +108,7 @@ Flywheel::SyncEvent CRT::get_next_horizontal_sync_event(bool hsync_is_requested, #define source_amplitude() next_run[SourceVertexOffsetOfPhaseTimeAndAmplitude + 2] #define source_phase_time() next_run[SourceVertexOffsetOfPhaseTimeAndAmplitude + 1] -void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divider, bool hsync_requested, bool vsync_requested, const bool vsync_charging, const Scan::Type type, uint16_t tex_x, uint16_t tex_y) +void CRT::advance_cycles(unsigned int number_of_cycles, bool hsync_requested, bool vsync_requested, const bool vsync_charging, const Scan::Type type) { std::unique_lock output_lock = openGL_output_builder_.get_output_lock(); number_of_cycles *= time_multiplier_; @@ -137,10 +137,8 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi if(next_run) { -// source_input_position_x1() = tex_x; -// source_input_position_y() = tex_y; + // output_y and texture locations will be written later; we won't necessarily know what it is outside of the locked region source_output_position_x1() = (uint16_t)horizontal_flywheel_->get_current_output_position(); - // Don't write output_y now, write it later; we won't necessarily know what it is outside of the locked region source_phase() = colour_burst_phase_; source_amplitude() = colour_burst_amplitude_; source_phase_time() = (uint8_t)colour_burst_time_; // assumption: burst was within the first 1/16 of the line @@ -162,10 +160,6 @@ void CRT::advance_cycles(unsigned int number_of_cycles, unsigned int source_divi if(next_run) { - // if this is a data run then advance the buffer pointer - if(type == Scan::Type::Data && source_divider) tex_x += next_run_length / (time_multiplier_ * source_divider); - - source_input_position_x2() = tex_x; source_output_position_x2() = (uint16_t)horizontal_flywheel_->get_current_output_position(); } @@ -291,7 +285,7 @@ void CRT::output_scan(const Scan *const scan) // TODO: inspect raw data for potential colour burst if required sync_period_ = is_receiving_sync_ ? (sync_period_ + scan->number_of_cycles) : 0; - advance_cycles(scan->number_of_cycles, scan->source_divider, hsync_requested, vsync_requested, this_is_sync, scan->type, scan->tex_x, scan->tex_y); + advance_cycles(scan->number_of_cycles, hsync_requested, vsync_requested, this_is_sync, scan->type); } /* @@ -341,7 +335,6 @@ void CRT::output_data(unsigned int number_of_cycles, unsigned int source_divider Scan scan { .type = Scan::Type::Data, .number_of_cycles = number_of_cycles, - .source_divider = source_divider }; output_scan(&scan); } diff --git a/Outputs/CRT/CRT.hpp b/Outputs/CRT/CRT.hpp index 85e495c95..d2482c25b 100644 --- a/Outputs/CRT/CRT.hpp +++ b/Outputs/CRT/CRT.hpp @@ -53,10 +53,6 @@ class CRT { } type; unsigned int number_of_cycles; union { - struct { - unsigned int source_divider; - uint16_t tex_x, tex_y; - }; struct { uint8_t phase, amplitude; }; @@ -69,7 +65,7 @@ class CRT { bool is_writing_composite_run_; // the outer entry point for dispatching output_sync, output_blank, output_level and output_data - void advance_cycles(unsigned int number_of_cycles, unsigned int source_divider, bool hsync_requested, bool vsync_requested, const bool vsync_charging, const Scan::Type type, uint16_t tex_x, uint16_t tex_y); + void advance_cycles(unsigned int number_of_cycles, bool hsync_requested, bool vsync_requested, const bool vsync_charging, const Scan::Type type); // the inner entry point that determines whether and when the next sync event will occur within // the current output window diff --git a/Outputs/CRT/Internals/TextureBuilder.hpp b/Outputs/CRT/Internals/TextureBuilder.hpp index 43b4a7b3a..d200a9b97 100644 --- a/Outputs/CRT/Internals/TextureBuilder.hpp +++ b/Outputs/CRT/Internals/TextureBuilder.hpp @@ -51,6 +51,9 @@ class TextureBuilder { struct WriteArea { uint16_t x, y, length; }; + /// Finalises all write areas allocated since the last call to @c flush. Only finalised areas will be + /// submitted upon the next @c submit. The supplied function will be called with a list of write areas + /// allocated, indicating their final resting locations and their lengths. void flush(const std::function &write_areas, size_t count)> &); private: