From 613d4b7c8ba0399ac91ed784e54686c1f571e686 Mon Sep 17 00:00:00 2001
From: Thomas Harte <thomas.harte@gmail.com>
Date: Sat, 7 Nov 2020 17:45:03 -0500
Subject: [PATCH] Migrates character ROM handling; supplies one for the IIgs.

---
 Machines/Apple/AppleII/AppleII.cpp       |   8 +-
 Machines/Apple/AppleII/Video.cpp         |  65 ++----------
 Machines/Apple/AppleII/Video.hpp         |  32 +-----
 Machines/Apple/AppleII/VideoSwitches.hpp | 127 ++++++++++++++++++++++-
 Machines/Apple/AppleIIgs/AppleIIgs.cpp   |   5 +-
 Machines/Apple/AppleIIgs/Video.cpp       |   7 +-
 Machines/Apple/AppleIIgs/Video.hpp       |   4 +-
 7 files changed, 142 insertions(+), 106 deletions(-)

diff --git a/Machines/Apple/AppleII/AppleII.cpp b/Machines/Apple/AppleII/AppleII.cpp
index 190b22182..8c939ba8d 100644
--- a/Machines/Apple/AppleII/AppleII.cpp
+++ b/Machines/Apple/AppleII/AppleII.cpp
@@ -353,21 +353,21 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 			size_t rom_size = 12*1024;
 			switch(target.model) {
 				default:
-					rom_descriptions.emplace_back(machine_name, "the basic Apple II character ROM", "apple2-character.rom", 2*1024, 0x64f415c6);
+					rom_descriptions.push_back(video_.rom_description(Video::VideoBase::CharacterROM::II));
 					rom_descriptions.emplace_back(machine_name, "the original Apple II ROM", "apple2o.rom", 12*1024, 0xba210588);
 				break;
 				case Target::Model::IIplus:
-					rom_descriptions.emplace_back(machine_name, "the basic Apple II character ROM", "apple2-character.rom", 2*1024, 0x64f415c6);
+					rom_descriptions.push_back(video_.rom_description(Video::VideoBase::CharacterROM::II));
 					rom_descriptions.emplace_back(machine_name, "the Apple II+ ROM", "apple2.rom", 12*1024, 0xf66f9c26);
 				break;
 				case Target::Model::IIe:
 					rom_size += 3840;
-					rom_descriptions.emplace_back(machine_name, "the Apple IIe character ROM", "apple2eu-character.rom", 4*1024, 0x816a86f1);
+					rom_descriptions.push_back(video_.rom_description(Video::VideoBase::CharacterROM::IIe));
 					rom_descriptions.emplace_back(machine_name, "the Apple IIe ROM", "apple2eu.rom", 32*1024, 0xe12be18d);
 				break;
 				case Target::Model::EnhancedIIe:
 					rom_size += 3840;
-					rom_descriptions.emplace_back(machine_name, "the Enhanced Apple IIe character ROM", "apple2e-character.rom", 4*1024, 0x2651014d);
+					rom_descriptions.push_back(video_.rom_description(Video::VideoBase::CharacterROM::EnhancedIIe));
 					rom_descriptions.emplace_back(machine_name, "the Enhanced Apple IIe ROM", "apple2e.rom", 32*1024, 0x65989942);
 				break;
 			}
diff --git a/Machines/Apple/AppleII/Video.cpp b/Machines/Apple/AppleII/Video.cpp
index 3ddbbe8b5..7b0df2baa 100644
--- a/Machines/Apple/AppleII/Video.cpp
+++ b/Machines/Apple/AppleII/Video.cpp
@@ -11,7 +11,7 @@
 using namespace Apple::II::Video;
 
 VideoBase::VideoBase(bool is_iie, std::function<void(Cycles)> &&target) :
-	VideoSwitches<Cycles>(Cycles(2), std::move(target)),
+	VideoSwitches<Cycles>(is_iie, Cycles(2), std::move(target)),
 	crt_(910, 1, Outputs::Display::Type::NTSC60, Outputs::Display::InputDataType::Luminance1),
 	is_iie_(is_iie) {
 
@@ -24,23 +24,6 @@ VideoBase::VideoBase(bool is_iie, std::function<void(Cycles)> &&target) :
 	// use default_colour_bursts elsewhere, though it otherwise should be. If/when
 	// it is, start doing so and return to setting the immediate phase up here.
 //	crt_.set_immediate_default_phase(0.5f);
-
-	character_zones[0].xor_mask = 0;
-	character_zones[0].address_mask = 0x3f;
-	character_zones[1].xor_mask = 0;
-	character_zones[1].address_mask = 0x3f;
-	character_zones[2].xor_mask = 0;
-	character_zones[2].address_mask = 0x3f;
-	character_zones[3].xor_mask = 0;
-	character_zones[3].address_mask = 0x3f;
-
-	if(is_iie) {
-		character_zones[0].xor_mask =
-		character_zones[2].xor_mask =
-		character_zones[3].xor_mask = 0xff;
-		character_zones[2].address_mask =
-		character_zones[3].address_mask = 0xff;
-	}
 }
 
 void VideoBase::set_scan_target(Outputs::Display::ScanTarget *scan_target) {
@@ -59,44 +42,10 @@ Outputs::Display::DisplayType VideoBase::get_display_type() const {
 	return crt_.get_display_type();
 }
 
-void VideoBase::did_set_alternative_character_set(bool alternative_character_set) {
-	alternative_character_set_ = alternative_character_set;
-	if(alternative_character_set) {
-		character_zones[1].address_mask = 0xff;
-		character_zones[1].xor_mask = 0;
-	} else {
-		character_zones[1].address_mask = 0x3f;
-		character_zones[1].xor_mask = flash_mask();
-		// The XOR mask is seeded here; it's dynamic, so updated elsewhere.
-	}
-}
-
-void VideoBase::did_set_annunciator_3(bool annunciator_3) {
-	high_resolution_mask_ = annunciator_3 ? 0x7f : 0xff;
-}
-
-void VideoBase::set_character_rom(const std::vector<uint8_t> &character_rom) {
-	character_rom_ = character_rom;
-
-	// Flip all character contents based on the second line of the $ graphic.
-	if(character_rom_[0x121] == 0x3c || character_rom_[0x122] == 0x3c) {
-		for(auto &graphic : character_rom_) {
-			graphic =
-				((graphic & 0x01) ? 0x40 : 0x00) |
-				((graphic & 0x02) ? 0x20 : 0x00) |
-				((graphic & 0x04) ? 0x10 : 0x00) |
-				((graphic & 0x08) ? 0x08 : 0x00) |
-				((graphic & 0x10) ? 0x04 : 0x00) |
-				((graphic & 0x20) ? 0x02 : 0x00) |
-				((graphic & 0x40) ? 0x01 : 0x00);
-		}
-	}
-}
-
 void VideoBase::output_text(uint8_t *target, const uint8_t *const source, size_t length, size_t pixel_row) const {
 	for(size_t c = 0; c < length; ++c) {
-		const int character = source[c] & character_zones[source[c] >> 6].address_mask;
-		const uint8_t xor_mask = character_zones[source[c] >> 6].xor_mask;
+		const int character = source[c] & character_zones_[source[c] >> 6].address_mask;
+		const uint8_t xor_mask = character_zones_[source[c] >> 6].xor_mask;
 		const std::size_t character_address = size_t(character << 3) + pixel_row;
 		const uint8_t character_pattern = character_rom_[character_address] ^ xor_mask;
 
@@ -117,19 +66,19 @@ void VideoBase::output_double_text(uint8_t *target, const uint8_t *const source,
 	for(size_t c = 0; c < length; ++c) {
 		const std::size_t character_addresses[2] = {
 			size_t(
-				(auxiliary_source[c] & character_zones[auxiliary_source[c] >> 6].address_mask) << 3
+				(auxiliary_source[c] & character_zones_[auxiliary_source[c] >> 6].address_mask) << 3
 			) + pixel_row,
 			size_t(
-				(source[c] & character_zones[source[c] >> 6].address_mask) << 3
+				(source[c] & character_zones_[source[c] >> 6].address_mask) << 3
 			) + pixel_row
 		};
 
 		const uint8_t character_patterns[2] = {
 			uint8_t(
-				character_rom_[character_addresses[0]] ^ character_zones[auxiliary_source[c] >> 6].xor_mask
+				character_rom_[character_addresses[0]] ^ character_zones_[auxiliary_source[c] >> 6].xor_mask
 			),
 			uint8_t(
-				character_rom_[character_addresses[1]] ^ character_zones[source[c] >> 6].xor_mask
+				character_rom_[character_addresses[1]] ^ character_zones_[source[c] >> 6].xor_mask
 			)
 		};
 
diff --git a/Machines/Apple/AppleII/Video.hpp b/Machines/Apple/AppleII/Video.hpp
index d246bea33..763f3c1cb 100644
--- a/Machines/Apple/AppleII/Video.hpp
+++ b/Machines/Apple/AppleII/Video.hpp
@@ -51,9 +51,6 @@ class VideoBase: public VideoSwitches<Cycles> {
 		/// Gets the type of output.
 		Outputs::Display::DisplayType get_display_type() const;
 
-		// Setup for text mode.
-		void set_character_rom(const std::vector<uint8_t> &);
-
 	protected:
 		Outputs::CRT::CRT crt_;
 
@@ -61,26 +58,13 @@ class VideoBase: public VideoSwitches<Cycles> {
 		uint8_t *pixel_pointer_ = nullptr;
 
 		// State affecting logical state.
-		int row_ = 0, column_ = 0, flash_ = 0;
-		uint8_t flash_mask() {
-			return uint8_t((flash_ / flash_length) * 0xff);
-		}
-
-		bool alternative_character_set_ = false;
-		void did_set_annunciator_3(bool) override;
-		void did_set_alternative_character_set(bool) override;
+		int row_ = 0, column_ = 0;
 
 		// Graphics carry is the final level output in a fetch window;
 		// it carries on into the next if it's high resolution with
 		// the delay bit set.
 		mutable uint8_t graphics_carry_ = 0;
 		bool was_double_ = false;
-		uint8_t high_resolution_mask_ = 0xff;
-
-		// This holds a copy of the character ROM. The regular character
-		// set is assumed to be in the first 64*8 bytes; the alternative
-		// is in the 128*8 bytes after that.
-		std::vector<uint8_t> character_rom_;
 
 		// Memory is fetched ahead of time into this array;
 		// this permits the correct delay between fetching
@@ -89,15 +73,6 @@ class VideoBase: public VideoSwitches<Cycles> {
 		std::array<uint8_t, 40> auxiliary_stream_;
 
 		const bool is_iie_ = false;
-		static constexpr int flash_length = 8406;
-
-		// Describes the current text mode mapping from in-memory character index
-		// to output character.
-		struct CharacterMapping {
-			uint8_t address_mask;
-			uint8_t xor_mask;
-		};
-		CharacterMapping character_zones[4];
 
 		/*!
 			Outputs 40-column text to @c target, using @c length bytes from @c source.
@@ -438,10 +413,7 @@ template <class BusHandler, bool is_iie> class Video: public VideoBase {
 				column_ = (column_ + cycles_this_line) % 65;
 				if(!column_) {
 					row_ = (row_ + 1) % 262;
-					flash_ = (flash_ + 1) % (2 * flash_length);
-					if(!alternative_character_set_) {
-						character_zones[1].xor_mask = flash_mask();
-					}
+					did_end_line();
 
 					// Add an extra half a colour cycle of blank; this isn't counted in the run_for
 					// count explicitly but is promised. If this is a vertical sync line, output sync
diff --git a/Machines/Apple/AppleII/VideoSwitches.hpp b/Machines/Apple/AppleII/VideoSwitches.hpp
index e5360a395..acad71834 100644
--- a/Machines/Apple/AppleII/VideoSwitches.hpp
+++ b/Machines/Apple/AppleII/VideoSwitches.hpp
@@ -11,6 +11,7 @@
 
 #include "../../../ClockReceiver/ClockReceiver.hpp"
 #include "../../../ClockReceiver/DeferredQueue.hpp"
+#include "../../ROMMachine.hpp"
 
 namespace Apple {
 namespace II {
@@ -33,8 +34,28 @@ template <typename TimeUnit> class VideoSwitches {
 		/*!
 			Constructs a new instance of VideoSwitches in which changes to relevant switches
 			affect the video mode only after @c delay cycles.
+
+			If @c is_iie is true, these switches will set up the character zones for an IIe-esque
+			set of potential flashing characters and alternate video modes.
 		*/
-		VideoSwitches(TimeUnit delay, std::function<void(TimeUnit)> &&target) : delay_(delay), deferrer_(std::move(target)) {}
+		VideoSwitches(bool is_iie, TimeUnit delay, std::function<void(TimeUnit)> &&target) : delay_(delay), deferrer_(std::move(target)) {
+			character_zones_[0].xor_mask = 0;
+			character_zones_[0].address_mask = 0x3f;
+			character_zones_[1].xor_mask = 0;
+			character_zones_[1].address_mask = 0x3f;
+			character_zones_[2].xor_mask = 0;
+			character_zones_[2].address_mask = 0x3f;
+			character_zones_[3].xor_mask = 0;
+			character_zones_[3].address_mask = 0x3f;
+
+			if(is_iie) {
+				character_zones_[0].xor_mask =
+				character_zones_[2].xor_mask =
+				character_zones_[3].xor_mask = 0xff;
+				character_zones_[2].address_mask =
+				character_zones_[3].address_mask = 0xff;
+			}
+		}
 
 		/*!
 			Advances @c cycles.
@@ -64,7 +85,15 @@ template <typename TimeUnit> class VideoSwitches {
 			external_.alternative_character_set = alternative_character_set;
 			deferrer_.defer(delay_, [this, alternative_character_set] {
 				internal_.alternative_character_set = alternative_character_set;
-				did_set_annunciator_3(alternative_character_set);
+
+				if(alternative_character_set) {
+					character_zones_[1].address_mask = 0xff;
+					character_zones_[1].xor_mask = 0;
+				} else {
+					character_zones_[1].address_mask = 0x3f;
+					character_zones_[1].xor_mask = flash_mask();
+					// The XOR mask is seeded here; it's dynamic, so updated elsewhere.
+				}
 			});
 		}
 		bool get_alternative_character_set() const {
@@ -191,13 +220,59 @@ template <typename TimeUnit> class VideoSwitches {
 			external_.annunciator_3 = annunciator_3;
 			deferrer_.defer(delay_, [this, annunciator_3] {
 				internal_.annunciator_3 = annunciator_3;
-				did_set_alternative_character_set(annunciator_3);
+				high_resolution_mask_ = annunciator_3 ? 0x7f : 0xff;
 			});
 		}
 		bool get_annunciator_3() const {
 			return external_.annunciator_3;
 		}
 
+		enum class CharacterROM {
+			/// The ROM that shipped with both the Apple II and the II+.
+			II,
+			/// The ROM that shipped with the original IIe.
+			IIe,
+			/// The ROM that shipped with the Enhanced IIe, the IIc and the IIgs.
+			EnhancedIIe
+		};
+
+		/// @returns A file-level description of @c rom.
+		static ROMMachine::ROM rom_description(CharacterROM rom) {
+			const std::string machine_name = "AppleII";
+			switch(rom) {
+				case CharacterROM::II:
+					return ROMMachine::ROM(machine_name, "the basic Apple II character ROM", "apple2-character.rom", 2*1024, 0x64f415c6);
+
+				case CharacterROM::IIe:
+					return ROMMachine::ROM(machine_name, "the Apple IIe character ROM", "apple2eu-character.rom", 4*1024, 0x816a86f1);
+
+				default:
+				case CharacterROM::EnhancedIIe:
+					return ROMMachine::ROM(machine_name, "the Enhanced Apple IIe character ROM", "apple2e-character.rom", 4*1024, 0x2651014d);
+			}
+		}
+
+		/// Set the character ROM for this video output.
+		void set_character_rom(const std::vector<uint8_t> &rom) {
+			character_rom_ = rom;
+
+			// There's some inconsistency in bit ordering amongst the common ROM dumps;
+			// detect that based arbitrarily on the second line of the $ graphic and
+			// ensure consistency.
+			if(character_rom_[0x121] == 0x3c || character_rom_[0x122] == 0x3c) {
+				for(auto &graphic : character_rom_) {
+					graphic =
+						((graphic & 0x01) ? 0x40 : 0x00) |
+						((graphic & 0x02) ? 0x20 : 0x00) |
+						((graphic & 0x04) ? 0x10 : 0x00) |
+						((graphic & 0x08) ? 0x08 : 0x00) |
+						((graphic & 0x10) ? 0x04 : 0x00) |
+						((graphic & 0x20) ? 0x02 : 0x00) |
+						((graphic & 0x40) ? 0x01 : 0x00);
+				}
+			}
+		}
+
 	protected:
 		GraphicsMode graphics_mode(int row) const {
 			if(
@@ -228,8 +303,16 @@ template <typename TimeUnit> class VideoSwitches {
 				uint16_t(((video_page()+1) * 0x400) + row_address);
 		}
 
-		virtual void did_set_annunciator_3(bool) = 0;
-		virtual void did_set_alternative_character_set(bool) = 0;
+		/*!
+			Should be called by subclasses at the end of each line of the display;
+			this gives the base class a peg on which to hang flashing-character updates.
+		*/
+		void did_end_line() {
+			// Update character set flashing; flashing is applied only when the alternative
+			// character set is not selected.
+			flash_ = (flash_ + 1) % (2 * flash_length);
+			character_zones_[1].xor_mask = flash_mask() * !internal_.alternative_character_set;
+		}
 
 	private:
 		// Maintain a DeferredQueue for delayed mode switches.
@@ -246,6 +329,40 @@ template <typename TimeUnit> class VideoSwitches {
 			bool high_resolution = false;
 			bool annunciator_3 = false;
 		} external_, internal_;
+
+		int flash_length = 8406;
+		int flash_ = 0;
+		uint8_t flash_mask() const {
+			return uint8_t((flash_ / flash_length) * 0xff);
+		}
+
+	protected:
+
+		// Describes the current text mode mapping from in-memory character index
+		// to output character; subclasses should:
+		//
+		//	(i)		use the top two-bits of the character code to index character_zones_;
+		//	(ii)	apply the address_mask to the character code in order to get a character
+		//			offset into the character ROM; and
+		//	(iii)	apply the XOR mask to the output of the character ROM.
+		//
+		// By this means they will properly handle the limited character sets of Apple IIs
+		// prior to the IIe as well as the IIe and onward's alternative character set toggle.
+		struct CharacterMapping {
+			uint8_t address_mask;
+			uint8_t xor_mask;
+		};
+		CharacterMapping character_zones_[4];
+
+		// A mask that should be applied to high-resolution graphics bytes before output;
+		// it acts to retain or remove the top bit, affecting whether the half-pixel delay
+		// bit is effective. On a IIe it's toggleable, on early Apple IIs it doesn't exist.
+		uint8_t high_resolution_mask_ = 0xff;
+
+		// This holds a copy of the character ROM. The regular character
+		// set is assumed to be in the first 64*8 bytes; the alternative
+		// is in the 128*8 bytes after that.
+		std::vector<uint8_t> character_rom_;
 };
 
 }
diff --git a/Machines/Apple/AppleIIgs/AppleIIgs.cpp b/Machines/Apple/AppleIIgs/AppleIIgs.cpp
index 992575eed..37b1e2bfa 100644
--- a/Machines/Apple/AppleIIgs/AppleIIgs.cpp
+++ b/Machines/Apple/AppleIIgs/AppleIIgs.cpp
@@ -61,11 +61,14 @@ class ConcreteMachine:
 					rom_descriptions.emplace_back(machine_name, "the Apple IIgs ROM03", "apple2gs.rom2", 256*1024, 0xde7ddf29);
 				break;
 			}
+			rom_descriptions.push_back(video_.rom_description(Video::VideoBase::CharacterROM::EnhancedIIe));
+
 			const auto roms = rom_fetcher(rom_descriptions);
-			if(!roms[0]) {
+			if(!roms[0] || !roms[1]) {
 				throw ROMMachine::Error::MissingROMs;
 			}
 			rom_ = *roms[0];
+			video_.set_character_rom(*roms[1]);
 
 			size_t ram_size = 0;
 			switch(target.memory_model) {
diff --git a/Machines/Apple/AppleIIgs/Video.cpp b/Machines/Apple/AppleIIgs/Video.cpp
index 8c461a554..5a6eec246 100644
--- a/Machines/Apple/AppleIIgs/Video.cpp
+++ b/Machines/Apple/AppleIIgs/Video.cpp
@@ -19,17 +19,14 @@ constexpr int FinalPixelLine = 192;
 }
 
 VideoBase::VideoBase() :
-	VideoSwitches<Cycles>(Cycles(2), [] (Cycles) {}) {
+	VideoSwitches<Cycles>(true, Cycles(2), [this] (Cycles cycles) { advance(cycles); }) {
 }
 
 void VideoBase::set_internal_ram(const uint8_t *ram) {
 	ram_ = ram;
 }
 
-void VideoBase::did_set_annunciator_3(bool) {}
-void VideoBase::did_set_alternative_character_set(bool) {}
-
-void VideoBase::run_for(Cycles cycles) {
+void VideoBase::advance(Cycles cycles) {
 	// TODO: everything else!
 	const auto old = cycles_into_frame_;
 	cycles_into_frame_ = (cycles_into_frame_ + cycles.as<int>()) % (CyclesPerLine * Lines);
diff --git a/Machines/Apple/AppleIIgs/Video.hpp b/Machines/Apple/AppleIIgs/Video.hpp
index 1e6038377..7f23bd73f 100644
--- a/Machines/Apple/AppleIIgs/Video.hpp
+++ b/Machines/Apple/AppleIIgs/Video.hpp
@@ -21,7 +21,6 @@ class VideoBase: public Apple::II::VideoSwitches<Cycles> {
 		VideoBase();
 		void set_internal_ram(const uint8_t *);
 
-		void run_for(Cycles);
 		bool get_is_vertical_blank();
 
 		void set_new_video(uint8_t);
@@ -34,8 +33,7 @@ class VideoBase: public Apple::II::VideoSwitches<Cycles> {
 		void notify_clock_tick();
 
 	private:
-		void did_set_annunciator_3(bool) override;
-		void did_set_alternative_character_set(bool) override;
+		void advance(Cycles);
 
 		uint8_t new_video_ = 0x01;
 		uint8_t interrupts_ = 0x00;