Improved I mode conversion by radarhere · Pull Request #3838 · python-pillow/Pillow

radarhere · 2019-05-09T01:39:04Z

Improves I mode conversion, scaling values, rather than clipping.

___________________________	Before	After
#3011
#3351
`hopper("I")`
`hopper("I;16")`
`hopper("F").convert("I")`

Also resolves the first part of #3159.

This change the results for a number of tests though, most significantly for ImageMath. The value of 'I' is changed now, being a scaled version of other modes instead of a clipped version, so when ImageMath returns a result with 'I' in it, that number is also scaled.

docs/handbook/concepts.rst

Co-Authored-By: Hugo <hugovk@users.noreply.github.com>

wiredfool · 2019-06-05T20:27:43Z

This one is going to need release notes as a potentially breaking change

wiredfool · 2019-06-05T20:30:37Z

docs/handbook/concepts.rst

-supports the following standard modes:
+A 1-bit pixel has a range of 0-1, an 8-bit pixel or a 32-bit floating point
+pixel has a range of 0-255, and a 32-bit signed integer has a range of 0-65535.
+These modes are supported:


Is that range actually correct? 32 bit signed is +-2 billion, that sounds more like 16bit unsigned.

According to numpy docs 16-bit is 0 - 65535. uint32 is 0 to 4294967295. Ps thanks guys! In desperate need of 16-bit functionality so will be watching this closely.

wiredfool · 2019-06-05T20:43:22Z

src/libImaging/Convert.c

-        else
-            *out = (UINT8) *in;
+        *out = (UINT8) (*in >> 8);
    }


I think that you’ve got the I;16 and I modes mixed up. I suspect that part of the problem with the referenced issues was likely I;16 image data being loaded in an I(;32) image because the i16 modes are complicated and only partially supported, but apart from the conversion to 8 bit, it’s safe (but inefficient) to operate on 16bit data in 32 bit storage.

Thanks for checking, and what you're saying makes sense. So in order to fix this properly, am I going to have to switch Pillow to reading I;16 actually as I;16 not I then?

I think it's more difficult than that.

I;16 and I;32 are not used quite like the 8 bit versions -- often times the actual image is only 15 or 10 bit depth, but there's no way to specify a 10 bit gray image, so it gets rounded up to the next byte. There are definitely files that I've seen with I;16 that are in the range of 0-8000 or so. So there's really no way to do a downconversion without extra info to tell you what the intent of the conversion is. Do you want to preserve the LSB, MSB, or autoscale?

The various I;16 modes (signed or unsigned) are not well supported, and really are only useful for bridging to numpy or file format conversions. I think it's basic things like resampling and filtering that don't work with I;16. But operating on a I;16 in 32 bit mode is fine, because none of the math is broken, so we went with that.

The reason that I never fixed this longstanding issue is that I never had a good handle on what I wanted the API to look like, and changing things as it is will break some users while fixing others.

I think that something like a convert mode for downsampling would make sense:

img = i32.convert('L', overflow=Image.CLIP)

could work, where we had an enum of Clip, Shift or Autoscale. Autoscale is definitely a new case and more work. Clip is what we have, Shift is what you wrote (mostly). (Though, this can work both ways, expanding is also a thing on upconversion). (naming is hard)

I think that fundamentally, you should be able to losslessly convert an I;16 to an I;32 and back by default, and similarly, I think that L->I;32->L should also work. I'd be ok with changing the defaults here on a major version, but probably not for a .x release.

Okay, I've created PR #3898 to revert this.

hugovk · 2019-06-08T18:17:36Z

This one is going to need release notes as a potentially breaking change

Noted in #3835. Or should we play it safe and hold off on this until major release 7.0.0 on 2020-01-01?

wiredfool · 2019-06-08T18:50:26Z

As I noted on some of the other comments, I’m not sure that this pr is correct, because it appears that the i16 and i32 modes got confused.

…sion" This reverts commit 41f3e7c, reversing changes made to 2f84482.

Revert pull request #3838

gokriznastic · 2019-09-12T09:35:23Z

Was this change released in version 6.1.0? I can see that it is referenced but I couldn't find it in the release notes.

radarhere · 2019-09-12T09:36:49Z

This change was reverted - it turns out it did not perfectly solve the problem.

gokriznastic · 2019-09-12T09:41:22Z

I see. Thank you for clarifying @radarhere
Meanwhile, as I see you did the PR, I am facing the similar problem when converting I;16 --> L --> I --> I;16. I find the values are being clipped. Can you tell me if there is some workaround to doing this until this is solved in a future release?

veonua · 2020-05-31T21:28:21Z

is the fix available in release 7.0.0?

radarhere · 2020-05-31T21:38:05Z

@veonua the fix is not part of any release. It was reverted, as it did not perfectly solve the problem

veonua · 2020-05-31T21:53:46Z

@wiredfool told that it can't be a part of a minor release,
The "imperfect" solution can not be merged with special experimental parameters like it was proposed

i32.convert('L', overflow=Image.SHIFT)

at this moment I'm facing an unexpected problem - around 5% of my pipeline output is blank images.

please let me know if there is an alternative library that can simply convert image to grayscale (8bit) png

CrackerHax · 2022-01-06T02:55:20Z

when fix?

radarhere · 2022-01-07T02:07:33Z

I don't know when this issue will be fixed. If you would like to, you could investigate and create a PR with a solution.

CrackerHax · 2022-01-07T10:40:26Z

It would be far more productive if someone who already knew the code handled this, obviously.

tmatsuzawa · 2024-01-05T04:24:06Z

so... is there a workaround to this problem in 2024??

radarhere · 2024-01-05T04:59:21Z

There is not a general workaround. If you have a specific situation, you can open a new issue and we can offer a specific solution, something along the lines of #5991

radarhere added 3 commits May 8, 2019 21:58

Improved I mode conversion

8a035b8

Updated ImageMath tests for scaled I mode conversion

ddc83fd

Added tests

2787b9b

This was referenced May 9, 2019

Converting MCIDAS to PDF results in empty file #3351

Closed

Converting this 16-bit grayscale image to 'L' mode destroys it #3011

Closed

Image conversion should scale pixel values accordingly #3159

Open

Updated pixel range documentation [ci skip]

64eff66

hugovk reviewed May 9, 2019

View reviewed changes

docs/handbook/concepts.rst Outdated Show resolved Hide resolved

Apply suggestions from code review [ci skip]

a258b8d

Co-Authored-By: Hugo <hugovk@users.noreply.github.com>

hugovk merged commit 41f3e7c into python-pillow:master Jun 5, 2019

radarhere deleted the i_conversion branch June 5, 2019 20:10

wiredfool reviewed Jun 5, 2019

View reviewed changes

radarhere mentioned this pull request Jun 8, 2019

Release Pillow 6.1.0 on July 1, 2019 #3835

Closed

22 tasks

radarhere added a commit to radarhere/Pillow that referenced this pull request Jun 11, 2019

Revert "Merge pull request python-pillow#3838 from radarhere/i_conver…

231fe4d

…sion" This reverts commit 41f3e7c, reversing changes made to 2f84482.

radarhere mentioned this pull request Jun 11, 2019

Revert pull request #3838 #3898

Merged

hugovk added a commit that referenced this pull request Jun 11, 2019

Revert pull request #3838 (#3898)

92dc8f4

Revert pull request #3838

bertsky mentioned this pull request Oct 18, 2020

Fix Pillow high depth images OCR-D/core#627

Merged

semjonsona mentioned this pull request Feb 9, 2023

Do not convert to JPEG if not possible AUTOMATIC1111/stable-diffusion-webui#7616

Closed

Uh oh!

Conversation

radarhere commented May 9, 2019

Uh oh!

Uh oh!

wiredfool commented Jun 5, 2019

Uh oh!

wiredfool Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

jamesjjcondon Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

wiredfool Jun 5, 2019

Choose a reason for hiding this comment

Uh oh!

radarhere Jun 10, 2019

Choose a reason for hiding this comment

Uh oh!

wiredfool Jun 10, 2019 • edited by radarhere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radarhere Jun 11, 2019

Choose a reason for hiding this comment

Uh oh!

hugovk commented Jun 8, 2019

Uh oh!

wiredfool commented Jun 8, 2019 • edited by radarhere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gokriznastic commented Sep 12, 2019

Uh oh!

radarhere commented Sep 12, 2019

Uh oh!

gokriznastic commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

veonua commented May 31, 2020

Uh oh!

radarhere commented May 31, 2020

Uh oh!

veonua commented May 31, 2020

Uh oh!

CrackerHax commented Jan 6, 2022 • edited by radarhere Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

radarhere commented Jan 7, 2022

Uh oh!

CrackerHax commented Jan 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tmatsuzawa commented Jan 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

radarhere commented Jan 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

wiredfool Jun 10, 2019 •

edited by radarhere

Loading

wiredfool commented Jun 8, 2019 •

edited by radarhere

Loading

gokriznastic commented Sep 12, 2019 •

edited

Loading

CrackerHax commented Jan 6, 2022 •

edited by radarhere

Loading

CrackerHax commented Jan 7, 2022 •

edited

Loading

tmatsuzawa commented Jan 5, 2024 •

edited

Loading

radarhere commented Jan 5, 2024 •

edited

Loading