Page MenuHome

TIFF encoding / decoding fix
Needs RevisionPublic

Authored by Troy Sobotka (sobotka) on Sun, Jan 6, 1:52 AM.

Details

Summary

Fixes T60221 where Blender would save as unassociated alpha and force load as associated.

Diff Detail

Repository
rB Blender

Event Timeline

Troy Sobotka (sobotka) edited the summary of this revision. (Show Details)Sun, Jan 6, 2:49 AM
Brecht Van Lommel (brecht) requested changes to this revision.EditedSun, Jan 6, 2:49 PM

This patch appears to be reversed?

I didn't test in detail, but glancing at the code this seems like it would cause:

  • Incorrect saving of 8 bit unassociated alpha image data to tif, by tagging them as having associated alpha but not actually doing the required premultiplication to convert them to that format.
  • Incorrect loading of 8 bit unassociated alpha tif images, by premultiplying them and then telling Blender it's still unassociated alpha.

If there is a bug, it's not clear to me that always lossy saving as associated alpha is the best solution.

This revision now requires changes to proceed.Sun, Jan 6, 2:49 PM

I was under the impression that the IB_PREMUL flags handled the saving? If not I can fix that.

Currently, the file will be flagged with extrasamples as unassociated, which would probably have been fine except for the ridiculous hack of flagging unassociated alpha imagery as associated. Some misconception about “preserving data” that ends up with a TIFF saved with unassociated alpha from Blender being mangled up on load.

If I am reading you correctly, this TIFF code won’t check the IB_PREMUL flag and properly handle the transform?

Can you give precise steps to reproduce the bug this is intended to fix? Otherwise this is confusing.

Currently, the file will be flagged with extrasamples as unassociated, which would probably have been fine except for the ridiculous hack of flagging unassociated alpha imagery as associated. Some misconception about “preserving data” that ends up with a TIFF saved with unassociated alpha from Blender being mangled up on load.

The hack only applies to file reading, and does not affect the metadata saved into any file. It avoids a lossy and unnecessary unassociated -> associated -> unassociated conversion on tif file read.

If I am reading you correctly, this TIFF code won’t check the IB_PREMUL flag and properly handle the transform?

IB_alphamode_premul is a flag for file reading, and tells Blender what kind of alpha format the image data has so it can do appropriate conversion. When writing files, float images are always associated and byte images unassociated, so there is no need to check a flag.

The hack only applies to file reading, and does not affect the metadata saved into any file.

First, this is because Blender's alpha handling is still screwed. It was and is a dumb as dirt decision to make the default for 8 bit unassociated alpha. I'm pretty sure you agree here after all these years.

The file reading breaks files saved from within Blender.

This is made worse by having the unacceptable PNG format as the default. It's complete garbage, and worse, we broke Blender to support it as opposed to doing the wise thing and insist on associated everywhere as per OIIO.

It avoids a lossy and unnecessary unassociated -> associated -> unassociated conversion on tif file read.

Again, byproduct of above. That said, if we add an associate / unassociate function with an intermediate promoted state, does that alleviate concerns?

Seriously though, the idea that you use the term "lossy" and "unnecessary" is somewhat ridiculous given that TIFF handling is completely broken in Blender, no?

Should we get our priorities straight before discussing the nuances of quantisation losses?