-
Notifications
You must be signed in to change notification settings - Fork 377
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
Exif rotation not supported? #130
Comments
Hi @xleon you are right Exif rotation is not yet handled. It's a good improvement for a future release. 👍 For reference this is what SDWebImage does: https://github.com/rs/SDWebImage/blob/b185621812f90a4eb4f7f44e0f547014da4d1958/SDWebImage/UIImage%2BMultiFormat.m And this is what Picasso does (there are other files involved): |
Cool. |
I ended up adding a rotate button that uses the rotate transformation to get around this in my app Sent from my iPhone
|
That´s not an option in apps that don´t allow the user to edit the image |
Even if it's a temporary hack I would suggest having it in ImageLoaderTask. bool transformPlaceholdersEnabled = Parameters.TransformPlaceholdersEnabled.HasValue ?
Parameters.TransformPlaceholdersEnabled.Value : ImageService.Config.TransformPlaceholders;
if (Parameters.Transformations != null && Parameters.Transformations.Count > 0
&& (!isPlaceholder || (isPlaceholder && transformPlaceholdersEnabled)))
{ |
This has also high priority for us. Would be awesome to get this build in FFImageLoading (we also have users who report their pictures uploaded with the wrong rotation). |
I guess Exif rotation isn't working on any platform: Android, iOS, WP (SL, UWP, RT). |
I'm not sure. On iOS we had rotation problems (doing a landscape photo ended up on the server in portrait). These problems occured on iOS when fetching the image from disk and resizing it then using core graphics. Since we use CachedImage on iOS with combination of image.GetImageAsync() the photos are rotated correctly. So my guess is the iOS ImageView automatically correctly rotates the taken photos? |
@coonmoo Yes, on iOS, UIImageView will read EXIFF orientation from image file and handle it ( |
I changed the issue title as we´re talking about other platforms |
Since reading the image orientation on android requires using ExifInterface with a "filepath" (which the CachedImage does not know), my proposal would be the following: Add an optional overload to: GetImageAsJpgAsync So I can use this method http://stackoverflow.com/a/11081918 in my app to determine the rotation. and call cachedImage.GetImageAsJpgAsync(rotation: 90) which should internally use a matrix: Matrix m = new Matrix(); The currently implemented method should be switched from Bitmap.CreateScaledBitmap, Would be super awesome, if you could add a rotation paramter :)!!! |
In a |
Sorry yeah, your approach (rotating the image also in the view) is the right way to do it. But, I had huge memory problems on low-end devices that's why in my head was the rotation overload for GetImageAsJpgAsync. I did the rotation previously like you in your exif-orientation-support branch, but on devices with < 1GB ram I run into frequent out of memory issues when For the web image support: I don't need it, because when users can upload their images correctly rotated, they are also fetched from the server correctly. |
@coonmoo As to memory problems, the key is to do this: https://github.com/molinch/FFImageLoading/blob/exif-orientation-support/source/FFImageLoading.Droid/Extensions/ExifExtensions.cs#L57-L61 BTW: Notice the rotation takes place after downsampling, so if you have it correctly configured (for low-end devices capabilities) - it would be less memory costly operation to do the rotation. |
@molinch Do you think we could optimize this method to try to reuse bitmap from a reuse-pool? That would be even less memory costly operation. |
@daniel-luberda I don't think we can do better than that... Unfortunately it does not seem to be possible to use the pool in this case. |
Btw, I don't know if it changes anything but there is an overload of Bitmap.CreateBitmap that accepts the matrix as a parameter. |
@molinch From what I've read, using canvas (as we do it) should be more efficient, but I didn't test the differences. If anyone has an explanation about which one is better, just let me know. |
BTW: Shouldn't we rotate bitmap before downsampling? |
To lower memory usage it's better to downsample and then rotate. But we should enforce correct width/height... Because of EXIF tags width can be height when we downsample. |
Another changes. What do you think? 7114cbc Testers needed! :P |
I'll test it. Is there any chance you could provide a -pre release nuget package? |
I uploaded dlls here: link |
Another idea: To avoid memory problems when loading image eg. from gallery. We could make a new ImageSource type which will always get an image thumbnail. It could use platform APIs to retrieve thumbnail, so it will be efficient. Perfect for gallery like apps. We could have |
Btw I agree with what @coonmoo said. If it comes from the web it's the responsability of the server to deliver correct images with correct orientation. It doesn't make sense to do it on device: it's slow and takes memory/cpu.
Which platform API are you thinking about? |
@molinch I agree, exif orientation for local files should be enough.
Currently, rotation is done only for images which have exif orientation specified. There's no performance penalty for files which don't have it. It could be easily done for web downloaded images as we already have them on a disk - we only need to specify the full file path (we could add some helper method to DiskCache implementations).
|
Ah definitely not this API on Android :) I'd rather not change anything people can just use |
Did you test it? Is it really that bad in newer Android versions?:P BTW:
|
@coonmoo Did you test it? |
Hey Daniel, thanks for your effort! But sadly I currently don't have access to the phone where the wrong orientation problems occured. So I cannot tell you if the orientation problem is fixed. At least the change is stable on the phones I tested. |
I rolled out the test dll changes to some testers. Some people report that their uploaded pictures are now completly black. So what I do is load the camera picture in the CachedImage view (preview is totally fine), then I call (after loading finished) image.GetImageAsJpgAsync(500.500. 95) and upload the resulting Byte Array to the Server. The Server stores this raw Byte Array then (without any further Image processing applied). So for some users the uploaded Picture (despite displaying correctly in preview) turns totally black. Any ideas on this? |
I'm not sure. I just tested it on a low end device, and sometimes even the images loaded from the server in CachedImage display completly black. |
@coonmoo do you have anything special in the logs when it happens? |
Here is the log from a low end device.
|
@coonmoo I don't see anything useful information in logs. My guess: Do you have |
I'm using the FFImageLoading defaults. CachedImage is configured like this: DownsampleToViewSize=true; |
Try with |
I sent you the image. Btw it happens on the specific device to every image captured from the camera. TransparencyEnabled=true makes not difference. |
The issue with black Images is definitely caused by the exif rotation change. I reverted to FFImageLoading 2.0.7 alpha and have no issues when capturing images. Then I tried again your dlls and the captured image was black again. Can I assist you any further with this issue? |
I spent a few hours debugging this issue on my low end devices. I finally have a solution which works very well across all devices: I modified: ExifExtensions.ToRotatedBitmap with the provided function. |
@coonmoo Thanks, sorry for answering so late, I had a very busy week. I'll test your changes and merge it into master. Great! Nice work :) |
Some devices rotate images in a way that a landscape picture can be shown in portrait mode and the opposite.
In another part of my app I´m handling / fixing this with android
ExifInterface
, butImageViewAsync
seams like not taking care of this (correct me if I´m wrong).This piece of code ilustrates the fix: https://gist.github.com/xleon/7c6e2eff4d9ca3c6db0b
Is there any actual workaround for this?
Am I doing something wrong?
Are you planning to handle exif rotation in the future?
I would make a PR but I´m not sure where´s the best place to write this code
The text was updated successfully, but these errors were encountered: