Add Qt-based config editor #3982

neobrain · 2024-08-20T18:58:44Z

Background

(Screenshots below)

Dear ImGui can be a great tool for quick prototyping and overlay GUIs. Conversely, it's also an unsuitable option for creating GUIs of moderate complexity without sacrificing usability and accessibility. This is acknowledged by the author of Dear ImGui himself, and will likely never change:

I do know that Dear ImGui unfortunately does not align with requirements of accessible software. It was designed as a technical/debug tooling software to be used as an overlay over 3d/graphics applications which themselves tends to have little to no accessibility features. It's not really my fault that people have started using it for desktop-ey applications. If you do use it for a tool please be mindful the tool won't be accessible to most screen readers

Historically Dear ImGui may have been a suitable tool for FEX when there was still a built-in debugger. Nowdays we only use it for FEXConfig, for which it provides no benefit over other desktop toolkits, while having all of the downsides of an immediate-mode framework.

This PR adds a rewrite of FEXConfig based on Qt, a widely available framework with decades of development progress. During the rewrite, I've also cleaned up and restructured some of the settings to make things easier to configure overall. The new application ~~ships as its own binary, called FEXQonfig~~ must be enabled by setting USE_FEXCONFIG_TOOLKIT=qt via CMake.

Improvements over current FEXConfig

Look and Feel matching the system:
- Font rendering: Anti-aliasing, font type, size, ...
- Keyboard navigation (tabs, file menu, ...)
- Styling (color theme, native controls, ...)
Architectural support for accessibility (screen reader, keyboard navigation, ...)
Native filesystem browser (file picker available in more places than before)
- All path settings can be configured with this dialog now
More robust widgets:
- E.g. dialogs are somehow really glitchy with Dear ImGui (might be a bug in how FEX uses them?)
Can be built without a custom ImGui fork, opening up the way for dropping another External
AppConfig support made more straightforward
- Not sure if anyone uses this, since current FEXConfig behaves very quirky
- Qt provides "tri-state" checkboxes that clearly highlight which settings aren't set in an AppConfig
- It's easier to delete individual settings in the "Advanced" tab now
- Overall, this makes it much easier to create minimal config files in a GUI
Code-wise, a declarative framework offers better separation of concerns by isolating the UI code from the config management logic
In principle we can add ~~Übersetzungen~~ ~~des traductions~~ translations!

Current state

FEXQonfig is ~~roughly~~ on feature-parity with FEXConfig while already offering a much more polished UX. Some things (documented via TODOs) still need to be implemented, and I haven't done much testing yet, so backing up Config.json is advised. For now, I figured I'd throw it out there to see whether it's worth finishing.

To test the Qt6 build on Ubuntu, I had to install the following packages:

qt6-declarative-dev
qt6-wayland
qml6-module-qtquick-dialogs
qml6-module-qtquick-templates
qml6-module-qtquick-window
qml6-module-qt-labs-folderlistmodel

Screenshots

Paths selectable via file dialog; special logging targets are selectable from a combobox;

TSO options are restructured:

The settings list is now type-aware and provides checkboxes/spinboxes where appropriate:

Tri-state buttons are used to highlight settings deleted from a config. Other controls (like comboboxes) are deselected or disabled:

Conan-Kudo · 2024-08-20T19:33:46Z

Thank you so much for this! 🥇

Conan-Kudo · 2024-08-20T19:41:20Z

Source/Tools/CMakeLists.txt

+    find_package(Qt5 COMPONENTS Qml Quick Widgets)
+    if (Qt5_FOUND)
+      add_subdirectory(FEXQonfig/)
+    endif()


Can we please use Qt 6 instead here? Qt 5 OSS is effectively EOL.

Sadly Kubuntu 24.04 LTS ships Qt 5 by default and figuring out what Qt 6 packages must be installed to compile FEXQonfig (and to not make it use the fallback theme!) seems to be nontrivial.

Supporting both simultaneously (and preferring Qt 6 over 5) should hopefully be possible without too much extra work though. The main breaking changes are in the Dialog APIs, which we should be able to handle via abstraction however.

I created a compatibility module to deal with the dialogs (surprisingly tricky to make Qt conditionally import the right one). Both Qt 5 and Qt 6 are supported now!

My testing with Qt 6 was limited due to what Ubuntu currently packages, so feel free to give it a spin if you want :)

jvoisin · 2024-08-20T19:47:00Z

Source/Tools/FEXQonfig/main.qml

+        id: openFileDialog
+        title: qsTr("Open FEX configuration")
+        nameFilters: [ "Config files(*.json)", "All files(*)" ]
+        // TODO: Set default folder to ~/.fex-emu?


Or even better, ~/.config/fex-emu/

You're not wrong. I'm not actually sure why we aren't yet using XDG_CONFIG_HOME for the config.

^ currently FEX uses it if it's explicitly set, but defaults to ~/.fex-emu

jvoisin · 2024-08-20T19:50:56Z

Source/Tools/FEXQonfig/main.qml

+            }
+        }
+
+        // Advanced settings


It would also be nice to add a small help/description for them I think.

Yup, though for now I'm just aiming to rebuild functionality that already exists (plus some things that were easy to add).

We already have descriptions written down in the Config.json file, but they're not readily available in C++ code yet, so this would need more work.

clear() emits the modelReset signal, which is redundant since we're about to add new data anyway.

Conan-Kudo · 2024-08-24T10:41:20Z

Source/Tools/CMakeLists.txt

@@ -3,6 +3,14 @@ add_subdirectory(CommonTools)
 if (NOT MINGW_BUILD)
  if (BUILD_FEXCONFIG)
    add_subdirectory(FEXConfig/)


Can we have a conditional so that we can only build this variant and not the other?

Sounds reasonable. BUILD_FEXCONFIG is now called USE_FEXCONFIG_TOOLKIT and can be set to either ""/"qt"/"imgui". Since it's an exclusive choice, FEXQonfig has been renamed to FEXConfig.

neobrain · 2024-08-27T16:15:41Z

This is on feature-parity with the existing FEXConfig now.

We could make it the default already but the CI runners don't have Qt libraries installed (qtdeclarative5-dev).

Sonicadvance1 · 2024-08-28T05:50:57Z

The rootfs list should scroll, since it consumes the whole window otherwise and it's hard to tell that there is more available in the window.

This provides better visibility when the RootFS list is large.

This orders the entries more like a normal file browser would.

neobrain · 2024-08-28T08:01:24Z

The rootfs list should scroll, since it consumes the whole window otherwise and it's hard to tell that there is more available in the window.

Ah yes, I limited the list's maximum height to around 4-6 items now (depending on the system theme). After that, it will display a scroll bar while leaving the two buttons visible.

Sonicadvance1 · 2024-08-28T08:02:50Z

Quite a bit better now

Conan-Kudo · 2024-08-28T11:03:43Z

CMakeLists.txt

@@ -7,7 +7,7 @@ CHECK_INCLUDE_FILES ("gdb/jit-reader.h" HAVE_GDB_JIT_READER_H)
 option(BUILD_TESTS "Build unit tests to ensure sanity" TRUE)
 option(BUILD_FEX_LINUX_TESTS "Build FEXLinuxTests, requires x86 compiler" FALSE)
 option(BUILD_THUNKS "Build thunks" FALSE)
-option(BUILD_FEXCONFIG "Build FEXConfig, requires SDL2 and X11" TRUE)
+set(USE_FEXCONFIG_TOOLKIT "imgui" CACHE STRING "If set, build FEXConfig (qt or imgui)")


Do we want to make qt the default before merging it?

We'll need to update our CI machines before it can be switched to default. Setting the cmake option until then should be fine.

Also probably doesn't hurt to ship this as opt-in for one release, just in case edge case bugs come up :)

neobrain · 2024-08-30T08:45:59Z

Gave this a final spin with a proper Qt 6 + Plasma 6 installation on openSUSE, working all fine. Let's do this!

Add Qt-based config editor

47f0fec

Conan-Kudo suggested changes Aug 20, 2024

View reviewed changes

jvoisin reviewed Aug 20, 2024

View reviewed changes

neobrain added 5 commits August 21, 2024 10:04

FEXQonfig: Load config from default location on startup

a6f9c51

FEXQonfig: Strip file:// prefixes when displaying file paths

9500842

FEXQonfig: Add status bar

3077fec

FEXQonfig: Save CacheObjectCodeCompilation on modification

d585063

FEXQonfig: Add Qt 6 support

ed0c045

neobrain force-pushed the feature_fexqonfic branch from 297c7a5 to ed0c045 Compare August 21, 2024 11:22

neobrain added 20 commits August 21, 2024 21:54

FEXQonfig: Fix double-save due to Qt 5 bug

6a7914a

FEXQonfig: Delay saving until after the user selects a filename

f553782

FEXQonfig: Drop leftover debug log

29fd62e

FEXQonfig: Make pages scrollable

8b6fe0c

FEXQonfig: Unindent after refactor

3077add

FEXQonfig: Fix formatting

5f5a06f

FEXQonfig: Tune window size constraints

5d5b411

FEXQonfig: Implement "Save As"

aa4205c

FEXQonfig: Enable file creation on save

3fcfd2c

FEXQonfig: Implement RootFS selection from file dialog

8232be6

FEXQonfig: Improve UI responsiveness

1e0273d

FEXQonfig: Further improve UI responsiveness

3a05b76

clear() emits the modelReset signal, which is redundant since we're about to add new data anyway.

FEXQonfig: Handle load failure

7176c71

FEXQonfig: Create config and RootFS folders on startup if needed

15fef17

FEXQonfig: Implement RootFS monitoring via inotify

86d1ce7

FEXQonfig: Implement environment variable editing

f4fca44

FEXQonfig: Rename header to Main.h

fd1f564

FEXQonfig: Use FolderDialog when selecting library forwarding paths

58cff72

FEXQonfig: Clean up object ownership

df86d80

FEXQonfig: Use better icon for RootFS file selection

68abe40

neobrain added 2 commits August 24, 2024 11:01

FEXQonfig: Fix Qt 6 build

bebf420

FEXQonfig: Remove unused code from CMakeLists.txt

6c92f94

Conan-Kudo suggested changes Aug 24, 2024

View reviewed changes

CMake: Build only one FEXConfig and make Qt/ImGui exclusive choices

ca9a94d

neobrain force-pushed the feature_fexqonfic branch from d55ed06 to ca9a94d Compare August 27, 2024 16:10

neobrain marked this pull request as ready for review August 27, 2024 16:11

Sonicadvance1 approved these changes Aug 28, 2024

View reviewed changes

neobrain added 3 commits August 28, 2024 09:51

FEXQonfig: Move fallback RootFS entry to the top of the list

e88c57b

This provides better visibility when the RootFS list is large.

FEXQonfig: Set a maximum height for RootFS list and make it scrollable

72d74ae

FEXQonfig: Use Unicode/locale-aware sorting for RootFS list

cc9ccf8

This orders the entries more like a normal file browser would.

Sonicadvance1 approved these changes Aug 28, 2024

View reviewed changes

Conan-Kudo reviewed Aug 28, 2024

View reviewed changes

neobrain added 4 commits August 29, 2024 11:53

FEXQonfig: Fix Qt6 runtime-warnings caused by unannotated return types

539d5ed

FEXQonfig: Fix unnecessary prompts for file selection on save

74da2eb

FEXQonfig: Use consistent orthography

a444db7

CMake: Build Dear ImGui only if needed

8a5388f

neobrain merged commit 3020a0d into FEX-Emu:main Aug 30, 2024
12 checks passed

neobrain deleted the feature_fexqonfic branch August 30, 2024 08:43

neobrain mentioned this pull request Sep 5, 2024

FEX 2409 FEX-Emu/fex-emu.com#5

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Qt-based config editor #3982

Add Qt-based config editor #3982

neobrain commented Aug 20, 2024 •

edited

Loading

Conan-Kudo commented Aug 20, 2024

Conan-Kudo Aug 20, 2024

neobrain Aug 20, 2024 •

edited

Loading

neobrain Aug 21, 2024

jvoisin Aug 20, 2024

neobrain Aug 20, 2024

bylaws Aug 20, 2024

jvoisin Aug 20, 2024

neobrain Aug 29, 2024

Conan-Kudo Aug 24, 2024

neobrain Aug 27, 2024

neobrain commented Aug 27, 2024

Sonicadvance1 commented Aug 28, 2024

neobrain commented Aug 28, 2024

Sonicadvance1 commented Aug 28, 2024

Conan-Kudo Aug 28, 2024

Sonicadvance1 Aug 28, 2024

neobrain Aug 28, 2024

neobrain commented Aug 30, 2024

Add Qt-based config editor #3982

Add Qt-based config editor #3982

Conversation

neobrain commented Aug 20, 2024 • edited Loading

Background

Improvements over current FEXConfig

Current state

Screenshots

Conan-Kudo commented Aug 20, 2024

Choose a reason for hiding this comment

neobrain Aug 20, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neobrain commented Aug 27, 2024

Sonicadvance1 commented Aug 28, 2024

neobrain commented Aug 28, 2024

Sonicadvance1 commented Aug 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neobrain commented Aug 30, 2024

neobrain commented Aug 20, 2024 •

edited

Loading

neobrain Aug 20, 2024 •

edited

Loading