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

jpeg output: fix density calculation #3861

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

mugulmd
Copy link
Contributor

@mugulmd mugulmd commented Jun 1, 2023

Description

In jpegoutput.cpp when computing the density fields the relationship between XRes, YRes and aspect is aspect = XRes / YRes.

However this relationship was not respected in one specific case, when aspect >= 1.0 and XRes and YRes are not set.

This PR solves this problem by enforcing the correct relationship.

Related to: #2822

Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

I think you are right. This part of the code is really confusing because we are trying to mimic a misunderstanding of several apps that appears to contradict the JFIF spec. But I think that you are just making the "neither one is set" conform to the twisted logic we had for ("only one of xres or yres is set").

@lgritz lgritz merged commit eaa2c2a into AcademySoftwareFoundation:master Jun 2, 2023
21 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Jun 2, 2023
…undation#3861)

In `jpegoutput.cpp` when computing the density fields the relationship
between `XRes`, `YRes` and `aspect` is `aspect = XRes / YRes`.

However this relationship was not respected in one specific case, when
`aspect >= 1.0` and `XRes` and `YRes` are not set.

This PR solves this problem by enforcing the correct relationship.

Related to: AcademySoftwareFoundation#2822
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.

None yet

2 participants