ImageCropper, SaveAs with BitmapFileFormat.Png seems like not PNG file but bitmap

[pkar70]

Describe the bug

A clear and concise description of what the bug is.

• Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work:

I have several PNG files (Android snapshots, 600x1024 pixels, with size about 400 kB. After cropping to about 300x350 pixels, and using imageCropper.SaveAs(.., BitmapFileFormat.Png) file size is almost same, and should be much smaller.
When I open such cropped file in Microsoft Imaging Suite Editor, and save as new PNG file, file size is four..five times smaller.

When I use ImageMagick to simply open/save (no edits):
magick convert Screenshot_20200413-223246.png magScreenshot_20200413-223246.png
file size is reduced from 452 KiB to 118 KiB.

Seems like imageCropper.SaveAs saves pixels in PNG but as bitmap, without any compression (should use deflate, as stated in PNG format description).

Steps to Reproduce

Steps to reproduce the behavior:

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Expected behavior

A clear and concise description of what you expected to happen.

Screenshots

If applicable, add screenshots to help explain your problem.

Environment

NuGet Package(s): 
6.0.0
Package Version(s): 

Windows 10 Build Number:
- [ ] Fall Creators Update (16299)
- [ ] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [X] second 2019 Update (18363)
- [ ] Insider Build (build number: )

App min and target version:
- [ ] Fall Creators Update (16299)
- [X] April 2018 Update (17134)
- [ ] October 2018 Update (17763)
- [ ] May 2019 Update (18362)
- [ ] Insider Build (xxxxx)

Device form factor:
- [X] Desktop
- [ ] Xbox
- [ ] Surface Hub
- [ ] IoT

Visual Studio 
- [ ] 2017 (version: )
- [X] 2019 (version: 16.4.2 ) 
- [ ] 2019 Preview (version: )

Additional context

Add any other context about the problem here.

Hello pkar70, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[michael-hawker]

I'm assuming it has something to do with the BitmapEncoder settings, but I'm not too familiar with these APIs and how they work. @HHChaos do you have any insights to this?

[JohnnyWestlake]

So, this is encoded with the standard WIC PNG encoder using the Automatic PNG Filter mode which does compress, it just happens that the WIC codec isn't that great at compressing. You can set the PNGFilterMode to Adaptive, but it usually doesn't make any big difference, and PNGFilterMode is pretty much the only control you get for PNG files using BitmapEncoder (all options are here)

ImageMagik uses a much better non-MIcrosoft encoder. As far as I know there's not much more you can do with standard UWP API's.

[pkar70]

Seems like it is not compression method problem at all...
Full picture (600x1024), 428 KB (439 180 bytes)
Cropped picture (295x344), 428 KB (439 180 bytes)
Same is true for all cropped pictures.

Binary comparison shows 41910 changed bytes (last changed offset A47B = (dec) 42107.
So, it writes good (compressed) data, but doesn't truncate file before write.

So, before Await flyoutImageCropper.SaveAsync(oStream, ..
I simpy insert
oStream.Size = 0
and file is smaller :)

Maybe such 'stream resetting' should be done in WCT (maybe not to zero, but to current stream pointer (seek)). Or, maybe better, add appropriate note to method documentation?

[JohnnyWestlake]

I would vote that as it is taking a stream in and not a storage file, the method itself shouldn't resize the stream - but it should definitely be documented, perhaps even on the method signature.

Another take would be to take an optional parameter that defaults to setting the stream size to zero before writing (as I would imagine that's the vast majority of cases), and optionally lets it just write from where it is - but that would be a breaking change.

The sample app should be updated too if it's decided to do nothing, as currently it has a high chance of leaving garbage data if overwritting.