Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[camera] Fix immediate camera animation on API level 23 or below - MAPSAND-614 #1842

Merged
merged 12 commits into from
Nov 17, 2022

Conversation

pengdev
Copy link
Member

@pengdev pengdev commented Nov 15, 2022

Summary of changes

This PR bypasses the immediate camera animation(where duration and start delay of the animation is 0) on the animator, and emit a single animator update instead of trying to start the ValueAnimator directly.

More context on the issue with animators on API 23 or below:

Devices with API <= 23 animating with duration = 0 will send initial value from Looper with some small delay as a result.
Hence multiple immediate animations sent close to each other may cancel previous ones.

User impact (optional)

This change particularly affects the camera animation and viewport plugin on API level 23 or below.
For example viewport plugin's FollowPuckViewportState behaviour will be fixed as shown below:

Before After
before.webm
after.webm

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Run make update-api to update generated api files, if there's public API changes, otherwise the verify-api-* CI steps might fail.
  • Update CHANGELOG.md or use the label 'skip changelog', otherwise check changelog CI step will fail.
  • If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from 481fdef to 1135b39 Compare November 15, 2022 16:35
@pengdev pengdev self-assigned this Nov 15, 2022
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch 2 times, most recently from 6558584 to 4ae85bb Compare November 16, 2022 09:50
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from 4ae85bb to e6a4060 Compare November 16, 2022 10:25
@pengdev pengdev changed the title [camera] Fix immediate camera animation on API level 23. [camera] Fix immediate camera animation on API level 23 or below. Nov 16, 2022
@kiryldz
Copy link
Contributor

kiryldz commented Nov 16, 2022

@pengdev can you try running tests locally on API = 23 device? we ignore them now as per

private fun ignoreTestForGivenAbi() = Build.VERSION.SDK_INT <= Build.VERSION_CODES.M

@pengdev
Copy link
Member Author

pengdev commented Nov 16, 2022

can you try running tests locally on API = 23 device? we ignore them now as per

@kiryldz I tried it, still testEaseToStartDelay and testEaseToStartDelayCancelled are failing, others are passing. Also I've tested the result is the same in the main branch.

Given the scope of this PR only helps immediate animation with both duration and startDelay is set to 0. Probably we should add separate tests this use case. I will try to improve the tests as well.

@pengdev pengdev changed the title [camera] Fix immediate camera animation on API level 23 or below. [camera] Fix immediate camera animation on API level 23 or below - MAPSAND-614 Nov 16, 2022
@pengdev pengdev marked this pull request as ready for review November 16, 2022 13:50
@pengdev pengdev requested review from a team as code owners November 16, 2022 13:50
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from 9835ab1 to c6f7fe9 Compare November 16, 2022 14:33
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from de277a8 to 5ceaf31 Compare November 17, 2022 09:32
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch 2 times, most recently from 199d60d to 05b59cc Compare November 17, 2022 10:32
.circleci/config.yml Outdated Show resolved Hide resolved
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from 05b59cc to a7f04ba Compare November 17, 2022 14:20
@pengdev pengdev force-pushed the peng-fix-immediate-camera-animator branch from a7f04ba to 8ecbfaa Compare November 17, 2022 14:25
@pengdev pengdev merged commit 2a3b4d4 into main Nov 17, 2022
@pengdev pengdev deleted the peng-fix-immediate-camera-animator branch November 17, 2022 15:33
pengdev added a commit that referenced this pull request Nov 17, 2022
pengdev added a commit that referenced this pull request Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants