Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[image_picker] Improve image_picker for iOS to handle more image types #6812

Merged
merged 10 commits into from
Dec 16, 2022

Conversation

vashworth
Copy link
Contributor

@vashworth vashworth commented Dec 8, 2022

Changes iOS image picker (iOS 14+) to use NSItemProvider's method loadDataRepresentationForTypeIdentifier:completionHandler: instead of loadObjectOfClass:completionHandler:.

The loadObjectOfClass method is unable to convert some image types directly into a UIImage. For example, WebP and ProRaw images return an error of Could not coerce an item to class UIImage. However, loadDataRepresentationForTypeIdentifier is able to load all image types that conform to UTTypeImage (UTTypeHEIC, UTTypeHEIF, UTTypeLivePhoto, UTTypeICO, UTTypeICNS, UTTypePNG, UTTypeGIF, UTTypeJPEG, UTTypeWebP, UTTypeTIFF, UTTypeBMP, UTTypeSVG, UTTypeRAWImage) into NSData, which can then be converted into a UIImage.

Fixes flutter/flutter#92100.
Fixes flutter/flutter#95885.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@mark8044
Copy link

This is much needed, thank you much @vashworth

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

Missed there was a previous similar PR about this #4665

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

If you make the Fixes #issue on its own lines then GitHub will handle linking the PR to the issues and will auto-close.

Fixes flutter/flutter#92100, flutter/flutter#95885.

Screen Shot 2022-12-13 at 1 37 52 PM

@cyanglaz anything to add, since you also reviewed #4665

@jmagman
Copy link
Member

jmagman commented Dec 13, 2022

Needs formatting:

patch -p1 <<DONE
diff --git a/packages/image_picker/image_picker_ios/ios/Classes/FLTPHPickerSaveImageToPathOperation.m b/packages/image_picker/image_picker_ios/ios/Classes/FLTPHPickerSaveImageToPathOperation.m
index 7cc50791a..16c205012 100644
--- a/packages/image_picker/image_picker_ios/ios/Classes/FLTPHPickerSaveImageToPathOperation.m
+++ b/packages/image_picker/image_picker_ios/ios/Classes/FLTPHPickerSaveImageToPathOperation.m
@@ -102,7 +102,8 @@ typedef void (^GetSavedPath)(NSString *);
                                     UIImage *image = [[UIImage alloc] initWithData:data];
                                     [self processImage:image];
                                   } else {
-                                    os_log_error(OS_LOG_DEFAULT, "Could not process image: %@", error);
+                                    os_log_error(OS_LOG_DEFAULT, "Could not process image: %@",
+                                                 error);
                                   }
                                 }];
     }

DONE

@@ -55,6 +55,76 @@ - (void)testSaveGIFImage API_AVAILABLE(ios(14)) {
[self verifySavingImageWithPickerResult:result fullMetadata:YES];
}

- (void)testSaveBMPImage API_AVAILABLE(ios(14)) {
Copy link
Member

@jmagman jmagman Dec 13, 2022

Choose a reason for hiding this comment

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

Are you able to test the pre-iOS 14 codepath without using PHPickerResult?

Copy link
Member

Choose a reason for hiding this comment

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

Never mind, that code is dead, FLTPHPickerSaveImageToPathOperation is only available on iOS 14.

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM!

@LabN36
Copy link

LabN36 commented Dec 16, 2022

I just tested vashworth:image_picker_images_not_loading branch and it doesn't seems to be working properly.

I have couple of RAW files in my real iPhone13 Pro device and it does not complete the future and throws error("Cancelled by a second request") next time when I'm about tho pick file.(which was the ultimate problem with this package)

I just simply ran image_picker/plugins/packages/image_picker/image_picker_ios/example/main.dart file
here is logs I get

[claims] grantAccessClaim reply is an error: Error Domain=NSCocoaErrorDomain Code=4097 "Couldn’t communicate with a helper application." UserInfo={NSUnderlyingError=0x283995c50 {Error Domain=NSCocoaErrorDomain Code=4097 "connection from pid 9395 on anonymousListener or serviceListener" UserInfo={NSDebugDescription=connection from pid 9395 on anonymousListener or serviceListener}}}
Could not process image: Error Domain=NSItemProviderErrorDomain Code=-1000 "Cannot load representation of type com.adobe.raw-image" UserInfo={NSLocalizedDescription=Cannot load representation of type com.adobe.raw-image, NSUnderlyingError=0x283995fe0 {Error Domain=NSCocoaErrorDomain Code=4097 "Couldn’t communicate with a helper application." UserInfo={NSUnderlyingError=0x283995c50 {Error Domain=NSCocoaErrorDomain Code=4097 "connection from pid 9395 on anonymousListener or serviceListener" UserInfo={NSDebugDescription=connection from pid 9395 on anonymousListener or serviceListener}}}}}

@LabN36
Copy link

LabN36 commented Dec 16, 2022

Also strangely it does work on some of RAW the file but for some RAW files it does.
FYI: I've ran on iOS 16.1.3 iPhone 13 Pro

Here are the sample files which doesn't seems to be working
https://drive.google.com/drive/folders/18S-5pMnJvQhAKpsuwabDCjoxhsk1RR_g?usp=share_link

File Specs
31.6Mb RAW Files
Wide Camera - 26 mm f1.5
ISO 100 | 26mm | 0 ev | f1.5 | 1/121 s

@jmagman
Copy link
Member

jmagman commented Dec 16, 2022

I have couple of RAW files in my real iPhone13 Pro device and it does not complete the future and throws error("Cancelled by a second request") next time when I'm about tho pick file.(which was the ultimate problem with this package)

The canceled by a second request issue is tracked with flutter/flutter#82602, which I'm working on now. However the underlying problem is that it can't load the image:

Could not process image: Error Domain=NSItemProviderErrorDomain Code=-1000 "Cannot load representation of type com.adobe.raw-image" UserInfo={NSLocalizedDescription=Cannot load representation of type com.adobe.raw-image, NSUnderlyingError=0x283995fe0 {Error Domain=NSCocoaErrorDomain Code=4097 "Couldn’t communicate with a helper application."

There are other Apple dev threads complaining about the same: https://developer.apple.com/forums/thread/672379 and https://developer.apple.com/forums/thread/661834.

I wonder if trying again works, or if it perma-fails for a given image once it's decided it can't be accessed.

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 16, 2022
@auto-submit auto-submit bot merged commit 32dcbf3 into flutter:main Dec 16, 2022
@jmagman
Copy link
Member

jmagman commented Dec 16, 2022

Talked with @vashworth about this one, we decided to merge even with the inconsistent selection of the RAW files since this change is still better than what we had before--it's supporting a lot more image formats. Plus there are reports of this happening on Apple developer forums which makes me think the Cannot load representation of type is an iOS bug. Asked @vashworth to file a new issue for that to see if we can come up with a workaround.

@vashworth
Copy link
Contributor Author

As @jmagman mentioned we decided to merge this. I was able to reproduce the issue @LabN36 had, and created a new issue for it with my findings: flutter/flutter#117240

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 19, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 19, 2022
* 5f62d21eb [local_auth] Fix failed biometric authentication not throwing error (flutter/plugins#6821)

* ca974ab0c [webview_flutter_web] Copies web implementation of webview_flutter from v4_webview (flutter/plugins#6854)

* 4d11be416 [image_picker] Don't store null paths in lost cache (flutter/plugins#6678)

* fd2841fd0 [webview_flutter_android] Fix timeouts in the integration tests (flutter/plugins#6857)

* abc9f9a9b [flutter_plugin_tools] If `clang-format` does not run, fall back to other executables in PATH (flutter/plugins#6853)

* 7efb5e89d [video_player] Add compatibility with the current platform interface (flutter/plugins#6855)

* 32dcbf3e3 [image_picker] Improve image_picker for iOS to handle more image types (flutter/plugins#6812)

* 840a04954 [webview_flutter] Copies app-facing implementation of webview_flutter from v4_webview (flutter/plugins#6856)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…#117314)

* 5f62d21eb [local_auth] Fix failed biometric authentication not throwing error (flutter/plugins#6821)

* ca974ab0c [webview_flutter_web] Copies web implementation of webview_flutter from v4_webview (flutter/plugins#6854)

* 4d11be416 [image_picker] Don't store null paths in lost cache (flutter/plugins#6678)

* fd2841fd0 [webview_flutter_android] Fix timeouts in the integration tests (flutter/plugins#6857)

* abc9f9a9b [flutter_plugin_tools] If `clang-format` does not run, fall back to other executables in PATH (flutter/plugins#6853)

* 7efb5e89d [video_player] Add compatibility with the current platform interface (flutter/plugins#6855)

* 32dcbf3e3 [image_picker] Improve image_picker for iOS to handle more image types (flutter/plugins#6812)

* 840a04954 [webview_flutter] Copies app-facing implementation of webview_flutter from v4_webview (flutter/plugins#6856)
mauricioluz pushed a commit to mauricioluz/plugins that referenced this pull request Jan 26, 2023
flutter#6812)

* Improve image picker for ios to handle more image types

* Update release info

* different svg, remove raw test

* change pro raw image

* change pro raw image

* add error log

* fix formatting

* fix image identifiers in test

* get image type identifier from itemProvider in test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: image_picker platform-ios
Projects
None yet
5 participants