Merge pull request #392 from TomHarte/VicPalette

Tweaks VIC palette, especially PAL.

Analyser/Static/Commodore/Target.hpp

@@ -22,7 +22,16 @@ struct Target: public ::Analyser::Static::Target {
 		ThirtyTwoKB
 	};
 
+	enum class Region {
+		American,
+		Danish,
+		Japanese,
+		European,
+		Swedish
+	};
+
 	MemoryModel memory_model = MemoryModel::Unexpanded;
+	Region region = Region::European;
 	bool has_c1540 = false;
 };
 

Components/6560/6560.hpp

@@ -95,25 +95,25 @@ template <class T> class MOS6560 {
 
 			// Lumunances are encoded trivially: on a 0–255 scale.
 			const uint8_t luminances[16] = {
-				0,		255,	109,	189,
-				199,	144,	159,	161,
-				126,	227,	227,	207,
-				235,	173,	188,	196
+				0,		255,	60,		189,
+				100,	144,	40,		227,
+				126,	161,	227,	207,
+				200,	196,	160,	196
 			};
 
 			// Chrominances are encoded such that 0–128 is a complete revolution of phase;
 			// anything above 191 disables the colour subcarrier. Phase is relative to the
 			// colour burst, so 0 is green.
 			const uint8_t pal_chrominances[16] = {
-				255,	255,	40,		112,
-				8,		88,		120,	56,
-				40,		48,		40,		112,
-				8,		88,		120,	56,
+				255,	255,	36,		112,
+				8,		88,		120,	48,
+				40,		56,		36,		112,
+				8,		72,		126,	56,
 			};
 			const uint8_t ntsc_chrominances[16] = {
 				255,	255,	8,		72,
-				32,		88,		48,		112,
-				0,		0,		8,		72,
+				32,		88,		40,		112,
+				0,		8,		12,		72,
 				32,		88,		48,		112,
 			};
 			const uint8_t *chrominances;
@@ -140,7 +140,7 @@ template <class T> class MOS6560 {
 			}
 
 			crt_->set_new_display_type(static_cast<unsigned int>(timing_.cycles_per_line*4), display_type);
-			crt_->set_visible_area(Outputs::CRT::Rect(0.05f, 0.05f, 0.9f, 0.9f));
+//			crt_->set_visible_area(Outputs::CRT::Rect(0.05f, 0.05f, 0.9f, 0.9f));
 
 //			switch(output_mode) {
 //				case OutputMode::PAL:

Machines/Commodore/1540/Implementation/C1540.cpp

@@ -165,14 +165,15 @@ void SerialPortVIA::set_port_output(MOS::MOS6522::Port port, uint8_t value, uint
 			attention_acknowledge_level_ = !(value&0x10);
 			data_level_output_ = (value&0x02);
 
-			serialPort->set_output(::Commodore::Serial::Line::Clock, (::Commodore::Serial::LineLevel)!(value&0x08));
+//			printf("[C1540] %s output is %s\n", StringForLine(::Commodore::Serial::Line::Clock), value ? "high" : "low");
+			serialPort->set_output(::Commodore::Serial::Line::Clock, static_cast<::Commodore::Serial::LineLevel>(!(value&0x08)));
 			update_data_line();
 		}
 	}
 }
 
 void SerialPortVIA::set_serial_line_state(::Commodore::Serial::Line line, bool value) {
-//	printf("[C1540] %s is %s\n", StringForLine(line), value ? "high" : "low");
+//	printf("[C1540] %s input is %s\n", StringForLine(line), value ? "high" : "low");
 
 	switch(line) {
 		default: break;
@@ -194,9 +195,10 @@ void SerialPortVIA::set_serial_port(const std::shared_ptr<::Commodore::Serial::P
 void SerialPortVIA::update_data_line() {
 	std::shared_ptr<::Commodore::Serial::Port> serialPort = serial_port_.lock();
 	if(serialPort) {
+//		printf("[C1540] %s output is %s\n", StringForLine(::Commodore::Serial::Line::Data), (!data_level_output_ && (attention_level_input_ != attention_acknowledge_level_)) ? "high" : "low");
 		// "ATN (Attention) is an input on pin 3 of P2 and P3 that is sensed at PB7 and CA1 of UC3 after being inverted by UA1"
 		serialPort->set_output(::Commodore::Serial::Line::Data,
-			(::Commodore::Serial::LineLevel)(!data_level_output_ && (attention_level_input_ != attention_acknowledge_level_)));
+			static_cast<::Commodore::Serial::LineLevel>(!data_level_output_ && (attention_level_input_ != attention_acknowledge_level_)));
 	}
 }
 

Machines/Commodore/Vic-20/Vic20.cpp

@@ -368,24 +368,12 @@ class ConcreteMachine:
 		}
 
 		void configure_as_target(const Analyser::Static::Target *target) override final {
-			auto *const commodore_target = dynamic_cast<const Analyser::Static::Commodore::Target *>(target);
+			commodore_target_ = *dynamic_cast<const Analyser::Static::Commodore::Target *>(target);
 
 			if(target->loading_command.length()) {
 				type_string(target->loading_command);
 			}
 
-			switch(commodore_target->memory_model) {
-				case Analyser::Static::Commodore::Target::MemoryModel::Unexpanded:
-					set_memory_size(Default);
-				break;
-				case Analyser::Static::Commodore::Target::MemoryModel::EightKB:
-					set_memory_size(ThreeKB);
-				break;
-				case Analyser::Static::Commodore::Target::MemoryModel::ThirtyTwoKB:
-					set_memory_size(ThirtyTwoKB);
-				break;
-			}
-
 			if(target->media.disks.size()) {
 				// construct the 1540
 				c1540_.reset(new ::Commodore::C1540::Machine(Commodore::C1540::Machine::C1540));
@@ -395,6 +383,9 @@ class ConcreteMachine:
 
 				// give it a means to obtain its ROM
 				c1540_->set_rom_fetcher(rom_fetcher_);
+
+				// give it a little warm up
+				c1540_->run_for(Cycles(2000000));
 			}
 
 			insert_media(target->media);
@@ -439,16 +430,6 @@ class ConcreteMachine:
 			return joysticks_;
 		}
 
-		void set_memory_size(MemorySize size) override final {
-			memory_size_ = size;
-			needs_configuration_ = true;
-		}
-
-		void set_region(Region region) override final {
-			region_ = region;
-			needs_configuration_ = true;
-		}
-
 		void set_ntsc_6560() {
 			set_clock_rate(1022727);
 			if(mos6560_) {
@@ -465,9 +446,9 @@ class ConcreteMachine:
 			}
 		}
 
-		void configure_memory() {
+		void set_memory_map(Analyser::Static::Commodore::Target::MemoryModel memory_model, Analyser::Static::Commodore::Target::Region region) {
 			// Determine PAL/NTSC
-			if(region_ == American || region_ == Japanese) {
+			if(region == Analyser::Static::Commodore::Target::Region::American || region == Analyser::Static::Commodore::Target::Region::Japanese) {
 				// NTSC
 				set_ntsc_6560();
 			} else {
@@ -479,13 +460,13 @@ class ConcreteMachine:
 			memset(processor_write_memory_map_, 0, sizeof(processor_write_memory_map_));
 			memset(mos6560_->video_memory_map, 0, sizeof(mos6560_->video_memory_map));
 
-			switch(memory_size_) {
+			switch(memory_model) {
 				default: break;
-				case ThreeKB:
+				case Analyser::Static::Commodore::Target::MemoryModel::EightKB:
 					write_to_map(processor_read_memory_map_, expansion_ram_, 0x0000, 0x1000);
 					write_to_map(processor_write_memory_map_, expansion_ram_, 0x0000, 0x1000);
 				break;
-				case ThirtyTwoKB:
+				case Analyser::Static::Commodore::Target::MemoryModel::ThirtyTwoKB:
 					write_to_map(processor_read_memory_map_, expansion_ram_, 0x0000, 0x8000);
 					write_to_map(processor_write_memory_map_, expansion_ram_, 0x0000, 0x8000);
 				break;
@@ -508,24 +489,24 @@ class ConcreteMachine:
 
 			ROM character_rom;
 			ROM kernel_rom;
-			switch(region_) {
+			switch(region) {
 				default:
 					character_rom = CharactersEnglish;
 					kernel_rom = KernelPAL;
 				break;
-				case American:
+				case Analyser::Static::Commodore::Target::Region::American:
 					character_rom = CharactersEnglish;
 					kernel_rom = KernelNTSC;
 				break;
-				case Danish:
+				case Analyser::Static::Commodore::Target::Region::Danish:
 					character_rom = CharactersDanish;
 					kernel_rom = KernelDanish;
 				break;
-				case Japanese:
+				case Analyser::Static::Commodore::Target::Region::Japanese:
 					character_rom = CharactersJapanese;
 					kernel_rom = KernelJapanese;
 				break;
-				case Swedish:
+				case Analyser::Static::Commodore::Target::Region::Swedish:
 					character_rom = CharactersSwedish;
 					kernel_rom = KernelSwedish;
 				break;
@@ -648,10 +629,6 @@ class ConcreteMachine:
 		}
 
 		void run_for(const Cycles cycles) override final {
-			if(needs_configuration_) {
-				needs_configuration_ = false;
-				configure_memory();
-			}
 			m6502_.run_for(cycles);
 		}
 
@@ -659,7 +636,7 @@ class ConcreteMachine:
 			mos6560_.reset(new Vic6560());
 			mos6560_->set_high_frequency_cutoff(1600);	// There is a 1.6Khz low-pass filter in the Vic-20.
 			// Make a guess: PAL. Without setting a clock rate the 6560 isn't fully set up so contractually something must be set.
-			set_pal_6560();
+			set_memory_map(commodore_target_.memory_model, commodore_target_.region);
 		}
 
 		void close_output() override final {
@@ -718,6 +695,8 @@ class ConcreteMachine:
 		}
 
 	private:
+		Analyser::Static::Commodore::Target commodore_target_;
+
 		CPU::MOS6502::Processor<ConcreteMachine, false> m6502_;
 
 		std::vector<uint8_t>  roms_[9];
@@ -748,10 +727,6 @@ class ConcreteMachine:
 			}
 		}
 
-		Region region_ = European;
-		MemorySize memory_size_ = MemorySize::Default;
-		bool needs_configuration_ = true;
-
 		Commodore::Vic20::KeyboardMapper keyboard_mapper_;
 		std::vector<std::unique_ptr<Inputs::Joystick>> joysticks_;
 

Machines/Commodore/Vic-20/Vic20.hpp

@@ -14,20 +14,6 @@
 namespace Commodore {
 namespace Vic20 {
 
-enum MemorySize {
-	Default,
-	ThreeKB,
-	ThirtyTwoKB
-};
-
-enum Region {
-	American,
-	Danish,
-	Japanese,
-	European,
-	Swedish
-};
-
 /// @returns The options available for a Vic-20.
 std::vector<std::unique_ptr<Configurable::Option>> get_options();
 
@@ -37,12 +23,6 @@ class Machine {
 
 		/// Creates and returns a Vic-20.
 		static Machine *Vic20();
-
-		/// Sets the memory size of this Vic-20.
-		virtual void set_memory_size(MemorySize size) = 0;
-
-		/// Sets the region of this Vic-20.
-		virtual void set_region(Region region) = 0;
 };
 
 }