Skip to content

[WIP] Skins :: fix play indicator in all skins#2389

Closed
ronso0 wants to merge 1 commit intomixxxdj:masterfrom
ronso0:play-buttons
Closed

[WIP] Skins :: fix play indicator in all skins#2389
ronso0 wants to merge 1 commit intomixxxdj:masterfrom
ronso0:play-buttons

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented Dec 9, 2019

https://bugs.launchpad.net/mixxx/+bug/1855620

In #2342 (comment) we discovered the play indicator used for Cue blinking is sometimes not updated correctly.
Right now one button with two connections is used for that purpose in all skins which creates issues with how the Cue blinking affects the Play button.
For example in LateNight the 'pressed' icon is used for blinking (just a red background would be less irritating), and in Deere the Pause icon (confusing).
For Cue blinking, just the background color should change.

I propose to use the existing pushbuttons, and add an indicator underneath (stacked layout) in order to separate the controls used and also have separate widgets for icons and background colors for the 'special case' of Cue blinking and play_from_cue:

a pushbutton on top takes care of the PLAY control & state (acually [group],play_indicator)

  • transparent + play/unpressed icon when paused
  • opaque + pause/pressed icon when playing

an indicator underneath shows Cue blinking and play_from_cue state with [group],play:

  • solid, 'inactive' background when paused (no blinking Cue mode)
  • covered by opaque Play button when deck is playing
  • colored background when playing from Cue or when Cue blinking is applied

This is how I implemented it in Tango back then, and it works reliably.

Another aspect of https://bugs.launchpad.net/mixxx/+bug/1855620 is that the skin didn't receive the proper state of cue_default and play_indicator with Numark for example mode when Play was quickly pressed repeatedly (easily reproducable with LateNight). When Play is engaged the Cue point is immediately moved to the 'pause position' and play_indicator is toggled. It seems skins may miss an CO update when they're still busy painting the previous state with pxmaps and additional styles.

@uklotzde
Copy link
Copy Markdown
Contributor

This seems to be incomplete?

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 10, 2019

..because I didn't start to work on it, yet ;)
For now this is just to start the discussion. I think there's also a C part necessary to solve the bug https://bugs.launchpad.net/mixxx/+bug/1855620 because

It seems skins may miss an CO update when they're still busy painting the previous state with pxmaps and additional styles.

@ronso0 ronso0 changed the title Skins :: fix play indicator in all skins [WIP] Skins :: fix play indicator in all skins Dec 10, 2019
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 10, 2019

I think there's also a C part necessary to solve the bug

Maybe it'll take too long to fix the skin lag. In that case the skin changes I proposed will probably reduce the symptoms and also make the CUe blinking consistent with the current skin styles.

@Holzhaus Holzhaus added the skins label Dec 10, 2019
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Jan 24, 2020

@Be-ing Did you find time to investigate the c++ part of the issue?
#2342 (comment)
I'm not sure if the skin changes I proposed would really alleviate the blinking issue.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 3, 2020

@Be-ing *ping
I'll start working on the skin side of this soonish, could you have a look what's causing the behaviour in C++?

@Holzhaus
Copy link
Copy Markdown
Member

@ronso0 Is this supposed to go into 2.2.4, 2.3 or 2.4? Can you add the appropriate milestone?

@ronso0 ronso0 added this to the 2.3.0 milestone Mar 20, 2020
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 20, 2020

Without the c++ changes it's a small improvement for 2.3
Can happen during beta phase, though, so I won't add it to the board now.
Edit okay, this hapens automatically now..

@ronso0 ronso0 self-assigned this Mar 20, 2020
@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Mar 28, 2020

Closing this as it's super tricky / impossible / errorprone to handle the states Play, Pay-from-cue and blinking (dpending on Cue mode) with just two controls ..,play and ..,play_indicator

I'm not sure if this linders the actul bug with Cue connections https://bugs.launchpad.net/mixxx/+bug/1855620

A fix to separate Play / Cue-blinking is ready and comes soonish.

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.

3 participants