Skip to content

[image_picker] updates to resize logic.#5527

Merged
auto-submit[bot] merged 11 commits intoflutter:mainfrom
tarrinneal:ip-resize-hw
Dec 12, 2023
Merged

[image_picker] updates to resize logic.#5527
auto-submit[bot] merged 11 commits intoflutter:mainfrom
tarrinneal:ip-resize-hw

Conversation

@tarrinneal
Copy link
Contributor

@tarrinneal tarrinneal commented Nov 30, 2023 •
edited
Loading

Simplifies resizing logic on ios and android.
Also fixes a bug with resizing flutter/flutter#88901.
Also changes rounding to be closer to original aspect ratio (and consistent across platforms)

fixes flutter/flutter#88901

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/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package 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.

@tarrinneal tarrinneal changed the title Ip resize hw [image_picker] updates to resize logic. Dec 6, 2023
@tarrinneal tarrinneal marked this pull request as ready for review December 6, 2023 18:59
vashworth
vashworth approved these changes Dec 12, 2023
View reviewed changes
Copy link
Contributor

@vashworth vashworth left a comment

Choose a reason for hiding this comment

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

iOS side LGTM.

Suggestion: You could make the width / height calculations into its own method that takes in the originalWidth and originalHeight as arguments. That way you could test the method without needing actual images. I think what you have is fine too, though.

@tarrinneal
Copy link
Contributor Author

tarrinneal commented Dec 12, 2023

Suggestion: You could make the width / height calculations into its own method that takes in the originalWidth and originalHeight as arguments. That way you could test the method without needing actual images. I think what you have is fine too, though.

That's not a bad idea. I think I'll let the tests stay as they are, since having a more robust "integration" type test is probably good anyway.

stuartmorgan-g
stuartmorgan-g approved these changes Dec 12, 2023
View reviewed changes
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

LGTM with small comments

@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 12, 2023
@auto-submit auto-submit bot merged commit 92bbf65 into flutter:main Dec 12, 2023
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll
92bbf65 [image_picker] updates to resize logic. (flutter/packages#5527)
83d26c7
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 13, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll
92bbf65 [image_picker] updates to resize logic. (flutter/packages#5527)
824ee91
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 13, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll
92bbf65 [image_picker] updates to resize logic. (flutter/packages#5527)
c61523d
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll
Roll Packages from 80aa46a to b5958e2 (10 revisions) (#140069)
3751005 
flutter/packages@80aa46a...b5958e2

2023-12-13 [email protected] [camera, camera_android] Re-enable passing integration tests (flutter/packages#5658)
2023-12-13 [email protected] Roll Flutter from 9719097 to 11a9cb7 (32 revisions) (flutter/packages#5665)
2023-12-13 [email protected] [url_launcher] Return false on Windows when there is no handler (flutter/packages#5359)
2023-12-13 [email protected] [url_launcher] Simplify Linux implementation (flutter/packages#5376)
2023-12-12 49699333+dependabot[bot]@users.noreply.github.com [local_auth]: Bump androidx.fragment:fragment from 1.6.1 to 1.6.2 in /packages/local_auth/local_auth_android/android (flutter/packages#5332)
2023-12-12 [email protected] [pigeon] Adds @CanIgnoreReturnValue annotation (flutter/packages#5601)
2023-12-12 [email protected] [image_picker] updates to resize logic. (flutter/packages#5527)
2023-12-12 [email protected] [various] Update examples using video_player (flutter/packages#5653)
2023-12-12 [email protected] [tools] Validate pubspec topic format (flutter/packages#5565)
2023-12-12 [email protected] [pigeon] Fix Kotlin generator to use provided errorClassName (flutter/packages#5480)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
Closed
@reidbaker reidbaker mentioned this pull request Jan 19, 2024
Merged
12 tasks
@huycozy huycozy mentioned this pull request Jan 23, 2024
Closed
@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
@jmagman jmagman mentioned this pull request Mar 7, 2024
Closed
@mark8044
Copy link

mark8044 commented Jun 8, 2024

@tarrinneal For whatever reason, this fix doesn't seem to work with images within the gallery that came from a camera photo. This does work perfectly from a photo saved from the web.

A part of me wonders if this has to do with exif metadata and images being landscape vs portrait. I know this was a problem for some time with uploading images to PHP servers which was later fixed in PHP7.x

Scenario 1:

  • Take a photo in portrait orientation (normal upright holding of the phone) with large MP size. For example if I snap a photo from a Samsung S21 it saves the image at 3000x4000 pixels

  • final List<XFile>? images = await _picker.pickMultiImage(maxWidth: 4032, maxHeight:3024, requestFullMetadata: false);

    DOES NOT WORK. This will return an image sized: 3000x4000

Scenario 2:

  • Save a photo that's 3000x4000 pixels to your gallery: https://picsum.photos/3000/4000

  • final List<XFile>? images = await _picker.pickMultiImage(maxWidth: 4032, maxHeight:3024, requestFullMetadata: false);

    WORKS. This will return an image sized: 2268x3024

@mark8044 mark8044 mentioned this pull request Jun 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@vashworth vashworth vashworth approved these changes

@gmackall gmackall Awaiting requested review from gmackall gmackall is a code owner

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App p: image_picker platform-android platform-ios

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[image_picker]: maxHeight and maxWidth not working as expected

4 participants

@tarrinneal @mark8044 @stuartmorgan-g @vashworth