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

[TIMOB-3887] Android: fix orientation of source image file #8742

Closed
wants to merge 3 commits into from

Conversation

frankieandone
Copy link
Contributor

@frankieandone frankieandone commented Jan 10, 2017

Test Case
Set permissions and take picture in four different orientations: up, rotated +- 90 degrees and upside down. Just to check go to your images directory and check that the orientation is the same as at the moment of capture.

/* 
 * @out is output of concerned function
 * @msg is the message to display describing @out
 * @flag is priority level of message. default is debug
 */
function log(out, msg, flag) {
	if (msg == undefined) {
		msg = " ";
	} else {
		msg = " " + msg + " ";
	}
	if (flag == "e" || flag == "error") {
		Ti.API.error("test msg error" + msg + out);
		return;
	}
	Ti.API.debug("test msg" + msg + out);
}

var win;

function fireCamera() {
	if (Ti.Platform.osname === 'android' || Ti.Platform.osname == "iphone" || Ti.Platform.osname == 'ipad') {
		win.removeEventListener('focus', fireCamera);
	}
	Titanium.Media.showCamera({
		
		success: function (event) {
			var cropRect = event.cropRect;
			var image = event.media;
			
			if (event.mediaType == Ti.Media.MEDIA_TYPE_PHOTO) {
				var imageView = Ti.UI.createImageView({
					width: win.width,
					height: win.height,
					image: event.media,
					autorotate: true
				});
				win.add(imageView);
			}
		},
		cancel: function () {
			
		},
		error: function (error) {
			if (error.code == Titanium.Media.NO_CAMERA) {
				log("", "device has no camera", error)
			}
			else {
				log(error.code, "unexpected error", error);
			}
		},
		saveToPhotoGallery: true,
		allowEditing: true,
		mediaTypes: [Ti.Media.MEDIA_TYPE_PHOTO]
	});
}

win = Titanium.UI.createWindow({
	title: "timob-3887"
});
win.open();

if (Ti.Platform.osname === 'android' || Ti.Platform.osname == "iphone" || Ti.Platform.osname == 'ipad') {
	win.addEventListener('focus', fireCamera);
} else {
	fireCamera();
}

JIRA: https://jira.appcelerator.org/browse/TIMOB-3887

@skypanther
Copy link
Contributor

It appears from my testing that the wrong EXIF orientation value is being detected when the camera is used. For example, with the phone held vertically/portrait the resulting file has an EXIF rotation value of ORIENTATION_ROTATE_90.

Based on that, it seems that this bug should be fixed in the base camera classes, not by rotating the imageview. I just happened to publish today a module that works around this bug by both rotating the bitmap and setting the exif orientation tag to portrait. See https://github.com/skypanther/TiRotate Feel free to use code from it. But really, the underlying problem with the captured image is what needs to be fixed here.

@hansemannn
Copy link
Collaborator

@skypanther Thanks for adding your thoughts here as well! 100 % agree, except that the portrait-in-landscape issue originally is a native Android issue which can be handled in those ways 🚀

@skypanther
Copy link
Contributor

Clicking through that SO post took me to one that says "ExifInterface doesn't work with all manufactures so use CameraInfo instead" which might explain why my module doesn't work on my Nexus 5x. And, gives me something to look into for resolving that. Thanks!

@frankieandone frankieandone changed the title [TIMOB-3887] Android: fixed imageView orientation issue [TIMOB-3887] Android: fix orientation of source image file Jan 18, 2017
@maggieaxway
Copy link
Contributor

maggieaxway commented Feb 28, 2017

@fmerzadyan Tested on Samsung S5 and Pixel. The stretching to fit behaviour and rotation looks strange. Can we try something like the Photos/Gallery app. The landscape image will center when device in portrait position, background in black, and fill the view when in device in landscape position?

https://goo.gl/photos/JvCeLDUQeqAgm99Z9

@maggieaxway
Copy link
Contributor

@fmerzadyan @jquick-axway
https://goo.gl/photos/uJUWEZY4egWQkzr3A
The first 4 photos are before PR, comparing with iOS behaviour
The last 4 photos are without the rotation code and with the CENTER_INSIDE only. I am not sure this is what they want. Would you help me to verify that?

@jquick-axway
Copy link
Contributor

We should not rotate the bitmap in memory like this.

Reason:
You're effectively loading the image twice in RAM. First, the code is decoding the image to a 32-bit uncompressed bitmap as-is in RAM. Then the call to rotateImage() creates another 32-bit uncompressed version of that bitmap in RAM of the same size memory wise, but rotated. Doing it this way is a major issue with camera photos (the most common case) since photos tend to be very large and low-end devices might not have enough heap memory to do this operation.

(Yes, I know a lot of naughty stackoverflow developers do it this way. They're doing it the wrong way.)

Better Solution:
We should load the JPEG image as-is and rotate the drawable instead. The following solution on stackoverflow is a good example of this where you can rotate it via the ImageView.setImageMatrix() method...
http://stackoverflow.com/questions/23607798/image-orientation-changes-after-downloaded-image-from-server-in-android/23607884#23607884

The JPEG file should not be changed. It's actually normal for JPEGs to have an EXIF orientation setting and all platforms have to deal with it. The difference is that Android forces a developer to rotate a JPEG manually versus other platforms (like Apple) will rotate it upright for you when displayed in UI.

Limitation:
Android's "ExifInterface" does not support accessing images under the APK's "res" directory on Android 6.x and older OS versions. This is a minor issue since developers control the JPEG files they bundle into their APK and can rotate it themselves.

Note:
I'll look into adding a new TiDrawableReference.getOrientation() method to the following pull request...
#8951

The above is a refactoring of our image loading code and involves routing most image loading to the TiDrawableReference class if it involves displaying it onscreen. Like camera photos that are too large to be displayed by the GPU.

@jquick-axway
Copy link
Contributor

Also, what @maggieaxway brought up appears to reveal a separate bug. An ImageView on iOS displays images "letterboxed" versus Android is "stretching" the image. I recommend that we remove the setScaleType(CENTER_IMAGE) change, figure out what the correct behavior should be between all platforms (Android, iOS, Windows Phone), and submit that as a separate PR bug fix.

@garymathews
Copy link
Contributor

Superseded by #9049

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants