From a3d0a50f4d02ef94e79be9d528ea4fff3442045a Mon Sep 17 00:00:00 2001
From: RoosterDragon <RoosterDragon@outlook.com>
Date: Sat, 9 Mar 2024 18:21:15 +0000
Subject: [PATCH] Improve sheet packing.

When sheet builders are adding sprites to a sheet, they work left to right along each row. They reserve height for the highest sprite seen along that row, resetting the height reservation when the row runs out of space and it moves down to the next row.

As the SpriteCache adds the sprites in a giant batch, it can optimise this operation by ordering the sprites by their height. This reduces wastage where shorter sprites don't use the the full height reserved within the row. The reduced wastage can help the sheet builder allocate fewer sheets, improving load times and improving GPU memory usage as less texture memory is required.
---
 OpenRA.Game/Graphics/SpriteCache.cs | 33 +++++++++++++++++++----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/OpenRA.Game/Graphics/SpriteCache.cs b/OpenRA.Game/Graphics/SpriteCache.cs
index e43d375446..adefd9088f 100644
--- a/OpenRA.Game/Graphics/SpriteCache.cs
+++ b/OpenRA.Game/Graphics/SpriteCache.cs
@@ -84,7 +84,7 @@ namespace OpenRA.Graphics
 			foreach (var sb in SheetBuilders.Values)
 				sb.Current.CreateBuffer();
 
-			var spriteCache = new Dictionary<int, Sprite>();
+			var pendingResolve = new List<(string Filename, int FrameIndex, bool Premultiplied, ISpriteFrame Frame, Sprite[] SpritesForToken)>();
 			foreach (var (filename, tokens) in reservationsByFilename)
 			{
 				modData.LoadScreen?.Display();
@@ -117,18 +117,11 @@ namespace OpenRA.Graphics
 						if (loadedFrames != null)
 						{
 							var resolved = new Sprite[loadedFrames.Length];
+							resolvedSprites[token] = resolved;
 							var frames = rs.Frames ?? Enumerable.Range(0, loadedFrames.Length);
 
-							// Premultiplied and non-premultiplied sprites must be cached separately
-							// to cover the case where the same image is requested in both versions.
-							// The premultiplied sprites are stored with an index offset for efficiency
-							// rather than allocating a second dictionary.
-							var di = rs.Premultiplied ? loadedFrames.Length : 0;
 							foreach (var i in frames)
-								resolved[i] = spriteCache.GetOrAdd(i + di,
-									f => SheetBuilders[SheetBuilder.FrameTypeToSheetType(loadedFrames[f - di].Type)].Add(loadedFrames[f - di], rs.Premultiplied));
-
-							resolvedSprites[token] = resolved;
+								pendingResolve.Add((filename, i, rs.Premultiplied, loadedFrames[i], resolved));
 						}
 						else
 						{
@@ -137,8 +130,26 @@ namespace OpenRA.Graphics
 						}
 					}
 				}
+			}
 
-				spriteCache.Clear();
+			// When the sheet builder is adding sprites, it reserves height for the tallest sprite seen along the row.
+			// We can achieve better sheet packing by keeping sprites with similar heights together.
+			var orderedPendingResolve = pendingResolve.OrderBy(x => x.Frame.Size.Height);
+
+			var spriteCache = new Dictionary<(string Filename, int FrameIndex, bool Premultiplied), Sprite>(pendingResolve.Count);
+			foreach (var (filename, frameIndex, premultiplied, frame, spritesForToken) in orderedPendingResolve)
+			{
+				// Premultiplied and non-premultiplied sprites must be cached separately
+				// to cover the case where the same image is requested in both versions.
+				spritesForToken[frameIndex] = spriteCache.GetOrAdd(
+					(filename, frameIndex, premultiplied),
+					_ =>
+					{
+						var sheetBuilder = SheetBuilders[SheetBuilder.FrameTypeToSheetType(frame.Type)];
+						return sheetBuilder.Add(frame, premultiplied);
+					});
+
+				modData.LoadScreen?.Display();
 			}
 
 			spriteReservations.Clear();