animate long press latching

[m0dB]

Animate long press latching (e.g. sync button) by capture the off-state in a pixmap and during the long press latch time, gradually draw less of it.

longpresslatching.mov

[JoergAtGithub]

I just tested this on Windows11, and the buttons appear fluidly animated and make the long press behavior really easier to understand.
And even for me as a experienced user, who is aware about the long press sync button functionality, it simplifies the use, because now I see how long I need to hold the button.

[m0dB]

Great! Happy you like it, and thanks for validating!

[m0dB]

@daschuer @Swiftb0y @ronso0 could you test on linux?

Any objections against this approach?

[acolombier]

Looking nice and smooth on Linux (PopOs 22.04 Wayland)

Kooha-2024-03-23-22-17-07.mp4

[m0dB]

Thanks, Antoine!

[ywwg]

my main note is that this gives every single pushbutton in the UI the latching logic and support objects even if they are never used. Would it be possible to make this a child object of a pushbutton (WPushButtonLatching) that inherits WPushButton and just adds on the new stuff? I think that would also be cleaner from a skin design perspective.

[ywwg]

if I understand correctly, this creates an independent 60fps timer to update this animation. Would it be possible / feasible to connect instead to the global UI update timer? or does it not really matter because it's just updating the position data, and the drawing is done elsewhere?

[m0dB]

I doesn't matter. The overhead of the timer is minimal and it doesn't do the actual drawing, it just triggers the drawing when Qt sees fit. This is the recommended way.

[ronso0]

my main note is that this gives every single pushbutton in the UI the latching logic and support objects even if they are never used.

These objects / connections can be created only if m_leftButtonMode == ControlPushButton::LONGPRESSLATCHING in setup()?

[m0dB]

my main note is that this gives every single pushbutton in the UI the latching logic and support objects even if they are never used. Would it be possible to make this a child object of a pushbutton (WPushButtonLatching) that inherits WPushButton and just adds on the new stuff? I think that would also be cleaner from a skin design perspective.

It would be possible but IMO this would lead to added complexity in the code rather than the opposite. Also, WPushButton already implements several modes / behaviours as it is, so why only isolating this one? The added overhead is minimal.

Also, as far as I remember, the skin parser is not instantiating different classes based on properties anywhere, so it would be strange to add that only for this particular case.

But I see your objection that this does add code to WPushButton that is only used for a very specific use case. What I could do is place the QTimer, the QPixmap, and the latching related code in a class that is a member of WPushButton, reducing the overhead to an std::unique_ptr and some function calls.

[m0dB]

my main note is that this gives every single pushbutton in the UI the latching logic and support objects even if they are never used.

These objects / connections can be created only if m_leftButtonMode == ControlPushButton::LONGPRESSLATCHING in setup()?

Yes.

I have moved the code and objects to nested class WPushButton::LongPressLatching, which is a instantiated as member std::unique<LongPressLatching> m_pLongPressLatching only if m_leftButtonMode == ControlPushButton::LONGPRESSLATCHING.

@fwcd let me know if this addresses your concerns.

[fwcd]

I think you wanted to ping @ywwg? 😄

[m0dB]

Indeed! All those 4 letter usernames! 😅

[ywwg]

I like this refactor! I agree since wpushbutton already does a lot of things it doesn't make sense to create a subclass. thanks.

One more question, is the animation triggered when someone long-presses on a controller? I am guessing right now it only works when initiated with the UI and a mouse. It seems out of scope for this PR to implement this for controllers but we probably want to do that too

[ywwg]

please add an explanatory comment for this new class that summarizes the feature

[m0dB]

Will do.

[m0dB]

Comment added. If you find it's sufficient, could you merge this? Thanks!

[ywwg]

@daschuer is the last outstanding reviewer

[daschuer]

/home/daniel/workspace/mixxx2/src/widget/wpushbutton.h:134:10: error: ‘unique_ptr’ in namespace ‘std’ does not name a template type
  134 |     std::unique_ptr<LongPressLatching> m_pLongPressLatching;
      |          ^~~~~~~~~~

#include <memory> is missing.

[daschuer]

I can confirm it is working nice after adding the include.

[ronso0]

@daschuer Okay to merge?

[daschuer]

After adding the missing include, yes.