[image_picker]fix load error when the image is BMP/HEIF/HEIC format #4665

xiaoxiaowesley · 2022-01-13T03:04:51Z

This PR attends to fix the image_picker no response error when picking the BMP images greater than iOS 14 in iOS simulator or iOS devices.

The reason why did not respond to the BMP image is that when picking the BMP format image the process step in FLTPHPickerSaveImageToPathOperation.mm will return nil in completionHandler and respond nothing back to dart.

The BMP image in videos is used from https://people.math.sc.edu/Burkardt/data/bmp/bmp_24.bmp in flutter/issues/95885

default.mov

This only happens in iOS 14 or greater version.

Also, reference in the flutter repo flutter/flutter#95885

Device info:iPhone13 iOS 15.0

After fix

default.mov

Pre-launch Checklist

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

flutter-dashboard · 2022-01-13T03:04:53Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

packages/image_picker/image_picker/CHANGELOG.md

cyanglaz · 2022-01-13T18:09:38Z

@xiaoxiaowesley Thanks preparing this PR! This PR will need tests to move forward :)

stuartmorgan · 2022-01-13T21:12:15Z

Thanks for the submission!

In the future, please do not delete the checklist that is in the PR template; it is there for a reason. This PR is missing required elements described in the checklist (I’ve restored it to the PR description), which need to be addressed before it moves forward with review. If you have any questions about completing those steps that aren’t addressed documentation, please let me know.

stuartmorgan · 2022-01-13T21:13:49Z

It looks like this will collide with #4448, so it would make sense to land that first then re-evaluate here.

xiaoxiaowesley · 2022-01-14T01:56:48Z

Thanks for the submission!

In the future, please do not delete the checklist that is in the PR template; it is there for a reason. This PR is missing required elements described in the checklist (I’ve restored it to the PR description), which need to be addressed before it moves forward with review. If you have any questions about completing those steps that aren’t addressed documentation, please let me know.

OK. This is my first PR to Plugins.No quite sure about the rule. Thank you for letting me know.

xiaoxiaowesley · 2022-01-14T08:11:56Z

It looks like this will collide with #4448, so it would make sense to land that first then re-evaluate here.
YES. It would conflict with it. And I will wait for #4448

stuartmorgan · 2022-02-10T21:25:58Z

Status from PR review: Still blocked on #4448, but that one is close.

stuartmorgan · 2022-02-28T17:20:20Z

#4448 has landed, so you can update this against the latest trunk and address the remaining items in the PR checklist, then we can review.

xiaoxiaowesley · 2022-03-01T02:23:32Z

#4448 has landed, so you can update this against the latest trunk and address the remaining items in the PR checklist, then we can review.

Great.On my way

flutter-dashboard · 2022-03-11T08:20:16Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

xiaoxiaowesley · 2022-03-14T01:45:36Z

#4448 has landed, so you can update this against the latest trunk and address the remaining items in the PR checklist, then we can review.

@stuartmorgan hi, I had added tests, and the checks all passed but remain one failed.
The failed check showed
android-platform_tests CHANNEL:master PLUGIN_SHARDING:--shardIndex 4 --shardCount 5: completed (failure)
I just modified iOS code, why show Android test failed? iI will be very happy if you can help me .

stuartmorgan · 2022-03-14T20:21:04Z

submit-queue is the live tree status; it's unrelated to your PR.

xiaoxiaowesley · 2022-03-19T03:47:10Z

@cyanglaz hi, It seems no problem now. Would you help me to review it?

cyanglaz · 2022-03-22T18:10:17Z

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

+                                    if (error == nil) {
+                                      __block UIImage *localImage =
+                                          [[UIImage alloc] initWithData:data];
+                                      PHAsset *originalAsset = [FLTImagePickerPhotoAssetUtil
+                                          getAssetFromPHPickerResult:self.result];
+
+                                      if (self.maxWidth != (id)[NSNull null] ||
+                                          self.maxHeight != (id)[NSNull null]) {
+                                        localImage = [FLTImagePickerImageUtil
+                                                    scaledImage:localImage
+                                                       maxWidth:self.maxWidth
+                                                      maxHeight:self.maxHeight
+                                            isMetadataAvailable:originalAsset != nil];
+                                      }
+                                      __block NSString *savedPath;
+                                      if (!originalAsset) {
+                                        // Image picked without an original asset (e.g. User pick
+                                        // image without permission)
+                                        savedPath = [FLTImagePickerPhotoAssetUtil
+                                            saveImageWithPickerInfo:nil
+                                                              image:localImage
+                                                       imageQuality:self.desiredImageQuality];
+                                        [self completeOperationWithPath:savedPath];
+                                      } else {
+                                        [[PHImageManager defaultManager]
+                                            requestImageDataForAsset:originalAsset
+                                                             options:nil
+                                                       resultHandler:^(
+                                                           NSData *_Nullable imageData,
+                                                           NSString *_Nullable dataUTI,
+                                                           UIImageOrientation orientation,
+                                                           NSDictionary *_Nullable info) {
+                                                         // maxWidth and maxHeight are used only for
+                                                         // GIF images.
+                                                         savedPath = [FLTImagePickerPhotoAssetUtil
+                                                             saveImageWithOriginalImageData:
+                                                                 imageData
+                                                                                      image:
+                                                                                          localImage
+                                                                                   maxWidth:
+                                                                                       self.maxWidth
+                                                                                  maxHeight:
+                                                                                      self.maxHeight
+                                                                               imageQuality:
+                                                                                   self.desiredImageQuality];
+                                                         [self completeOperationWithPath:savedPath];
+                                                       }];
+                                      }
+                                    } else {
+                                      [self completeOperationWithPath:nil];
+                                    }


Can this be replaced wtih [self processImage:image];?

ok。I modified it.

cyanglaz

LGTM % nits.
@stuartmorgan as a second reviewer.

packages/image_picker/image_picker/CHANGELOG.md

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

stuartmorgan

You'll need to update this for the federation of the iOS implementation, but it should be straightforward; all the changes just need to be to image_picker_ios instead.

stuartmorgan · 2022-04-11T20:25:56Z

packages/image_picker/image_picker/CHANGELOG.md

+
+## 0.8.4+12
+
+* iOS：support images with heic,heif,raw,tiff formats.


Suggested change

* iOS：support images with heic,heif,raw,tiff formats.

* Adds support for HEIC, HEIF, RAW, and TIFF formats on iOS.

stuartmorgan · 2022-04-11T20:40:20Z

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

@@ -4,8 +4,8 @@

 #import <UniformTypeIdentifiers/UniformTypeIdentifiers.h>

+#import <UniformTypeIdentifiers/UTCoreTypes.h>


This needs a blank line after it; they are different import sections.

stuartmorgan · 2022-04-11T20:40:27Z

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

 #import "FLTPHPickerSaveImageToPathOperation.h"
-


Please undo this change.

stuartmorgan · 2022-04-11T20:42:21Z

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

+      UTTypeHEIC.identifier,
+      UTTypeJPEG.identifier,
+      UTTypeWebP.identifier,
+      UTTypeGIF.identifier,


Why are we changing the codepath for types that already worked? Is this a preferred API?

stuartmorgan · 2022-04-11T20:44:35Z

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m

-                                  [self processImage:image];
-                                }];
-      return;
+    NSArray *supportedRepresentations = @[


"Supported" seems misleading here; this isn't an exhaustive list since there's the other codepath as well. What is distinct about these vs other types that makes them need to use a different code path (so that we can name the list accordingly)?

stuartmorgan · 2022-05-05T20:41:49Z

Are you still planning on updating this PR to address the review comments above?

stuartmorgan · 2022-05-19T20:24:23Z

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to submit a new PR if you have the time to address the review comments. Thanks!

fix load error when the image is BMP format

04e2e82

xiaoxiaowesley requested a review from cyanglaz as a code owner January 13, 2022 03:04

flutter-dashboard bot added the needs tests label Jan 13, 2022

github-actions bot added p: image_picker platform-ios labels Jan 13, 2022

update no response when picking BMP CHANGELOG

fefbe88

cyanglaz reviewed Jan 13, 2022

View reviewed changes

packages/image_picker/image_picker/CHANGELOG.md Outdated Show resolved Hide resolved

xiaoxiaowesley changed the title ~~fix load error when the image is BMP format~~ [image_picker]fix load error when the image is BMP format Jan 14, 2022

fix CHANGELOG.md mistake

4063085

xiaoxiaowesley and others added 4 commits March 11, 2022 14:33

Merge branch 'main' into main

a23135e

update CHANGELOG.md

c201bcd

format file

5dc0b50

update yaml version to 0.8.4+12

29ce9b1

xiaoxiaowesley changed the title ~~[image_picker]fix load error when the image is BMP format~~ [image_picker]fix load error when the image is BMP/HEIF/HEIC format Mar 11, 2022

xiaoxiaowesley added 2 commits March 11, 2022 17:01

remove useless space

0d4d2b7

add heif,heic,raw,tiff image and unit test

e2a8fd6

xiaoxiaowesley force-pushed the main branch from e3d4494 to e2a8fd6 Compare March 13, 2022 14:24

xiaoxiaowesley requested a review from cyanglaz March 13, 2022 15:20

cyanglaz reviewed Mar 22, 2022

View reviewed changes

replace long process with processImage method

36da2b6

cyanglaz approved these changes Mar 23, 2022

View reviewed changes

packages/image_picker/image_picker/CHANGELOG.md Outdated Show resolved Hide resolved

packages/image_picker/image_picker/ios/Classes/FLTPHPickerSaveImageToPathOperation.m Outdated Show resolved Hide resolved

cyanglaz requested a review from stuartmorgan March 23, 2022 18:34

xiaoxiaowesley added 2 commits March 24, 2022 09:55

specify what new formats support

d130454

revert blank line

e584c96

stuartmorgan suggested changes Apr 11, 2022

View reviewed changes

stuartmorgan closed this May 19, 2022

stuartmorgan mentioned this pull request May 19, 2022

[image_picker] No response when picking a BMP image on iOS flutter/flutter#95885

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[image_picker]fix load error when the image is BMP/HEIF/HEIC format #4665

[image_picker]fix load error when the image is BMP/HEIF/HEIC format #4665

xiaoxiaowesley commented Jan 13, 2022 •

edited

Loading

flutter-dashboard bot commented Jan 13, 2022

cyanglaz commented Jan 13, 2022

stuartmorgan commented Jan 13, 2022

stuartmorgan commented Jan 13, 2022

xiaoxiaowesley commented Jan 14, 2022

xiaoxiaowesley commented Jan 14, 2022

stuartmorgan commented Feb 10, 2022

stuartmorgan commented Feb 28, 2022

xiaoxiaowesley commented Mar 1, 2022

flutter-dashboard bot commented Mar 11, 2022

xiaoxiaowesley commented Mar 14, 2022

stuartmorgan commented Mar 14, 2022

xiaoxiaowesley commented Mar 19, 2022

cyanglaz Mar 22, 2022

xiaoxiaowesley Mar 23, 2022

cyanglaz left a comment

stuartmorgan left a comment

stuartmorgan Apr 11, 2022

stuartmorgan Apr 11, 2022

stuartmorgan Apr 11, 2022

stuartmorgan Apr 11, 2022

stuartmorgan Apr 11, 2022

stuartmorgan commented May 5, 2022

stuartmorgan commented May 19, 2022


		## 0.8.4+12

		* iOS：support images with heic,heif,raw,tiff formats.

	* iOS：support images with heic,heif,raw,tiff formats.
	* Adds support for HEIC, HEIF, RAW, and TIFF formats on iOS.

		@@ -4,8 +4,8 @@

		#import <UniformTypeIdentifiers/UniformTypeIdentifiers.h>

		#import <UniformTypeIdentifiers/UTCoreTypes.h>

[image_picker]fix load error when the image is BMP/HEIF/HEIC format #4665

[image_picker]fix load error when the image is BMP/HEIF/HEIC format #4665

Conversation

xiaoxiaowesley commented Jan 13, 2022 • edited Loading

Pre-launch Checklist

flutter-dashboard bot commented Jan 13, 2022

cyanglaz commented Jan 13, 2022

stuartmorgan commented Jan 13, 2022

stuartmorgan commented Jan 13, 2022

xiaoxiaowesley commented Jan 14, 2022

xiaoxiaowesley commented Jan 14, 2022

stuartmorgan commented Feb 10, 2022

stuartmorgan commented Feb 28, 2022

xiaoxiaowesley commented Mar 1, 2022

flutter-dashboard bot commented Mar 11, 2022

xiaoxiaowesley commented Mar 14, 2022

stuartmorgan commented Mar 14, 2022

xiaoxiaowesley commented Mar 19, 2022

cyanglaz Mar 22, 2022

Choose a reason for hiding this comment

xiaoxiaowesley Mar 23, 2022

Choose a reason for hiding this comment

cyanglaz left a comment

Choose a reason for hiding this comment

stuartmorgan left a comment

Choose a reason for hiding this comment

stuartmorgan Apr 11, 2022

Choose a reason for hiding this comment

stuartmorgan Apr 11, 2022

Choose a reason for hiding this comment

stuartmorgan Apr 11, 2022

Choose a reason for hiding this comment

stuartmorgan Apr 11, 2022

Choose a reason for hiding this comment

stuartmorgan Apr 11, 2022

Choose a reason for hiding this comment

stuartmorgan commented May 5, 2022

stuartmorgan commented May 19, 2022

xiaoxiaowesley commented Jan 13, 2022 •

edited

Loading