From 8101c70c91e63e4232afcafc94151bf2220daeed Mon Sep 17 00:00:00 2001
From: RoosterDragon <RoosterDragon@outlook.com>
Date: Sun, 18 Aug 2024 14:38:11 +0100
Subject: [PATCH] Allow sheet buffers to be reused in SheetBuilder.

Sheets can be buffered - where a copy of their data is kept in RAM. This allows for modifications to the sheet to be batched in the RAM buffer, before later being committed to VRAM in a single operation. The SheetBuilder class allows for sprites to be allocated onto one or more sheets. Each time a sheet is filled, it will allocate a new sheet. Sheets allocated by the sheet builder will typically use the buffering mechanism.

Previously each time the builder allocated a new sheet, the buffer would get thrown away, and the next sheet would allocate a fresh buffer. These buffers can be large and may accumulate before the GC cleans them up. So although only one buffer will be live at a time, they can cause a spike in memory used by the process during loading.

We can avoid this issue by allowing the buffer from the previous sheet to be reused by the next sheet. This is possible because the sheet builder only has one live sheet for modifications at a time, and they are all the same type and size. By allocating only one buffer per builder instead of one per sheet, we reduce the peak memory required during loading.
---
 OpenRA.Game/Graphics/Sheet.cs        | 20 ++++++++++++++++++++
 OpenRA.Game/Graphics/SheetBuilder.cs |  7 ++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/OpenRA.Game/Graphics/Sheet.cs b/OpenRA.Game/Graphics/Sheet.cs
index 2df6a0dd2b..2de2d1ce81 100644
--- a/OpenRA.Game/Graphics/Sheet.cs
+++ b/OpenRA.Game/Graphics/Sheet.cs
@@ -132,6 +132,7 @@ namespace OpenRA.Graphics
 		{
 			if (!Buffered)
 				return;
+
 			dirty = true;
 			releaseBufferOnCommit = true;
 
@@ -140,6 +141,25 @@ namespace OpenRA.Graphics
 				GetTexture();
 		}
 
+		public bool ReleaseBufferAndTryTransferTo(Sheet destination)
+		{
+			if (Size != destination.Size)
+				throw new ArgumentException("Destination sheet does not have the same size", nameof(destination));
+
+			var buffer = data;
+			ReleaseBuffer();
+
+			// Only transfer if the destination has no data that would be lost by overwriting.
+			if (buffer != null && destination.data == null && destination.texture == null)
+			{
+				Array.Clear(buffer, 0, buffer.Length);
+				destination.data = buffer;
+				return true;
+			}
+
+			return false;
+		}
+
 		public void Dispose()
 		{
 			texture?.Dispose();
diff --git a/OpenRA.Game/Graphics/SheetBuilder.cs b/OpenRA.Game/Graphics/SheetBuilder.cs
index 78dfb5f8e4..1237972726 100644
--- a/OpenRA.Game/Graphics/SheetBuilder.cs
+++ b/OpenRA.Game/Graphics/SheetBuilder.cs
@@ -130,8 +130,13 @@ namespace OpenRA.Graphics
 				var next = NextChannel(CurrentChannel);
 				if (next == null)
 				{
-					Current.ReleaseBuffer();
+					var previous = Current;
 					Current = allocateSheet();
+
+					// Reuse the backing buffer between sheets where possible.
+					// This avoids allocating additional buffers which the GC must clean up.
+					previous.ReleaseBufferAndTryTransferTo(Current);
+
 					sheets.Add(Current);
 					CurrentChannel = Type == SheetType.Indexed ? TextureChannel.Red : TextureChannel.RGBA;
 				}