-
-
Notifications
You must be signed in to change notification settings - Fork 258
UI entirely rewritten in QT6 (PySide6 bindings) #1164
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for this PR! This PR seems to include #749. Is it possible to split your PR into 2, so we have a Qt PR against current main without #749? Your PR is difficult to review otherwise (because many changes that seem unrelated, like in models/). And it would have to wait for #749 otherwise, whenever this is. |
|
No, Some guidance for review:
Regarding PR #749, the only files concerning it are:
As I did not touch the backend, JXL support is not implemented, so the integration of #749 is not necessary to review this PR.
EDIT: All the bugs I could identify on my own are now fixed. I will wait on your feedback for things that I may have missed. |
|
Did you evaluate my idea from earlier, to implement wrappers for the components in You have opted for a complete rewrite of all UI code instead. Maybe that is the right decision, but I'd like to understand it, because I feel like it will be very difficult to do a big bang switch without introducing many new bugs. |
|
At first I tried a conservative approach, however, coupling was so strong, that it was too difficult to carry on, and I ultimately decided to rewrite everything (attempting to move as many blocks of code as they were implemented as possible). In hindsight, I think that pushing towards a conservative approach would have bitten us later, and could have transformed in a considerable technical debt, nobody would be incentivized to fix later. As mentioned above, this also opens the doors to unit testing, but I understand that it will slow reviewing considerably for now. PS: no, EDIT: of course, there is no need for a big bang switch, we can proceed in parallel, as long as the main branch does not touch anything in Anyway, here is the validation checklist of the controls that I could not initialize on my own with sensible values. The following are the parameters which currently DO NOT have any sanity check (everything else has been assigned values I believed to be safe, but let me know otherwise):
|
|
I have cleared my backlog of changes to be made, and fixed all the bugs (except one in |
|
will have a look when I find the time |
|
cant wait for this update to become final i hope it becomes much faster UI |








Executive summary
This PR rewrites entirely the UI of OneTrainer with a modern graphical engine (QT6). Conflicts should be minimal, as changes are limited to
modules.uiandmodules.util.enum, performance improvements are given by a more efficient rendering engine (compared to tkinter) and a completely rewritten concurrent execution model.The old UI code has been decoupled into a Model-View-Controller architecture, making future maintenance easier.
It works on my machine™
Closes
Issues: #584 #618
PRs: #749
Breaks
Every PR modifying something in
modules.ui(anything else can be updated transparently and without issues). The sooner this PR is accepted, the fewer conflicts we should expect.Simplifies (but does not solve yet)
Issues: #220 #316 #570 #765 #1022 #958 #956 #955
PRs: #908 (must be reimplemented), possibly renders #1111 obsolete?
Expected bugs
Bugs found so far
These bugs do not affect training (I have successfully trained SD1.5 and SDXL fine tunes and loras, it seems parameters are read correctly), but help is definitely needed to check for all the cases.
Contents of
modules.ui.README.mdQT6 GUI Overview
QT6 GUI Overview
Overall Architecture
The GUI has been completely re-implemented as a Model-View-Controller architecture, for better future-proofing.
The folder structure is the following:
modules/ui/models: OneTrainer functionalities, abstracted from GUI implementationmodules/ui/controllers: Linker classes, managing how models should be invoked, validating (complex) user inputs and orchestrating GUI behaviormodules/ui/views:*.uifiles drawing each component, in a way which is as data-agnostic as possiblemodules/ui/utils: auxiliary classes.Models
Model classes collect original OneTrainer functionalities, abstracting from the specific user interface.
As models can potentially be invoked from different processes/threads/event loops, each operation modifying internal states must be thread-safe.
Models subclassing
SingletonConfigModelwrapmodules.util.configclasses, exposing a singleton interface and a thread-safe dot-notation-based read/write mechanism.Other models provide auxiliary utilities (e.g., open the browser, load files, etc.) and are mostly grouped conceptually (i.e., all file operations are handled by the same class).
Thread-safe access to model objects is mediated by a global QSimpleMutex, shared by every subclass of
SingletonConfigModel. Multiple levels of synchronization are possible:self.configattribute which can be accessed safely withWhatever.instance().get_state(var)andWhatever.instance().set_state(var, value)(or unsafely withWhatever.instance().config.var)self.bulk_read()andself.bulk_write()methods. These should be used to make sure that users editing UI controls while a multiple variables are read consecutively do not result in an inconsistent state.with self.critical_region_read()andwith self.critical_region_write()mediate access to a shared resource with an instance-specific reentrant read-write lock. Most, if not all, synchronizations should use these two context managers.with self.critical_region()uses a generic reentrant lock which is instance-specificwith self.critical_region_global()uses a generic reentrant lock which is shared across every subclass ofSingletonConfigModel.Controllers
Controller classes are finite-state machines that initialize themselves with a specific sequence of events, and then react to external events (slots/signals).
Each controller is associated with a view (
self.ui) and is optionally associated with a parent controller (self.parent), creating a hierarchy with theOneTrainerControllerat the root.At construction, each controller executes these operations:
BaseController.__init__: initializes the view_setup(): setups additional attributes (e.g., references to model classes)_loadPresets(): for controls that contain variable data (e.g., QComboBox), loads the list of values (typically from amodules.util.enumclass, or from files)self.state_ui_connectionsdict: connects ui elements toStateModelvariables bidirectionally (every time a control is changed, theTrainConfigis updated, and every timestateChangedis emitted, the control is updated)_connectUIBehavior(): forms static connections between signals and slots (e.g., button behaviors)_connectInputValidation(): associates complex validation functions (QValidators, slots, or other mechanisms) to each control (simple validations are defined in view files)update_after_connect=Trueself.__init__: Additional controller-specific initializations.The
state_ui_connectionsdictionary contains pairs{'train_config_variable': 'ui_element'}for ease of connection, and a similar pattern is often used for other connections. This dictionary involves only connections withStateModel.Other models are connected to controls manually in
_connectUIBehavior(), using a similar pattern on a local dictionary.Every interaction with non-GUI code (e.g., progress bar updates, training, etc.) is mediated by signals/slots which invoke model methods.
Controllers also have the responsibility of owning and handling additional threads. This is to guarantee better encapsulation and future-proofing, as changing libraries or invocation patterns will allow to keep the models untouched.
Views
View files are created with QtCreator, or QtDesigner, and assumed to expose, whenever possible,data-agnostic controls (e.g., a QComboBox for data types, the values of which are populated at runtime).
Naming convention: each widget within a
*.uifile is either a decoration (e.g., a static label or a spacer) with its default name (e.g.label_42), or is associated with a meaningful name in the formcamelCaseControlNameXxx,where
Xxxis a class identifier:Lbl: QLabelLed: QLineEditTed: QTextEditCbx: QComboBoxSbx: QSpinBox or QDoubleSpinBoxCmb: QComboBoxLay: QVerticalLayout, QHorizontalLayout or QGridLayoutBtn: QPushButton or QToolButton.This convention has no real use, other than allowing contributors to quickly tell from the name which signals/slots are supported by a given UI element.
Suggested development checklist:
Note that
*.uifiles allow for simple Signal-Slot connections to be defined directly from the WYSIWYG editor, however this can lead to maintenance headaches, when a connection is accidentally made both on the View and the Controller. I strongly advice to connect controls only in the_connectUIBehavior()andconnectInputValidation()methods of the Controller.Violations of the Model-View-Controller architecture:
.uifile.Utils
Auxiliary, but QT-dependent, classes.
FigureWidget: Figure widget for plots and images. Can be instantiated with a toolbar (separateMaskDrawingToolbarclass) for inspection or image editing (arbitrary tools are managed by the controller instantiating the widget).OneTrainerApplication: Subclass of QApplication defining global signals which can be connected from any ControllerWorkerPool: Generic threaded processor executing functions on a thread pool automatically managed. Functions can be made reentrant (i.e., they will be executed once, even when multiple calls are made, useful for example when a user attempts to scan the same folder before the previous operation terminated) if they are associated with a name.QT6 Notions
The following are some basic notions for useful QT6 features.
Signal-slot connections: QT's interactions are asynchronous and based on message passing. Each widget exposes two types of methods:
emit()ed. Some signals are associated with data with known type (e.g.,QLineEdit.textChangedalso transmits the text in a string parameter).@Slot(types)decorator, but arbitrary python functions can act as slots, as long as their parameters match the signal.@Slotdecorator does not accept the idiomtype|None, you can either use "normal" functions, or decorate them with@Slot(object)for nullable parameters.A signal-slot connection can be created (
connect()) and destroyed (disconnect()) dynamically.Every time a signal is emitted, all the slots connected to it are executed.
Important considerations:
widget.blockSignals(True)before changing its value.QLineEdit.textChanged(str)andQLabel.text(str)will automatically copy the text from the line edit to the label) from the UI editor, this is convenient, but there is the risk of forgetting to connect something, or connecting it twice (once in the UI editor and then again in python code)Buddies: Events involving QLabels can be redirected to different controls (e.g., clicking on a label may activate a text box on its right), to improve the user experience.
Buddies can be associated statically in
*.uifiles, or associated programmatically (e.g., when a label is created from python code).Widget promotion: Widgets can be subclassed to provide additional functionalities, without losing the possibility of exploiting the WYSIWYG editor. It is sufficient to define a widget as belonging to a particular class, and registering at runtime the promotion.
Text masks and validators: Invalid QLineEdit input can be rejected automatically with either of two mechanisms:
hh:mm:ssformat for times, or#hhhhhhfor RGB colors) by altering the text as it is editedreturnPressedandeditingFinishedsignals as long as the text entered does not pass the checks, and optionally expose methods to correct invalid text (default QValidators, such a QRegexValidator, use these additional methods to automatically cancel invalid characters as they are typed).Localization: Each string defined in
*.uifiles, as well as each string processed by QTranslator,tr()orQCoreApplication.translate()can be automatically extracted into an xml file by thelupdatetool, translated by native-language contributors, and loaded at runtime.Since
lupdateis a static analyzer, it is important that each string can be inspected from the source file (i.e.,tr("A static string")will be translatable,my_var = "A not-so-static string"; tr(my_var)will not).Concurrent Execution Model
The application uses multiple approaches for concurrent execution.
QThreadPoolmodel. This is transparent from the programmer's perspective, but using only Signals and Slots for every non-trivial communication mechanism is important to prevent unintended behavior arising from this execution model.ImageModelandBulkModelrely on the MapReduce paradigm, therefore are implemented with a standardmultiprocessing.Pool.mapapproach. Note that since it internally relies onpickle, not every function can be run on it (namely, class methods and lambdas are not pickleable)WorkerPooloffers three execution mechanisms, all exposing the same Signal-Slot-based interface:QRunnablefunctions automatically handled byQThreadPool, which can be enqueued arbitrarily many times.QRunnablefunctions automatically handled byQThreadPool, which are reentrant based on a name (if aQRunnablewith the same name is already running, the new worker is not enqueued).threading.Threadfunctions, manually launched. This addresses two limitations ofQRunnable, at the expenses of sacrificing automatic load balancing: exceptions in the underlying C++ code can crash the application, and the absence ofjoin().Decisions and Caveats
modules/util/enumhave been extended with methods for GUI pretty-printing (modules.util.enum.BaseEnum.BaseEnumclass), without altering their existing functionalityQCoreApplication.translate(), because it groups them by context (e.g.QCoreApplication.translate(context="model_tab", sourceText="Data Type")), allowing explicit disambiguation every time, and providing translators with a somewhat ordered xml (every string with the same context will be close together).BaseController._connect()to easily manage reconnections of dynamic widgets, and the "low level" methods should not be called directly.modelChanged,openedConcept(int), etc.), which are used to guarantee data consistency across all UI elements, by letting slots handle updates. This should be cleaner than asking the caller to modify UI elements other than its own.modules.ui.modelsclasses simply handle the backend functionalities that were implemented in the old UI. In the future it may be reasonable to merge it withmodules.util.configinto thread-safe global states.