Not using quantization for Grey8 images when encoding 8Bit Bitmaps #909

brianpopow · 2019-05-15T19:18:14Z

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 matches 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

The title already says it: When its an Gray8 image, color quantization is actually not needed.

codecov · 2019-05-16T18:52:20Z

Codecov Report

Merging #909 into master will increase coverage by <.01%.
The diff coverage is 93.22%.

@@            Coverage Diff             @@
##           master     #909      +/-   ##
==========================================
+ Coverage   89.24%   89.25%   +<.01%     
==========================================
  Files        1083     1083              
  Lines       47394    47442      +48     
  Branches     3293     3299       +6     
==========================================
+ Hits        42299    42345      +46     
  Misses       4393     4393              
- Partials      702      704       +2

Impacted Files	Coverage Δ
tests/ImageSharp.Tests/Drawing/DrawImageTests.cs	`98.97% <100%> (ø)`	⬆️
src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs	`95.33% <100%> (+0.8%)`	⬆️
...ts/ImageSharp.Tests/Formats/Bmp/BmpEncoderTests.cs	`94.94% <86.66%> (-3.69%)`	⬇️
...geSharp/PixelFormats/PixelImplementations/Gray8.cs	`79.59% <0%> (-2.05%)`	⬇️
.../Processing/Processors/Quantization/WuQuantizer.cs	`100% <0%> (+15%)`	⬆️

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 ef736e0...bb8911d. Read the comment docs.

brianpopow · 2019-05-18T09:07:19Z

I would like to add two additional test cases to test the quantization of a 32 bit color image to a 8 bit color image see:#10. This may require again comparing with a tolerance, because quantization could produce different results on .net462 vs netcoreapp2.1, but i think thats still better than not testing it.

Or is it maybe enough that the quantization processors already are tested?

JimBobSquarePants · 2019-05-18T09:11:17Z

@brianpopow I've merged those images for you 👍

antonfirsov · 2019-05-18T11:51:07Z

@brianpopow 0.6686% difference is being reported on 32bit .NET 4.7.2. Can you reproduce this locally by enforcing 32 bit execution?

brianpopow · 2019-05-18T13:01:52Z

@antonfirsov yes this time i can actually reproduce it locally. I will try to see if i can find out why this is happening.

brianpopow · 2019-05-18T16:54:09Z

@antonfirsov it is, what you already said previously, the quantization produces slightly different results on net472 than on netcore. It looks to me as if the quantized colors are in many case (not all) identical in net472 and netcore, but they appear in a slightly different position in the palette. For example:

net472 : idx: 112, R: 237, G: 144, B: 144
netcore: idx: 112, R: 128, G: 55, B: 128
---------------------------------------------
net472 : idx: 113, R: 128, G: 55, B: 128
netcore: idx: 113, R: 237, G: 144, B: 144
---------------------------------------------
net472 : idx: 115, R: 10, G: 99, B: 99
netcore: idx: 115, R: 221, G: 99, B: 99
---------------------------------------------
net472 : idx: 116, R: 221, G: 99, B: 99
netcore: idx: 116, R: 10, G: 99, B: 99
---------------------------------------------

Here is a zip file containing the quantized palette of net472 and netcore21. Also a comparison of which pixels differ.
[quantize_issue_net472.zip] https://github.com/SixLabors/ImageSharp/files/3194282/quantize_issue_net472.zip)

~~I will increase the tolerance of the bitmap color encoding test further.~~
I will remove the tests, i dont think its useful with such a high tolerance.

… with net472 and netcore

JimBobSquarePants · 2019-05-19T00:57:20Z

There’s a lot of floating point calculation going on in Wu which will be causing the issue. If the output looks ok I’m good with dropping the tests.

antonfirsov · 2019-05-19T09:04:04Z

My tecnhique is to check TestEnvironment.Is64BitProcess at the beginning of the test. This way we can keep the coverage for conforming platforms.

brianpopow · 2019-05-19T09:43:27Z

@antonfirsov ok that makes sense, i will do that.

antonfirsov · 2019-05-19T18:07:11Z

src/ImageSharp/Formats/Bmp/BmpEncoderCore.cs

+                for (int y = image.Height - 1; y >= 0; y--)
+                {
+                    Span<TPixel> inputPixelRow = image.GetPixelRowSpan(y);
+                    PixelOperations<TPixel>.Instance.ToGray8Bytes(


Can't we replace this with inputPixelRow.AsBytes().CopyTo(outputPixelRow) here? Seems more explanatory about our intention.

I can not find AsBytes(), is that an extension method im missing?

Apologies, it's MemoryMarshal.AsBytes(spanOfThings) 😄

i have changed that, please check it again.

Looks very good, thanks!

…antizeOnGray8 # Conflicts: # tests/ImageSharp.Tests/Drawing/DrawImageTests.cs

brianpopow · 2019-05-19T19:13:08Z

@antonfirsov i was unsure about DrawImageTests.cs while resolving the merge conflict: should this tests be removed or should we keep it?

antonfirsov · 2019-05-19T19:18:52Z

DrawImageTests should be kept in it's original form, strange you had a conflict here. Can you easily revert all changes on that class? Or just replace the code in your file with the code on current master?

brianpopow · 2019-05-19T19:23:03Z

after i integrated the master branch the DrawImageTests.cs was named: DrawImageTest.cs (without an s), so the tests failed because the reference images could not be found. I just changed that to fix the failing tests.
I will take the code from master

antonfirsov · 2019-05-19T20:24:13Z

All good now! Thanks a lot for taking the time, and sorry for the conflicts!

…izeOnGray8 Not using quantization for Grey8 images when encoding 8Bit Bitmaps

brianpopow added 2 commits May 15, 2019 21:15

Not using quantization for Grey8 images when encoding 8Bit Bitmaps

89d09ee

Refactor Write8Bit into two methods: one for gray and one for color

a1829bc

JimBobSquarePants approved these changes May 18, 2019

View reviewed changes

JimBobSquarePants added the formats:bmp label May 18, 2019

brianpopow added 3 commits May 18, 2019 11:44

Add tests for quantization of 32 bit bitmap to 8 bit color image

ec0e913

Renamed DrawImageTest to DrawImageTests, fixed namespace

18cda2e

Using tolerant comparer for the Encode_8BitColor tests

5733661

brianpopow changed the title ~~WIP: Not using quantization for Grey8 images when encoding 8Bit Bitmaps~~ Not using quantization for Grey8 images when encoding 8Bit Bitmaps May 18, 2019

Removed the Encode_8BitColor, because of the quantization differences…

d98f003

… with net472 and netcore

Re-enabled Encode_8BitColor tests, if its 64 bit process

e62ae79

antonfirsov reviewed May 19, 2019

View reviewed changes

brianpopow added 2 commits May 19, 2019 20:56

Using MemoryMarshal.AsBytes in Write8BitGray to get the bytes of a row

8809765

Merge remote-tracking branch 'upstream/master' into feature/dontUseQu…

3da351a

…antizeOnGray8 # Conflicts: # tests/ImageSharp.Tests/Drawing/DrawImageTests.cs

Fix merge mistake: Using DrawImageTests from current master branch

bb8911d

antonfirsov merged commit 2b5a575 into SixLabors:master May 19, 2019

antonfirsov added a commit to antonfirsov/ImageSharp that referenced this pull request Nov 11, 2019

Merge pull request SixLabors#909 from brianpopow/feature/dontUseQuant…

77b18a3

…izeOnGray8 Not using quantization for Grey8 images when encoding 8Bit Bitmaps

dependabot bot mentioned this pull request Jul 30, 2025

Bump SixLabors.ImageSharp from 1.0.0-beta0006 to 2.1.11 sphexator/Wonderland#1

Merged

dependabot bot mentioned this pull request Oct 19, 2025

Bump the nuget group with 1 update ewdlop/bepuphysics2Fork#1

Open

Uh oh!

Not using quantization for Grey8 images when encoding 8Bit Bitmaps #909

Not using quantization for Grey8 images when encoding 8Bit Bitmaps #909

Uh oh!

Conversation

brianpopow commented May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Prerequisites

Description

Uh oh!

codecov bot commented May 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

brianpopow commented May 18, 2019

Uh oh!

JimBobSquarePants commented May 18, 2019

Uh oh!

antonfirsov commented May 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brianpopow commented May 18, 2019

Uh oh!

brianpopow commented May 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JimBobSquarePants commented May 19, 2019

Uh oh!

antonfirsov commented May 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brianpopow commented May 19, 2019

Uh oh!

antonfirsov May 19, 2019

Choose a reason for hiding this comment

Uh oh!

brianpopow May 19, 2019

Choose a reason for hiding this comment

Uh oh!

antonfirsov May 19, 2019

Choose a reason for hiding this comment

Uh oh!

brianpopow May 19, 2019

Choose a reason for hiding this comment

Uh oh!

antonfirsov May 19, 2019

Choose a reason for hiding this comment

Uh oh!

brianpopow commented May 19, 2019

Uh oh!

antonfirsov commented May 19, 2019

Uh oh!

brianpopow commented May 19, 2019

Uh oh!

antonfirsov commented May 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

brianpopow commented May 15, 2019 •

edited

Loading

codecov bot commented May 16, 2019 •

edited

Loading

antonfirsov commented May 18, 2019 •

edited

Loading

brianpopow commented May 18, 2019 •

edited

Loading

antonfirsov commented May 19, 2019 •

edited

Loading