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

[camera] android-rework part 10: fixed final orientation issues. #4153

Closed
wants to merge 81 commits into from

Conversation

BeMacized
Copy link
Contributor

This PR fixes some regressions regarding setting the focus point, as well as some other related issues regarding orientation:

  • Rotation of the preview widget now stays in sync with the UI orientation.
  • Setting the focus point now properly triggers the autofocus sequence.
  • Coordinates for setting the focus- and exposure point now properly rotate according to the ui orientation.

This is the final tenth PR in a series of pull-requests which will gradually introduce changes from PR #3651, making it easier to review the code (as discussed with @stuartmorgan).

As of right now, this PR will still be kept in draft mode, as it also contains changes it depends on from the previous parts. Once those have been merged and any changes from those have been merged back into this PR, the draft status will be removed.

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]. (Note that 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].
    • Although these PRs technically did not introduce a breaking change, it likely still warrants a major version bump due to the scale of this refactor. (See here)
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

mvanbeusekom and others added 30 commits April 20, 2021 09:35
…nsor_features' into camera-android/supporting_functionality
@google-cla
Copy link

google-cla bot commented Jul 13, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Jul 13, 2021
@stuartmorgan
Copy link
Contributor

This PR fixes some regressions

Can you elaborate on what these regressions are from? Earlier PRs in the series?

If so, why are we landing the switch with known regressions and then fixing them, rather than fixing the in-progress PR?

@BeMacized
Copy link
Contributor Author

BeMacized commented Jul 13, 2021

Can you elaborate on what these regressions are from? Earlier PRs in the series?

Setting the focus point would retrigger the autofocus sequence before the android refactor, but no longer in part 9.
As for the other issues listed, I am not fully sure if they were present before the refactor, but they had to be fixed regardless.

If so, why are we landing the switch with known regressions and then fixing them, rather than fixing the in-progress PR?

As mentioned in part 9, after discussion with @mvanbeusekom, to keep the size of the PRs down so they're easier to review. Other than that, there is no consideration. Part 9 and 10 might as well be a single PR as far as I'm concerned.

EDIT: If they're both considered small enough I can close this PR and just merge these changes into the part 9 PR.

@stuartmorgan
Copy link
Contributor

Setting the focus point would retrigger the autofocus sequence before the android refactor, but no longer in part 9.

We should not deliberately land known regressions.

As mentioned in part 9, after discussion with @mvanbeusekom, to keep the size of the PRs down so they're easier to review. Other than that, there is no consideration. Part 9 and 10 might as well be a single PR as far as I'm concerned.

I'm not sure what the combined size is, but if it's too big with the regression fixed we should see if there's another way to cut it.

@BeMacized
Copy link
Contributor Author

Closing this in favour of being merged with #4059 instead.

@BeMacized BeMacized closed this Jul 13, 2021
@mvanbeusekom mvanbeusekom deleted the fix/camera-orientation branch September 21, 2021 09:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants