Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allocate empty sheets #21370

Open
wants to merge 1 commit into
base: bleed
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions OpenRA.Game/Graphics/SheetBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ public SheetBuilder(SheetType t, Func<Sheet> allocateSheet, int margin = 1)
{
CurrentChannel = t == SheetType.Indexed ? TextureChannel.Red : TextureChannel.RGBA;
Type = t;
Current = allocateSheet();
sheets.Add(Current);
this.allocateSheet = allocateSheet;
this.margin = margin;
}
Expand All @@ -86,6 +84,13 @@ public SheetBuilder(SheetType t, Func<Sheet> allocateSheet, int margin = 1)
public Sprite Add(byte[] src, SpriteFrameType type, Size size, bool premultiplied = false) { return Add(src, type, size, 0, float3.Zero, premultiplied); }
public Sprite Add(byte[] src, SpriteFrameType type, Size size, float zRamp, in float3 spriteOffset, bool premultiplied = false)
{
if (Current == null)
{
Current = allocateSheet();
Current.CreateBuffer();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can get away without the CreateBuffer call here and below, they'll be created when Util.FastCopyIntoChannel accesses the sheet data anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we can. I feel like we need a bit more unified handling of empty sprites

sheets.Add(Current);
}

// Don't bother allocating empty sprites
if (size.Width == 0 || size.Height == 0)
return new Sprite(Current, Rectangle.Empty, 0, spriteOffset, CurrentChannel, BlendMode.Alpha);
Expand Down Expand Up @@ -116,6 +121,13 @@ public Sprite Add(Png src, float scale = 1f)
public Sprite Allocate(Size imageSize, float scale = 1f) { return Allocate(imageSize, 0, float3.Zero, scale); }
public Sprite Allocate(Size imageSize, float zRamp, in float3 spriteOffset, float scale = 1f)
{
if (Current == null)
{
Current = allocateSheet();
Current.CreateBuffer();
sheets.Add(Current);
}

if (imageSize.Width + p.X + margin > Current.Size.Width)
{
p = new int2(0, p.Y + rowHeight + margin);
Expand Down
5 changes: 1 addition & 4 deletions OpenRA.Game/Graphics/SpriteCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ static ISpriteFrame[] GetFrames(IReadOnlyFileSystem fileSystem, string filename,

public void LoadReservations(ModData modData)
{
foreach (var sb in SheetBuilders.Values)
sb.Current.CreateBuffer();

var pendingResolve = new List<(
string Filename,
int FrameIndex,
Expand Down Expand Up @@ -141,7 +138,7 @@ public void LoadReservations(ModData modData)
reservationsByFilename.Clear();

foreach (var sb in SheetBuilders.Values)
sb.Current.ReleaseBuffer();
sb.Current?.ReleaseBuffer();
}

public Sprite[] ResolveSprites(int token)
Expand Down
4 changes: 3 additions & 1 deletion OpenRA.Game/Map/MapCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ void LoadAsyncInternal()
}

// Release the buffer by forcing changes to be written out to the texture, allowing the buffer to be reclaimed by GC.
Game.RunAfterTick(sheetBuilder.Current.ReleaseBuffer);
if (sheetBuilder.Current != null)
Game.RunAfterTick(sheetBuilder.Current.ReleaseBuffer);

Log.Write("debug", "MapCache.LoadAsyncInternal ended");
}

Expand Down
4 changes: 2 additions & 2 deletions OpenRA.Mods.Cnc/Graphics/VoxelLoader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public ModelRenderData GenerateRenderData(VxlLimb l)
{
// Sheet overflow - allocate a new sheet and try once more
Log.Write("debug", "Voxel sheet overflow! Generating new sheet");
sheetBuilder.Current.ReleaseBuffer();
sheetBuilder.Current?.ReleaseBuffer();
sheetBuilder = CreateSheetBuilder();
v = GenerateSlicePlanes(l).SelectMany(x => x).ToArray();
}
Expand Down Expand Up @@ -229,7 +229,7 @@ public Voxel Load(string vxl, string hva)

public void Finish()
{
sheetBuilder.Current.ReleaseBuffer();
sheetBuilder.Current?.ReleaseBuffer();
}

public void Dispose()
Expand Down
2 changes: 1 addition & 1 deletion OpenRA.Mods.Common/Terrain/DefaultTileCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public DefaultTileCache(DefaultTerrain terrainInfo, Action<uint, string> onMissi

MissingTile = sheetBuilders[missingSheetType].Add(new byte[missingDataLength], missingFrameType, new Size(1, 1));
foreach (var sb in sheetBuilders.Values)
sb.Current.ReleaseBuffer();
sb.Current?.ReleaseBuffer();
}

public bool HasTileSprite(TerrainTile r, int? variant = null)
Expand Down
13 changes: 8 additions & 5 deletions OpenRA.Mods.Common/UtilityCommands/DumpSequenceSheetsCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,16 @@ void IUtilityCommand.Run(Utility utility, string[] args)
var count = 0;

var sb = sequences.SpriteCache.SheetBuilders[SheetType.Indexed];
foreach (var s in sb.AllSheets)
if (sb.Current != null)
{
var max = s == sb.Current ? (int)sb.CurrentChannel + 1 : 4;
for (var i = 0; i < max; i++)
s.AsPng((TextureChannel)ChannelMasks[i], palette).Save($"{count}.{i}.png");
foreach (var s in sb.AllSheets)
{
var max = s == sb.Current ? (int)sb.CurrentChannel + 1 : 4;
for (var i = 0; i < max; i++)
s.AsPng((TextureChannel)ChannelMasks[i], palette).Save($"{count}.{i}.png");

count++;
count++;
}
}

sb = sequences.SpriteCache.SheetBuilders[SheetType.BGRA];
Expand Down