danyspin97 / wpaperd

Modern wallpaper daemon for Wayland
GNU General Public License v3.0
332 stars 26 forks source link

feat: pause/resume auto wallpaper sequence #68

Closed Rolv-Apneseth closed 5 months ago

Rolv-Apneseth commented 5 months ago

Hey again. This is in relation to #48.

What do you think of this fairly basic approach? I can take a whole different one if you prefer / have any suggestions.

Edit: This should also fix the current issue with the Cargo.lock file as it appears it wasn't updated

danyspin97 commented 5 months ago

Hey @Rolv-Apneseth, I am always happy to see a PR from you :)

The basic approach is fine, but I'd like to entirely remove the timer event, so that wpaperd doesn't wake up. This might be especially good for energy saving scenarios. Implementing this approach requires playing with the event queue handle in the main loop, but it shouldn't be too complex either. The event_source in Surface could become an enum like this:

enum Timer {
   Running(RegistrationToken),
   NotSet,
   // the time passed after the last timer sent an event
   // so that we might start the timer with this duration
   Paused(Duration), 
}

We still need paused property (or rather to_pause) so that the main loop knows which surfaces to modify (the event queue handle is only accesible from the main loop).

Take these suggestions as ideas, it might be possible I am missing some details and that the implementation might be different. What do you think about this approach?

Rolv-Apneseth commented 5 months ago

Sure yeah, I'll have a look at following this approach and come back if I have any issues/questions, thanks

Rolv-Apneseth commented 5 months ago

Ok - let me know what you think as I'm not too sure I took the correct approach. Also let me know if I should add any docs to the README or anywhere else

danyspin97 commented 5 months ago

The PR looks good to me, thank you for adding this feature!

Rolv-Apneseth commented 5 months ago

Nice, and no problem.

Rolv-Apneseth commented 5 months ago

Can #48 be closed or would you also like a toggle rather than just pause/resume?

danyspin97 commented 5 months ago

I think a toggle would be helpful, especially for people that uses keybindings for controlling wpaperd behavior, especially since its implementation is straightforward.

Rolv-Apneseth commented 5 months ago

Alright sure, I'll do a quick PR when I get the time then. Just adding an option to toggle right, not replacing the pause/resume ones?

danyspin97 commented 5 months ago

That would be perfect, thank you! Yes, just a toggle command.