Skip to content

Conversation

@197g
Copy link
Member

@197g 197g commented Aug 31, 2025

This was found through the fax4 decoding that is always bilevel.

No longer silently ignore an unknown color type when a photometric interpretation requires an inversion. DICOM requires that WhiteIsZero and BlackIsZero are only used for samples-per-pixel=1 but we do not strictly enforce that early. Previously for the result Multiband color we just ignored the tag which is also not right.

Closes: #296

@Shnatsel the image you linked is now in the test set and image's convert example gives the correct colors in a test. I'm still puzzled about the code block in fax where bits seem flipped, to me at least. Maybe fax flipped their Black/White colors in the returned stream, too? It's puzzling.

This was found through the fax4 decoding that is always bilevel.

No longer silently ignore an unknown color type when a photometric
interpretation requires an inversion. DICOM requires that WhiteIsZero
and BlackIsZero are only used for samples-per-pixel=1 but we do not
strictly enforce that early. Previously for the result Multiband color
we just ignored the tag which is also not right.
@Shnatsel
Copy link
Member

Well, it fixes decoding of imagemagick output (which you added to the repo) and the libtiff fax4 test file. And the BlackIsZero image committed originally still works. So good enough for me.

@Shnatsel
Copy link
Member

Shnatsel commented Aug 31, 2025

There is a non-fax-encoded 1-bit TIFF in the image test suite, and with this change it still decodes fine: https://github.com/image-rs/image/blob/main/tests/images/tiff/testsuite/l1.tiff

It is in BlackIsZero mode

@197g 197g marked this pull request as ready for review August 31, 2025 23:48
@197g
Copy link
Member Author

197g commented Aug 31, 2025

I've verified with libtiff. It seems the old 'convention' or default behavior of treating whiteiszero might have eaten its tail in here somehwere. Here's the runlength decoding loop for libtiff: https://gitlab.com/libtiff/libtiff/-/blob/85f2ac8e0b01cb7db2bbecf4a3b891bdbef67938/libtiff/tif_fax3.c (_TIFFFax3fillruns)

The critical portion is that its loop always does white-black at the same time, fax starts with white. And in that loop we have:

// white section
                        for (; n && !isAligned(cp, int64_t); n--)
                            *cp++ = 0x00;
[…]
// black section
                        for (; n && !isAligned(cp, int64_t); n--)
                            *cp++ = 0xff;

Whatever history is behind here it is consistent with treating the run that is, per ITU, white as 0 and black as 1. I'll add comments and we can merge this.

@197g 197g force-pushed the fax4-photometric-interpretation branch from 49f2d66 to 1556713 Compare September 1, 2025 00:12
@197g 197g merged commit 12c9853 into main Sep 1, 2025
15 checks passed
@197g 197g deleted the fax4-photometric-interpretation branch September 1, 2025 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Photometric interpretation for Fax4 / 1-bit images

4 participants