Skip to content

Conversation

@nikoloc
Copy link

@nikoloc nikoloc commented Mar 27, 2025

idea here is that this is the logic thats going to be used by compositors anyway, and having these helpers will remove all this boilerplate.
compositor maintainers are free to implement them in any way they want (with output frame events or with timers for example), and just call appropriate functions to start, update and destroy them.

todos

  • add documentation in the header
  • add the example implementation to tinywl
    • timers example
    • output frame events example

i dont know how would i go about adding both examples to tinywl tho, so maybe i could just add the first option, as thats what mwc does and what i am familiar with.

@nikoloc nikoloc changed the title added some bacis stuff for animations added some basic stuff for animations Mar 27, 2025
@WillPower3309
Copy link
Member

I like the idea of providing some help to compositors for animations. I think the timers example would be more than adequate for tinywl!

@WillPower3309
Copy link
Member

WillPower3309 commented Apr 1, 2025

What are your thoughts on allowing compositors to add their own update and complete function pointers? I'd think complete would be required. We can then remove the finished boolean, and this would let us keep as much of the animation structs as private as possible, to ensure compositors won't mess around with the process. We could even manage the animation timer itself and enforce that as the preferred way of implementing animations, unless there are distinct advantages to the output frame events method (I'd assume compositors would want as little latency in that path as possible).

@nikoloc
Copy link
Author

nikoloc commented Apr 1, 2025

i have greatly improved on the initial idea; one of the things i did was add timers by default with user callbacks. i have been doing it locally in mwc, but forgot to push the code yesterday, so hopefully i will finish it today so we have more ground for discussion.

@nikoloc
Copy link
Author

nikoloc commented Apr 1, 2025

What are your thoughts on allowing compositors to add their own update and complete function pointers? I'd think complete would be required. We can then remove the finished boolean, and this would let us keep as much of the animation structs as private as possible, to ensure compositors won't mess around with the process. We could even manage the animation timer itself and enforce that as the preferred way of implementing animations, unless there are distinct advantages to the output frame events method (I'd assume compositors would want as little latency in that path as possible).

you edited the response in the meantime, but i have already mostly answered things here.

@nikoloc
Copy link
Author

nikoloc commented Apr 11, 2025

this should be done now, i have also added an example to tinywl :)

Copy link
Member

@ErikReider ErikReider left a comment

Choose a reason for hiding this comment

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

Also, please use tabs instead of spaces.

I don't think we need an output frame events example :)

Otherwise, this LGTM


/* we dont take the commits from a client if its animating currently.
* this should be handled better, but for showing animations off this is fine */
if(toplevel->popin_animation) return;
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Space between if and (

}

struct fx_animation_curve {
double params[4];
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to know what these 4 parameters are, and what they do as a comment?

https://developer.mozilla.org/en-US/docs/Web/CSS/easing-function/cubic-bezier#parameters

@nikoloc
Copy link
Author

nikoloc commented May 5, 2025

i guess a .clang-format would be nice to have, but i will format the code manually.

but on the more important note, we may want to also add 'general purpose' animations (struct fx_animation) that would just provide current factor in the callback for things like fade-in/out.

what are your thought on it?

@nikoloc
Copy link
Author

nikoloc commented May 5, 2025

formatting issues should be resolved. i have also added some resources for bezier curves, but in a header file instead, as it seemed better that way.

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