StrikerX3/Ymir

Rework frontend code

Open

#20 opened on Apr 30, 2025

View on GitHub
 (0 comments) (1 reaction) (0 assignees)C++ (52 forks)auto 404
frontend-sdl3help wanted

Repository metrics

Stars
 (988 stars)
PR merge metrics
 (PR metrics pending)

Description

The current SDL3 frontend code grew unwieldy, with lots of complexity added by the use of threads and several UI components that need to talk to each other. There's a simple and limited async one-directional event system in place that works for simple cases, but can't be used to run things like futures/promises or Qt-style signals and slots. The entire GUI was built on top of ImGui which wasn't designed for some of the use cases employed at the moment.

The pain points

Let's begin with the biggest of the issues I've had to deal with: the Backup Memory Manager window. This window alone revealed the worst cracks in the current design of the frontend app: lack of two-way communication, complex thread synchronization, limited GUI features and poor application architecture.

The Saturn contains a fixed internal backup memory and an optional, external backup memory in the form of a cartridge. The intention of the Backup Memory Manager window is to allow the user to manage their backup files in a friendly and intuitive UI without having to go through the Saturn's memory manager UI. I did manage to accomplish this in a satisfying way for the user, but had to commit several atrocities in the code for it to happen.

Dealing with the internal backup memory was easy enough. However, due to the current design of the emulator core and the use of threads, dealing with the cartridge was a massive pain in the rear. In order to understand why, you'll first need to understand how the application is architected and how the emulator implements cartridges and backup memory.

Threads and the cartridge slot

The application runs two threads: the main thread with all the GUI and input logic, and the emulator thread dedicated to run the emulator core alone. This is done to decouple GUI updates from emulator execution speed and to improve emulation performance. As you can imagine, having the GUI thread access emulator state directly in this scenario can be dangerous, especially when accessing dynamically-allocated objects such as the cartridges.

Since the Saturn supports different types of cartridges, they are implemented as a simple hierarchy of classes, with BaseCartridge serving as the base class for all cartridge types and concrete implementations for each cartridge handling their specific logic. Among those, we have the BackupMemoryCartridge.

The cartridge slot stores a std::unique_ptr<BaseCartridge> that's always populated with a concrete instance of a cartridge, even when there is no cartridge (there's a special NoCartridge type specifically for that case - this is also known as the Null Object design pattern). It also offers a method that allows you to dynamically cast the cartridge to a pointer of the concrete type of cartridge. If the inserted cartridge is of that type, you get a valid pointer, otherwise you get a nullptr.

Obviously, the pointer is only valid as long as the cartridge isn't replaced, and this is pain point number one: because the emulator runs on its own thread, there's always the risk that this thread could replace the cartridge while the GUI holds a pointer to a cartridge object. In order to prevent this, I introduced a mutex that the GUI holds while it is working with the cartridge, and the emulator thread holds when it wants to replace the cartridge instance. The issue is, you have to remember to lock the cartridge mutex whenever you're working with the cartridge in the GUI thread. This is not an automatic process, it's something that needs to be constantly remembered, and you have to pepper the code with std::unique_lock lock{m_context.locks.cart}, which leads me to pain point number two:

The God object, SharedContext

Various parts of the application need access to certain "global" elements, like fonts, the Saturn instance, settings, locks, and many more. The SharedContext struct was introduced to grant easy access to those objects, but it has essentially turned into a God struct that contains almost everything in the application. In many cases this is completely unnecessary, since not every UI widget needs to access the input context, or every lock, or the save states, or what have you. This also increase compilation times for the application, since changes to any of the SharedContext's components cause a nearly full recompilation of the entire application.

There has to be a better way to structure these components such that the application's subsystems only have access to what they need. Before somene comes up with the lazy idea of storing it in globals: global state is strictly prohibited in this code base. Everything must belong to a struct/class or be a local variable, or static variable in a function or struct at most if it makes sense to have exactly one instance of it. The only things allowed to be globals are constants or process-wide variables (e.g. process ID, priority, a handle to raw mouse input device, etc.).

Asynchronous event queues, callbacks, file dialogs, UI management complexity

The SharedContext also holds two event queues: one for the GUI/main thread, and one for the emulator thread. These events are fire-and-forget, meant to perform simple actions in a thread-safe manner, but it has been used to do more than just that. The most complex interactions I had to implement in the Backup Memory Manager window were the import and export of backup files and full backup memory images. SDL3's file dialog system is invoked with a fire-and-forget function that takes a callback that is then invoked by an arbitrary thread (which may or may not be the main thread). So, as you can imagine, this is callback hell on top of multithreading hell and event queuing hell.

On top of having those callbacks, I also wanted to display modal windows with the results of importing or exporting the files, which meant adding yet another layer of complexity on an already complex window. Because you can't just call ImGui::OpenPopup from random places, I had to store the results to be displayed later on the drawing function that also handles the main window. The window class is littered with random fields meant to be temporary storage for these modals, which is extremely annoying and messy, and I also had to remember to clean up those fields when the popups are closed to limit memory usage, which is doubly annoying.

The debugger

The backend support for debugging was relatively straightforward to implement. I had prior experience with another emulator and I've been inspired by a few designs in the wild, so I knew what I was getting into and came up with a great architecture that works perfectly well.

Most of the debugger's UI elements were fairly simple to implement too. Some required a bit of attention due to the multithreaded design of the application, but nothing as terrible as the Backup Memory Manager window. The pain point here is the disproportionate amount of work required to implement the GUI counterpart to the debugger backend.

Designing GUIs is a slow and painful process

Most of the time was spent writing the same boilerplate code over and over again. The same table structure, the same PushFont/PopFont sequence to draw a hex field, the same ImGui::Text()/ImGui::SameLine()/ImGui::SomeWidget() to place the label before the widget, etc. etc. etc.

Granted, I could've written some wrappers for common cases, but it's not often that simple. Take for instance the hex input fields. I wanted those to have a specific width, be left-padded with zeros and use the monospace font. You'll find this boilerplate repeated many, many, many times on the debugger UI code:

uint8 vector = intc.GetVector(sh2::InterruptSource::DIVU_OVFI);
ImGui::SetNextItemWidth(ImGui::GetStyle().FramePadding.x * 2 + hexCharWidth * 2);
ImGui::PushFont(m_context.fonts.monospace.medium.regular);
if (ImGui::InputScalar("##vcrdiv", ImGuiDataType_U8, &vector, nullptr, nullptr, "%02X",
                       ImGuiInputTextFlags_CharsHexadecimal)) {
    intc.SetVector(sh2::InterruptSource::DIVU_OVFI, vector);
}
ImGui::PopFont();

Is it possible to move this to a function? Yes, everything is possible. But taking a closer look at it, we need to specify:

  • The input label (##vcrdiv in this case)
  • The data type (which would be nice if it was derived automatically from the data variable)
  • The value itself (which can just be a simple variable)
  • The field width (which may not necessarily be as wide as the data type allows)
  • A pointer to the font (or the context)
  • The format string (which also would be nice if it was automatically derived from the data variable)
  • Optional minimum/maximum constraints for the value (rarely used, but it is used)

Some of these are pretty trivial to solve. Some are extremely annoying. Ideally, the boilerplate would reduce to:

uint8 vector = intc.GetVector(sh2::InterruptSource::DIVU_OVFI);
// implied: width = 2 * sizeof(uint8)
if (widgets::HexField("##vcrdiv", &vector, m_context.fonts.monospace.medium.regular)) {
    intc.SetVector(sh2::InterruptSource::DIVU_OVFI, vector);
}

A bit less verbose, for sure, but have fun figuring out how to implement HexField to handle every case across the codebase.

On top of the constant boilerplate repetition, you also have to deal with ImGui's limitations. Things like centering widgets horizontally or vertically take extra effort to implement due to the immediate nature of ImGui. Tables have their own limitations too - you can't draw something spanning multiple columns, so your choices are either to draw a single column with the dynamic elements (see the SCU DSP debugger window for an example of this) or forego tables and manually align elements. Tables also have an annoying tendency to squish columns more than desired, so you'll see a lot of fixed-width columns everywhere.

Then, of course, you want the GUI to look nice, comfortable, and most importantly, usable. That requires a lot of iterations. And because there's no easy way to visualize ImGui windows, you have to compile the entire application over and over again. Luckily, Ninja build helps a lot with build speeds, but it still isn't quite as fast as using a visual editor like Qt Creator.

Recap and final thoughts

The application's UI and most of the logic runs on the main thread by necessity, since the event loop and several SDL3 actions must be invoked from that thread. The emulator runs on a dedicated thread for improved performance. Both threads have their own dedicated concurrent event queues which works fine for simple fire-and-forget commands, but there are certain cases where a more advanced signaling system would be more appropriate.

Most of the issues I've had while implementing the GUI were due to the multithreaded design, ImGui's limitations or architectural decisions that hindered implementation of certain features.

The emulator running in a dedicated thread is non-negotiable for two reasons:

  • Performance, as mentioned before. It runs up to 50% faster than running it on the main thread because there's no UI logic competing for CPU time and that also likely leads to better L1 cache utilization.
  • Detaching the emulator from the GUI thread enables the GUI to run at the monitor's refresh rate regardless of the emulated system's frame rate. This does make video sync more difficult, but it's well worth it for the overall user experience. I use a 240 Hz monitor and the difference between running the emulator in the main thread vs. its own thread is night and day.

The multithreaded design introduces several challenges:

  • Anything that accesses dynamically allocated objects (cartridges, peripherals, etc.) needs to be properly mutexed between the emulator and GUI threads. Replacing those instances must happen on the emulator thread, otherwise there's a risk of a race condition where the emulator thread attempts to access a dynamic object that's being reallocated by the GUI thread. Conversely, if the GUI is reading from those objects, it also needs to hold the mutex to make sure the emulator thread doesn't deallocate it before it's done working with it.
  • Any complex writes coming from the GUI thread must be enqueued to run on the emulator thread. This includes all memory editor writes, all debugger components trying to write to registers, and so on. Simple writes like those done to WRAM are fine, if racy. One could argue that writing data while the emulator is running is dangerous and unpredictable, but Ymir allows it and handles it as gracefully as possible.

The event system has its own limitations too:

  • The use of std::variant makes it cumbersome to add new events. The emulator queue alleviates this by allowing code to send a std::function to be executed in the context of the thread, but it still requires effort to build those events.
  • Events are fire-and-forget. You can't safely send back results like in a future/promise design.
  • Events are asynchronous and come with all the challenges of dealing with asynchronous execution too

Finally, while ImGui is a great choice for designing real-time GUIs, it's not a comfortable option for anything more advanced than debugger displays. It takes a lot of boilerplate to draw anything more complex than a simple series of fields, and performing complex interactions such as opening file dialogs and dealing with callbacks requires a ton of coordination between several parts of the application.

I don't think a full rewrite is warranted, but if you wish to do so, I won't stop you.

One potential limitation to keep in mind is that the current frontend uses the SDL3 Renderer API, which doesn't support shaders. If users want to use those CRT shaders and whatnot, that won't be viable at the moment. My attempt to switch to the SDL3 GPU API resulted in some weird graphics artifacts with Vulkan, which seems to be a driver issue that's likely that other users may face. I also get the same artifacts on other applications, however.

P.S: You'll also find slightly less filtered mini-rants about frontend development on app.cpp and backup_memory_view.cpp

Contributor guide