Skip to content

Conversation

tarrinneal
Copy link
Contributor

@tarrinneal tarrinneal commented Nov 30, 2023

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
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

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.

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

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 13, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 13, 2023
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
@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: image_picker platform-android platform-ios
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[image_picker]: maxHeight and maxWidth not working as expected
4 participants