Merge branch 'master' into EnchantedWoods

Analyser/Static/Commodore/StaticAnalyser.cpp

@@ -16,6 +16,7 @@
 #include "../../../Outputs/Log.hpp"
 
 #include <algorithm>
+#include <cstring>
 #include <sstream>
 
 using namespace Analyser::Static::Commodore;
@@ -78,7 +79,7 @@ Analyser::Static::TargetList Analyser::Static::Commodore::GetTargets(const Media
 	}
 
 	if(!files.empty()) {
-		target->memory_model = Target::MemoryModel::Unexpanded;
+		auto memory_model = Target::MemoryModel::Unexpanded;
 		std::ostringstream string_stream;
 		string_stream << "LOAD\"" << (is_disk ? "*" : "") << "\"," << device << ",";
 		if(files.front().is_basic()) {
@@ -94,16 +95,18 @@ Analyser::Static::TargetList Analyser::Static::Commodore::GetTargets(const Media
 			default:
 				LOG("Unrecognised loading address for Commodore program: " << PADHEX(4) <<  files.front().starting_address);
 			case 0x1001:
-				target->memory_model = Target::MemoryModel::Unexpanded;
+				memory_model = Target::MemoryModel::Unexpanded;
 			break;
 			case 0x1201:
-				target->memory_model = Target::MemoryModel::ThirtyTwoKB;
+				memory_model = Target::MemoryModel::ThirtyTwoKB;
 			break;
 			case 0x0401:
-				target->memory_model = Target::MemoryModel::EightKB;
+				memory_model = Target::MemoryModel::EightKB;
 			break;
 		}
 
+		target->set_memory_model(memory_model);
+
 		// General approach: increase memory size conservatively such that the largest file found will fit.
 //		for(File &file : files) {
 //			std::size_t file_size = file.data.size();
@@ -145,13 +148,52 @@ Analyser::Static::TargetList Analyser::Static::Commodore::GetTargets(const Media
 	}
 
 	if(!target->media.empty()) {
-		// Inspect filename for a region hint.
+		// Inspect filename for configuration hints.
 		std::string lowercase_name = file_name;
 		std::transform(lowercase_name.begin(), lowercase_name.end(), lowercase_name.begin(), ::tolower);
+
+		// Hint 1: 'ntsc' anywhere in the name implies America.
 		if(lowercase_name.find("ntsc") != std::string::npos) {
 			target->region = Analyser::Static::Commodore::Target::Region::American;
 		}
 
+		// Potential additional hints: check for TheC64 tags.
+		auto final_underscore = lowercase_name.find_last_of('_');
+		if(final_underscore != std::string::npos) {
+			auto iterator = lowercase_name.begin() + ssize_t(final_underscore) + 1;
+
+			while(iterator != lowercase_name.end()) {
+				// Grab the next tag.
+				char next_tag[3] = {0, 0, 0};
+				next_tag[0] = *iterator++;
+				if(iterator == lowercase_name.end()) break;
+				next_tag[1] = *iterator++;
+
+				// Exit early if attempting to read another tag has run over the file extension.
+				if(next_tag[0] == '.' || next_tag[1] == '.') break;
+
+				// Check whether it's anything.
+				target->enabled_ram.bank0 |= !strcmp(next_tag, "b0");
+				target->enabled_ram.bank1 |= !strcmp(next_tag, "b1");
+				target->enabled_ram.bank2 |= !strcmp(next_tag, "b2");
+				target->enabled_ram.bank3 |= !strcmp(next_tag, "b3");
+				target->enabled_ram.bank5 |= !strcmp(next_tag, "b5");
+				if(!strcmp(next_tag, "tn")) {	// i.e. NTSC.
+					target->region = Analyser::Static::Commodore::Target::Region::American;
+				}
+				if(!strcmp(next_tag, "tp")) {	// i.e. PAL.
+					target->region = Analyser::Static::Commodore::Target::Region::European;
+				}
+
+				// Unhandled:
+				//
+				//	M6: 	this is a C64 file.
+				//	MV: 	this is a Vic-20 file.
+				//	J1/J2:	this C64 file should have the primary joystick in slot 1/2.
+				//	RO:		this disk image should be treated as read-only.
+			}
+		}
+
 		// Attach a 1540 if there are any disks here.
 		target->has_c1540 = !target->media.disks.empty();
 

Analyser/Static/Commodore/Target.hpp

@@ -31,7 +31,26 @@ struct Target: public ::Analyser::Static::Target {
 		Swedish
 	};
 
-	MemoryModel memory_model = MemoryModel::Unexpanded;
+	/// Maps from a named memory model to a bank enabled/disabled set.
+	void set_memory_model(MemoryModel memory_model) {
+		// This is correct for unexpanded and 32kb memory models.
+		enabled_ram.bank0 = enabled_ram.bank1 =
+		enabled_ram.bank2 = enabled_ram.bank3 =
+		enabled_ram.bank5 = memory_model == MemoryModel::ThirtyTwoKB;
+
+		// Bank 0 will need to be enabled if this is an 8kb machine.
+		enabled_ram.bank0 |= memory_model == MemoryModel::EightKB;
+	}
+	struct {
+		bool bank0 = false;
+		bool bank1 = false;
+		bool bank2 = false;
+		bool bank3 = false;
+		bool bank5 = false;
+					// Sic. There is no bank 4; this is because the area that logically would be
+					// bank 4 is occupied by the character ROM, colour RAM, hardware registers, etc.
+	} enabled_ram;
+
 	Region region = Region::European;
 	bool has_c1540 = false;
 	std::string loading_command;

ClockReceiver/JustInTime.hpp

@@ -29,7 +29,7 @@ template <class T, int multiplier = 1, int divider = 1, class LocalTimeScale = H
 
 		/// Adds time to the actor.
 		forceinline void operator += (const LocalTimeScale &rhs) {
-			if(multiplier != 1) {
+			if constexpr (multiplier != 1) {
 				time_since_update_ += rhs * multiplier;
 			} else {
 				time_since_update_ += rhs;
@@ -52,7 +52,7 @@ template <class T, int multiplier = 1, int divider = 1, class LocalTimeScale = H
 		forceinline void flush() {
 			if(!is_flushed_) {
 				is_flushed_ = true;
-				if(divider == 1) {
+				if constexpr (divider == 1) {
 					object_.run_for(time_since_update_.template flush<TargetTimeScale>());
 				} else {
 					const auto duration = time_since_update_.template divide<TargetTimeScale>(LocalTimeScale(divider));

Machines/Atari/ST/AtariST.cpp

@@ -176,7 +176,10 @@ class ConcreteMachine:
 			// An interrupt acknowledge, perhaps?
 			if(cycle.operation & Microcycle::InterruptAcknowledge) {
 				// Current implementation: everything other than 6 (i.e. the MFP is autovectored.
-				if((cycle.word_address()&7) != 6) {
+				const int interrupt_level = cycle.word_address()&7;
+				if(interrupt_level != 6) {
+					video_interrupts_pending_ &= ~interrupt_level;
+					update_interrupt_input();
 					mc68000_.set_is_peripheral_address(true);
 					return HalfCycles(0);
 				} else {
@@ -552,12 +555,27 @@ class ConcreteMachine:
 			update_interrupt_input();
 		}
 
+		int video_interrupts_pending_ = 0;
+		bool previous_hsync_ = false, previous_vsync_ = false;
 		void update_interrupt_input() {
+			// Complete guess: set video interrupts pending if/when hsync of vsync
+			// go inactive. Reset upon IACK.
+			const bool hsync = video_.last_valid()->hsync();
+			const bool vsync = video_.last_valid()->vsync();
+			if(previous_hsync_ != hsync && previous_hsync_) {
+				video_interrupts_pending_ |= 2;
+			}
+			if(previous_vsync_ != vsync && previous_vsync_) {
+				video_interrupts_pending_ |= 4;
+			}
+			previous_vsync_ = vsync;
+			previous_hsync_ = hsync;
+
 			if(mfp_->get_interrupt_line()) {
 				mc68000_.set_interrupt_level(6);
-			} else if(video_->vsync()) {
+			} else if(video_interrupts_pending_ & 4) {
 				mc68000_.set_interrupt_level(4);
-			} else if(video_->hsync()) {
+			} else if(video_interrupts_pending_ & 2) {
 				mc68000_.set_interrupt_level(2);
 			} else {
 				mc68000_.set_interrupt_level(0);

Machines/Atari/ST/Video.cpp

@@ -95,19 +95,24 @@ struct Checker {
 #endif
 
 const int de_delay_period = CYCLE(28);		// Number of half cycles after DE that observed DE changes.
-const int vsync_x_position = CYCLE(54);	// Horizontal cycle on which vertical sync changes happen.
+const int vsync_x_position = CYCLE(56);		// Horizontal cycle on which vertical sync changes happen.
+
 const int hsync_start = CYCLE(48);			// Cycles before end of line when hsync starts.
 const int hsync_end = CYCLE(8);				// Cycles before end of line when hsync ends.
 
+const int hsync_delay_period = hsync_end;			// Signal hsync at the end of the line.
+const int vsync_delay_period = hsync_delay_period;	// Signal vsync with the same delay as hsync.
+
 // "VSYNC starts 104 cycles after the start of the previous line's HSYNC, so that's 4 cycles before DE would be activated. ";
-// hsync is at -50, so that's +54
+// that's an inconsistent statement since it would imply VSYNC at +54, which is 2 cycles before DE in 60Hz mode and 6 before
+// in 50Hz mode. I've gone with 56, to be four cycles ahead of DE in 50Hz mode.
 
 }
 
 Video::Video() :
 	deferrer_([=] (HalfCycles duration) { advance(duration); }),
 	crt_(1024, 1, Outputs::Display::Type::PAL50, Outputs::Display::InputDataType::Red4Green4Blue4),
-	shifter_(crt_, palette_) {
+	video_stream_(crt_, palette_) {
 
 	// Show a total of 260 lines; a little short for PAL but a compromise between that and the ST's
 	// usual output height of 200 lines.
@@ -177,6 +182,8 @@ void Video::advance(HalfCycles duration) {
 		// Determine current output mode and number of cycles to output for.
 		const int run_length = std::min(integer_duration, next_event - x_);
 		const bool display_enable = vertical_.enable && horizontal_.enable;
+		const bool hsync = horizontal_.sync;
+		const bool vsync = vertical_.sync;
 
 		// Ensure proper fetching irrespective of the output.
 		if(load_) {
@@ -196,16 +203,12 @@ void Video::advance(HalfCycles duration) {
 			}
 		}
 
-		// TODO: if I'm asserting that sync and blank override the shifter (but, presumably,
-		// the shifter keeps shifting), then output_sync and output_blank need to have an effect
-		// inside the shifter on the temporary register values.
-
 		if(horizontal_.sync || vertical_.sync) {
-			shifter_.output_sync(run_length);
+			video_stream_.output(run_length, VideoStream::OutputMode::Sync);
 		} else if(horizontal_.blank || vertical_.blank) {
-			shifter_.output_blank(run_length);
+			video_stream_.output(run_length, VideoStream::OutputMode::Blank);
 		} else if(!load_) {
-			shifter_.output_border(run_length, output_bpp_);
+			video_stream_.output(run_length, VideoStream::OutputMode::Pixels);
 		} else {
 			const int since_load = x_ - load_base_;
 
@@ -222,12 +225,12 @@ void Video::advance(HalfCycles duration) {
 			//	was reloaded by the fetch depends on the FIFO.
 
 			if(start_column == end_column) {
-				shifter_.output_pixels(run_length, output_bpp_);
+				video_stream_.output(run_length, VideoStream::OutputMode::Pixels);
 			} else {
 				// Continue the current column if partway across.
 				if(since_load&7) {
 					// If at least one column boundary is crossed, complete this column.
-					shifter_.output_pixels(8 - (since_load & 7), output_bpp_);
+					video_stream_.output(8 - (since_load & 7), VideoStream::OutputMode::Pixels);
 					++start_column;	// This starts a new column, so latch a new word.
 					push_latched_data();
 				}
@@ -235,13 +238,13 @@ void Video::advance(HalfCycles duration) {
 				// Run for all columns that have their starts in this time period.
 				int complete_columns = end_column - start_column;
 				while(complete_columns--) {
-					shifter_.output_pixels(8, output_bpp_);
+					video_stream_.output(8, VideoStream::OutputMode::Pixels);
 					push_latched_data();
 				}
 
 				// Output the start of the next column, if necessary.
 				if((since_load + run_length) & 7) {
-					shifter_.output_pixels((since_load + run_length) & 7, output_bpp_);
+					video_stream_.output((since_load + run_length) & 7, VideoStream::OutputMode::Pixels);
 				}
 			}
 		}
@@ -265,22 +268,10 @@ void Video::advance(HalfCycles duration) {
 			} else if(y_ == vertical_timings.reset_enable) {
 				next_vertical_.enable = false;
 			} else if(next_y_ == vertical_timings.height) {
-				next_y_ = 0;
-			} else if(y_ == 0) {
 				next_vertical_.sync_schedule = VerticalState::SyncSchedule::Begin;
-			} else if(y_ == 3) {
+				next_y_ = 0;
+			} else if(next_y_ == 2) {
 				next_vertical_.sync_schedule = VerticalState::SyncSchedule::End;
-
-				current_address_ = base_address_ >> 1;
-				reset_fifo();	// TODO: remove this, I think, once otherwise stable.
-
-				// Consider a shout out to the range observer.
-				if(previous_base_address_ != base_address_) {
-					previous_base_address_ = base_address_;
-					if(range_observer_) {
-						range_observer_->video_did_change_access_range(this);
-					}
-				}
 			}
 		}
 
@@ -288,14 +279,17 @@ void Video::advance(HalfCycles duration) {
 		x_ += run_length;
 		integer_duration -= run_length;
 
-		// Check horizontal events.
+		// Check horizontal events; the first six are guaranteed to occur separately.
 		if(horizontal_timings.reset_blank == x_)		horizontal_.blank = false;
 		else if(horizontal_timings.set_blank == x_)		horizontal_.blank = true;
 		else if(horizontal_timings.reset_enable == x_)	horizontal_.enable = false;
 		else if(horizontal_timings.set_enable == x_) 	horizontal_.enable = true;
 		else if(line_length_ - hsync_start == x_)		{ horizontal_.sync = true; horizontal_.enable = false; }
 		else if(line_length_ - hsync_end == x_)			horizontal_.sync = false;
-		else if(next_load_toggle_ == x_)				{
+
+		// next_load_toggle_ is less predictable; test separately because it may coincide
+		// with one of the above tests.
+		if(next_load_toggle_ == x_)	{
 			next_load_toggle_ = -1;
 			load_ ^= true;
 			load_base_ = x_;
@@ -305,6 +299,8 @@ void Video::advance(HalfCycles duration) {
 		if(vertical_.sync_schedule != VerticalState::SyncSchedule::None && x_ == vsync_x_position) {
 			vertical_.sync = vertical_.sync_schedule == VerticalState::SyncSchedule::Begin;
 			vertical_.enable &= !vertical_.sync;
+
+			reset_fifo();	// TODO: remove this, probably, once otherwise stable?
 		}
 
 		// Check whether the terminating event was end-of-line; if so then advance
@@ -315,6 +311,20 @@ void Video::advance(HalfCycles duration) {
 			y_ = next_y_;
 		}
 
+		// The address is reloaded during the entire period of vertical sync.
+		// Cf. http://www.atari-forum.com/viewtopic.php?t=31954&start=50#p324730
+		if(vertical_.sync) {
+			current_address_ = base_address_ >> 1;
+
+			// Consider a shout out to the range observer.
+			if(previous_base_address_ != base_address_) {
+				previous_base_address_ = base_address_;
+				if(range_observer_) {
+					range_observer_->video_did_change_access_range(this);
+				}
+			}
+		}
+
 		// Chuck any deferred output changes into the queue.
 		const bool next_display_enable = vertical_.enable && horizontal_.enable;
 		if(display_enable != next_display_enable) {
@@ -324,6 +334,16 @@ void Video::advance(HalfCycles duration) {
 			// Schedule change in inwardly-visible effect.
 			next_load_toggle_ = x_ + 8;	// 4 cycles = 8 half-cycles
 		}
+
+		if(horizontal_.sync != hsync) {
+			// Schedule change in outwardly-visible hsync line.
+			add_event(hsync_delay_period - integer_duration, horizontal_.sync ? Event::Type::SetHsync : Event::Type::ResetHsync);
+		}
+
+		if(vertical_.sync != vsync) {
+			// Schedule change in outwardly-visible hsync line.
+			add_event(vsync_delay_period - integer_duration, vertical_.sync ? Event::Type::SetVsync : Event::Type::ResetVsync);
+		}
 	}
 }
 
@@ -331,7 +351,7 @@ void Video::push_latched_data() {
 	data_latch_read_position_ = (data_latch_read_position_ + 1) & 127;
 
 	if(!(data_latch_read_position_ & 3)) {
-		shifter_.load(
+		video_stream_.load(
 			(uint64_t(data_latch_[(data_latch_read_position_ - 4) & 127]) << 48) |
 			(uint64_t(data_latch_[(data_latch_read_position_ - 3) & 127]) << 32) |
 			(uint64_t(data_latch_[(data_latch_read_position_ - 2) & 127]) << 16) |
@@ -345,11 +365,11 @@ void Video::reset_fifo() {
 }
 
 bool Video::hsync() {
-	return horizontal_.sync;
+	return public_state_.hsync;
 }
 
 bool Video::vsync() {
-	return vertical_.sync;
+	return public_state_.vsync;
 }
 
 bool Video::display_enabled() {
@@ -379,7 +399,7 @@ HalfCycles Video::get_next_sequence_point() {
 
 	// If any events are pending, give the first of those the chance to be next.
 	if(!pending_events_.empty()) {
-		event_time = std::min(event_time, x_ + event_time);
+		event_time = std::min(event_time, x_ + pending_events_.front().delay);
 	}
 
 	// If this is a vertically-enabled line, check for the display enable boundaries, + the standard delay.
@@ -398,10 +418,13 @@ HalfCycles Video::get_next_sequence_point() {
 	}
 
 	// Test for beginning and end of horizontal sync.
-	if(x_ < line_length_ - hsync_start) 	event_time = std::min(line_length_ - hsync_start, event_time);
+	if(x_ < line_length_ - hsync_start + hsync_delay_period) {
-	else if(x_ < line_length_ - hsync_end)	event_time = std::min(line_length_ - hsync_end, event_time);
+		event_time = std::min(line_length_ - hsync_start + hsync_delay_period, event_time);
+	}
+	/* Hereby assumed: hsync end will be communicated at end of line: */
+	static_assert(hsync_end == hsync_delay_period);
 
-	// It wasn't any of those, so as a temporary expedient, just supply end of line.
+	// It wasn't any of those, just supply end of line. That's when the static_assert above assumes a visible hsync transition.
 	return HalfCycles(event_time - x_);
 }
 
@@ -456,6 +479,8 @@ void Video::write(int address, uint16_t value) {
 		case 0x24:	case 0x25:	case 0x26:	case 0x27:
 		case 0x28:	case 0x29:	case 0x2a:	case 0x2b:
 		case 0x2c:	case 0x2d:	case 0x2e:	case 0x2f: {
+			if(address == 0x20) video_stream_.will_change_border_colour();
+
 			raw_palette_[address - 0x20] = value;
 			uint8_t *const entry = reinterpret_cast<uint8_t *>(&palette_[address - 0x20]);
 			entry[0] = uint8_t((value & 0x700) >> 7);
@@ -484,154 +509,176 @@ void Video::update_output_mode() {
 	if(output_bpp_ != old_bpp_) {
 		// "the 71-Hz-switch does something like a shifter-reset." (and some people use a high-low resolutions switch instead)
 		reset_fifo();
+		video_stream_.set_bpp(output_bpp_);
 	}
+
+//	const int freqs[] = {50, 60, 72};
+//	printf("%d, %d -> %d [%d %d]\n", x_ / 2, y_, freqs[int(field_frequency_)], horizontal_.enable, vertical_.enable);
 }
 
 // MARK: - The shifter
 
-void Video::Shifter::flush_output(OutputMode next_mode) {
+void Video::VideoStream::output(int duration, OutputMode mode) {
-	switch(output_mode_) {
+	// If this is a transition from sync to blank, actually transition to colour burst.
+	if(output_mode_ == OutputMode::Sync && mode == OutputMode::Blank) {
+		mode = OutputMode::ColourBurst;
+	}
+
+	// If this is seeming a transition from blank to colour burst, obey it only if/when
+	// sufficient colour burst has been output.
+	if(output_mode_ == OutputMode::Blank && mode == OutputMode::ColourBurst) {
+		if(duration_ + duration >= 40) {
+			const int overage = duration + duration_ - 40;
+			duration_ = 40;
+
+			generate(overage, OutputMode::ColourBurst, true);
+		} else {
+			mode = OutputMode::ColourBurst;
+		}
+	}
+
+	// If this is a transition, or if we're doing pixels, output whatever has been accumulated.
+	if(mode != output_mode_ || output_mode_ == OutputMode::Pixels) {
+		generate(duration, output_mode_, mode != output_mode_);
+	} else {
+		duration_ += duration;
+	}
+
+	// Accumulate time in the current mode.
+	output_mode_ = mode;
+}
+
+void Video::VideoStream::generate(int duration, OutputMode mode, bool is_terminal) {
+	// Three of these are trivial; deal with them upfront. They don't care about the duration of
+	// whatever is new, just about how much was accumulated prior to now.
+	if(mode != OutputMode::Pixels) {
+		switch(mode) {
+			default:
 			case OutputMode::Sync:			crt_.output_sync(duration_);					break;
 			case OutputMode::Blank:			crt_.output_blank(duration_);					break;
 			case OutputMode::ColourBurst:	crt_.output_default_colour_burst(duration_);	break;
-		case OutputMode::Border: {
-//			if(!border_colour_) {
-//				crt_.output_blank(duration_);
-//			} else {
-				uint16_t *const colour_pointer = reinterpret_cast<uint16_t *>(crt_.begin_data(1));
-				if(colour_pointer) *colour_pointer = border_colour_;
-				crt_.output_level(duration_);
-//			}
-		} break;
-		case OutputMode::Pixels: {
-			crt_.output_data(duration_, pixel_pointer_);
-			pixel_buffer_ = nullptr;
-			pixel_pointer_ = 0;
-		} break;
 		}
-	duration_ = 0;
-	output_mode_ = next_mode;
-}
 
-void Video::Shifter::output_colour_burst(int duration) {
+		// Reseed duration
-	// More hackery afoot here; if and when duration_ crosses a threshold of 40,
+		duration_ = duration;
-	// output 40 cycles of colour burst and then redirect to blank.
+
-	if(output_mode_ != OutputMode::ColourBurst) {
+		// The shifter should keep running, so throw away the proper amount of content.
-		flush_output(OutputMode::ColourBurst);
+		shift(duration_);
-	}
-	duration_ += duration;
-	if(duration_ >= 40) {
-		const int blank_duration = duration_ - 40;
-		duration_ = 40;
-		flush_output(OutputMode::Blank);
-		output_blank(blank_duration);
-	}
-}
 
-void Video::Shifter::output_blank(int duration) {
-	if(output_mode_ != OutputMode::Blank) {
-		// Bit of a hack: if this is a transition from sync or we're really in
-		// colour burst, divert into that.
-		if(output_mode_ == OutputMode::Sync || output_mode_ == OutputMode::ColourBurst) {
-			output_colour_burst(duration);
 		return;
 	}
-		flush_output(OutputMode::Blank);
-	}
-	duration_ += duration;
-}
 
-void Video::Shifter::output_sync(int duration) {
+	// If the shifter is empty, accumulate in duration_ a promise to draw border later.
-	if(output_mode_ != OutputMode::Sync) {
-		flush_output(OutputMode::Sync);
-	}
-	duration_ += duration;
-}
-
-void Video::Shifter::output_border(int duration, OutputBpp bpp) {
-	// If there's still anything in the shifter, redirect this to an output_pixels call.
-	if(output_shifter_) {
-		// This doesn't take an opinion on how much of the shifter remains populated;
-		// it assumes the worst case.
-		const int pixel_length = std::min(32, duration);
-		output_pixels(pixel_length, bpp);
-		duration -= pixel_length;
-		if(!duration) {
-			return;
-		}
-	}
-
-	// Flush anything that isn't level output *in the current border colour*.
-	if(output_mode_ != OutputMode::Border || border_colour_ != palette_[0]) {
-		flush_output(OutputMode::Border);
-		border_colour_ = palette_[0];
-	}
-	duration_ += duration;
-}
-
-void Video::Shifter::output_pixels(int duration, OutputBpp bpp) {
-	// If the shifter is empty and there's no pixel buffer at present,
-	// redirect this to an output_level call. Otherwise, do a quick
-	// memset-type fill, since the special case has been detected anyway.
 	if(!output_shifter_) {
-		if(!pixel_buffer_) {
+		if(pixel_pointer_) {
-			output_border(duration, bpp);
+			flush_pixels();
-		} else {
+		}
+
 		duration_ += duration;
 
-			switch(bpp_) {
+		// If this is terminal, we'll need to draw now. But if it isn't, job done.
-				case OutputBpp::One: {
+		if(is_terminal) {
-					const size_t pixels = size_t(duration << 1);
+			flush_border();
-					memset(&pixel_buffer_[pixel_pointer_], 0, pixels * sizeof(uint16_t));
+		}
-					pixel_pointer_ += pixels;
-				} break;
 
-				default:
-				case OutputBpp::Four:
-					assert(!(duration & 1));
-					duration >>= 1;
-				case OutputBpp::Two: {
-					while(duration--) {
-						pixel_buffer_[pixel_pointer_] = palette_[0];
-						++pixel_pointer_;
-					}
-				} break;
-			}
-		}
 		return;
 	}
 
-	// Flush anything that isn't pixel output in the proper bpp; also flush if there's nowhere
+	// There's definitely some pixels to convey, but perhaps there's some border first?
-	// left to put pixels.
+	if(duration_) {
-	if(output_mode_ != OutputMode::Pixels || bpp_ != bpp || pixel_pointer_ >= 320) {
+		flush_border();
-		flush_output(OutputMode::Pixels);
-		bpp_ = bpp;
-		pixel_buffer_ = reinterpret_cast<uint16_t *>(crt_.begin_data(320 + 32));
 	}
-	duration_ += duration;
+
+	// Time to do some pixels!
+	output_pixels(duration);
+
+	// If was terminal, make sure any transient storage is output.
+	if(is_terminal) {
+		flush_pixels();
+	}
+}
+
+void Video::VideoStream::will_change_border_colour() {
+	// Flush the accumulated border if it'd be adversely affected.
+	if(duration_ && output_mode_ == OutputMode::Pixels) {
+		flush_border();
+	}
+}
+
+void Video::VideoStream::flush_border() {
+	// Output colour 0 for the entirety of duration_ (or black, if this is 1bpp mode).
+	uint16_t *const colour_pointer = reinterpret_cast<uint16_t *>(crt_.begin_data(1));
+	if(colour_pointer) *colour_pointer = (bpp_ != OutputBpp::One) ?  palette_[0] : 0;
+	crt_.output_level(duration_);
+
+	duration_ = 0;
+}
+
+namespace {
+#if TARGET_RT_BIG_ENDIAN
+	constexpr int upper = 0;
+#else
+	constexpr int upper = 1;
+#endif
+}
+
+void Video::VideoStream::shift(int duration) {
+	switch(bpp_) {
+		case OutputBpp::One:
+			output_shifter_ <<= (duration << 1);
+		break;
+		case OutputBpp::Two:
+			while(duration--) {
+				shifter_halves_[upper] = (shifter_halves_[upper] << 1) & 0xfffefffe;
+				shifter_halves_[upper] |= (shifter_halves_[upper^1] & 0x80008000) >> 15;
+				shifter_halves_[upper^1] = (shifter_halves_[upper^1] << 1) & 0xfffefffe;
+			}
+		break;
+		case OutputBpp::Four:
+			while(duration) {
+				output_shifter_ = (output_shifter_ << 1) & 0xfffefffefffefffe;
+				duration -= 2;
+			}
+		break;
+	}
+}
+
+// TODO: turn this into a template on current BPP, perhaps? Would avoid reevaluation of the conditional.
+void Video::VideoStream::output_pixels(int duration) {
+	constexpr int allocation_size = 352;	// i.e. 320 plus a spare 32.
+
+	// Convert from duration to pixels.
+	int pixels = duration;
+	switch(bpp_) {
+		case OutputBpp::One: pixels <<= 1;	break;
+		default: break;
+		case OutputBpp::Four: pixels >>= 1;	break;
+	}
+
+	while(pixels) {
+		// If no buffer is currently available, attempt to allocate one.
+		if(!pixel_buffer_) {
+			pixel_buffer_ = reinterpret_cast<uint16_t *>(crt_.begin_data(allocation_size, 2));
+
+			// Stop the loop if no buffer is available.
+			if(!pixel_buffer_) break;
+		}
+
+		int pixels_to_draw = std::min(allocation_size - pixel_pointer_, pixels);
+		pixels -= pixels_to_draw;
 
 		switch(bpp_) {
-		case OutputBpp::One: {
+			case OutputBpp::One:
-			int pixels = duration << 1;
+				while(pixels_to_draw--) {
-			if(pixel_buffer_) {
-				while(pixels--) {
 					pixel_buffer_[pixel_pointer_] = ((output_shifter_ >> 63) & 1) * 0xffff;
 					output_shifter_ <<= 1;
+
 					++pixel_pointer_;
 				}
-			} else {
+			break;
-				pixel_pointer_ += size_t(pixels);
+
-				output_shifter_ <<= pixels;
+			case OutputBpp::Two:
-			}
+				while(pixels_to_draw--) {
-		} break;
-		case OutputBpp::Two: {
-	#if TARGET_RT_BIG_ENDIAN
-			constexpr int upper = 0;
-	#else
-			constexpr int upper = 1;
-	#endif
-			if(pixel_buffer_) {
-				while(duration--) {
 					pixel_buffer_[pixel_pointer_] = palette_[
 						((output_shifter_ >> 63) & 1) |
 						((output_shifter_ >> 46) & 2)
@@ -645,20 +692,10 @@ void Video::Shifter::output_pixels(int duration, OutputBpp bpp) {
 
 					++pixel_pointer_;
 				}
-			} else {
+			break;
-				pixel_pointer_ += size_t(duration);
+
-				while(duration--) {
-					shifter_halves_[upper] = (shifter_halves_[upper] << 1) & 0xfffefffe;
-					shifter_halves_[upper] |= (shifter_halves_[upper^1] & 0x80008000) >> 15;
-					shifter_halves_[upper^1] = (shifter_halves_[upper^1] << 1) & 0xfffefffe;
-				}
-			}
-		} break;
-		default:
 			case OutputBpp::Four:
-			assert(!(duration & 1));
+				while(pixels_to_draw--) {
-			if(pixel_buffer_) {
-				while(duration) {
 					pixel_buffer_[pixel_pointer_] = palette_[
 						((output_shifter_ >> 63) & 1) |
 						((output_shifter_ >> 46) & 2) |
@@ -666,21 +703,50 @@ void Video::Shifter::output_pixels(int duration, OutputBpp bpp) {
 						((output_shifter_ >> 12) & 8)
 					];
 					output_shifter_ = (output_shifter_ << 1) & 0xfffefffefffefffe;
+
 					++pixel_pointer_;
-					duration -= 2;
-				}
-			} else {
-				pixel_pointer_ += size_t(duration >> 1);
-				while(duration) {
-					output_shifter_ = (output_shifter_ << 1) & 0xfffefffefffefffe;
-					duration -= 2;
-				}
 				}
 			break;
 		}
+
+		// Check whether the limit has been reached.
+		if(pixel_pointer_ == allocation_size) {
+			flush_pixels();
+		}
+	}
+
+	// If duration remains, that implies no buffer was available, so
+	// just do the corresponding shifting and provide proper timing to the CRT.
+	if(pixels) {
+		int leftover_duration = pixels;
+		switch(bpp_) {
+			case OutputBpp::One: leftover_duration >>= 1;	break;
+			default: break;
+			case OutputBpp::Four: leftover_duration <<= 1;	break;
+		}
+		shift(leftover_duration);
+		crt_.output_data(leftover_duration);
+	}
 }
 
-void Video::Shifter::load(uint64_t value) {
+void Video::VideoStream::flush_pixels() {
+	switch(bpp_) {
+		case OutputBpp::One:	crt_.output_data(pixel_pointer_ >> 1, size_t(pixel_pointer_)); break;
+		default: 				crt_.output_data(pixel_pointer_); break;
+		case OutputBpp::Four:	crt_.output_data(pixel_pointer_ << 1, size_t(pixel_pointer_)); break;
+	}
+
+	pixel_pointer_ = 0;
+	pixel_buffer_ = nullptr;
+}
+
+void Video::VideoStream::set_bpp(OutputBpp bpp) {
+	// TODO: is flushing like this correct?
+	output_shifter_ = 0;
+	bpp_ = bpp;
+}
+
+void Video::VideoStream::load(uint64_t value) {
 	output_shifter_ = value;
 }
 

Machines/Atari/ST/Video.hpp

@@ -15,6 +15,9 @@
 
 #include <vector>
 
+// Testing hook; not for any other user.
+class VideoTester;
+
 namespace Atari {
 namespace ST {
 
@@ -166,47 +169,77 @@ class Video {
 
 		void reset_fifo();
 
-		class Shifter {
+		/*!
-			public:
+			Provides a target for control over the output video stream, which is considered to be
-				Shifter(Outputs::CRT::CRT &crt, uint16_t *palette) : crt_(crt), palette_(palette) {}
+			a permanently shifting shifter, that you need to reload when appropriate, which can be
-				void output_blank(int duration);
+			overridden by the blank and sync levels.
-				void output_sync(int duration);
-				void output_border(int duration, OutputBpp bpp);
-				void output_pixels(int duration, OutputBpp bpp);
-				void output_colour_burst(int duration);
 
+			This stream will automatically insert a colour burst.
+		*/
+		class VideoStream {
+			public:
+				VideoStream(Outputs::CRT::CRT &crt, uint16_t *palette) : crt_(crt), palette_(palette) {}
+
+				enum class OutputMode {
+					Sync, Blank, ColourBurst, Pixels,
+				};
+
+				/// Sets the current data format for the shifter. Changes in output BPP flush the shifter.
+				void set_bpp(OutputBpp bpp);
+
+				/// Outputs signal of type @c mode for @c duration.
+				void output(int duration, OutputMode mode);
+
+				/// Warns the video stream that the border colour, included in the palette that it holds a pointer to,
+				/// will change momentarily. This should be called after the relevant @c output() updates, and
+				/// is used to help elide border-regio output.
+				void will_change_border_colour();
+
+				/// Loads 64 bits into the Shifter. The shifter shifts continuously. If you also declare
+				/// a pixels region then whatever is being shifted will reach the display, in a form that
+				/// depends on the current output BPP.
 				void load(uint64_t value);
 
 			private:
+				// The target CRT and the palette to use.
+				Outputs::CRT::CRT &crt_;
+				uint16_t *palette_ = nullptr;
+
+				// Internal stateful processes.
+				void generate(int duration, OutputMode mode, bool is_terminal);
+
+				void flush_border();
+				void flush_pixels();
+				void shift(int duration);
+				void output_pixels(int duration);
+
+				// Internal state that is a function of output intent.
 				int duration_ = 0;
-				enum class OutputMode {
+				OutputMode output_mode_ = OutputMode::Sync;
-					Sync, Blank, Border, Pixels, ColourBurst
-				} output_mode_ = OutputMode::Sync;
-				uint16_t border_colour_ = 0;
 				OutputBpp bpp_ = OutputBpp::Four;
 				union {
 					uint64_t output_shifter_;
 					uint32_t shifter_halves_[2];
 				};
 
-				void flush_output(OutputMode next_mode);
+				// Internal state for handling output serialisation.
-
 				uint16_t *pixel_buffer_ = nullptr;
-				size_t pixel_pointer_ = 0;
+				int pixel_pointer_ = 0;
-
+		} video_stream_;
-				Outputs::CRT::CRT &crt_;
-				uint16_t *palette_ = nullptr;
-		} shifter_;
 
 		/// Contains copies of the various observeable fields, after the relevant propagation delay.
 		struct PublicState {
 			bool display_enable = false;
+			bool hsync = false;
+			bool vsync = false;
 		} public_state_;
 
 		struct Event {
 			int delay;
 			enum class Type {
-				SetDisplayEnable, ResetDisplayEnable
+				SetDisplayEnable, ResetDisplayEnable,
+				SetHsync, ResetHsync,
+				SetVsync, ResetVsync,
 			} type;
 
 			Event(Type type, int delay) : delay(delay), type(type) {}
@@ -220,6 +253,10 @@ class Video {
 					default:
 					case Type::SetDisplayEnable:	state.display_enable = true;	break;
 					case Type::ResetDisplayEnable:	state.display_enable = false;	break;
+					case Type::SetHsync:			state.hsync = true;				break;
+					case Type::ResetHsync:			state.hsync = false;			break;
+					case Type::SetVsync:			state.vsync = true;				break;
+					case Type::ResetVsync:			state.vsync = false;			break;
 				}
 			}
 		};
@@ -232,18 +269,25 @@ class Video {
 				return;
 			}
 
+			if(!pending_events_.empty()) {
 				// Otherwise enqueue, having subtracted the delay for any preceding events,
 				// and subtracting from the subsequent, if any.
 				auto insertion_point = pending_events_.begin();
-			while(insertion_point != pending_events_.end() && insertion_point->delay > delay) {
+				while(insertion_point != pending_events_.end() && insertion_point->delay < delay) {
 					delay -= insertion_point->delay;
 					++insertion_point;
 				}
 				if(insertion_point != pending_events_.end()) {
 					insertion_point->delay -= delay;
 				}
+
 				pending_events_.emplace(insertion_point, type, delay);
+			} else {
+				pending_events_.emplace_back(type, delay);
 			}
+		}
+
+		friend class ::VideoTester;
 };
 
 }

Machines/Commodore/Vic-20/Vic20.cpp

@@ -320,7 +320,7 @@ class ConcreteMachine:
 			tape_->set_delegate(this);
 			tape_->set_clocking_hint_observer(this);
 
-			// install a joystick
+			// Install a joystick.
 			joysticks_.emplace_back(new Joystick(*user_port_via_port_handler_, *keyboard_via_port_handler_));
 
 			const std::string machine_name = "Vic20";
@@ -401,22 +401,14 @@ class ConcreteMachine:
 	write_to_map(processor_read_memory_map_, &ram_[baseaddr], baseaddr, length);	\
 	write_to_map(processor_write_memory_map_, &ram_[baseaddr], baseaddr, length);
 
-			// Add 6502-visible RAM as requested
+			// Add 6502-visible RAM as requested.
-			switch(target.memory_model) {
-				case Analyser::Static::Commodore::Target::MemoryModel::Unexpanded:
-					// The default Vic-20 memory map has 1kb at address 0 and another 4kb at address 0x1000.
 			set_ram(0x0000, 0x0400);
-					set_ram(0x1000, 0x1000);
+			set_ram(0x1000, 0x1000);	// Built-in RAM.
-				break;
+			if(target.enabled_ram.bank0) set_ram(0x0400, 0x0c00);	// Bank 0:	0x0400 -> 0x1000.
-				case Analyser::Static::Commodore::Target::MemoryModel::EightKB:
+			if(target.enabled_ram.bank1) set_ram(0x2000, 0x2000);	// Bank 1:	0x2000 -> 0x4000.
-					// An 8kb Vic-20 fills in the gap between the two blocks of RAM on an unexpanded machine.
+			if(target.enabled_ram.bank2) set_ram(0x4000, 0x2000);	// Bank 2:	0x4000 -> 0x6000.
-					set_ram(0x0000, 0x2000);
+			if(target.enabled_ram.bank3) set_ram(0x6000, 0x2000);	// Bank 3:	0x6000 -> 0x8000.
-				break;
+			if(target.enabled_ram.bank5) set_ram(0xa000, 0x2000);	// Bank 5:	0xa000 -> 0xc000.
-				case Analyser::Static::Commodore::Target::MemoryModel::ThirtyTwoKB:
-					// A 32kb Vic-20 fills the entire lower 32kb with RAM.
-					set_ram(0x0000, 0x8000);
-				break;
-			}
 
 #undef set_ram
 
@@ -453,6 +445,8 @@ class ConcreteMachine:
 			write_to_map(mos6560_bus_handler_.video_memory_map, character_rom_.data(), 0x0000, static_cast<uint16_t>(character_rom_.size()));
 			write_to_map(processor_read_memory_map_, kernel_rom_.data(), 0xe000, static_cast<uint16_t>(kernel_rom_.size()));
 
+			// The insert_media occurs last, so if there's a conflict between cartridges and RAM,
+			// the cartridge wins.
 			insert_media(target.media);
 			if(!target.loading_command.empty()) {
 				type_string(target.loading_command);
@@ -711,7 +705,7 @@ class ConcreteMachine:
 
 		std::vector<uint8_t> rom_;
 		uint16_t rom_address_, rom_length_;
-		uint8_t ram_[0x8000];
+		uint8_t ram_[0x10000];
 		uint8_t colour_ram_[0x0400];
 
 		uint8_t *processor_read_memory_map_[64];

OSBindings/Mac/Clock Signal.xcodeproj/xcshareddata/xcschemes/Clock Signal.xcscheme

@@ -23,9 +23,9 @@
       </BuildActionEntries>
    </BuildAction>
    <TestAction
-      buildConfiguration = "Release"
+      buildConfiguration = "Debug"
-      selectedDebuggerIdentifier = ""
+      selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
-      selectedLauncherIdentifier = "Xcode.IDEFoundation.Launcher.PosixSpawn"
+      selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
       shouldUseLaunchSchemeArgsEnv = "YES"
       disableMainThreadChecker = "YES"
       codeCoverageEnabled = "YES">

OSBindings/Mac/Clock Signal/Machine/StaticAnalyser/CSStaticAnalyser.mm

@@ -121,11 +121,13 @@
 			case CSMachineVic20RegionEuropean:	target->region = Target::Region::European;	break;
 			case CSMachineVic20RegionJapanese:	target->region = Target::Region::Japanese;	break;
 		}
+		auto memory_model = Target::MemoryModel::Unexpanded;
 		switch(memorySize) {
-			default:	target->memory_model = Target::MemoryModel::Unexpanded;		break;
+			default:	break;
-			case 8:		target->memory_model = Target::MemoryModel::EightKB;		break;
+			case 8:		memory_model = Target::MemoryModel::EightKB;		break;
-			case 32:	target->memory_model = Target::MemoryModel::ThirtyTwoKB;	break;
+			case 32:	memory_model = Target::MemoryModel::ThirtyTwoKB;	break;
 		}
+		target->set_memory_model(memory_model);
 		target->has_c1540 = !!hasC1540;
 		_targets.push_back(std::move(target));
 	}

OSBindings/Mac/Clock SignalTests/68000ComparativeTests.mm

@@ -27,7 +27,7 @@
 
 - (void)setUp {
 	// To limit tests run to a subset of files and/or of tests, uncomment and fill in below.
-//	_fileSet = [NSSet setWithArray:@[@"eor.json"]];
+//	_fileSet = [NSSet setWithArray:@[@"rtr.json"]];
 //	_testSet = [NSSet setWithArray:@[@"EOR b0c0", @"EOR b0c2", @"EOR b0c3", @"EOR b0c5", @"EOR b0c6", @"EOR b0c7", @"OR b0c7"]];
 }
 
@@ -172,6 +172,7 @@
 		XCTAssertEqual(state.supervisor_stack_pointer, [finalState[@"a7"] integerValue], @"%@: A7 inconsistent", name);
 		XCTAssertEqual(state.user_stack_pointer, [finalState[@"usp"] integerValue], @"%@: USP inconsistent", name);
 		XCTAssertEqual(state.status, [finalState[@"sr"] integerValue], @"%@: Status inconsistent", name);
+		XCTAssertEqual(state.program_counter - 4, [finalState[@"pc"] integerValue], @"%@: Program counter inconsistent", name);
 
 		// Test final memory state.
 		NSArray<NSNumber *> *const finalMemory = test[@"final memory"];

OSBindings/Mac/Clock SignalTests/AtariSTVideoTests.mm

@@ -8,30 +8,97 @@
 
 #import <XCTest/XCTest.h>
 
+#include <memory>
+
 #include "../../../Machines/Atari/ST/Video.hpp"
 
+// Implement Atari::ST::Video's friend class, to expose some
+// internal state.
+struct VideoTester {
+	static bool vsync(Atari::ST::Video &video) {
+		return video.vertical_.sync;
+	}
+};
+
 @interface AtariSTVideoTests : XCTestCase
 @end
 
-@implementation AtariSTVideoTests
+@implementation AtariSTVideoTests {
+	std::unique_ptr<Atari::ST::Video> _video;
+	uint16_t _ram[256*1024];
+}
+
+// MARK: - Setup and tear down.
 
 - (void)setUp {
 	[super setUp];
+
+	// Establish an instance of video.
+	_video = std::make_unique<Atari::ST::Video>();
+	_video->set_ram(_ram, sizeof(_ram));
 }
 
 - (void)tearDown {
 	[super tearDown];
+
+	// Release the video instance.
+	_video = nullptr;
 }
 
-- (void)testSequencePoints {
+// MARK: - Helpers
-	// Establish an instance of video.
-	Atari::ST::Video video;
-	uint16_t ram[256*1024];
-	video.set_ram(ram, sizeof(ram));
 
+- (void)runVideoForCycles:(int)cycles {
+	while(cycles--) {
+		_video->run_for(Cycles(1));
+	}
+}
+
+- (void)syncToStartOfLine {
+	// Run until the visible fetch address changes, to get to the start of the pixel zone.
+	const uint32_t original_address = [self currentVideoAddress];
+	while(original_address == [self currentVideoAddress]) {
+		_video->run_for(Cycles(1));
+	}
+
+	// Run until start of hsync.
+	while(!_video->hsync()) {
+		_video->run_for(Cycles(1));
+	}
+
+	// Run until end of hsync.
+	while(_video->hsync()) {
+		_video->run_for(Cycles(1));
+	}
+}
+
+- (void)setFrequency:(int)frequency {
+	switch(frequency) {
+		default:
+		case 50:	_video->write(0x05, 0x200);	_video->write(0x30, 0x000);	break;
+		case 60:	_video->write(0x05, 0x000); _video->write(0x30, 0x000);	break;
+		case 72:								_video->write(0x30, 0x200);	break;
+	}
+}
+
+- (uint32_t)currentVideoAddress {
+	return
+		(_video->read(0x04) & 0xff) |
+		((_video->read(0x03) & 0xff) << 8) |
+		((_video->read(0x02) & 0xff) << 16);
+}
+
+- (void)setVideoBaseAddress:(uint32_t)baseAddress {
+	_video->write(0x00, baseAddress >> 16);
+	_video->write(0x01, baseAddress >> 8);
+}
+
+// MARK: - Sequence Point Prediction Tests
+
+/// Tests that no events occur outside of the sequence points the video predicts.
+- (void)testSequencePoints {
 	// Set 4bpp, 50Hz.
-	video.write(0x05, 0x0200);
+	_video->write(0x05, 0x0200);
-	video.write(0x30, 0x0000);
+	_video->write(0x30, 0x0000);
 
 	// Run for [more than] a whole frame making sure that no observeable outputs
 	// change at any time other than a sequence point.
@@ -43,18 +110,231 @@
 		const bool is_transition_point = next_event == HalfCycles(0);
 
 		if(is_transition_point) {
-			display_enable = video.display_enabled();
+			display_enable = _video->display_enabled();
-			vsync = video.vsync();
+			vsync = _video->vsync();
-			hsync = video.hsync();
+			hsync = _video->hsync();
-			next_event = video.get_next_sequence_point();
+			next_event = _video->get_next_sequence_point();
 		} else {
-			NSAssert(display_enable == video.display_enabled(), @"Unannounced change in display enabled at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
+			NSAssert(display_enable == _video->display_enabled(), @"Unannounced change in display enabled at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
-			NSAssert(vsync == video.vsync(), @"Unannounced change in vsync at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
+			NSAssert(vsync == _video->vsync(), @"Unannounced change in vsync at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
-			NSAssert(hsync == video.hsync(), @"Unannounced change in hsync at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
+			NSAssert(hsync == _video->hsync(), @"Unannounced change in hsync at cycle %zu [%d before next sequence point]", c, next_event.as<int>());
 		}
-		video.run_for(HalfCycles(2));
+		_video->run_for(HalfCycles(2));
 		next_event -= HalfCycles(2);
 	}
 }
 
+// MARK: - Sync Line Length Tests
+
+struct RunLength {
+	int frequency;
+	int length;
+};
+
+- (void)testSequence:(const RunLength *)sequence targetLength:(int)duration {
+	[self syncToStartOfLine];
+
+	const uint32_t start_address = [self currentVideoAddress];
+
+	while(sequence->frequency != -1) {
+		[self setFrequency:sequence->frequency];
+		[self runVideoForCycles:sequence->length];
+		++sequence;
+	}
+	const uint32_t final_address = [self currentVideoAddress];
+
+	XCTAssertEqual(final_address - start_address, duration);
+}
+
+- (void)testLineLength54 {
+	// Run as though a regular 50Hz line at least until cycle 52;
+	// then switch to 72 Hz by 164, and allow the line to finish.
+	const RunLength test[] = {
+		{50, 60},
+		{72, 452},
+		{-1}
+	};
+	[self testSequence:test targetLength:54];
+}
+
+- (void)testLineLength56 {
+	// Run as though a regular 60Hz line at least until cycle 52;
+	// then switch to 72 Hz by 164, and allow the line to finish.
+	const RunLength test[] = {
+		{60, 60},
+		{72, 452},
+		{-1}
+	};
+	[self testSequence:test targetLength:56];
+}
+
+- (void)testLineLength80 {
+	// Run a standard 72Hz line.
+	const RunLength test[] = {
+		{72, 224},
+		{-1}
+	};
+	[self testSequence:test targetLength:80];
+}
+
+- (void)testLineLengthLong80 {
+	// Run a 72Hz line with a switch through 50Hz to extend the length to 512 cycles.
+	const RunLength test[] = {
+		{72, 50},
+		{50, 20},
+		{72, 442},
+		{-1}
+	};
+	[self testSequence:test targetLength:80];
+}
+
+- (void)testLineLength158 {
+	// Transition from 50Hz to 60Hz mid-line.
+	const RunLength test[] = {
+		{50, 60},
+		{60, 458},
+		{-1}
+	};
+	[self testSequence:test targetLength:158];
+}
+
+- (void)testLineLength160_60Hz {
+	const RunLength test[] = {
+		{60, 508},
+		{-1}
+	};
+	[self testSequence:test targetLength:160];
+}
+
+- (void)testLineLength160_50Hz {
+	const RunLength test[] = {
+		{50, 512},
+		{-1}
+	};
+	[self testSequence:test targetLength:160];
+}
+
+- (void)testLineLength162 {
+	// Transition from 60Hz to 50Hz mid-line.
+	const RunLength test[] = {
+		{60, 54},
+		{50, 458},
+		{-1}
+	};
+	[self testSequence:test targetLength:162];
+}
+
+- (void)testLineLength184 {
+	// Start off in 72Hz, switch to 60 during pixels.
+	const RunLength test[] = {
+		{72, 8},
+		{60, 500},
+		{-1}
+	};
+	[self testSequence:test targetLength:184];
+}
+
+- (void)testLineLength186 {
+	// Start off in 72Hz, switch to 50 during pixels.
+	const RunLength test[] = {
+		{72, 8},
+		{50, 504},
+		{-1}
+	};
+	[self testSequence:test targetLength:186];
+}
+
+- (void)testLineLength204 {
+	// Start in 50Hz, avoid DE disable.
+	const RunLength test[] = {
+		{50, 374},
+		{60, 138},
+		{-1}
+	};
+	[self testSequence:test targetLength:204];
+}
+
+- (void)testLineLength206 {
+	// Start in 60Hz, get a 50Hz line length, avoid DE disable.
+	const RunLength test[] = {
+		{60, 53},
+		{50, 3},		// To 56.
+		{60, 314},		// 370.
+		{50, 4},		// 374.
+		{60, 138},		// 512.
+		{-1}
+	};
+	[self testSequence:test targetLength:206];
+}
+
+- (void)testLineLength230 {
+	// Start in 72Hz, avoid DE disable.
+	const RunLength test[] = {
+		{72, 8},
+		{50, 366},
+		{60, 138},
+		{-1}
+	};
+	[self testSequence:test targetLength:230];
+}
+
+// MARK: - Address Reload Timing tests
+
+/// Tests that the current video address is reloaded constantly throughout vsync (subject to caveat: observed .
+- (void)testVsyncReload {
+
+	// Set an initial video address of 0.
+	[self setVideoBaseAddress:0];
+
+	// Find next area of non-vsync.
+	while(VideoTester::vsync(*_video)) {
+		_video->run_for(Cycles(1));
+	}
+
+	// Set a different base video address.
+	[self setVideoBaseAddress:0x800000];
+
+	// Find next area of vsync, checking that the address isn't
+	// reloaded before then.
+	while(!VideoTester::vsync(*_video)) {
+		XCTAssertNotEqual([self currentVideoAddress], 0x800000);
+		_video->run_for(Cycles(1));
+	}
+
+	// Vsync has now started, test that video address has been set.
+	XCTAssertEqual([self currentVideoAddress], 0x800000);
+
+	// Run a few cycles, set a different video base address,
+	// confirm that has been set.
+	[self runVideoForCycles:200];
+	XCTAssertEqual([self currentVideoAddress], 0x800000);
+	[self setVideoBaseAddress:0xc00000];
+	[self runVideoForCycles:1];
+	XCTAssertEqual([self currentVideoAddress], 0xc00000);
+
+	// Find end of vertical sync, set a different base address,
+	// check that it doesn't become current.
+	while(VideoTester::vsync(*_video)) {
+		_video->run_for(Cycles(1));
+	}
+	[self setVideoBaseAddress:0];
+	[self runVideoForCycles:1];
+	XCTAssertNotEqual([self currentVideoAddress], 0);
+}
+
+// MARK: - Tests Correlating To Exact Pieces of Software
+
+- (void)testUnionDemoScroller {
+	const RunLength test[] = {
+		{72, 8},
+		{50, 365},
+		{60, 8},
+		{50, 59},
+		{72, 12},
+		{50, 60},
+		{-1}
+	};
+	[self testSequence:test targetLength:230];
+}
+
 @end

Processors/68000/Implementation/68000Implementation.hpp

@@ -1854,11 +1854,11 @@ template <class T, bool dtack_is_implicit, bool signal_will_perform> void Proces
 									RTE and RTR share an implementation.
 								*/
 								case Operation::RTE_RTR:
-									// If this is RTR, patch out the is_supervisor bit.
+									// If this is RTR, patch out the supervisor half of the status register.
 									if(decoded_instruction_.full == 0x4e77) {
-										source_bus_data_[0].full =
+										const auto current_status = get_status();
-											(source_bus_data_[0].full & uint32_t(~(1 << 13))) |
+										source_bus_data_[0].halves.low.halves.high =
-											uint32_t(is_supervisor_ << 13);
+											uint8_t(current_status >> 8);
 									}
 									set_status(source_bus_data_[0].full);
 								break;

Processors/68000/Implementation/68000Storage.cpp

@@ -1013,7 +1013,11 @@ struct ProcessorStorageConstructor {
 						case Decoder::RTE_RTR: {
 							program.set_requires_supervisor(instruction == 0x4e73);
 
-							// TODO: something explicit to ensure the nR nr nr is exclusively linked.
+							// The targets of the nR nr nr below are reset to the program counter elsewhere;
+							// look for the comment "relink the RTE and RTR bus steps". It is currently not
+							// explicitly tested that these bus steps are not shared with a non-RTE/RTR operation,
+							// just assumed because the repetition of nr is fairly silly. A more explicit soution
+							// might be preferable in the future.
 							op(Action::PrepareRTE_RTR, seq("nR nr nr", { &storage_.precomputed_addresses_[0], &storage_.precomputed_addresses_[1], &storage_.precomputed_addresses_[2] } ));
 							op(Action::PerformOperation, seq("np np"));
 							op();