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

feat: add "uprightWidth" and "uprightHeight" properties to Ti.Blob #11936

Merged
merged 4 commits into from Sep 2, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Aug 21, 2020

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

Summary:

  • These new properties provide image width/height when factoring in JPEG EXIF orientation.
  • Needed since Android does not auto-rotate the image when loading into memory. Will only auto-rotate when displaying on-screen. Meaning "width" and "height" represent the image's unrotated dimensions.
  • In the future, we should refactor our Android image loading code to load it upright like iOS, but this would be a large undertaking that is best done in a major version of Titanium

Note:
If we're not comfortable with adding these properties to the SDK, then alternatively we can add a method to our "ti.imagefactory" to fetch an image's upright dimensions instead.

Test:

  1. Build and run DownloadImageBlobTest.js attached to TIMOB-28093 on Android.
  2. Make sure device has Internet access.
  3. Tap on the "Download" button.
  4. Select "JPEG - EXIF Rotate 90" in dialog.
  5. Verify image is upright and scaled proportionally (not stretched) to fit the window.
  6. Repeat steps 3-5 for "JPEG - EXIF Rotate 180".
  7. Repeat steps 3-5 for "JPEG - EXIF Rotate 270".
  8. Repeat steps 3-5 for "JPEG - EXIF Flip Horizontal".
  9. Repeat steps 3-5 for "JPEG - EXIF Flip Vertical".
  10. Repeat steps 3-5 for "JPEG - EXIF Transpose".
  11. Repeat steps 3-5 for "JPEG - EXIF Transverse".
  12. Repeat all of the above on iOS.

(Note that the EXIF Flip, Transpose, and Transverse tests will appear as mirrored. Fixing the mirroring effect would require a larger refactoring of our code in the future.)

@build
Copy link
Contributor

build commented Aug 21, 2020

Fails
🚫 Tests have failed, see below for more information.
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 1 tests have failed There are 1 tests failing and 711 skipped out of 8120 total tests.

Tests:

ClassnameNameTimeError
ios.ipad.Titanium.UI.iOS.CollisionBehavior.exampleworks (14.0)10.001
Error: timeout of 10000ms exceeded
file:///Users/build/Library/Developer/CoreSimulator/Devices/8397E161-2778-43CE-AB1B-BDA17BDD4023/data/Containers/Bundle/Application/4B6B024F-9CAF-4EDD-B90E-266ACA0E119E/mocha.app/ti-mocha.js:4326:27

Generated by 🚫 dangerJS against bd1cc01

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

Although I'm okay with this change, I think it would have been nice to add Ti.Blob.exif where some EXIF properties could be exposed. We could start with the corrected width, height and rotation then add things like latitude, longitude and model in the future. This means we wouldn't need to deprecate these properties in the future when we eventually refactor our image loading code and they wouldn't be redundant on iOS (as width and height is already corrected for rotation).

@jquick-axway
Copy link
Contributor Author

@garymathews , I think adding an "exif" property would be useful too. It just wouldn't be an easy drop-in for existing code that's calculating things based off the blob's width/height. The new "uprightWidth"/"uprightHeight" properties are an easy drop-in. I'm not thrilled about adding these properties either, but we need to do something.

@ssjsamir ssjsamir self-requested a review August 26, 2020 13:49
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed, Tested using the instructions mentioned in the description.

Note:
Android shows mirrored images for EXIF Flip, Transpose, and Transverse while on iOS in is upright.

Test Environment

MacOS Big Sur: 11.0 Beta 5
Xcode: 12.0 Beta 5
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.1.0-master.11""
iphone 8 Sim (14.0 Beta)
iphone 11 (14.0 Beta 5)
Pixel XL (Android 10)

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