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

Replace Ellipse-based Ripple with Composition API #316

Merged
merged 2 commits into from
Nov 18, 2023

Conversation

maxkatz6
Copy link
Contributor

Just an experiment, which potentially can be a nice improvement for low-end devices.
Currently, Ripple effect is implemented with an Ellipse shape animated with multiple transitions on the UI thread.
This PR rewrites this effect to be executed on the Render thread, and UI thread is responsible for starting animation and triggering second step (fading out opacity).

On my local machine with Debug build (to make it slower), I can see animation working way better in navigation buttons, but not really different on normal buttons. It must be because navigation buttons are loading new pages on click, blocking UI thread for a moment - that's where Render thread animations work better, as they can be rendered and computed in parallel.

Breaking changes:

  • Ripple class is not an Ellipse, and is a static class that allows some Easing and Duration customizations (like it was before).
  • There is no canvas in the default template (minor)

I also moved default properties from setters to the styled property definitions. Ideally, I would also avoid "^:not(.no-transitions)" selector, as it's going to be applied on each button, adding some styling overhead. But that's not in the scope of this PR.

Math in this PR is a bit off, and the ripple effect doesn't fill whole parent. I guess the old code had Margin transition for that, but I haven't dive too deep.

@maxkatz6
Copy link
Contributor Author

Before:

demo1.mp4

After:

demo2.mp4

@JanTamis
Copy link
Collaborator

wow really nice!

@SKProCH SKProCH self-assigned this Nov 16, 2023
@SKProCH SKProCH marked this pull request as ready for review November 16, 2023 21:28
@SKProCH SKProCH self-requested a review November 16, 2023 21:36
Copy link
Collaborator

@SKProCH SKProCH left a comment

Choose a reason for hiding this comment

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

Look fine for me, but i should investigate how to make it more simular to current implementation.

@SKProCH SKProCH merged commit fe86323 into AvaloniaCommunity:master Nov 18, 2023
@maxkatz6 maxkatz6 deleted the rippleRework branch November 29, 2023 00:48
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.

3 participants