[image_picker_for_web] Added image resize functionality #3439

jesusrp98 · 2021-01-17T20:02:35Z

Added image resize functionality to the web implementation of image_picker to make it par with the mobile implementation of the plugin.

This implementation comes from @postacik, detailed here on #2802.
Huge thanks to @ditman as well for all the help!

This SHOULD NOT be a breaking change.

Also this is my first PR here so bare with me :)

Pre-launch Checklist

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

google-cla · 2021-01-17T20:02:41Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

jesusrp98 · 2021-01-17T20:03:50Z

@googlebot I signed it!

ditman

This needs some tests (here), also, I made a couple of comments in the general approach.

Have you verified this works with the example app of the plugin in web?

packages/image_picker/image_picker_for_web/lib/image_picker_for_web.dart

ditman · 2021-01-19T21:50:15Z

packages/image_picker/image_picker_for_web/lib/image_picker_for_web.dart

+    int imageQuality,
+  ) {
+    final completer = Completer<String>();
+    final img = html.ImageElement();


Don't you need to add this img to the DOM for the events to fire?

I'm haven't had many experience of developing flutter web plugins and I don't know how to proceed here.
Could you elaborate on this @ditman please?

Hm, some times for some DOM elements to fire their load/error events they must be injected into the markup of the page (anywhere, but they need to be injected).

I'm not sure this is the case, but I'd test this code across all main browsers (Edge/Safari/Firefox/Chrome) to ensure that the on*** events of the ImageElement are firing as expected.

Same approach for the comment below where we're resetting the image to something blank, and then the desired source again :)

@ditman , you are right, events are generally fired if the element is added to DOM tree but while converting my JavaScript code to Dart, I realized that it worked without adding it to DOM for Chrome. I haven't tested for other mentioned browsers.

OK I understand what you're saying here, but I don't know how to exactly add this element to the DOM tree :/

I'd need some help here @postacik @ditman .

ditman · 2021-01-19T21:52:27Z

packages/image_picker/image_picker_for_web/lib/image_picker_for_web.dart

+        ratio = maxWidth / img.height;
+      }
+      if (img.width > img.height) {
+        if (img.width < maxHeight) ratio = maxHeight / img.width;


This is odd, comparing a width to a height. I think there's some typos in this algo to determine the aspect ratio / scale factor (?)

I'll take a look a the proper algorithm here, maybe I can refine this.

@ditman , the scaling algorithm was just a copy paste from an old JavaScript project. I really don't recall how I constructed it or tested it thoroughly.

The best algorithm for the resize function would be using the algorithm used in the mobile counter part. Maybe the algorithm may be copied from the following Java file so that it will be consistent with the mobile version.

plugins/packages/image_picker/image_picker/android/src/main/java/io/flutter/plugins/imagepicker/ImageResizer.java

Line 56 in 6783cd5

private File resizedImage(

I agree with you @postacik. I'll update the current algorith to resemble something similar to the Android implementation.

ditman · 2021-01-19T21:54:04Z

packages/image_picker/image_picker_for_web/lib/image_picker_for_web.dart

+      // Flush cache
+      img.src =
+          'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///ywAAAAAAQABAAACAUwAOw==';


Will this trigger the onLoad event above? If so, you might want to check for this special image to do nothing with it; otherwise you'll attempt to complete the completer several times, and it'll throw an exception (you can only complete a completer once)

Yes it's a bit weird here and I don't really understand what's going on.
Maybe @postacik could help us here (?).

@ditman, you are right, if it does trigger the onload event several times, it would cause exceptions. The JavaScript code I used was not using a Promise (or Completer) but simply copying the result to the canvas so the hack to avoid cases where onload did not fire did not have any side effects but here it would. The base64 image is a 1x1 gif. Maybe the completer may be avoided if the loaded image size is 1x1.

Added a simple img.width > 1 && img.height > 1 check before performing the resize logic.
Give me your feedback here!

jesusrp98 · 2021-01-22T11:57:22Z

Sorry @ditman. The first commit was just copying and pasting the resize function to the code base. I'll continue with the work now and implement all your feedback. I'm really thankful for that because I haven't really tinkered with flutter web plugins before :)

ditman · 2021-01-23T01:23:40Z

Haha thanks for the clarification @jesusrp98! I didn't realize this was a Draft PR 🤦 :P

Hixie · 2021-10-12T23:18:17Z

@jesusrp98 I'm going to close this PR for now since it's not ready for landing but please don't hesitate to reopen it (or submit a new one) if you are still working on this. Thanks!

ditman · 2021-10-14T19:37:50Z

Coincidentally, this is being worked on here as of late: #4389

Initial implementation

f1c4680

google-cla bot added the cla: no label Jan 17, 2021

google-cla bot added cla: yes and removed cla: no labels Jan 17, 2021

ditman reviewed Jan 19, 2021

View reviewed changes

Applied several suggestions to implementation

4e828e5

jesusrp98 requested a review from ditman January 22, 2021 14:11

jesusrp98 added 2 commits January 25, 2021 19:47

Improved algorithm

3d84e18

Added a new completer check

0b9057c

stuartmorgan-g added the p: image_picker label Jan 29, 2021

Hixie closed this Oct 12, 2021

github-actions bot added the platform-web label Oct 12, 2021

[image_picker_for_web] Added image resize functionality #3439

[image_picker_for_web] Added image resize functionality #3439

Uh oh!

Conversation

jesusrp98 commented Jan 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

google-cla bot commented Jan 17, 2021

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

jesusrp98 commented Jan 17, 2021

Uh oh!

ditman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesusrp98 commented Jan 22, 2021

Uh oh!

ditman commented Jan 23, 2021

Uh oh!

Hixie commented Oct 12, 2021

Uh oh!

ditman commented Oct 14, 2021

Uh oh!

Uh oh!

jesusrp98 commented Jan 17, 2021 •

edited

Loading