QML client rewrite #3457

danryu · 2024-12-30T07:01:42Z

Description of changes

NOTE: This PR is posted at the request of @ann0see, see https://github.com/orgs/jamulussoftware/discussions/3454.
It is intended as a reference or baseline to assist in the effort of converting to a QML-based version in the future.

N.B. I had to implement a freeze on new upstream merges due to the intricate nature of these changes. So it may be necessary to cherry-pick / re-implement / port any necessary upstream changes during the last 4-6 months of 2024 to the files in question, particularly client.cpp and audiomixerboard.cpp

While setting myself the challenge to learn QML, I have succeeded in rewriting the core Jamulus client GUI with QML, removing all widget code in the process.
To put it mildly, it was a large, non-trivial effort to disentangle all the widget display logic from the business logic.

All existing business logic is largely preserved, with small changes and additions here and there.
In general, things are much simplified when it comes to UI element handling. No need, for example, for the clever audiomixerboard template stuff, or creating and hiding 250 channelfader objects :)

Key Changes:

clientdlg -> removed, business logic moved to client, display moved to QML
settingsdlg -> removed, business logic moved to settings, display moved to QML
chatdlg -> converted to chatbox
audiomixerboard -> CChannelFader and CAudioMixerBoard classes refactored as necessary
all *.qml files representing key components in the UI

Design
The UI design is just a demo to contain all the functioning elements. It can easily be adapted as necessary.
For this reason it has almost no styling applied.

Features
All the core features of the client have been reproduced in the QML version. There may still be some subtle bugs, but it is already quite usable (only tested on Windows 11).

Exclusions
To reduce the scope of the project (already large enough) I have not implemented the following, but they should not be challenging to re-introduce:

server chooser dialog
instruments/skills/flags
reverb
what's this stuff
translations / language chooser stuff

I invite you to build and test it locally, and offer any suggestions or enhancements.
Especially on macOS as I don't have any Apple hardware at the moment.

CHANGELOG:

Context: Fixes an issue?

Does this change need documentation? What needs to be documented and how?

Status of this Pull Request

What is missing until this pull request can be merged?

Checklist

I've verified that this Pull Request follows the general code principles
I tested my code and it does what I want
My code follows the style guide
I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
I've filled all the content above

ann0see · 2024-12-30T07:39:51Z

Jamulus.pro

You did quite a lot of changes to Jamulus.pro
It seems to be overkill.

Can you be specific?
In this commit there are a few extra changes in Jamulus.pro included from earlier dev / simplification work. There is quite a lot of cruft IMO anyway in the .pro file, but feel free to push or suggest changes.

ann0see · 2024-12-30T07:43:34Z

src/audiomixerboard.cpp

@@ -25,388 +25,156 @@
 #include "audiomixerboard.h"

 /******************************************************************************\
-* CChanneFader                                                                 *


ann0see · 2024-12-30T07:48:54Z

src/chatbox.cpp

+    emit chatHistoryChanged();
+}
+
+// void CChatBox::OnAnchorClicked ( const QUrl& Url )


How do message boxes work in qml?

Closest equivalent is the Popup (https://doc.qt.io/qt-6/qml-qtquick-controls-popup.html) which I used in a few cases, see AppWindow.qml.

ann0see · 2024-12-30T07:52:53Z

src/client.cpp

+    // update mixer board with the additional client infos
+    audioMixerBoard.ApplyNewConClientList( vecChanInfo );
+    // set session status
+    setSessionStatus("CONNECTED");


I don’t think we need to necessarily always have a text saying „Connected“. The state should be conveyed differently.
if the connection stands faders are visible, else not. Potentially we‘d want to say „Disconnected“ in this case.

ann0see · 2024-12-30T07:53:55Z

src/client.cpp

@@ -431,139 +472,97 @@ void CClient::SetDoAutoSockBufSize ( const bool bValue )
    CreateServerJitterBufferMessage();
 }

-// In order not to flood the server with gain or pan change messages, particularly when using
+// In order not to flood the server with gain change messages, particularly when using


Seems like you revert some recent changes like the pan timer.

Eesh, that's possible, I tried to avoid that but with the length of time this took me, I may have missed a few commits.

I see a few other recent changes in client.cpp, such as 49eb608. I'm not sure what this client<->server channel mapping is about, or what server channels are, or what the requirement for this was.
For things like this, it might be best to cherry-pick the relevant commits, as the number of files affected will be minimal.

ann0see · 2024-12-30T07:55:22Z

src/client.cpp

-    for ( int i = 0; i < MAX_NUM_CHANNELS; i++ )
+        qDebug() << "Feedback detected ... ";
+        // show message about feedback issue
+        setUserMsg( tr ( "Audio feedback or loud signal detected.\n\n"


Ok. That’s how errors can be shown.

ann0see · 2024-12-30T07:57:09Z

src/client.cpp

 }

-void CClient::FreeClientChannel ( const int iServerChannelID )
+
+void CClient::Connect( const QString& strAddress )


This might be useful for #3372

ann0see · 2024-12-30T08:00:04Z

src/main.cpp

@@ -342,9 +345,6 @@ int main ( int argc, char** argv )
        // HTML status file ----------------------------------------------------
        if ( GetStringArgument ( argc, argv, i, "-m", "--htmlstatus", strArgument ) )
        {
-            qWarning() << qUtf8Printable (


It seems like some of the latest changes are reverted. The question is how we can have a clean state.

ann0see

I think one of the main challenges is to keep it up to date with the changes on main.

The question is how can we progress.

In the best case, after 3.12.0 is out, we‘d have a codebase which is clean and we could make the qml changes.

ann0see · 2024-12-30T08:08:42Z

Maybe out of scope, maybe not:

We'd need to adapt the CI.

ann0see · 2024-12-30T08:14:49Z

I could imagine the following:

Get an approval from other main developers that we want to move to qml in Jamulus 4
Create a separate branch for Jamulus 4 as soon as 3.12.0 is mostly done (=3.12.0 is frozen)
Merge further changes into the Jamulus 4 branch

ann0see · 2024-12-30T08:39:12Z

I think we can delay reverb implementation. It's not even clear if this would even be part of Jamulus 4 in the current way.

The connect dialog would be more important.

ann0see · 2024-12-30T08:41:12Z

If I find time in the following months I'd like to look at getting the CI working.

working QML client

658d131

ann0see marked this pull request as draft December 30, 2024 07:34

ann0see reviewed Dec 30, 2024

View reviewed changes

ann0see added this to the Release 4.0.0 milestone Dec 30, 2024

danryu added 2 commits December 31, 2024 10:00

QGuiApplication not QGUIApplication

6ff632e

remove redundant qguiapplication.h

f83ed52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QML client rewrite #3457

QML client rewrite #3457

danryu commented Dec 30, 2024 •

edited

Loading

ann0see Dec 30, 2024

danryu Dec 30, 2024

ann0see Dec 30, 2024

ann0see Dec 30, 2024

danryu Dec 30, 2024

ann0see Dec 30, 2024 •

edited

Loading

ann0see Dec 30, 2024

danryu Dec 30, 2024

danryu Dec 30, 2024

ann0see Dec 30, 2024

ann0see Dec 30, 2024

ann0see Dec 30, 2024

ann0see left a comment

ann0see commented Dec 30, 2024

ann0see commented Dec 30, 2024 •

edited

Loading

ann0see commented Dec 30, 2024

ann0see commented Dec 30, 2024

QML client rewrite #3457

Are you sure you want to change the base?

QML client rewrite #3457

Conversation

danryu commented Dec 30, 2024 • edited Loading

Checklist

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

Choose a reason for hiding this comment

ann0see Dec 30, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ann0see left a comment

Choose a reason for hiding this comment

ann0see commented Dec 30, 2024

ann0see commented Dec 30, 2024 • edited Loading

ann0see commented Dec 30, 2024

ann0see commented Dec 30, 2024

danryu commented Dec 30, 2024 •

edited

Loading

ann0see Dec 30, 2024 •

edited

Loading

ann0see commented Dec 30, 2024 •

edited

Loading