[C64] DiskTrack implementation to simplify delta tracking

src/BizHawk.Emulation.Cores/Computers/Commodore64/Media/Disk.cs

@@ -1,5 +1,5 @@
 using System.Collections.Generic;
-using System.Linq;
+
 using BizHawk.Common;
 
 namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media
@@ -9,8 +9,7 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media
 		public const int FluxBitsPerEntry = 32;
 		public const int FluxBitsPerTrack = 16000000 / 5;
 		public const int FluxEntriesPerTrack = FluxBitsPerTrack / FluxBitsPerEntry;
-		private readonly int[][] _tracks;
+		private readonly DiskTrack[] _tracks;
-		private readonly int[][] _originalMedia;
 		private bool[] _usedTracks;
 		public bool Valid;
 		public bool WriteProtected;
@@ -18,12 +17,12 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media
 		/// <summary>
 		/// Create a blank, unformatted disk.
 		/// </summary>
-		public Disk(int trackCapacity)
+		public Disk(int trackCount)
 		{
 			WriteProtected = false;
-			_tracks = new int[trackCapacity][];
+			_tracks = new DiskTrack[trackCount];
+			_usedTracks = new bool[trackCount];
 			FillMissingTracks();
-			_originalMedia = _tracks.Select(t => (int[])t.Clone()).ToArray();
 			Valid = true;
 		}
 
@@ -37,76 +36,17 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media
 		public Disk(IList<byte[]> trackData, IList<int> trackNumbers, IList<int> trackDensities, int trackCapacity)
 		{
 			WriteProtected = true;
-			_tracks = new int[trackCapacity][];
+			_tracks = new DiskTrack[trackCapacity];
+			_usedTracks = new bool[trackCapacity];
 			for (var i = 0; i < trackData.Count; i++)
 			{
-				_tracks[trackNumbers[i]] = ConvertToFluxTransitions(trackDensities[i], trackData[i], 0);
+				var track = new DiskTrack();
+				track.ReadFromGCR(trackDensities[i], trackData[i], 0);
+				_tracks[trackNumbers[i]] = track;
 			}
 
 			FillMissingTracks();
 			Valid = true;
-			_originalMedia = _tracks.Select(t => (int[])t.Clone()).ToArray();
-		}
-
-		private int[] ConvertToFluxTransitions(int density, byte[] bytes, int fluxBitOffset)
-		{
-			var paddedLength = bytes.Length;
-			switch (density)
-			{
-				case 3:
-					paddedLength = Math.Max(bytes.Length, 7692);
-					break;
-				case 2:
-					paddedLength = Math.Max(bytes.Length, 7142);
-					break;
-				case 1:
-					paddedLength = Math.Max(bytes.Length, 6666);
-					break;
-				case 0:
-					paddedLength = Math.Max(bytes.Length, 6250);
-					break;
-			}
-
-			paddedLength++;
-			var paddedBytes = new byte[paddedLength];
-			Array.Copy(bytes, paddedBytes, bytes.Length);
-			for (var i = bytes.Length; i < paddedLength; i++)
-			{
-				paddedBytes[i] = 0xAA;
-			}
-			var result = new int[FluxEntriesPerTrack];
-			var lengthBits = (paddedLength * 8) - 7;
-			var offsets = new List<long>();
-			var remainingBits = lengthBits;
-
-			const long bitsNum = FluxEntriesPerTrack * FluxBitsPerEntry;
-			long bitsDen = lengthBits;
-
-			for (var i = 0; i < paddedLength; i++)
-			{
-				var byteData = paddedBytes[i];
-				for (var j = 0; j < 8; j++)
-				{
-					var offset = fluxBitOffset + ((i * 8 + j) * bitsNum / bitsDen);
-					var byteOffset = (int)(offset / FluxBitsPerEntry);
-					var bitOffset = (int)(offset % FluxBitsPerEntry);
-					offsets.Add(offset);
-					result[byteOffset] |= ((byteData & 0x80) != 0 ? 1 : 0) << bitOffset;
-					byteData <<= 1;
-					remainingBits--;
-					if (remainingBits <= 0)
-					{
-						break;
-					}
-				}
-
-				if (remainingBits <= 0)
-				{
-					break;
-				}
-			}
-
-			return result;
 		}
 
 		private void FillMissingTracks()
@@ -116,52 +56,35 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media
 			{
 				if (_tracks[i] == null && _tracks[i - 1] != null)
 				{
-					_tracks[i] = new int[FluxEntriesPerTrack];
+					_tracks[i] = _tracks[i - 1].Clone();
-					Array.Copy(_tracks[i - 1], _tracks[i], FluxEntriesPerTrack);
 				}
 			}
 
 			// Fill vacant tracks
 			for (var i = 0; i < _tracks.Length; i++)
 			{
-				if (_tracks[i] == null)
+				_tracks[i] ??= new();
-				{
-					_tracks[i] = new int[FluxEntriesPerTrack];
-				}
 			}
 		}
 
-		public void AttachTracker(bool[] usedTracks)
-		{
-			if (_tracks.Length != usedTracks.Length)
-			{
-				throw new InvalidOperationException("track and tracker length mismatch! (this should be impossible, please report)");
-			}
-
-			_usedTracks = usedTracks;
-		}
-
 		/// <summary>
 		/// Generic update of the deltas stored in Drive1541's ISaveRam implementation.
 		/// deltaUpdateCallback will be called for each track which has been possibly dirtied
 		/// </summary>
 		/// <param name="deltaUpdateCallback">callback</param>
-		public void DeltaUpdate(Action<int, int[], int[]> deltaUpdateCallback)
+		public void DeltaUpdate(Action<int, DiskTrack> deltaUpdateCallback)
 		{
 			for (var i = 0; i < _tracks.Length; i++)
 			{
 				if (_usedTracks[i])
 				{
-					deltaUpdateCallback(i, _originalMedia[i], _tracks[i]);
+					deltaUpdateCallback(i, _tracks[i]);
 				}
 			}
 		}
 
-		public int[] GetDataForTrack(int halftrack)
+		public DiskTrack GetTrack(int trackNumber)
-		{
+			=> _tracks[trackNumber];
-			_usedTracks[halftrack] = true;
-			return _tracks[halftrack];
-		}
 
 		public void SyncState(Serializer ser)
 		{

src/BizHawk.Emulation.Cores/Computers/Commodore64/Media/DiskTrack.cs

@@ -0,0 +1,217 @@
+using System.Buffers;
+
+using BizHawk.Common;
+
+namespace BizHawk.Emulation.Cores.Computers.Commodore64.Media;
+
+/// <summary>
+/// Represents the magnetic flux transitions for one rotation of floppy disk media. Each bit represents
+/// the transition of the signal level from 1 to 0, or from 0 to 1.
+/// </summary>
+public sealed class DiskTrack
+{
+	/// <summary>
+	/// The master clock rate for synchronization.
+	/// </summary>
+	private const int ClockRateHz = 16000000;
+
+	/// <summary>
+	/// Number of bytes per element in the Bits array.
+	/// </summary>
+	private const int BytesPerEntry = sizeof(int);
+
+	/// <summary>
+	/// Number of bits contained in a single value of the Bits array.
+	/// </summary>
+	private const int FluxBitsPerEntry = BytesPerEntry * 8;
+
+	/// <summary>
+	/// The number of flux transition bits stored for each track.
+	/// </summary>
+	private const int FluxBitsPerTrack = ClockRateHz / 5;
+
+	/// <summary>
+	/// The fixed size of the Bits array.
+	/// </summary>
+	private const int FluxEntriesPerTrack = FluxBitsPerTrack / FluxBitsPerEntry;
+
+	/// <summary>
+	/// The number of bytes contained in the cached delta, for use with save states.
+	/// </summary>
+	private const int DeltaBytesPerTrack = FluxEntriesPerTrack * BytesPerEntry + 4;
+
+	private int[] _bits = new int[FluxEntriesPerTrack];
+	private int[] _original = new int[FluxEntriesPerTrack];
+	private byte[] _delta = new byte[DeltaBytesPerTrack];
+	private bool _dirty = true;
+	private bool _modified = false;
+
+	/// <summary>
+	/// Current state of the disk, which may be changed from the original media.
+	/// </summary>
+	public ReadOnlySpan<int> Bits => _bits;
+
+	/// <summary>
+	/// Fixed state of the original media, from which deltas will be calculated.
+	/// </summary>
+	public ReadOnlySpan<int> Original => _original;
+
+	/// <summary>
+	/// The compressed difference between
+	/// </summary>
+	public byte[] Delta => _delta;
+
+	/// <summary>
+	/// If true, the delta needs to be recalculated.
+	/// </summary>
+	public bool IsDirty => _dirty;
+
+	/// <summary>
+	/// If true, the track data has been modified.
+	/// </summary>
+	public bool IsModified => _modified;
+
+	/// <summary>
+	/// Create a clone of the DiskTrack.
+	/// </summary>
+	/// <returns>
+	/// A new DiskTrack with an identical copy of <see cref="Bits"/>.
+	/// </returns>
+	public DiskTrack Clone()
+	{
+		var clone = new DiskTrack();
+		Bits.CopyTo(clone._bits.AsSpan());
+		clone._original = _original;
+		return clone;
+	}
+
+	/// <summary>
+	/// Prepare the <see cref="IsModified"/> property.
+	/// </summary>
+	/// <returns>
+	/// The new value of <see cref="IsModified"/>.
+	/// </returns>
+	private bool CheckModified()
+		=> _modified = !_original.AsSpan().SequenceEqual(_bits);
+
+	/// <summary>
+	/// Apply a compressed delta over the original media.
+	/// </summary>
+	/// <param name="delta">
+	/// Compressed delta data.
+	/// </param>
+	public void ApplyDelta(ReadOnlySpan<byte> delta)
+	{
+		DeltaSerializer.ApplyDelta<int>(_original, _bits, delta);
+		_delta = delta.ToArray();
+		_dirty = false;
+		CheckModified();
+	}
+
+	/// <summary>
+	/// Updates the delta for this track.
+	/// </summary>
+	/// <returns>
+	/// True if the delta has updated, false otherwise.
+	/// </returns>
+	public bool UpdateDelta()
+	{
+		if (!_dirty) return false;
+
+		_delta = DeltaSerializer.GetDelta<int>(_original, _bits).ToArray();
+		_dirty = false;
+		return true;
+	}
+
+	/// <summary>
+	/// Resets this track to the state of the original media.
+	/// </summary>
+	public void Reset()
+	{
+		_original.CopyTo(_bits.AsSpan());
+		_delta = Array.Empty<byte>();
+		_dirty = false;
+	}
+
+	/// <summary>
+	/// Synchronize state.
+	/// </summary>
+	/// <param name="ser">
+	/// Serializer with which to synchronize.
+	/// </param>
+	public void SyncState(Serializer ser, string deltaId)
+	{
+		ser.Sync(deltaId, ref _delta, useNull: true);
+	}
+
+	public void Write(int index, int bits)
+	{
+		// We only need to update delta if the bits actually changed.
+
+		if (_bits[index] == bits) return;
+
+		_bits[index] = bits;
+		_dirty = true;
+	}
+
+	public void ReadFromGCR(int density, ReadOnlySpan<byte> bytes, int fluxBitOffset)
+	{
+		// There are four levels of track density correlated with the four different clock dividers
+		// in the 1541 disk drive. Outer tracks have more surface area, so a technique is used to read
+		// bits at a higher rate.
+
+		var paddedLength = density switch
+		{
+			3 => Math.Max(bytes.Length, 7692),
+			2 => Math.Max(bytes.Length, 7142),
+			1 => Math.Max(bytes.Length, 6666),
+			0 => Math.Max(bytes.Length, 6250),
+			_ => bytes.Length
+		};
+
+		// One extra byte is added at the end to break up tracks so that if the data is perfectly
+		// aligned in an unfortunate way, loaders don't seize up trying to find data. Some copy protections
+		// will read the same track repeatedly to account for variations in drive mechanics, and this should get
+		// the more temperamental ones to load eventually.
+
+		paddedLength++;
+
+		// It is possible that there are more or fewer bits than the specification due to any number
+		// of reasons (e.g. copy protection, tiny variations in motor speed) so we pad out with the "default"
+		// bit pattern.
+
+		using var paddedBytesMem = MemoryPool<byte>.Shared.Rent(paddedLength);
+		var paddedBytes = paddedBytesMem.Memory.Span.Slice(0, paddedLength);
+		bytes.CopyTo(paddedBytes);
+		paddedBytes.Slice(bytes.Length).Fill(0xAA);
+
+		var lengthBits = paddedLength * 8 - 7;
+		var remainingBits = lengthBits;
+
+		const long bitsNum = FluxEntriesPerTrack * FluxBitsPerEntry;
+		long bitsDen = lengthBits;
+
+		for (var i = 0; i < paddedLength; i++)
+		{
+			var byteData = paddedBytes[i];
+			for (var j = 0; j < 8; j++)
+			{
+				var offset = fluxBitOffset + ((i * 8 + j) * bitsNum / bitsDen);
+				var byteOffset = (int)(offset / FluxBitsPerEntry);
+				var bitOffset = (int)(offset % FluxBitsPerEntry);
+				_bits[byteOffset] |= (byteData >> 7) << bitOffset;
+				byteData <<= 1;
+				remainingBits--;
+				if (remainingBits <= 0)
+				{
+					break;
+				}
+			}
+
+			if (remainingBits <= 0)
+			{
+				break;
+			}
+		}
+	}
+}

src/BizHawk.Emulation.Cores/Computers/Commodore64/Serial/Drive1541.FluxTransitions.cs

@@ -38,6 +38,9 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 
 		private void ExecuteFlux()
 		{
+			var track = _disk.GetTrack(_trackNumber);
+			var bits = track.Bits;
+
 			// This actually executes the main 16mhz clock
 			while (_clocks > 0)
 			{
@@ -56,7 +59,7 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 						if (_diskBitsLeft <= 0)
 						{
 							if (_diskWriteEnabled)
-								_trackImageData[_diskByteOffset] = _diskOutputBits;
+								track.Write(_diskByteOffset, _diskOutputBits);
 
 							_diskByteOffset++;
 
@@ -64,7 +67,7 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 								_diskByteOffset = 0;
 
 							if (!_diskWriteEnabled)
-								_diskBits = _trackImageData[_diskByteOffset];
+								_diskBits = bits[_diskByteOffset];
 
 							_diskOutputBits = 0;
 							_diskBitsLeft = Disk.FluxBitsPerEntry;
@@ -197,6 +200,8 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 				_diskDensityCounter++;
 				_diskCycle = (_diskCycle + 1) & 0xF;
 			}
+
+			if (_diskWriteEnabled && track.UpdateDelta()) SaveDelta(_trackNumber, track.Delta);
 		}
 	}
 }

src/BizHawk.Emulation.Cores/Computers/Commodore64/Serial/Drive1541.SaveRam.cs

@@ -13,20 +13,24 @@ public sealed partial class Drive1541 : ISaveRam
 	// we keep it here for all disks as we need to remember it when swapping disks around
 	// _usedDiskTracks.Length also doubles as a way to remember the disk count
 	private bool[][] _usedDiskTracks;
-	private byte[,][] _diskDeltas;
 	private readonly Func<int> _getCurrentDiskNumber;
 
 	public void InitSaveRam(int diskCount)
 	{
 		_usedDiskTracks = new bool[diskCount][];
-		_diskDeltas = new byte[diskCount, 84][];
+		_diskDeltas = new byte[diskCount][][];
 		for (var i = 0; i < diskCount; i++)
 		{
 			_usedDiskTracks[i] = new bool[84];
+			_diskDeltas[i] = new byte[84][];
+			for (var j = 0; j < 84; j++)
+			{
+				_diskDeltas[i][j] = Array.Empty<byte>();
+			}
 		}
 	}
 
-	public bool SaveRamModified => true;
+	public bool SaveRamModified { get; private set; } = false;
 
 	public byte[] CloneSaveRam()
 	{
@@ -35,13 +39,13 @@ public sealed partial class Drive1541 : ISaveRam
 		using var ms = new MemoryStream();
 		using var bw = new BinaryWriter(ms);
 		bw.Write(_usedDiskTracks.Length);
-		for (var i = 0; i < _usedDiskTracks.Length; i++)
+		for (var diskNumber = 0; diskNumber < _usedDiskTracks.Length; diskNumber++)
 		{
-			bw.WriteByteBuffer(_usedDiskTracks[i]
+			bw.WriteByteBuffer(_usedDiskTracks[diskNumber]
 				.ToUByteBuffer());
-			for (var j = 0; j < 84; j++)
+			for (var trackNumber = 0; trackNumber < 84; trackNumber++)
 			{
-				bw.WriteByteBuffer(_diskDeltas[i, j]);
+				bw.WriteByteBuffer(_diskDeltas[diskNumber][trackNumber]);
 			}
 		}
 
@@ -66,38 +70,54 @@ public sealed partial class Drive1541 : ISaveRam
 			_usedDiskTracks[i] = br.ReadByteBuffer(returnNull: false)!.ToBoolBuffer();
 			for (var j = 0; j < 84; j++)
 			{
-				_diskDeltas[i, j] = br.ReadByteBuffer(returnNull: true);
+				_diskDeltas[i][j] = br.ReadByteBuffer(returnNull: true);
 			}
 		}
 
-		_disk?.AttachTracker(_usedDiskTracks[_getCurrentDiskNumber()]);
 		LoadDeltas(); // load up new deltas
 		_usedDiskTracks[_getCurrentDiskNumber()][_trackNumber] = true; // make sure this gets set to true now
 	}
 
 	public void SaveDeltas()
 	{
-		_disk?.DeltaUpdate((tracknum, original, current) =>
+		_disk?.DeltaUpdate((tracknum, track) =>
 		{
-			_diskDeltas[_getCurrentDiskNumber(), tracknum] = DeltaSerializer.GetDelta<int>(original, current)
+			SaveDelta(tracknum, track.Delta);
-				.ToArray();
 		});
 	}
 
 	public void LoadDeltas()
 	{
-		_disk?.DeltaUpdate((tracknum, original, current) =>
+		_disk?.DeltaUpdate((tracknum, track) =>
 		{
-			DeltaSerializer.ApplyDelta<int>(original, current, _diskDeltas[_getCurrentDiskNumber(), tracknum]);
+			LoadDelta(tracknum, track.Delta);
 		});
 	}
 
 	private void ResetDeltas()
 	{
-		_disk?.DeltaUpdate(static (_, original, current) =>
+		_disk?.DeltaUpdate(static (_, track) =>
 		{
-			original.AsSpan()
+			track.Reset();
-				.CopyTo(current);
 		});
 	}
+
+	private void SaveDelta(int trackNumber, byte[] delta)
+	{
+		SaveRamModified = true;
+		_diskDeltas[_getCurrentDiskNumber()][trackNumber] = delta;
+	}
+
+	private void LoadDelta(int trackNumber, byte[] delta)
+	{
+		_diskDeltas[_getCurrentDiskNumber()][trackNumber] = delta;
+		_disk.GetTrack(trackNumber).ApplyDelta(delta);
+	}
+
+	private void ResetDelta(int trackNumber)
+	{
+		SaveRamModified = true;
+		_diskDeltas[_getCurrentDiskNumber()][trackNumber] = Array.Empty<byte>();
+		_disk.GetTrack(trackNumber).Reset();
+	}
 }

src/BizHawk.Emulation.Cores/Computers/Commodore64/Serial/Drive1541.cs

@@ -7,6 +7,7 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 {
 	public sealed partial class Drive1541 : SerialPortDevice
 	{
+		private byte[][][] _diskDeltas;
 		private Disk _disk;
 		private int _bitHistory;
 		private int _bitsRemainingInLatchedByte;
@@ -24,7 +25,6 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 		private int _cpuClockNum;
 		private int _ratioDifference;
 		private int _driveLightOffTime;
-		private int[] _trackImageData;
 		public Func<int> ReadIec = () => 0xFF;
 		public Action DebuggerStep;
 		public readonly Chip23128 DriveRom;
@@ -129,24 +129,19 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 				SaveDeltas();
 			}
 
-			for (var i = 0; i < _usedDiskTracks.Length; i++)
+			for (var diskNumber = 0; diskNumber < _usedDiskTracks.Length; diskNumber++)
 			{
-				ser.Sync($"_usedDiskTracks{i}", ref _usedDiskTracks[i], useNull: false);
+				ser.Sync($"_usedDiskTracks{diskNumber}", ref _usedDiskTracks[diskNumber], useNull: false);
-				for (var j = 0; j < 84; j++)
+				for (var trackNumber = 0; trackNumber < 84; trackNumber++)
 				{
-					ser.Sync($"DiskDeltas{i},{j}", ref _diskDeltas[i, j], useNull: true);
+					ser.Sync($"DiskDeltas{diskNumber},{trackNumber}", ref _diskDeltas[diskNumber][trackNumber], useNull: true);
 				}
 			}
 
-			_disk?.AttachTracker(_usedDiskTracks[_getCurrentDiskNumber()]);
-
 			if (ser.IsReader)
 			{
 				LoadDeltas();
 			}
-
-			// set _trackImageData back to the correct reference
-			_trackImageData = _disk?.GetDataForTrack(_trackNumber);
 		}
 
 		public override void ExecutePhase()
@@ -230,7 +225,6 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 		public void InsertMedia(Disk disk)
 		{
 			_disk = disk;
-			_disk?.AttachTracker(_usedDiskTracks[_getCurrentDiskNumber()]);
 			UpdateMediaData();
 		}
 
@@ -238,8 +232,8 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 		{
 			if (_disk != null)
 			{
-				_trackImageData = _disk.GetDataForTrack(_trackNumber);
+				var track = _disk.GetTrack(_trackNumber);
-				_diskBits = _trackImageData[_diskByteOffset] >> (Disk.FluxBitsPerEntry - _diskBitsLeft);
+				_diskBits = track.Bits[_diskByteOffset] >> (Disk.FluxBitsPerEntry - _diskBitsLeft);
 				_diskWriteProtected = _disk.WriteProtected;
 			}
 			else
@@ -251,7 +245,6 @@ namespace BizHawk.Emulation.Cores.Computers.Commodore64.Serial
 		public void RemoveMedia()
 		{
 			_disk = null;
-			_trackImageData = null;
 			_diskBits = 0;
 		}