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

ImfAcesFile.cpp: Fix primary conversion edge case. #650

Conversation

arkellr
Copy link
Contributor

@arkellr arkellr commented Jan 31, 2020

This fixes the edge case when the file chromaticities.white != file adopted
neutral.

In such a scenario the adoptedNeutral file attr must be used as the
display white point value passed into RGBtoXYZ() method since
the adaption matrix (Bradform Transform) is computed using the adopted neutral as its
input white point.

Signed-off-by: Arkell Rasiah arasiah@pixsystem.com

This fixes the edge case when the file chromaticities.white != file adopted
neutral.

In such a scenario the adoptedNeutral file attr must be used as the
display white point value passed into RGBtoXYZ() method since
the adaption matrix (Bradform Transform)  is computed using the adopted neutral as its
input white point.

Signed-off-by: Arkell Rasiah <arasiah@pixsystem.com>
@cary-ilm cary-ilm added the Needs Discussion To be discussed in the technical steering committee label Feb 5, 2020
Since fileChr.white is 'fileNeutral' and acesChr.white is 'acesNeutral'
always.

Signed-off-by: Arkell Rasiah <arasiah@pixsystem.com>
Copy link
Contributor

@JGoldstone JGoldstone left a comment

Choose a reason for hiding this comment

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

If I had a time machine I would go back to 2007 and persuade Florian, based on the definitions given in ISO 22028-1:2003 that we used in ACES discussions, to call the attribute 'adaptedWhite' rather than 'adoptedWhite'. But what's done is done. LGTM.

Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

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

LGTM.

@cary-ilm cary-ilm merged commit 48c2106 into AcademySoftwareFoundation:master Feb 26, 2020
@cary-ilm cary-ilm added this to the v2.5.0 milestone Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Discussion To be discussed in the technical steering committee v2.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants