Merge pull request #1075 from TomHarte/PlayfieldMasking

Add comments, fix playfield sprite masking.

Machines/Amiga/Bitplanes.hpp

@@ -64,7 +64,7 @@ class BitplaneShifter {
 			int odd_delay,
 			int even_delay);
 
-		/// Shifts either two pixels (in low-res mode) and four pixels (in high-res).
+		/// Shifts either two pixels (in low-res mode) or four pixels (in high-res).
 		void shift(bool high_res) {
 			constexpr int shifts[] = {16, 32};
 
@@ -73,8 +73,14 @@ class BitplaneShifter {
 		}
 
 		/// @returns The next four pixels to output; in low-resolution mode only two
-		/// of them will be unique. The value is arranges so that MSB = first pixel to output,
+		/// of them will be unique.
-		/// LSB = last. Each byte is formed as 00[bitplane 5][bitplane 4]...[bitplane 0].
+		///
+		/// The value is arranges so that MSB = first pixel to output, LSB = last.
+		///
+		/// Each byte is swizzled to provide easier playfield separation, being in the form:
+		/// 	b6, b7 = 0;
+		/// 	b3–b5: planes 1, 3 and 5;
+		/// 	b0–b2: planes 0, 2 and 4.
 		uint32_t get(bool high_res) {
 			if(high_res) {
 				return uint32_t(data_[1] >> 32);

Machines/Amiga/Chipset.cpp

@@ -199,17 +199,37 @@ void Chipset::output_pixels(int cycles_until_sync) {
 		}
 	}
 
-	// Compute masks potentially to obscure sprites.
+	// This will store flags to indicate presence or absence of sprite pixels for the four shifters.
-	int playfield_odd_pixel_mask =
+	int collision_masks[4] = {0, 0, 0, 0};
-		(((playfield >> 22) | (playfield >> 24) | (playfield >> 26)) & 8) |
+
-		(((playfield >> 15) | (playfield >> 17) | (playfield >> 19)) & 4) |
+	// If there are sprites visible, bother to figure out the playfield masks here.
-		(((playfield >> 8) | (playfield >> 10) | (playfield >> 12)) & 2) |
+	if(sprite_shifters_[0].get() | sprite_shifters_[1].get() | sprite_shifters_[2].get() | sprite_shifters_[3].get()) {
-		(((playfield >> 1) | (playfield >> 3) | (playfield >> 5)) & 1);
+		// The playfield value is arranged as:
-	int playfield_even_pixel_mask =
+		//
-		(((playfield >> 21) | (playfield >> 23) | (playfield >> 25)) & 8) |
+		//	pixel = [0 0 b5 b3 b1 b4 b2 b0]
-		(((playfield >> 14) | (playfield >> 16) | (playfield >> 18)) & 4) |
+		//	full value = [pixel] [pixel] [pixel] [pixel]
-		(((playfield >> 7) | (playfield >> 9) | (playfield >> 11)) & 2) |
+		//
-		(((playfield >> 0) | (playfield >> 2) | (playfield >> 4)) & 1);
+		// i.e. the odd pixel mask is:
+		//	b0 = bits 3, 4, 5;
+		//	b1 = bits 11, 12, 13;
+		//	b2 = bits 19, 20, 21;
+		//	b3 = bits 27, 28, 29.
+		//
+		// ... and the even pixel mask is the other set.
+
+		// Ensure that b0, b8, b16, b24 are the complete mask state of the even playfields,
+		// and b3, b11, b19, b27 are the complete mask state of the odd playfields.
+		const uint32_t merged_playfield = playfield | (playfield >> 1) | (playfield >> 2);
+
+		// Collect b0, b8, b16 and b24 as b0, b1, b2, b3 (and give no regard to the other bits).
+		uint32_t playfield_even_pixel_mask = merged_playfield & 0x01010101;
+		playfield_even_pixel_mask |= playfield_even_pixel_mask >> 7;
+		playfield_even_pixel_mask |= playfield_even_pixel_mask >> 14;
+
+		// Collect b3, b11, b19 and b27 as b0, b1, b2, b3 (and give no regard to the other bits).
+		uint32_t playfield_odd_pixel_mask = (merged_playfield >> 3) & 0x01010101;
+		playfield_odd_pixel_mask |= playfield_odd_pixel_mask >> 7;
+		playfield_odd_pixel_mask |= playfield_odd_pixel_mask >> 14;
 
 		// If only a single playfield is in use, treat the mask as playing
 		// into the priority selected for the even bitfields.
@@ -218,8 +238,7 @@ void Chipset::output_pixels(int cycles_until_sync) {
 			playfield_odd_pixel_mask = 0;
 		}
 
-	// Process sprites.
+		// Draw sprites.
-	int collision_masks[4] = {0, 0, 0, 0};
 		int index = int(sprite_shifters_.size());
 		for(auto shifter = sprite_shifters_.rbegin(); shifter != sprite_shifters_.rend(); ++shifter) {
 			// Update the index, and skip this shifter entirely if it's empty.
@@ -234,14 +253,26 @@ void Chipset::output_pixels(int cycles_until_sync) {
 			}
 			collision_masks[index] = (collision_masks[index] & 0x01) | ((collision_masks[index] & 0x10) >> 3);
 
-		// Get the specific pixel mask.
+			// Get the specific pixel mask;
-		const int pixel_mask =
+			//
+			// Playfield priority meanings:
+			//
+			//	4: behind all sprites;
+			//	3: in front of sprites 6 & 7, behind all others;
+			//	2: in front of 4, 5, 6 & 7; behind all others;
+			//	1: in front of 2, 3, 4, 5, 6, & 7; behind 0 & 1;
+			//	0: in front of all sprites.
+			//
+			// i.e. the playfield is in front of the two sprites in shifter n
+			// if and only if it has a priority of n or less.
+			const auto pixel_mask =
 				(
 					((odd_priority_ <= index) ? playfield_odd_pixel_mask : 0) |
 					((even_priority_ <= index) ? playfield_even_pixel_mask : 0)
 				);
 
-		// Output pixels, if a buffer exists.
+			// Output pixels, if a buffer exists and only where the pixel
+			// mask allows. TODO: try to find a less branchy version of the below.
 			const auto base = (index << 2) + 16;
 			if(pixels_) {
 				if(sprites_[size_t((index << 1) + 1)].attached) {
@@ -279,6 +310,7 @@ void Chipset::output_pixels(int cycles_until_sync) {
 				}
 			}
 		}
+	}
 
 	// Compute playfield collision mask and populate collisions register.
 	const uint32_t playfield_collisions = (playfield & playfield_collision_mask_) ^ playfield_collision_complement_;