QML: Redesign demo skin #3928

Holzhaus · 2021-05-30T17:33:51Z

I played around with the QML skin controls and tried to redesign the skin from scratch to check if these controls if they actually suit our purposes. I think they do. Some of the images were adapted from the SVG linked here.

Swiftb0y

Is there a reason why you preferred using anchors instead of containers provided by the Layout module?

Also as a general API design rule, I'd like to avoid boolean properties on the Mixxx provided components and instead use enums. At least that would be more in-line of what Qt does. Thoughts?

res/skins/QMLDemo/CrossfaderRow.qml

Swiftb0y · 2021-05-31T11:43:58Z

res/skins/QMLDemo/Button.qml

+    property color normalColor: Theme.buttonNormalColor
+    required property color activeColor


why is the normal color part of the theme but not the active color?

Because buttons may have different active colors depending on context. E. g. PFL (Mixer) is blue, but CUE (Deck) is green.

Right, but if they just want to use a default, the have to specify that explicitly instead of the button falling back on the Theme automatically

Correct. There is no sensible default, so making it required to specify ensures I don't forget it and prevents that the skin looks bad.

Is there really no sensible default for the active color of a button? Couldn't the active color just default to Qt.lighter(normalColor, 1.5)?

Or just have a active default color in the Theme?

I mean this is just a non-reusable skin component.

Also, In c++ code we usually avoid default arguments when not needed, so I don't really understand why we should add it here.

Can you elaborate why this is needed?

I agree, I'm sorry I'm nagging so much about the polish of a prototype.

I'd say its needed because it removed redundancy in the skin (technically, not sure if this is actually an annoyance, I didn't write the code ;) )

What does @ronso0 think about this?

No problem, I'm not annoyed :D I just wondered why you think it's necessary/helpful.

res/skins/QMLDemo/DeckInfoBar.qml

res/skins/QMLDemo/Deck.qml

Swiftb0y · 2021-05-31T13:12:35Z

res/skins/QMLDemo/InfoBarButton.qml

+        value: root.checked || root.down
+    }
+
+    Item {


Suggested change

Item {

foreground: Item {

and then expose the foreground as a property? maybe it would even be possible to use an alias property?

I don't think thats possible but I'll have a look. Generally, only the stuff in the Mixxx directory is meant to be reused by other skins, so I only added customization properties where actually needed.

Thats understandable, I'm just suggesting it because you are actually exposing the foreground on L13 if I understand correctly.

Now I get what you mean.

Your original suggestion won't work, this tries to assign the foreground property which doesn't exist (and If I declare it, QML doesn't know where to display it, because it's not part of the body).

And If I make foreground an alias property directly, the property is read-only.

Mhmm right, how did I not notice that this would clash with the regular property assignment syntax...
SixtyFPS supports this using id := Object {...} syntax instead of having the id declared as a property on the object.
I wish that was possible because it would allow to shorten the creation of simple objects on the lines of playCO := Mixxx.createCO(group, "play") instead of requiring 3 times the amount of code.

Holzhaus · 2021-05-31T13:29:02Z

Is there a reason why you preferred using anchors instead of containers provided by the Layout module?

Do you mean Row/Column or ColumnLayout/RowLayout?

I tried to avoid the latter because this makes it very hard to add animations later on.

Also as a general API design rule, I'd like to avoid boolean properties on the Mixxx provided components and instead use enums. At least that would be more in-line of what Qt does. Thoughts?

I agree, but can you point out the specific source lines?

Holzhaus · 2021-05-31T13:41:16Z

I played around with the QML skin controls and tried to redesign the skin from scratch to check if these controls if they actually suit our purposes. I think they do.

Actually, Mixxx.ToggleButton is rather useless, so we might as well remove it.

Swiftb0y · 2021-05-31T13:55:37Z

Do you mean Row/Column or ColumnLayout/RowLayout?

In this particular case I even meant GridLayout (with the contents then being laid out using the row&column and minimum/maximumWidth/Height properties. I don't know how that hinders animation, nor even if it makes much sense to animate that component in particular. I just thought using layouts might be easier to read or and more difficult mess up than using anchors.

I agree, but can you point out the specific source lines?

most of the Mixxx Components have some extra eyecandy feature whose visibility can be configured using a boolean (Spinny.indicatorVisible, Slider.bar, Knob.arc), most of that could be just a generic visibility attribute containing an enum that describes which feature are actually visible (this would also accommodate the case where there is more than one feature). Since you are asking for specific line numbers this would be them in current master: Knob.qml:15 Slider.qml:13 Spinny.qml:9

ronso0 · 2021-05-31T14:58:00Z

what's missing when loading the QML skin fails with ... ?
some dependencies? (sorry I'm lazy when it comes to cleaning up the build dir and reconfigure..)

debug [Main] Now in rebootMixxxView...
debug [Main] ~DlgAutoDJ()
warning [Main] file:///src/master/res/skins/QMLDemo/main.qml:33:17: Type Skin.Button unavailable 
                     Skin.Button { 
                     ^
warning [Main] file:///src/master/res/skins/QMLDemo/Button.qml:12:14: Expected token `:' 
         required property color activeColor 
                  ^
warning [Main] Skin "QMLDemo" failed to load!

Holzhaus · 2021-05-31T15:13:26Z

Yikes, looks like the required keyword was only added in Qt 5.15.

Try running

find res/skins/QMLDemo -iname "*.qml" -exec sed -i 's/required property/property/g' {} \;

And try again. Maybe that works.

ronso0 · 2021-05-31T15:47:23Z

nope, unfortunately not

warning [Main] file:///src/master/res/skins/QMLDemo/main.qml:45:9: Type Skin.DeckRow unavailable 
             Skin.DeckRow { 
             ^
warning [Main] file:///src/master/res/skins/QMLDemo/DeckRow.qml:13:5: Type Deck unavailable 
         Deck { 
         ^
warning [Main] file:///src/master/res/skins/QMLDemo/Deck.qml:14:5: Type Skin.DeckInfoBar unavailable 
         Skin.DeckInfoBar { 
         ^
warning [Main] file:///src/master/res/skins/QMLDemo/DeckInfoBar.qml:45:9: Type MixxxControls.Spinny unavailable 
             MixxxControls.Spinny { 
             ^
warning [Main] file:///src/master/res/skins/QMLDemo/Mixxx/Controls/Spinny.qml:3:1: module "QtQuick.Controls" version 2.15 is not installed 
     import QtQuick.Controls 2.15 
     ^

I have those installed

||/ Name                                    Version              Architecture Description
+++-=======================================-====================-============-====================================
ii  qml-module-qtquick-controls:amd64       5.12.8-0ubuntu2      amd64        Qt 5 Quick Controls QML module
ii  qml-module-qtquick-controls2:amd64      5.12.8+dfsg-0ubuntu1 amd64        Qt 5 Qt Quick Controls 2 QML module
ii  qml-module-qtquick-dialogs:amd64        5.12.8-0ubuntu2      amd64        Qt 5 Dialogs QML module
ii  qml-module-qtquick-layouts:amd64        5.12.8-0ubuntu1      amd64        Qt 5 Quick Layouts QML module
ii  qml-module-qtquick-privatewidgets:amd64 5.12.8-0ubuntu2      amd64        Qt 5 Private Widgets QML module
ii  qml-module-qtquick-templates2:amd64     5.12.8+dfsg-0ubuntu1 amd64        Qt 5 Qt Quick Templates 2 QML module
ii  qml-module-qtquick-window2:amd64        5.12.8-0ubuntu1      amd64        Qt 5 window 2 QML module
ii  qml-module-qtquick-xmllistmodel:amd64   5.12.8-0ubuntu1      amd64        Qt 5 xmllistmodel QML module

Swiftb0y · 2021-05-31T15:51:32Z

good catch, replace import QtQuick.Controls 2.15 in Mixxx/Controls/Spinny.qml:3 with import QtQuick.Controls 2.12

Swiftb0y · 2021-05-31T17:26:11Z

the last hotcue shadow seems to be cutoff (at least perceptually, not sure if this is intended).

Swiftb0y · 2021-05-31T17:31:02Z

text and non-horizontal/vertical lines look awful in general but I guess this is because of missing antialiasing?

Performance stats also seem worse than they would need to be. I guess this is because lots of elements are being drawn transparently which doesn't need to be transparent and transparency (and clipping) break batching.

ronso0 · 2021-05-31T19:12:45Z

okay, got it running.
though I had to install a few more packages

qml-module-qtquick-controls
qml-module-qtquick-controls2
qml-module-qtquick-shapes
qml-module-qt-labs-qmlmodels

and change
import QtQuick.Shapes 1.15 > 1.12

Looks and feels nice, chapeau @Holzhaus !

Swiftb0y · 2021-05-31T19:23:27Z

Thank you. The only minor remaining issue I have now is the hotcue buttons. We can worry about performance once its actually starts to be a problem.

Holzhaus · 2021-05-31T21:55:00Z

Thank you. The only minor remaining issue I have now is the hotcue buttons. We can worry about performance once its actually starts to be a problem.

We can have a look at that soon, but Mixxx shartup time is super fast compared to LateNight. I hope it stays like that when this skin becomes similarily feature-packed :D

Btw, hotcue buttons should be fixed now.

Holzhaus · 2021-05-31T21:57:03Z

Looks and feels nice, chapeau @Holzhaus !

Thanks. Make sure to try out the 4 deck toggle and the crossfader orientation buttons ;-)

ronso0 · 2021-05-31T22:08:07Z

Yeah noticed the animations.
Wonder how it performs on low-spec hardware.

Btw, great idea to pick up the Gudron source!

ronso0 · 2021-05-31T22:18:09Z

hmm..with Qt 5.12 packages I don't see any track related info (loaded in Deere before).
when I hit play after switching to Qml Mixxx segfaults.

Holzhaus · 2021-05-31T22:31:05Z

hmm..with Qt 5.12 packages I don't see any track related info (loaded in Deere before).

Expected, requires #3911, because track info is not accessible via control proxies ;-)

when I hit play after switching to Qml Mixxx segfaults.

Hmm, I can't reproduce. Weird. Can you give me a backtrace?

ronso0 · 2021-05-31T22:40:40Z

here's the backtrace. segfault happens after the spinny made it from 12 to almost 6 o'clock
qml-crash-when-playing.txt

Holzhaus · 2021-05-31T22:48:15Z

Does this also happen when you start Mixxx with the QML skin instead of switching to it?

Maybe some qwidget skin does not clean up correctly.

Swiftb0y · 2021-06-01T09:32:04Z

now the repeater index is broken:
warning [Main] file:///home/swiftb0y/Sourcerepositories/mixxx/res/skins/QMLDemo/Deck.qml:0: ReferenceError: index is not defined

Swiftb0y · 2021-06-01T09:35:16Z

Also, if you want to fix some other bugs I've found, they're documented in project#4

Holzhaus · 2021-06-01T10:19:10Z

now the repeater index is broken:
warning [Main] file:///home/swiftb0y/Sourcerepositories/mixxx/res/skins/QMLDemo/Deck.qml:0: ReferenceError: index is not defined

You need to recompile, had to remove REQUIRED for everyone, even on newer Qt versions due to the QTBUG linked in the comment.

Swiftb0y · 2021-06-01T10:33:50Z

Thanks, forgot you had to introduce C++ to make it 5.12 compatible.

Swiftb0y

LGTM, merge?

Holzhaus · 2021-06-01T10:47:04Z

LGTM, merge?

Added some minor fixes, ready now. The knob clamping warning is a bug in cpp.

Holzhaus added the skins label May 30, 2021

QML: Redesign demo skin

aba26be

Holzhaus force-pushed the qml-redesign branch from 7af6d30 to aba26be Compare May 30, 2021 18:41

QML: Fix Spinny indicator override using contentItem

edf2cf6

Holzhaus force-pushed the qml-redesign branch from 7d7cc61 to 176bf21 Compare May 30, 2021 21:27

Swiftb0y self-assigned this May 30, 2021

Holzhaus added 2 commits May 31, 2021 00:31

skins/QMLDemo: Add support for Hotcue Buttons

06a068b

skins/QMLDemo: Clean up deck info bars and add FX unit assign buttons

daa577e

Holzhaus force-pushed the qml-redesign branch from 176bf21 to daa577e Compare May 30, 2021 22:34

Swiftb0y reviewed May 31, 2021

View reviewed changes

Holzhaus added 4 commits May 31, 2021 20:31

skins/QMLDemo: Add IDs to effect unit placeholders

7816e3e

skins/QMLDemo: Improve Deck time JS code

ff5da67

QML: Make re-usable QML controls Qt 5.12 compatible

794da58

skins/QMLDemo: Use Qt 5.12 compatible QtQuick.Shapes import in EqColumn

f9dba93

Holzhaus force-pushed the qml-redesign branch from afc7622 to 974e81f Compare May 31, 2021 19:13

Holzhaus force-pushed the qml-redesign branch from 5700ef6 to b295d62 Compare May 31, 2021 22:24

Holzhaus added 4 commits June 1, 2021 01:06

QML: Remove "required" keyword from demo skin for Qt 5.12 compatibility

cc4c0f0

QML: Remove useless ToggleButton QML control

b2d7ce5

skin/QMLDemo: Fix clipping of buttons with low width

aa56a7c

skins/QMLDemo: Add EQ kill buttons

1528ede

Holzhaus force-pushed the qml-redesign branch from b295d62 to 1528ede Compare May 31, 2021 23:09

Holzhaus requested a review from Swiftb0y May 31, 2021 23:09

Swiftb0y approved these changes Jun 1, 2021

View reviewed changes

Holzhaus added 3 commits June 1, 2021 12:43

skins/QMLDemo: Reduce duplication in EqKnob

d9f0bd5

QML: Replace transparent Rectangles with Items in Knob component

c6a0d7d

QML: Fix Knob component parameter clamping

e6c13e3

Holzhaus force-pushed the qml-redesign branch from 9e4cf25 to e6c13e3 Compare June 1, 2021 10:45

Swiftb0y approved these changes Jun 1, 2021

View reviewed changes

Swiftb0y merged commit 0bef3dd into mixxxdj:main Jun 1, 2021

		property color normalColor: Theme.buttonNormalColor
		required property color activeColor

QML: Redesign demo skin #3928

QML: Redesign demo skin #3928

Conversation

Holzhaus commented May 30, 2021

Swiftb0y left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y May 31, 2021 • edited Loading

Choose a reason for hiding this comment

Holzhaus May 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Holzhaus May 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Holzhaus May 31, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Holzhaus commented May 31, 2021

Holzhaus commented May 31, 2021

Swiftb0y commented May 31, 2021

ronso0 commented May 31, 2021

Holzhaus commented May 31, 2021 • edited Loading

ronso0 commented May 31, 2021

Swiftb0y commented May 31, 2021

Swiftb0y commented May 31, 2021 • edited Loading

Swiftb0y commented May 31, 2021 • edited Loading

ronso0 commented May 31, 2021

Swiftb0y commented May 31, 2021

Holzhaus commented May 31, 2021 • edited Loading

Holzhaus commented May 31, 2021

ronso0 commented May 31, 2021

ronso0 commented May 31, 2021

Holzhaus commented May 31, 2021 • edited Loading

ronso0 commented May 31, 2021

Holzhaus commented May 31, 2021 • edited Loading

Swiftb0y commented Jun 1, 2021

Swiftb0y commented Jun 1, 2021

Holzhaus commented Jun 1, 2021

Swiftb0y commented Jun 1, 2021

Swiftb0y left a comment • edited Loading

Choose a reason for hiding this comment

Holzhaus commented Jun 1, 2021 • edited Loading

Swiftb0y May 31, 2021 •

edited

Loading

Holzhaus May 31, 2021 •

edited

Loading

Holzhaus May 31, 2021 •

edited

Loading

Holzhaus May 31, 2021 •

edited

Loading

Holzhaus commented May 31, 2021 •

edited

Loading

Swiftb0y commented May 31, 2021 •

edited

Loading

Swiftb0y commented May 31, 2021 •

edited

Loading

Holzhaus commented May 31, 2021 •

edited

Loading

Holzhaus commented May 31, 2021 •

edited

Loading

Holzhaus commented May 31, 2021 •

edited

Loading

Swiftb0y left a comment •

edited

Loading

Holzhaus commented Jun 1, 2021 •

edited

Loading