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 load unused sprite frames into video ram #15098

Merged
merged 3 commits into from May 2, 2018

Conversation

Projects
None yet
3 participants
@pchote
Copy link
Member

pchote commented Apr 30, 2018

This PR reworks the SpriteCache and DefaultSpriteSequence loaders to defer loading sprite frames into the SheetBuilder until they are actually used. Frames that are not used do not get loaded at all. This is particularly useful in TD where the infantry artwork contains a lot of unused frames. It also helps in D2K, which otherwise loads all of DATA.R8 and BLOXBASE.R8 into the sheet.

Unused frames remain stored in memory: the sprite loading code does not have a concept of loading being completed, so we need to hold on to them "just in case". This overhead shouldn't grow to more than a few MB, so IMO is an acceptable tradeoff until @RoosterDragon or somebody else can rework the Sequence plumbing to fix this.

Supersedes #15097.

Testcase: use #15094 to inspect sheet usage with and without this PR.

allSprites.Add(sprite);
}

// HACK: The sequency code relies on side-effects from getUsedFrames

This comment has been minimized.

@pchote

pchote Apr 30, 2018

Author Member

Supporting Length: * in the sequence yaml puts us in the ugly recursive situation where the yaml metadata depends on the sprite data which depends on the yaml data. This is not the first time that this has bit us. Our sequence parsing code would simplify a lot if we could drop that and require all lengths to be explicitly defined.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Apr 30, 2018

Got an exception after a few seconds on the RA shellmap:

OpenRA engine version {DEV_VERSION}
Red Alert mod version {DEV_VERSION}
on map 28ae41d4def5caaaaf79f08870dfb08f56d8d7eb (Desert Shellmap by Scott_NZ).
Date: 2018-04-30 23:02:15Z
Operating System: Windows (Microsoft Windows NT 6.2.9200.0)
Runtime Version: .NET CLR 4.0.30319.42000
Exception of type `System.NullReferenceException`: Der Objektverweis wurde nicht auf eine Objektinstanz festgelegt.
   bei OpenRA.Graphics.SpriteRenderable.ScreenPosition(WorldRenderer wr) in d:\dev\OpenRA\OpenRA.Game\Graphics\SpriteRenderable.cs:Zeile 54.
   bei OpenRA.Graphics.SpriteRenderable.Render(WorldRenderer wr) in d:\dev\OpenRA\OpenRA.Game\Graphics\SpriteRenderable.cs:Zeile 63.
   bei OpenRA.Graphics.WorldRenderer.Draw() in d:\dev\OpenRA\OpenRA.Game\Graphics\WorldRenderer.cs:Zeile 187.
   bei OpenRA.Game.RenderTick() in d:\dev\OpenRA\OpenRA.Game\Game.cs:Zeile 650.
   bei OpenRA.Game.Loop() in d:\dev\OpenRA\OpenRA.Game\Game.cs:Zeile 781.
   bei OpenRA.Game.Run() in d:\dev\OpenRA\OpenRA.Game\Game.cs:Zeile 799.
   bei OpenRA.Game.InitializeAndRun(String[] args) in d:\dev\OpenRA\OpenRA.Game\Game.cs:Zeile 253.
   bei OpenRA.Program.Main(String[] args) in d:\dev\OpenRA\OpenRA.Game\Support\Program.cs:Zeile 37.

pchote added some commits Apr 30, 2018

Add support on-demand sprite frame loading.
Passing the getUsedFrames argument will cause frames to be buffered
locally (and not added to the SheetBuilder) until they are explicitly
requested. This allows unused frames in sprite files to be skipped
instead of wasting space in GPU memory.

@pchote pchote force-pushed the pchote:on-demand-spriteloader branch from 76b74b6 to ad5428a May 1, 2018

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented May 1, 2018

Fixed. Also fixed the SpriteBounds calculation, which is what I had copied the wrong calculation from.

@reaperrr
Copy link
Contributor

reaperrr left a comment

Utility output looks good, no crashes or other issues encountered on RA shellmap

@abcdefg30 abcdefg30 merged commit bf4b917 into OpenRA:bleed May 2, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented May 2, 2018

@pchote pchote deleted the pchote:on-demand-spriteloader branch Jul 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.