Amps up the documentation.

Machines/Atari/ST/Video.cpp

@@ -33,7 +33,7 @@ const struct VerticalParams {
 };
 
 /// @returns The correct @c VerticalParams for output at @c frequency.
-const VerticalParams &vertical_parameters(FieldFrequency frequency) {
+const VerticalParams &vertical_parameters(Video::FieldFrequency frequency) {
 	return vertical_params[int(frequency)];
 }
 
@@ -63,7 +63,7 @@ const struct HorizontalParams {
 	{4*2, 164*2,	184*2, 2*2,		224*2}
 };
 
-const HorizontalParams &horizontal_parameters(FieldFrequency frequency) {
+const HorizontalParams &horizontal_parameters(Video::FieldFrequency frequency) {
 	return horizontal_params[int(frequency)];
 }
 
@@ -72,14 +72,14 @@ struct Checker {
 	Checker() {
 		for(int c = 0; c < 3; ++c) {
 			// Expected horizontal order of events: reset blank, enable display, disable display, enable blank (at least 50 before end of line), end of line
-			const auto horizontal = horizontal_parameters(FieldFrequency(c));
+			const auto horizontal = horizontal_parameters(Video::FieldFrequency(c));
 			assert(horizontal.reset_blank < horizontal.set_enable);
 			assert(horizontal.set_enable < horizontal.reset_enable);
 			assert(horizontal.reset_enable < horizontal.set_blank);
 			assert(horizontal.set_blank+50 < horizontal.length);
 
 			// Expected vertical order of events: reset blank, enable display, disable display, enable blank (at least 50 before end of line), end of line
-			const auto vertical = vertical_parameters(FieldFrequency(c));
+			const auto vertical = vertical_parameters(Video::FieldFrequency(c));
 			assert(vertical.set_enable < vertical.reset_enable);
 			assert(vertical.reset_enable < vertical.height);
 		}

Machines/Atari/ST/Video.hpp

@@ -15,14 +15,20 @@
 namespace Atari {
 namespace ST {
 
-enum class FieldFrequency {
+/*!
-	Fifty = 0, Sixty = 1, SeventyTwo = 2
+	Models a combination of the parts of the GLUE, MMU and Shifter that in net
-};
+	form the video subsystem of the Atari ST. So not accurate to a real chip, but
-
+	(hopefully) to a subsystem.
+*/
 class Video {
 	public:
 		Video();
 
+		/*!
+			Sets the memory pool that provides video, and its size.
+		*/
+		void set_ram(uint16_t *, size_t size);
+
 		/*!
 			Sets the target device for video data.
 		*/
@@ -33,21 +39,51 @@ class Video {
 		*/
 		void run_for(HalfCycles duration);
 
+
 		/*!
 			@returns the number of cycles until there is next a change in the hsync,
 			vsync or display_enable outputs.
 		*/
 		HalfCycles get_next_sequence_point();
 
+		/*!
+			@returns @c true if the horizontal sync output is currently active; @c false otherwise.
+
+			@discussion On an Atari ST, this generates a VPA-style interrupt, which is often erroneously
+			documented as being triggered by horizontal blank.
+		*/
 		bool hsync();
+
+		/*!
+			@returns @c true if the vertical sync output is currently active; @c false otherwise.
+
+			@discussion On an Atari ST, this generates a VPA-style interrupt, which is often erroneously
+			documented as being triggered by vertical blank.
+		*/
 		bool vsync();
+
+		/*!
+			@returns @c true if the display enabled output is currently active; @c false otherwise.
+
+			@discussion On an Atari ST this is fed to the MFP. The documentation that I've been able to
+			find implies a total 28-cycle delay between the real delay enabled signal changing and its effect
+			on the 68000 interrupt input via the MFP. As I have yet to determine how much delay is caused
+			by the MFP a full 28-cycle delay is applied by this class. This should be dialled down when the
+			MFP's responsibility is clarified.
+		*/
 		bool display_enabled();
 
-		void set_ram(uint16_t *, size_t size);
+		/// @returns the effect of reading from @c address; only the low 6 bits are decoded.
-
 		uint16_t read(int address);
+
+		/// Writes @c value to @c address, of which only the low 6 bits are decoded.
 		void write(int address, uint16_t value);
 
+		/// Used internally to track state.
+		enum class FieldFrequency {
+			Fifty = 0, Sixty = 1, SeventyTwo = 2
+		};
+
 	private:
 		Outputs::CRT::CRT crt_;
 
@@ -56,7 +92,7 @@ class Video {
 		int base_address_ = 0;
 		int current_address_ = 0;
 
-		uint16_t *ram_;
+		uint16_t *ram_ = nullptr;
 		uint16_t line_buffer_[256];
 
 		int x_ = 0, y_ = 0, next_y_ = 0;
@@ -67,7 +103,7 @@ class Video {
 		FieldFrequency field_frequency_ = FieldFrequency::Fifty;
 		enum class OutputBpp {
 			One, Two, Four
-		} output_bpp_;
+		} output_bpp_ = OutputBpp::Four;
 		void update_output_mode();
 
 		struct HorizontalState {
@@ -92,7 +128,7 @@ class Video {
 		int line_length_ = 1024;
 
 		int data_latch_position_ = 0;
-		uint16_t data_latch_[4];
+		uint16_t data_latch_[4] = {0, 0, 0, 0};
 		void latch_word();
 
 		class Shifter {
@@ -110,8 +146,8 @@ class Video {
 				enum class OutputMode {
 					Sync, Blank, Border, Pixels
 				} output_mode_ = OutputMode::Sync;
-				uint16_t border_colour_;
+				uint16_t border_colour_ = 0;
-				OutputBpp bpp_;
+				OutputBpp bpp_ = OutputBpp::Four;
 				union {
 					uint64_t output_shifter_;
 					uint32_t shifter_halves_[2];
@@ -119,7 +155,7 @@ class Video {
 
 				void flush_output(OutputMode next_mode);
 
-				uint16_t *pixel_buffer_;
+				uint16_t *pixel_buffer_ = nullptr;
 				size_t pixel_pointer_ = 0;
 
 				Outputs::CRT::CRT &crt_;