Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compact GUI design, new features, combined widgets, editable text fields, movable docks, and more... #485

Open
wants to merge 85 commits into
base: master
Choose a base branch
from

Conversation

mehw
Copy link
Contributor

@mehw mehw commented Mar 23, 2023

Hello, I'll be really grateful to anyone who could give me some feedback about this design.

Thank you ;)

Summary

  • Can undock, dock, and move Navigation, Codec, and Tool Bar.

  • Custom title bar in dock windows with mnemonic shortcuts to dock, undock.

  • Integrates smart buttons in editable text fields. Can edit times directly.

  • Buttons to open a dialog to edit Current Time, Marker A and B.

  • Buttons to Save Script and Run Project/Script.

  • Also buttons to Jump/Reset Marker A/B, Append Media, Cut, Copy, Paste, Undo, Redo.

  • Grayed out Reset Marker A/B buttons suggest when a marker is reset, and vice versa.

  • Can hide sets of components and save their visible state as preferences.

  • Views->Toolbars is accessible as context menu on Menu Bar, Tool Bar, Navigation, and Codec widgets to hide components.

  • And tooltips all the way...

NEW GUI

compact
components
context
floating
icons0
icons1
icons2

CURRENT GUI

Notice how the Selection Duration is truncated... Which is what pushed me to modify the GUI and add new features...
default

mehw added 30 commits March 23, 2023 08:43
Properly (re)name the gui's XML elements to suggest their meaning.

Group the gui's bottom components together into the Navigation widget.
This serves the purpose of making the interface more compact, and the
mouse navigation easier.

Shrinking the gui's window should not clutter or overlap elements.  A
minimum size should be respected to allow displaying every widget.

The bottom components are put into individual sets that can be hidden
independently.

Enable floating Navigation and Codec widgets, and allow to dock them
up/down and left/right respectively.  Also allow the Toolbar to dock
both up/down and left/right.

Add a close button to the Navigation and Codec Options widgets, and
enable context menus in the widgets main Menu, Toolbar, Navigation,
and Codec Options.  The View->Toolbars->Navigation menu option will
toggle the Navigation visibility, hiding all of its components.

Allow to select and copy into the clipboard current time, total time,
selection duration, marker A time, and marker B time by highlighting
the content of their own element.

Redesign current time, marker A time, and marker B time elements to
combine action buttons and text fields.

This design fixes the displaying of the selection duration's seconds,
that were cut due to insufficient space for the label element.
Follow the convention dictated by T_vumeter.{cpp,h}.
Docking features require the presence of the title bar.
Bottom dock widgets have been combined into the Navigation widget as
frame elements.  Remove residual code about setting title bars.
Bottom dock widgets have been combined into the Navigation widget as
frame elements.  Remove residual code about checking heights.
Buttons float/dock and close to the left, add a title if set.  Use the
title's mnemonic & to switch to float/dock state.

This solves the problem of Light and Dark themes that do not read dock
widgets mnemonic correctly, putting a & into the widget's title.
Slightly darkening the Light theme windows' background helps to spot
the dock widgets' resize handle, otherwise not visible enough.
Add new hideable items and resort Toolbars order, bottom dock widgets
have been combined into the Navigation widget as frame elements.
Initialize new hideable items visibility, the Navigation dock widget
contains new frame elements.  Sort top to botton following the gui.
Use the status bar as a XML widget.  The status bar can be hidden via
'View->Toolbars->Status Bar'.
The gui interface has space for a taller volume meter.
Reading left-to-right, the segment A to B has the former letter before
the latter.  It makes sense to express this positioning in the goMarkA
icons (A< vs >A, where goMarkB is >B, to represent the segment A<->B).
Visually center the letter B in its box.
@mehw
Copy link
Contributor Author

mehw commented Mar 23, 2023

  • select the Marker B text field

  • then select the Marker A text field

this changes Marker B here

I can't reproduce the problem... I tried with the mouse cursor and by hitting tab... Would you take a video too, please?

2023-03-23.22-27-36.mov

@szlldm
Copy link
Contributor

szlldm commented Mar 23, 2023

asc.mp4

@mehw
Copy link
Contributor Author

mehw commented Mar 23, 2023

Thanks. I'm trying with different videos... maybe I can get the same behavior...

What system are you using? Have you tried disabling/enabling the Swap markers option?

swap

@eumagga0x2a
Copy link
Collaborator

Thank you for the huge amount of thought put into this POC. Making the time display editable was on my wishlist for quite a while.

The code doesn't build with Qt6 and renaming navButtonsLayout has broken compilation on macOS specifically, but fixing all the obsolete QRegExp stuff, gone since Qt6, was luckily straightforward, so that now I am able to play with it.

I need to spend some time with the new concept to evaluate it beyond the immediate knee-jerk rejection (I don't like cramped interfaces filled with small hit targets).

@eumagga0x2a
Copy link
Collaborator

Below the build fix for reference.

index d1b33286f..3f52795ec 100644
--- a/avidemux/qt4/ADM_userInterfaces/ADM_gui/Q_gui2.cpp
+++ b/avidemux/qt4/ADM_userInterfaces/ADM_gui/Q_gui2.cpp
@@ -143,7 +143,13 @@ static bool uiIsMaximized=false;
 
 static bool needsResizing=false;
 
-static QRegExp timeRegExp("^([0-9]{2}):([0-5][0-9]):([0-5][0-9])\\.([0-9]{3})$");
+static
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+               timeRegExp("^([0-9]{2}):([0-5][0-9]):([0-5][0-9])\\.([0-9]{3})$");
 
 static QAction *findAction(std::vector<MenuEntry> *list, Action action);
 static QAction *findActionInToolBar(QToolBar *tb, Action action);
@@ -714,7 +720,7 @@ MainWindow::MainWindow(const vector<IScriptEngine*>& scriptEngines) : _scriptEng
 #endif
 
 #ifdef __APPLE__
-    ui.navButtonsLayout->setSpacing(2);
+    ui.controlsWidgetLayout->setSpacing(2);
     // Qt upscales 2x sized icons in the toolbar, making them huge and pixelated in HiDPI conditions, WTF?
     ui.toolBar->setIconSize(QSize(24,24));
 #endif
@@ -780,7 +786,11 @@ MainWindow::MainWindow(const vector<IScriptEngine*>& scriptEngines) : _scriptEng
     connect(ui.spinBox_TimeValue,SIGNAL(valueChanged(int)),this,SLOT(timeChanged(int)));
     connect(ui.spinBox_TimeValue, SIGNAL(editingFinished()), this, SLOT(timeChangeFinished()));
 #if 1 /* disable if read-only */
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     QRegExpValidator *timeValidator = new QRegExpValidator(timeRegExp, this);
+#else
+    QRegularExpressionValidator *timeValidator = new QRegularExpressionValidator(timeRegExp, this);
+#endif
     ui.currentTime->setValidator(timeValidator);
     ui.currentTime->setInputMask("99:99:99.999");
     ui.selectionMarkerA->setValidator(timeValidator);
@@ -3727,14 +3737,29 @@ admUITaskBarProgress *UI_getTaskBarProgress()
 */
 bool UI_getCurrentTime(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 {
-    QRegExp rx(timeRegExp);
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+    rx(timeRegExp);
 
     // Previous state
     uint64_t pts = admPreview::getCurrentPts();
 
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     if(rx.exactMatch(WIDGET(currentTime)->text()))
+#else
+    QRegularExpressionMatch match = rx.match(WIDGET(currentTime)->text());
+    if(match.hasMatch())
+#endif
     {
-        QStringList results = rx.capturedTexts();
+        QStringList results =
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+        rx.capturedTexts();
+#else
+        match.capturedTexts();
+#endif
 
         *hh = results.at(1).toInt(NULL, 10);
         *mm = results.at(2).toInt(NULL, 10);
@@ -3763,15 +3788,30 @@ bool UI_getCurrentTime(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 */
 bool UI_getMarkerA(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 {
-    QRegExp rx(timeRegExp);
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+    rx(timeRegExp);
 
     // Previous state
     uint64_t ptsA = video_body->getMarkerAPts();
     uint64_t ptsB = video_body->getMarkerBPts();
 
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     if(rx.exactMatch(WIDGET(selectionMarkerA)->text()))
+#else
+    QRegularExpressionMatch match = rx.match(WIDGET(selectionMarkerA)->text());
+    if(match.hasMatch())
+#endif
     {
-        QStringList results = rx.capturedTexts();
+        QStringList results =
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+        rx.capturedTexts();
+#else
+        match.capturedTexts();
+#endif
 
         *hh = results.at(1).toInt(NULL, 10);
         *mm = results.at(2).toInt(NULL, 10);
@@ -3800,15 +3840,30 @@ bool UI_getMarkerA(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 */
 bool UI_getMarkerB(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 {
-    QRegExp rx(timeRegExp);
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+    rx(timeRegExp);
 
     // Previous state
     uint64_t ptsA = video_body->getMarkerAPts();
     uint64_t ptsB = video_body->getMarkerBPts();
 
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     if(rx.exactMatch(WIDGET(selectionMarkerB)->text()))
+#else
+    QRegularExpressionMatch match = rx.match(WIDGET(selectionMarkerB)->text());
+    if(match.hasMatch())
+#endif
     {
-        QStringList results = rx.capturedTexts();
+        QStringList results =
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+        rx.capturedTexts();
+#else
+        match.capturedTexts();
+#endif
 
         *hh = results.at(1).toInt(NULL, 10);
         *mm = results.at(2).toInt(NULL, 10);
@@ -3837,15 +3892,30 @@ bool UI_getMarkerB(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 */
 bool UI_getSelectionTime(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 {
-    QRegExp rx(timeRegExp);
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+    rx(timeRegExp);
 
     // Previous state
     uint64_t ptsA = video_body->getMarkerAPts();
     uint64_t ptsB = video_body->getMarkerBPts();
 
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     if(rx.exactMatch(WIDGET(selectionDuration)->text()))
+#else
+    QRegularExpressionMatch match = rx.match(WIDGET(selectionDuration)->text());
+    if(match.hasMatch())
+#endif
     {
-        QStringList results = rx.capturedTexts();
+        QStringList results =
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+        rx.capturedTexts();
+#else
+        match.capturedTexts();
+#endif
 
         *hh = results.at(1).toInt(NULL, 10);
         *mm = results.at(2).toInt(NULL, 10);
@@ -3875,14 +3945,29 @@ bool UI_getSelectionTime(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 bool UI_getTotalTime(uint32_t *hh, uint32_t *mm, uint32_t *ss, uint32_t *ms)
 {
     bool status = true;
-    QRegExp rx(timeRegExp);
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+    QRegExp
+#else
+    QRegularExpression
+#endif
+    rx(timeRegExp);
 
     // Previous state
     uint64_t tot = video_body->getVideoDuration();
 
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
     if(rx.exactMatch(WIDGET(totalTime)->text()))
+#else
+    QRegularExpressionMatch match = rx.match(WIDGET(totalTime)->text());
+    if(match.hasMatch())
+#endif
     {
-        QStringList results = rx.capturedTexts();
+        QStringList results =
+#if QT_VERSION < QT_VERSION_CHECK(5,1,0)
+        rx.capturedTexts();
+#else
+        match.capturedTexts();
+#endif
 
         *hh = results.at(1).toInt(NULL, 10);
         *mm = results.at(2).toInt(NULL, 10);

@mehw
Copy link
Contributor Author

mehw commented Mar 23, 2023

The code doesn't build with Qt6 and renaming navButtonsLayout has broken compilation on macOS specifically, but fixing all the obsolete QRegExp stuff, gone since Qt6, was luckily straightforward, so that now I am able to play with it.

I feel sorry to hear that. Currently I'm working with Qt 5.15.8 on Gentoo GNU/Linux. Tell me if I can be of any help.

I need to spend some time with the new concept to evaluate it beyond the immediate knee-jerk rejection (I don't like cramped interfaces filled with small hit targets).

I was trying to sort out a configuration giving some visual hints and control over tedious tuning and adjustments, with the objective of rearrangeable widgets and togglable features.

I'm debugging a problem highlighted by @szlldm right now, I hope to get it fixed soon.

Below the build fix for reference

Thanks.

@mehw
Copy link
Contributor Author

mehw commented Mar 23, 2023

By the way, icons, buttons, styles, etc. are just a mean to focus on the infrastructure, pushing to implement useful features that then can be expressed and used in any other creative/conceptual way.

@mehw
Copy link
Contributor Author

mehw commented Mar 23, 2023

  • select the Marker B text field

  • then select the Marker A text field

this changes Marker B here

@szlldm I've an insight on the problem now, thank you so much for your help ;)

A uint64_t type cast gives the right precision to uint32_t values when
converting to a hh:mm:ss.mm,us time expressed in microseconds.
New ADM_QLineEditPTS class to add precision to time fields.  GUI text
elements now store time at microsecond precisions.  This prevents the
jittering bug when switching focus out from a time field.

Jittering bug: Time fields signal editingFinished after switching the
focus or pressing Enter.  Fields signaled as edited are compared with
the previous real time to confirm that they have been truly modified.
A field time is converted from milli to the micro seconds of the real
time.  The equality fails when the real time microseconds part is not
all zero.  The jitter is when the field time, differing from the real
time, is replaced with the closest frame real time (usually the frame
before).

Thanks to szlldm for spotting the jittering bug.

Thanks to eumagga0x2a for fixing obsolete regexp code.
Editing the markers A/B fields directly, by changing the text, may
result in a computed frame real position not diffrent from before.

Different timings may resolve to the same frame.  When an A/B text
refresh is not performed anyway, the edited value may be obsolete.

Editing A/B directly is more flexible than the 'Go to Time' dialog
but, as said before, the difference is that the former changes the
field text before the frame computation is done, the latter intead
changes the text after, and only if the computed time differs from
the currently one set.
Confirming the total time edit should be followed by a refresh.

The total time field can be edited to prepare a cut selection from the
marker A to B starting from the end of the video.  This feature can be
used to set a cut to reduce the video length of a specific amount.

After refreshing the total time field, the video length specified will
be indicated by the marker A time.
Time fields will show microsecond precision timings when hovered,
i.e. 99:59:59.999,999.  Tooltips will be updated when a field has
been programmatically changed, or a user's edit is confirmed.

This could also be used for debugging purposes.
$ cd avidemux_core/ADM_coreUtils/src
$ nano prefs2.conf
$ chmod +x update_prefs.sh
$ ./update_prefs.sh
This is a workaround about editingFinished events.  Even when time
fields are set to readonly, the signal triggers just one time when
the field is clicked and then the focus is changed, with Tab or by
clicking another element.

This could be a better solution than disabling the time fields, if
there are action buttons that should be kept enabled in a field.
@mehw
Copy link
Contributor Author

mehw commented Mar 25, 2023

@eumagga0x2a Hi, there are different fixes in the latest commits. The direct editing feature of the time fields got more precision. There's a new class overloading QLineEdit to manage the time fields and to confine regular expressions to one location only. I sincerely hope that you could enjoy some of the new additions ;) Let me know, thanks.

Editing the total time prepares a selection to cut a video to a specific length, the marker A is placed at the new length, and B after the last frame.

Editing the selection duration will narrow the selection from A to B, moving B toward A, to the left.

tooltips0
tooltips1

@eumagga0x2a
Copy link
Collaborator

Thank you, I've finally found a few hours to spend with the new GUI on Linux and on Windows 10, but unfortunately didn't have time to debug the functional issues, mainly the direct time input being broken (probably regressed by the newly added changes). I wonder how the new concept works with multiple displays, with detached dock widgets placed on a different display than the main window, something I cannot test at all. Everything below reflects just my impression and is entirely IMHO:

  1. Multiple similar / uniform input fields strain the eye which has to identify the right one quickly. The brain has to process quite subtle visual cues, causing each time a brief but persistent moment of confusion.
  2. The font imitating an LCD clock, used on Linux and macOS (but not on Windows) for time display was forced by me over the heads of users protesting against its poor legibility for the specific reason of avoiding high frequency shifting of the digits during playback. With multiple fields, used for static or very seldom changing time values, the poor legibility of this font bites myself.
  3. I don't like the very idea of allowing "ordinary" users to set selection start and end to arbitrary time values (I understand that advanced users can do exactly that via scripting). Accepting input and then immediately replacing it with a corrected value feels very confusing and may be outright infuriating.
  4. Tweaking QPalette::Window color results in the "Light" theme being much darker than the default theme on Windows, makes the application look immediately alien in a desktop environment.
  5. Short mouse travel distances come at a cost of a very unbalanced / purely utilitarian appearance.
  6. I discourage using preferences for stuff specific to Qt GUI, there are QSettings for that.
  7. The huge scope of changes makes it hard to isolate desired modifications and additions and would shift the focus for long time to Qt GUI structure and features while IMVHO the real burning GUI-related issues exist below, like...
  • OpenGL being finally totally broken on Linux after recent Qt6 updates and OpenGL performance greatly worsened since Qt 5.12 IIRC
  • Wayland support missing
  • OpenGL being deprecated and phased out on macOS and Avidemux losing the only accelerated video display on that platform
  • Right and bottom part of video display is cut off if screen size is insufficient, the complete picture cannot be accessed at 100% zoom in such circumstances (make the video frame scrollable?)
  • Identifying and solving issues running Avidemux in multi-screen setups.

@mehw
Copy link
Contributor Author

mehw commented Mar 30, 2023

@eumagga0x2a Hi, thank you very much for all your considerations. I'm currently implementing a way to allow docking to a diffrent area and undocking w/o showing a title bar when a widget is docked, if it is possible, and I'll read your points one by one thoroughly as soon as my mind is at ease. May I ask you for pictures showing interface differences from a GNU/Linux platform? I you prefer to use an email, my address is in the commits.

Thank you, I've finally found a few hours to spend with the new GUI on Linux and on Windows 10, but unfortunately didn't have time to debug the functional issues, mainly the direct time input being broken (probably regressed by the newly added changes).

I see that now there are merge conflicts. I still have to look into what have been changed since my PR. I did a lot of labeling on gui2.ui, and I don't know if is this the problem... Right now, I'm focusing my energies to fine-tune new features, hoping that they could be helpful to the community in any way possible, looking back at what I based my work on may put a strain on this. I'm sure that you understand that going on refining the PR is easier than over thinking about what is changed since then. Anyway, I'm open to suggestions on what could be a better direction to take.

I wonder how the new concept works with multiple displays, with detached dock widgets placed on a different display than the main window, something I cannot test at all.

Allow to make use of multiple displays is one of my aims. I opened the PR as soon as I had something to start a discussion. I can modify my perspective only by sharing concepts, and for this I thank you. The interface needs a lot of polishing, I'm building a TODO list while coding. For instance, dock widgets decoration when floating and keeping them streamline when docked.

Expect new commits soon to this PR. I believe that first-hand experience is by comparison the best way to conceptualize and give birth to new ideas. Some issues may be just aesthetic related, and easy addressable once the infrastructure they build on is solid.

Can you still compile this PR?

@eumagga0x2a
Copy link
Collaborator

I see that now there are merge conflicts.

I'm sorry for the inconvenience, the work on the status bar added by szlldm prior to this PR needs to be continued until it has reached some presentable shape and official nightly builds can be offered to the public. I would like to avoid blocking the project for a prolonged period of time beyond the already unsightly state of affairs following the move to FFmpeg 5.x which has left VA-API hw accel support in a half-broken state.

May I ask you for pictures showing interface differences from a GNU/Linux platform?

Will do soon.

I keep your PR locally on a separate branch, so it compiles, yes. It just doesn't pick up any ongoing work in other areas of the application.

@mehw
Copy link
Contributor Author

mehw commented Apr 8, 2023

@eumagga0x2a Hi, the context menu and dock widgets decoration are almost done. Rotating a custom title bar properly was not so easy to tweak...

@eumagga0x2a
Copy link
Collaborator

FYI: here are some of the promised screenshots of the new GUI on non-Linux platforms, here showing an empty Avidemux window on Windows 10 and on macOS Monterey with the corresponding default theme.

adm-new-gui-win10-empty-window-default-theme
avidemux-new-gui-macos-dark-default-theme-01

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants