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

fix(exif-orientation): added support for exif values 2, 4, 5 and 7 #409

Merged
merged 5 commits into from Jul 20, 2022
Merged

fix(exif-orientation): added support for exif values 2, 4, 5 and 7 #409

merged 5 commits into from Jul 20, 2022

Conversation

Mouwrice
Copy link
Contributor

@Mouwrice Mouwrice commented Jul 18, 2022

Closes #408

Bug

Cause:

The BitMapUtils did not take all EXIF orientation values into account causing some images to show mirrored and rotated wrongly.

Reproduce

GIVEN: An image with an EXIF value of 2, 4, 5 or 7
WHEN: Opening the image in the crop view
THEN: Images have the wrong orientation

How the bug was solved:

Added the remaining of the ORIENTATION tags from the ExifInterface https://developer.android.com/reference/androidx/exifinterface/media/ExifInterface#TAG_ORIENTATION

Before:

2 4 5 7
image image image image

After:

2 4 5 7
image image image image

@Canato Canato self-assigned this Jul 18, 2022
@Mouwrice Mouwrice marked this pull request as ready for review July 18, 2022 12:35
@Mouwrice Mouwrice requested a review from a team as a code owner July 18, 2022 12:35
@Mouwrice
Copy link
Contributor Author

@Canato heya, I thinks this PR should do the trick to resolve my issue from a couple hours ago

exif.getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL)
val degrees: Int = when (orientationAttributeInt) {
ExifInterface.ORIENTATION_ROTATE_90, ExifInterface.ORIENTATION_TRANSVERSE,
ExifInterface.ORIENTATION_TRANSPOSE -> 90

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers.

This expression contains a magic number. Consider defining it to a well named constant.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just an enum, mr bot, dont you worry

@Canato
Copy link
Member

Canato commented Jul 19, 2022

@Mouwrice on the steps to reproduce could you provide an image for testing and which buttons are you clicking(and the order)?

Not sure if I was able to reproduce

@Mouwrice
Copy link
Contributor Author

@Canato Of course: https://github.com/recurser/exif-orientation-examples
You can the images from this repo, I've also linked them in the issue, the number on the images is the EXIF value it has.

To reproduces just open the app, SAMPLE USING CROPIMAGEVIEW choose an image that has EXIF value 2, 4, 5 or 7 and see that they are orientated wrongly.

The exact images used in my PR description are:
https://github.com/recurser/exif-orientation-examples/blob/master/Landscape_2.jpg
https://github.com/recurser/exif-orientation-examples/blob/master/Landscape_4.jpg
https://github.com/recurser/exif-orientation-examples/blob/master/Landscape_5.jpg
https://github.com/recurser/exif-orientation-examples/blob/master/Landscape_7.jpg

The other EXIF values caused no issues, as you had already taken them into account.

Canato
Canato previously approved these changes Jul 19, 2022
Copy link
Member

@Canato Canato left a comment

Choose a reason for hiding this comment

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

I could not reproduce it, but the code looks very nice and the new version is all good!

Thanks for the improvement

@Mouwrice
Copy link
Contributor Author

I could not reproduce it, but the code looks very nice and the new version is all good!

Thanks for the improvement

@Canato Thanks you and you are welcome. Do you have any ETA for when this will get release into a new version. I am kind off in urgent need for this fix.

Also, are you sure you tried it on the main branch with the images supplied and not this branch (which no longer has the issue of course) hahahah :p

@Canato
Copy link
Member

Canato commented Jul 20, 2022

@Mouwrice the CI passing we merge and release a fix today.

I'm going to Tomorrowland tomorrow and back next Wednesday, so is time for it XD

@Mouwrice
Copy link
Contributor Author

@Mouwrice the CI passing we merge and release a fix today.

I'm going to Tomorrowland tomorrow and back next Wednesday, so is time for it XD

oooooh, hahahahah have fun! I want to go too someday

@Canato Canato merged commit 212e7a8 into CanHub:main Jul 20, 2022
@Mouwrice Mouwrice deleted the fix/exif-orientations branch July 20, 2022 09:12
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.

[BUG] - Not all EXIF values are supported
2 participants