CommunityToolkit / WindowsCommunityToolkit

The Windows Community Toolkit is a collection of helpers, extensions, and custom controls. It simplifies and demonstrates common developer tasks building .NET apps with UWP and the Windows App SDK / WinUI 3 for Windows 10 and Windows 11. The toolkit is part of the .NET Foundation.
https://docs.microsoft.com/windows/communitytoolkit/
Other
5.89k stars 1.37k forks source link

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

Open pkar70 opened 4 years ago

pkar70 commented 4 years ago

Describe the bug

A clear and concise description of what the bug is.

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.

ghost commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.

michael-hawker commented 3 years ago

@Kyaa-dost can we get a summary breakdown report e-mail type thing from GitHub or the bot of things like new open issues or issues that need attention, etc...?

Kyaa-dost commented 3 years ago

@michael-hawker I have explored and not found anything from the GitHub side that supports emailing the labels or issues etc. Perhaps there can be third party apps or actions but not certain how reliable or credible they are if this is what we are planning to utilize continuously. This leaves us to our last option Bot that currently does not support this feature but I can draft an email and forward it to the team and see if they can implement this in the near future release/s 😊