Skip to content

Conversation

@brianpopow
Copy link
Collaborator

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Add support for encoding 1 bit per pixel bitmaps.

@JimBobSquarePants: The bitmap encoder uses Octree Quantizer as a default. This quantizer does seem to have problems with just 2 colors. Im not so familiar with the Octree Quantizer. Is not possible to quantize with just to colors or is it maybe an issue we should look into? All indices seem to be 0.
If i change it to Wu quantizer it works with two colors.

@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #1623 (33192d3) into master (6fa903c) will increase coverage by 0.00%.
The diff coverage is 88.57%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1623   +/-   ##
=======================================
  Coverage   83.67%   83.67%           
=======================================
  Files         749      749           
  Lines       33082    33111   +29     
  Branches     3707     3714    +7     
=======================================
+ Hits        27682    27707   +25     
- Misses       4681     4682    +1     
- Partials      719      722    +3     
Flag Coverage Δ
unittests 83.67% <88.57%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ImageSharp/Formats/Bmp/BmpEncoder.cs 100.00% <ø> (ø)
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs 93.83% <88.23%> (-1.08%) ⬇️
src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs 94.21% <100.00%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6fa903c...33192d3. Read the comment docs.

@JimBobSquarePants
Copy link
Member

This quantizer does seem to have problems with just 2 colors. Im not so familiar with the Octree Quantizer. Is not possible to quantize with just to colors or is it maybe an issue we should look into? All indices seem to be 0.

@brianpopow Yep there's issues with how the Octree Quantizer currently distributes and reduces colors. (In addition to rampant memory usage issues)

It does look like we should be able to do a fairly quick port based on the code here to a version that should allow accurate reduction.
#1350 (comment)

It's something I keep meaning to take the time to attempt.

@brianpopow
Copy link
Collaborator Author

@brianpopow Yep there's issues with how the Octree Quantizer currently distributes and reduces colors. (In addition to rampant memory usage issues)

@JimBobSquarePants: Should I switch the default quantizer then to the Wu-Quantizer?

@JimBobSquarePants
Copy link
Member

@JimBobSquarePants: Should I switch the default quantizer then to the Wu-Quantizer?

For bitmap so we can get this PR in then yeah. I'll likely move back after depending on the results of profiling both once things have been fixed.

Copy link
Member

@JimBobSquarePants JimBobSquarePants left a comment

Choose a reason for hiding this comment

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

Awesome work. I can't believe we have all the bit depth support now! 😄

@JimBobSquarePants JimBobSquarePants merged commit a8cb711 into master May 5, 2021
@JimBobSquarePants JimBobSquarePants deleted the bp/bmp1bit branch May 5, 2021 23:37
@rickbrew
Copy link

rickbrew commented May 6, 2021

It does look like we should be able to do a fairly quick port based on the code here to a version that should allow accurate reduction.
#1350 (comment)

I can confirm that my new Octree code correctly quantizes down to 1-bit depth. Paint.NET uses this in production today with the latest 4.2.16 release.

@JimBobSquarePants
Copy link
Member

@rickbrew Awesome, that's great to hear, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants