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

Added a capability to rotate Thumbnail images according to exif orientation property #67

Merged
merged 4 commits into from
Jan 20, 2018

Conversation

mis94
Copy link
Contributor

@mis94 mis94 commented Jan 20, 2018

Currently the library displays thumbnail images regardless of their orientation values which may cause images with exif orientation property values not equal 1 to be rotated as in this screenshot:
problem

I've added a slight change to thumbnailStyle method in Image class and now it rotates images according to their orientation value, after adding this change and using it in my project, now images are displayed in a proper way regardless of their orientation values:
capture

@benhowell
Copy link
Owner

Hi @mis94
Thanks for the PR!

Am I right in understanding that if the optional orientation prop is not used then images are rendered in their natural orientation?

Also, for completeness, I think the PR needs to account for all transformations. A list of the missing ones are below:
2: 'rotateY(180deg)',
4: 'rotate(180deg) rotateY(180deg)',
5: 'rotate(270deg) rotateY(180deg)',
7: 'rotate(90deg) rotateY(180deg)',

Can you add these and re-submit?

@mis94
Copy link
Contributor Author

mis94 commented Jan 20, 2018

Yes, if the orientation prop is not provided the images are rendered in their natural orientation (the same case as if orientation is equal 1).
I didn't handle the rest of the orientation values as I've read here:
https://www.impulseadventure.com/photo/exif-orientation.html
"Note that one would only expect the four orientation settings shown to be possible with a digital camera. The other four settings would imply that the resulting image was flipped horizontally before recording."
But if you see if necessary to handle I can do so and re-submit.

@benhowell
Copy link
Owner

I think for completeness with the spec, it'd be better to include the other cases (perhaps there's a case where someone would want to flip an image deliberately, who knows?)

@mis94
Copy link
Contributor Author

mis94 commented Jan 20, 2018

Done. I've added the rest of the cases.

@benhowell benhowell merged commit e7eba44 into benhowell:master Jan 20, 2018
@benhowell
Copy link
Owner

Excellent, thanks!

In a future version I'll probably extend this so that images are scaled according to the transformation to be performed (e.g. if we rotate 90 degrees due to orientation 6, then scale according to the transformed image). At the moment scaling does not take this into account, hence the bars on either side of the transformed images in your demo image.

Merged. Will release v0.4.8 shortly.

@benhowell
Copy link
Owner

benhowell commented Jan 20, 2018

Published in v0.4.8 👍

@mis94 mis94 deleted the orientation-change branch January 20, 2018 05:01
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.

2 participants