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

WIP: Run the core on the main thread. #111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

comex
Copy link

@comex comex commented May 29, 2017

(Instead of using a worker thread as is done currently.)

Why would you want to do that? To fix no-vidext mode.

In mupen64plus-core, main_check_inputs calls SDL_PumpEvents. SDL_PumpEvents checks if there is a SDL_VideoDevice active - which in this case is only be true when running without vidext - and if so, calls the video backend's PumpEvents. On Mac, that's Cocoa_PumpEvents, which tries to pump the Cocoa event loop, which promptly crashes because you're only supposed to do that from the main thread.

According to the SDL FAQ, it's not just SDL_PumpEvents that should be called on the main thread, but all video functions:

Can I call SDL video functions from multiple threads?

No, most graphics back ends are not thread-safe, so you should only call SDL video functions from the main thread of your application.

So to make this correct, the core's no-vidext code (that uses SDL video) would have to be modified to do everything asynchronously on the main thread. I think that would be possible and nice to have, but in lieu of doing all the work for that, here's a patch to just run the core on the main thread.

I've verified on Mac that the UI is still usable while playing, in both vidext and no-vidext mode. But I haven't tested this on other platforms at all.

Caveats:

  • On Mac, opening a menu does its own "synchronous loop that pumps events" thing - it pumps events, but doesn't return to the code that sent it the menu-opened event until the menu is closed. So opening a menu causes emulation to freeze. While unintended, I'm not sure this behavior is actually undesirable, since the user probably opened the menu to select pause or stop anyway. Still, it suggests this approach is not optimal in the long term, and there may be similar or worse side-effects on other platforms.

  • In vidext mode, the script has the opportunity to call QCoreApplication.processEvents explicitly on each frame, but in no-vidext mode, it relies on the assumption that the core's SDL_PumpEvents also pumps Qt events, because they both run on top of the platform's native event loop. This assumption is true on Mac, and my guess is that it's also true on other platforms, but I haven't tested it. If it doesn't work somewhere, I guess the worker could be changed to support both modes, either running as a thread or borrowing the main thread.

@comex
Copy link
Author

comex commented May 29, 2017

Oh, and it also freezes the UI when paused in the debugger, because that waits on a semaphore (condition variable in my branch). Sigh… I really should try the other approach.

@comex
Copy link
Author

comex commented Jun 6, 2017

Going back to this – unfortunately, I don't think rendering off the main thread is feasible with SDL video (i.e. without the vidext). In this Stack Overflow post, a poster using Linux got X server errors when trying to render off thread, and 'solved' it by calling SDL_Init(SDL_INIT_VIDEO) in the rendering thread rather than the main thread. But that's clearly one of the "video functions" that the FAQ entry I quoted says should be called on the main thread. Even if we were to disregard the FAQ and init video on a separate thread, the lack of thread safety means that we should at least call other video functions from that same thread, yet Cocoa really wants video functions to be called from the system-defined main thread.

So I do think this patch might be the best approach, even if it's a bit icky. The debugger, for its part, can be fixed to pump the event loop so it doesn't freeze the UI when paused.

For the record, potential alternatives include:

  • Modifying the core to be capable of backing out of the call stack and returning to the caller on request, then resuming with a later call. That would be elegant, but would require some serious refactoring.
  • Using fibers/explicit stack switching to hackily achieve the same effect - i.e. the core would have its own stack, and could 'suspend' and switch back to the main stack (which could return to the caller), while keeping its own call stack in place. But from the system's perspective this would all be on the main thread. Fibers are kind of a relic of an older era, though, and not very portable.
  • Modifying the core to offload all rendering (not just mode setting and stuff) to the main thread, while keeping CPU emulation and the rest in a separate thread.
  • Just requiring the vidext to be enabled...

By the way, I think there's also a minor issue with the vidext, where the OpenGL context is obtained from Qt. Qt explicitly supports off-thread rendering – but it expects a QGLContext to be explicitly transferred to another thread, using moveToThread, before that thread can call makeCurrent on it. As far as I can tell, the Python code never calls moveToThread, so that should be fixed...

@gen2brain
Copy link
Member

I just tried your patch. It looks that non vidext mode is broken. In vidext mode UI can freeze when I for example open ROM from external hard drive that needs some time to wake up. Before, unpacking ROM was running in other thread so that was not an issue. Opening some dialogs will freeze/pause emulation, but some, like cheats dialog doesn't pause. I do like that it pauses emulation, I wanted to do something similar but when focus is lost.

For X11, there used to be a code that called X11InitThreads via ctypes before Qt app is created, then it was later changed to use Qt::AA_X11InitThreads attribute, but that was for Qt4 http://doc.qt.io/qt-4.8/qt.html , now in Qt5 it is still used but docs say that it is obsolete http://doc.qt.io/qt-5/qt.html .

I tried also to switch to QOpenGLWidget now that QGLWidget is deprecated #90 . I didn't check to see if getProcAddress is now added to PyQt for QOpenGLContext . Maybe newer OpenGL widget can help, a lot of things work differently with new widget.

What exactly are the issues in cocoa? I usually just try in OS X in vmware when I pack binaries, but cannot test everything in VM.

@loganmc10
Copy link
Member

Have you tested QOpenGLWidget with GLideN64? I don't think it will work. QOpenGLWidget renders into a Framebuffer object, which conflicts with GLideN64. You'll probably need to use one that renders onto the native window, like QOpenGLWindow

@gen2brain
Copy link
Member

@loganmc10 Just tried, you are right, it doesn't work. If native window must be used I guess QGLWidget is fine then, except that it is deprecated.

@gen2brain
Copy link
Member

Not sure if QMainWindow can be replaced with QOpenGLWindow (to add menus and statusbar), if that can work that is also an option. But again, only rice plugin can work correctly with vidext and resizing, for other plugins you need to stop ROM, resize, then start again.
Maybe vidext can be disabled/not allowed for other plugins but I will prefer not to do it like that. Anyway something will needs to be done, better before old QGLWidget is removed from Qt.

@loganmc10
Copy link
Member

You can treat QOpenGLWindow like a widget using QWidget::createWindowContainer().

You create like QOpenGLWindow like:

QOpenGLWindow *my_window = new QOpenGLWindow();
QWidget *container = QWidget::createWindowContainer(my_window);

And then use the "container" widget like you would any other widget, you can put it inside a QMainWindow. I'm not sure if this works in PyQt but it works in C++

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

Successfully merging this pull request may close these issues.

3 participants