Fix dithering and improve readability #9338
Open
+32
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Are you saying this PR is not yet ready for review? |
radarhere
changed the title
Author
I edited my comment, It builds fine and works as intented: the result is the same as before with a bunch of test images I tried. But I it is not like we can test that with a ground truth and improve the test suite to that end, so someone's second opinion would be nice. |
Member
|
You can see that our test suite is failing with your changes. I've created WildRackoon#1 to update our expected results. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Somes changes related to the dithering conversion code:
Fix RGB cache error values at the end of a row, not set to the right values (Blue values was used for R,G and B ?)
I believe this is a mistake, this affects only the last column of the image, so it is likely very minimal, but I still really would like a second opinion on that.
Improved error diffusion readability, now the Floyd–Steinberg weights are readable through the code.
We should add more comments for posterity IMO, most of this dates back to the original PIL fork, the implementation is nicely optimized, but that makes the code slightly more confusing.
I mean look at this 😅:
Pillow/src/libImaging/Convert.c
Line 1284 in 8494b06
Future improvement includes adding more dithering kernels in the same optimized manner.
If anyone needs more details on those changes, I published a gist for that here: https://gist.github.com/WildRackoon/0030fc3d4db3f4af73e2cdac19bbdf6d