Skip to content

Tango :: small fixes + Master/Head mixer toggle#1423

Merged
daschuer merged 19 commits intomixxxdj:masterfrom
ronso0:tango-tiny-fixes
Dec 23, 2017
Merged

Tango :: small fixes + Master/Head mixer toggle#1423
daschuer merged 19 commits intomixxxdj:masterfrom
ronso0:tango-tiny-fixes

Conversation

@ronso0
Copy link
Copy Markdown
Member

@ronso0 ronso0 commented Dec 19, 2017

Bug 1707991 samplers don't scale well: Time to load and shut down Mixxx increases perceptibly if skins are loaded with a total of 64 sample decks.

This is a quick workaround that reduces the number of available sample decks to 16, and introduces a second version of the skin with 64 sample decks. Both versions are identical, except the number of available sample decks, of course.

The number of samplers is set in three places in skin.xml only:

  • <attribute config_key="[Master],num_samplers">16</attribute>
    defines the number of samplers available in the engine
  • <Template src="skin:skin_settings_sampler_rows_16.xml"/>
    this is a Singleton definition that loads the respective row selection button for the skin's settings menu
  • <Template src="skin:sample_decks_16.xml"/>
    this loads the appropriate sample decks template

When changing Tango in the future, just work on the now-standard version with 16 samplers, "Tango".
When all changes are in place

  • delete the original 64-samplers version
  • add a copy of the 16-samplers version to /res/skins, name it "Tango (64 sample decks)"
  • change "16" to "64" in all three places listed above

This PR also contains some fixes some bugs that slipped through my fingers in the previous PR:

  • some old COs in skin settings menu, some typos (now 4 FX units are available again :)
  • consolidate some COs so that visibility settings persist when switching skins
  • right-hand decks' track time is now aligned properly when set to 'symmetrical"
  • use Q also for main Cue point mark on waveforms
  • minor changes behind the scenes that reduce flickering when components are shown/hidden

One of the first things I did when I started working on Tango: remove Master & Headphone controls, they are (hard-wired) on my controller and superflous in the skin.
So to complete the "controller focus" of Tango I added a toggle to hide those controls from top bar, as well as a icon in case you want them back. CO is [Tango],master_mixer and "1" by default.

ronso0 added 15 commits December 15, 2017 22:53
With <attribute persist="true" config_key="[EffectRack1],show">1</attribute>
in skin.xml, the effect rack would always show up, but quick toggle in top bar
indicates it's off...
This was the first thing I'd hide when making my personal branch.
To make it discoverable, there's a nice mixer icon when Master & Headphone
mixer controls are hidden
in skin.xml change these in order to switch between 16 or 64 samplers
 <attribute config_key="[Master],num_samplers">XX</attribute>
 <Template src="skin:skin_settings_sampler_rows_16.xml"/>
 <Template src="skin:sample_decks_16.xml"/>
@MK-42
Copy link
Copy Markdown
Contributor

MK-42 commented Dec 19, 2017

No more 'mixxx is not responding' under gnome3 with the 16 Samplers version. Thats nice, shutdown time is reasonable now.

What I noticed (also in current master): when you toggle the Channel-Mixer, the top-bar buttons will jump outwards on larger screens, for me that feels strange. I like the spacing that produces between crossfader and buttons with channel-mixers, but I think the spacing could also be used if they are not shown.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 19, 2017

What I noticed (also in current master): when you toggle the Channel-Mixer, the top-bar buttons will jump outwards on larger screens, for me that feels strange.

The quick toggles are supposed to be aligned with the decks (excl. mixer or cover/spinny) and on small screens they're always aligned to decks incl. mixer. Both makes sense, especially when the stacked waveforms are hidden.
I didn't expect users would renounce a big spinny/cover when the screen allows it ;)

For small screens (<1260px) I'll keep it as is, and for big screens I'll just ditch the alignment, it's to complicated anyway.

@daschuer
Copy link
Copy Markdown
Member

This just duplicates the whole skin right? Not a very good idea for repository size and maintenance.
A Version with just a duplicated skin file would work for me.
I think I will Implement the view menu version, to get around this.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 19, 2017

I agree that duplicating the skins is not a good idea because it creates a big maintenance burden. I don't think the view menu is the right place for this option either. IMO we should work towards removing the top menu bar, not add more to it. I think the preferences would be a better place, so let's finish up #1377 first.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 19, 2017

This just duplicates the whole skin right? Not a very good idea for repository size and maintenance.
A Version with just a duplicated skin file would work for me.
I think I will Implement the view menu version, to get around this.

Sure, it would be way more efficient to just swap skin.xml. But considering a) this is a temporary workaround until your proposal is in place b) size of master is ~400MB and c) any skin is <1MB, I think this okay for now, and avoids complaints.

Also I think the View menu is not a good place for this option. All existing controls there react instantly (except rescanning Library and dev options), but switching sampler count reloads the whole skin and would therefore unexpectedly make the GUI unresponsive for at least a couple of seconds, on slow machines maybe half a minute. Let's put this option to Preferences > Interface. If this window is open, users won't expect the GUI to be accessible.

@daschuer
Copy link
Copy Markdown
Member

I think I have found a better solution: Lazy loading for singleton containers.
This way we do not need even a preferences option.

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 19, 2017

That sounds like a good idea. 👍

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 19, 2017

Hmm, that will still require wasting RAM for the samplers that are not shown because those still need to be created before the skin widgets can connect to the COs. But the startup and shutdown times should improve.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 19, 2017

Lazy loading for singleton containers.
Sounds fun! What does it mean?

@daschuer
Copy link
Copy Markdown
Member

That the skin is parsed only if the container is switched to visible.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 22, 2017

@daschuer
The fixes presented here should really be in the beta.
Should I roll back the 16/64 samplers modification (delete 2nd directory and 64-version just Tango) or do we merge this as well?

@Be-ing
Copy link
Copy Markdown
Contributor

Be-ing commented Dec 22, 2017

Should I roll back the 16/64 samplers modification (delete 2nd directory and 64-version just Tango) or do we merge this as well?

Please revert the changes to the samplers.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 22, 2017

okay, now this only contains

  • some old COs in skin settings menu, some typos (now 4 FX units are available again :)
  • consolidate some COs so that visibility settings persist when switching skins
  • right-hand decks' track time is now aligned properly when set to 'symmetrical"
  • use Q also for main Cue point mark on waveforms
  • minor changes behind the scenes that reduce flickering when components are shown/hidden

and a toggle for Master/Headphone mixer in top bar

@daschuer
Copy link
Copy Markdown
Member

Please rabase -i this on upstream master and remove the duplicating skin commits.
This avoids that everyone has these big size binary commits in the git history to eternity.
Thank you.

@ronso0
Copy link
Copy Markdown
Member Author

ronso0 commented Dec 22, 2017

Please rabase -i this on upstream master and remove the duplicating skin commits.

Done.
Today I learned another way not create a mess in the first place...

@Be-ing Be-ing mentioned this pull request Dec 22, 2017
8 tasks
@ronso0 ronso0 changed the title Tango :: 16 or 64 sample decks + small fixes Tango :: small fixes + Master/Head mixer toggle Dec 22, 2017
@daschuer
Copy link
Copy Markdown
Member

Super, thank you.

@daschuer daschuer merged commit 78c3ae8 into mixxxdj:master Dec 23, 2017
@ronso0 ronso0 deleted the tango-tiny-fixes branch December 23, 2017 00:56
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.

4 participants