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

Replace System.Drawing.Bitmap with Png for image saving #16218

Merged
merged 8 commits into from Feb 24, 2019

Conversation

@pchote
Copy link
Member

commented Feb 18, 2019

This PR continues the work from #15973 by extending our Png class to support image saving (using Bitmap internally for now, to be replaced in a followup PR), and porting all consumers to the new API.

This also changes the way screenshot saving works so that more work can be done from the worker thread. This should reduce the size of the perf hitch when saving screenshots.

Dependency for #15973.

@reaperrr
Copy link
Contributor

left a comment

The code is out of my league, but screenshots and map preview saving work fine and no other regressions spotted either, so 👍

OpenRA.Game/Graphics/Sheet.cs Outdated Show resolved Hide resolved
OpenRA.Platforms.Default/ThreadedGraphicsContext.cs Outdated Show resolved Hide resolved

// Add embedded metadata to the end of the image
bitmapStream.Position = 0;
var outputStream = new MemoryStream();

This comment has been minimized.

Copy link
@teinarss

teinarss Feb 22, 2019

Contributor

Missing using()

This comment has been minimized.

Copy link
@pchote

pchote Feb 22, 2019

Author Member

The BinaryWriter disposes the stream at the end of its using - this one of the reasons I don't like BinaryWriter, but I didn't see a strong reason to rewrite @IceReaper's initial use of it as this whole method needs to be replaced in the short term to remove System.Bitmap.

@pchote pchote force-pushed the pchote:png-write branch 2 times, most recently from 0d6f541 to 164b3ec Feb 22, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Feb 22, 2019

Rebased and updated.

@pchote pchote force-pushed the pchote:png-write branch from 164b3ec to 3dcb8dd Feb 24, 2019

@pchote

This comment has been minimized.

Copy link
Member Author

commented Feb 24, 2019

Fixed and rebased again.

@reaperrr reaperrr merged commit 94f7f6f into OpenRA:bleed Feb 24, 2019

2 checks passed

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

@pchote pchote deleted the pchote:png-write branch Feb 24, 2019

@IceReaper

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2019

Ah already merged :) Anyway would have gotten an👍 from me

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