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

raw input: fix user_flip usage #3858

Merged

Conversation

mugulmd
Copy link
Contributor

@mugulmd mugulmd commented May 30, 2023

Description

When reading a raw image, setting the user_flip to 1 meant ignoring the flip value (to avoid buffer rotation from LibRaw).

However the value 1 for the user_flip is not neutral: values lie between 0 and 7 (see https://www.libraw.org/docs/API-datastruct-eng.html#libraw_image_sizes_t), and 1 corresponds to a horizontal mirror (in my understanding of the source code).

The proposed fix is to use 0 as user_flip value for ignoring the flip value.

This modification invalidates the changes made in this PR: #3847.
Therefore, flip to orientation conversion has been adapted to match exactly LibRaw's documentation, which handles only rotations (+90, +180, -90 degrees).

Tests

The previous PR was only tested on images of landscape and human faces, hence the mirroring issue remained unnoticed.
When reading images containing text however the problem is clearly visible.
This PR fixes this mirroring issue on the following 4 orientations: 0, +90, +180 and -90 degrees rotation.
Other orientations could not be tested as they don't seem to ever appear on raw images datasets.

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.

LGTM, and thanks for the fix. Before I merge, can you please fix the formatting as indicated by examining the output of the failed "clang-format" test in CI? Thanks.

@mugulmd mugulmd force-pushed the raw/fix_flip_to_orientation branch from c6c9b18 to 30f1f1e Compare May 31, 2023 07:38
@mugulmd
Copy link
Contributor Author

mugulmd commented May 31, 2023

LGTM, and thanks for the fix. Before I merge, can you please fix the formatting as indicated by examining the output of the failed "clang-format" test in CI? Thanks.

Done 😄

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.

LGTM, thanks

@lgritz lgritz merged commit cf2b7d7 into AcademySoftwareFoundation:master May 31, 2023
20 of 21 checks passed
lgritz pushed a commit to lgritz/OpenImageIO that referenced this pull request Jun 1, 2023
When reading a raw image, setting the `user_flip` to 1 meant ignoring
the `flip` value (to avoid buffer rotation from LibRaw).

However the value 1 for the `user_flip` is **not neutral**: values lie
between 0 and 7 (see
https://www.libraw.org/docs/API-datastruct-eng.html#libraw_image_sizes_t),
and 1 corresponds to a horizontal mirror (in my understanding of the
source code).

The proposed fix is to use 0 as `user_flip` value for ignoring the
`flip` value.

This modification invalidates the changes made in this PR:
AcademySoftwareFoundation#3847.
Therefore, flip to orientation conversion has been adapted to match
exactly LibRaw's documentation, which handles only rotations (+90, +180,
-90 degrees).

The previous PR was only tested on images of landscape and human faces,
hence the mirroring issue remained unnoticed.
When reading images containing text however the problem is clearly
visible.
This PR fixes this mirroring issue on the following 4 orientations: 0,
+90, +180 and -90 degrees rotation.
Other orientations could not be tested as they don't seem to ever appear
on raw images datasets.
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