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

Less jiggly animation on moveTo #189

Merged
merged 2 commits into from Nov 4, 2020

Conversation

markusressel
Copy link
Collaborator

@markusressel markusressel commented Nov 4, 2020

Solution

This replaces the usage of PropertyValuesHolder.ofObject with two PropertyValuesHolder.ofFloat instances, to work around the weird behavior which is not respecting the set animation interpolator.

This also pretty much eliminates the "wiggly" animation when using moveTo, although it still has problems, like "bashing" into walls due to overpan not being allowed during the animation.

@markusressel
Copy link
Collaborator Author

Maybe it would be a good idea to add a comment about this to the code, since I will certainly forget about this detail in T-10 seconds...

Copy link
Owner

@natario1 natario1 left a comment

Choose a reason for hiding this comment

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

Comment would help! No idea about why this happens.

On a side note, should AbsolutePoint and ScaledPoint be inline classes? I wonder if this would avoid JVM allocations which are especially painful during animations.

@markusressel
Copy link
Collaborator Author

On a side note, should AbsolutePoint and ScaledPoint be inline classes? I wonder if this would avoid JVM allocations which are especially painful during animations.

Probably, however, according to the Kotlin docs:

An inline class must have a single property initialized in the primary constructor.

Which doesn't seem like a very intuitive thing for an object that is meant to describe two values.

@natario1
Copy link
Owner

natario1 commented Nov 4, 2020

Sounds good 😄

@markusressel markusressel merged commit 373da27 into master Nov 4, 2020
@markusressel markusressel deleted the bugfix/#188_animation_duration_fix branch November 4, 2020 23:12
@luisangelsm
Copy link

Hey, I have been testing ZoomLayout and I don't think this problem is actually fixed. If you use the sample app and call zoomLayou.getEngine().moveToCenter you can clearly see how the pan animation is completely wrong, the center of the content moves around during the animation.

There are also hiccups when the content hits the edge of the container, I think they are what @markusressel calls bashing into walls. Enabling overpan doesn't really fix these jumps (and the animation can easily end in a wrong state), after logging the actual pan applied to the view you can see some weirdness in it.

@natario1 @markusressel Any ideas about what is going on? I have been trying to fix the problem(s), any help is really welcome.

@markusressel
Copy link
Collaborator Author

Well, since it was over 2 years ago when I last looked at this, sadly I have no idea 😢 All of the research that I have done has been documented in #188, and it may very well not be trivial to fix this this.

@luisangelsm
Copy link

Thanks for the reply @markusressel

One thing I think it causes problems is trying to fix zoom and pan during the animation (checkBounds and ensurePan) and maybe other calculations inside the pan manager. Before starting the animation the library should calculate a valid end state in advance. Not all zoom and pan combinations can be fulfilled, but given a target zoom and a target pan it should be possible to know exactly the final state, and the animation should happen between those two states allowing any intermediate state even if it is not valid. I am not sure if this is something it can just happen inside MatrixController or if the zoom/pan managers need tweaking.

This is what happens with a zoom + pan animation, it is the actual panX that the ZoomLayout is getting. It is full of bumps before reaching 0, where you can clearly see a hard transition, an then you get a nice curve from there. I think those jumps happen because of the corrections applied during the animation.

x

That hard transition is probably caused by how the gravity affects the position of the content view before it gets bigger than the container view, gravity should not have any effect during the animation and it just needs to be considered when calculating the final state before hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants