From 4fca85f63d9f661b102f777f562089f368b80436 Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 24 Feb 2024 18:44:13 +0000 Subject: [PATCH] Improve sheet packing in Dune 2000. In a3d0a50f4d02ef94e79be9d528ea4fff3442045a, SpriteCache is updated to sort sprites by height before adding them onto the sheet. This improves packing by reducing wasted space as the sprites are packed onto the sheet. D2kSpriteSequence does not fully benefit from this change, as it creates additional sprites afterwards in the ResolveSprites method. These are not sorted, so they often waste space due to height changes between adjacent sprites and cause an inefficient packing. Sorting them in place is insufficient, as each sequence performs the operation independently. So sets of sprites across different sequences end up with poor packing overall. We need all the sprites to be collected together and sorted in one place for best effect. We restructure SpriteCache to allow a frame mutation function to be provided when reserving sprites. This removes the need for the ReserveFrames and ResolveFrames methods in SpriteCache. D2kSpriteSequence can use this new function to pass in the required modification, and no longer has to add frames to the sheet builder itself. Now the SpriteCache can apply the desired frame mutations, it can batch together these mutated frames with the other frames and sort them all as a single batch. With all frames sorted together the maximum benefit of this packing approach is realised. This reduces the number of BGRA sheets required for the d2k mod from 3 to 2. --- OpenRA.Game/Graphics/SpriteCache.cs | 74 ++++++------------- OpenRA.Mods.D2k/Graphics/D2kSpriteSequence.cs | 34 +++------ 2 files changed, 34 insertions(+), 74 deletions(-) diff --git a/OpenRA.Game/Graphics/SpriteCache.cs b/OpenRA.Game/Graphics/SpriteCache.cs index adefd9088f..97ed93b66f 100644 --- a/OpenRA.Game/Graphics/SpriteCache.cs +++ b/OpenRA.Game/Graphics/SpriteCache.cs @@ -24,11 +24,9 @@ namespace OpenRA.Graphics readonly ISpriteLoader[] loaders; readonly IReadOnlyFileSystem fileSystem; - readonly Dictionary spriteReservations = new(); - readonly Dictionary frameReservations = new(); + readonly Dictionary AdjustFrame, bool Premultiplied)> spriteReservations = new(); readonly Dictionary> reservationsByFilename = new(); - readonly Dictionary resolvedFrames = new(); readonly Dictionary resolvedSprites = new(); readonly Dictionary missingFiles = new(); @@ -47,18 +45,10 @@ namespace OpenRA.Graphics this.loaders = loaders; } - public int ReserveSprites(string filename, IEnumerable frames, MiniYamlNode.SourceLocation location, bool premultiplied = false) + public int ReserveSprites(string filename, IEnumerable frames, MiniYamlNode.SourceLocation location, Func adjustFrame = null, bool premultiplied = false) { var token = nextReservationToken++; - spriteReservations[token] = (frames?.ToArray(), location, premultiplied); - reservationsByFilename.GetOrAdd(filename, _ => new List()).Add(token); - return token; - } - - public int ReserveFrames(string filename, IEnumerable frames, MiniYamlNode.SourceLocation location) - { - var token = nextReservationToken++; - frameReservations[token] = (frames?.ToArray(), location); + spriteReservations[token] = (frames?.ToArray(), location, adjustFrame, premultiplied); reservationsByFilename.GetOrAdd(filename, _ => new List()).Add(token); return token; } @@ -84,34 +74,19 @@ namespace OpenRA.Graphics foreach (var sb in SheetBuilders.Values) sb.Current.CreateBuffer(); - var pendingResolve = new List<(string Filename, int FrameIndex, bool Premultiplied, ISpriteFrame Frame, Sprite[] SpritesForToken)>(); + var pendingResolve = new List<( + string Filename, + int FrameIndex, + bool Premultiplied, + Func AdjustFrame, + ISpriteFrame Frame, + Sprite[] SpritesForToken)>(); foreach (var (filename, tokens) in reservationsByFilename) { modData.LoadScreen?.Display(); var loadedFrames = GetFrames(fileSystem, filename, loaders, out _); foreach (var token in tokens) { - if (frameReservations.TryGetValue(token, out var rf)) - { - if (loadedFrames != null) - { - if (rf.Frames != null) - { - var resolved = new ISpriteFrame[loadedFrames.Length]; - foreach (var i in rf.Frames) - resolved[i] = loadedFrames[i]; - resolvedFrames[token] = resolved; - } - else - resolvedFrames[token] = loadedFrames; - } - else - { - resolvedFrames[token] = null; - missingFiles[token] = (filename, rf.Location); - } - } - if (spriteReservations.TryGetValue(token, out var rs)) { if (loadedFrames != null) @@ -121,7 +96,12 @@ namespace OpenRA.Graphics var frames = rs.Frames ?? Enumerable.Range(0, loadedFrames.Length); foreach (var i in frames) - pendingResolve.Add((filename, i, rs.Premultiplied, loadedFrames[i], resolved)); + { + var frame = loadedFrames[i]; + if (rs.AdjustFrame != null) + frame = rs.AdjustFrame(frame); + pendingResolve.Add((filename, i, rs.Premultiplied, rs.AdjustFrame, frame, resolved)); + } } else { @@ -136,13 +116,18 @@ namespace OpenRA.Graphics // 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) + var spriteCache = new Dictionary<( + string Filename, + int FrameIndex, + bool Premultiplied, + Func AdjustFrame), + Sprite>(pendingResolve.Count); + foreach (var (filename, frameIndex, premultiplied, adjustFrame, 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), + (filename, frameIndex, premultiplied, adjustFrame), _ => { var sheetBuilder = SheetBuilders[SheetBuilder.FrameTypeToSheetType(frame.Type)]; @@ -153,7 +138,6 @@ namespace OpenRA.Graphics } spriteReservations.Clear(); - frameReservations.Clear(); reservationsByFilename.Clear(); foreach (var sb in SheetBuilders.Values) @@ -170,16 +154,6 @@ namespace OpenRA.Graphics return resolved; } - public ISpriteFrame[] ResolveFrames(int token) - { - var resolved = resolvedFrames[token]; - resolvedFrames.Remove(token); - if (missingFiles.TryGetValue(token, out var r)) - throw new FileNotFoundException($"{r.Location}: {r.Filename} not found", r.Filename); - - return resolved; - } - public IEnumerable<(string Filename, MiniYamlNode.SourceLocation Location)> MissingFiles => missingFiles.Values.ToHashSet(); public void Dispose() diff --git a/OpenRA.Mods.D2k/Graphics/D2kSpriteSequence.cs b/OpenRA.Mods.D2k/Graphics/D2kSpriteSequence.cs index 3a881836b7..caf4b5517e 100644 --- a/OpenRA.Mods.D2k/Graphics/D2kSpriteSequence.cs +++ b/OpenRA.Mods.D2k/Graphics/D2kSpriteSequence.cs @@ -62,6 +62,13 @@ namespace OpenRA.Mods.Cnc.Graphics var offset = LoadField(Offset, data, defaults); var blendMode = LoadField(BlendMode, data, defaults); + Func adjustFrame = null; + if (remapColor != default || convertShroudToFog) + adjustFrame = RemapFrame; + + ISpriteFrame RemapFrame(ISpriteFrame f) => + (f is R8Loader.RemappableFrame rf) ? rf.WithSequenceFlags(useShadow, convertShroudToFog, remapColor) : f; + var combineNode = data.NodeWithKeyOrDefault(Combine.Key); if (combineNode != null) { @@ -75,11 +82,7 @@ namespace OpenRA.Mods.Cnc.Graphics foreach (var f in ParseCombineFilenames(modData, tileset, subFrames, subData)) { - int token; - if (remapColor != default || convertShroudToFog) - token = cache.ReserveFrames(f.Filename, f.LoadFrames, f.Location); - else - token = cache.ReserveSprites(f.Filename, f.LoadFrames, f.Location); + var token = cache.ReserveSprites(f.Filename, f.LoadFrames, f.Location, adjustFrame); spritesToLoad.Add(new SpriteReservation { @@ -98,11 +101,7 @@ namespace OpenRA.Mods.Cnc.Graphics { foreach (var f in ParseFilenames(modData, tileset, frames, data, defaults)) { - int token; - if (remapColor != default || convertShroudToFog) - token = cache.ReserveFrames(f.Filename, f.LoadFrames, f.Location); - else - token = cache.ReserveSprites(f.Filename, f.LoadFrames, f.Location); + var token = cache.ReserveSprites(f.Filename, f.LoadFrames, f.Location, adjustFrame); spritesToLoad.Add(new SpriteReservation { @@ -129,20 +128,7 @@ namespace OpenRA.Mods.Cnc.Graphics var allSprites = spritesToLoad.SelectMany(r => { - Sprite[] resolved; - if (remapColor != default || convertShroudToFog) - resolved = cache.ResolveFrames(r.Token) - .Select(f => (f is R8Loader.RemappableFrame rf) ? rf.WithSequenceFlags(useShadow, convertShroudToFog, remapColor) : f) - .Select(f => - { - if (f == null) - return null; - - return cache.SheetBuilders[SheetBuilder.FrameTypeToSheetType(f.Type)] - .Add(f.Data, f.Type, f.Size, 0, f.Offset); - }).ToArray(); - else - resolved = cache.ResolveSprites(r.Token); + var resolved = cache.ResolveSprites(r.Token); if (r.Frames != null) resolved = r.Frames.Select(f => resolved[f]).ToArray();