Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TiffSaver: produce valid 4-channel RGBA images #4059

Merged
merged 5 commits into from
Nov 28, 2023
Merged

Conversation

CGDogan
Copy link
Contributor

@CGDogan CGDogan commented Jul 26, 2023

As I asked on https://forum.image.sc/t/bfconvert-breaks-images-with-alpha-layer-wrong-interleave/83482, bfconvert, when given 4-channel images, outputs with the monochrome tag. This means that many image decoders will try to read it as monochrome and show an invalid image. In general, we should save 4-channel images like 3-channel ones, because RGBA tagged as RGB is a supported configuration in the TIFF specification (https://www.itu.int/itudoc/itu-t/com16/tiff-fx/docs/tiff6.pdf pg 77), where even the order is specified: R,G,B, then A. This will result in BioFormats producing much more compatible tiff files.

EDIT: Rebased, considering that the TIFF specification does not limit the additional alpha channel to RGB, but it limits its premultipliedness property only to RGB. As such, YCbCr can also have an alpha channel

@CGDogan CGDogan changed the title TiffSaver handle RGBA TiffSaver: produce valid 4-channel RGBA images Jul 26, 2023
@melissalinkert
Copy link
Member

For reference, the original logic was introduced in #3057. It would help to know which viewer(s) can't handle TIFFs converted without this change, so that we can better evaluate the behavior of this change across a range of use cases.

@CGDogan
Copy link
Contributor Author

CGDogan commented Jul 26, 2023

My mac can’t, but the reason I’m concerned with this is that I bfconvert images to tiff so that I can view with OpenSlide, but OpenSlide then shows it as monochrome. But for comparison, I searched for “view tiff online” and opened 5 different sites, none of them could view it. Here's the file: https://github.com/ome/bioformats/files/11995909/out5.tiff.zip

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/bfconvert-breaks-images-with-alpha-layer-wrong-interleave/83482/5

@dgault dgault added this to the 7.0.1 milestone Sep 4, 2023
@dgault dgault modified the milestones: 7.0.1, 7.1.0 Oct 11, 2023
Copy link
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

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

Running bfconvert with an RGBA input file and test.tiff as the output file name does result in a 4-channel RGB TIFF which can be displayed with showinf, ImageMagick's display, and Windows Photos. However, display shows this warning:

display-im6.q16: Sum of Photometric type-related color channels and ExtraSamples doesn't match SamplesPerPixel. Defining non-color channels as ExtraSamples.. `TIFFReadDirectory' @ warning/tiff.c/TIFFWarnings/912.

The ExtraSamples tag should be set in this case, with a single value (likely 0). See pages 31-32 of the TIFF specification.

@CGDogan
Copy link
Contributor Author

CGDogan commented Nov 22, 2023

It should work now?

Copy link
Member

@melissalinkert melissalinkert left a comment

Choose a reason for hiding this comment

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

This looks fine to me now. If I convert a 4-channel RGBA PNG to TIFF with bfconvert, the resulting TIFF has the PhotometricInterpretation, BitsPerSample, and ExtraSamples values that are expected. Opening the file with ImageMagick's display command shows a correct image with no warnings.

Converting the same file with bfconvert -separate continues to produce 4 separate IFDs, none of which have an ExtraSamples tag. The PhotometricInterpretation in this case is 1 (BLACK_IS_ZERO) and BitsPerSample has one entry, all as expected.

@dgault dgault merged commit e265284 into ome:develop Nov 28, 2023
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants