search

sarah-walker-pcem / pcem

PCem
http://pcem-emulator.co.uk
GNU General Public License v2.0
1.52k stars 207 forks source link

wxWidget Issues under Wayland #128

Open lunakittyyy opened 2 years ago

lunakittyyy commented 2 years ago

Describe the bug Right clicking on the emulated PC on a Wayland environment such as stock Ubuntu 21.10 (My current configuration) causes the right click context menu to disappear shortly after, and it will not function again until PCem is restarted. Running PCem with the environment variable GDK_BACKEND=x11 appears to work around this bug.

To Reproduce Use a Wayland-based desktop and right click on an emulated PC without any GDK_BACKEND environment var specified

Expected behavior Menu works and allows you to configure settings such as media connected.

Screenshots Errors in console: image

Emulator configuration

Host machine

lunakittyyy commented 2 years ago

I'll try to reproduce this on 22.04 LTS when that releases some time next month.

robdaemon commented 2 years ago

I can reproduce this on Fedora 35 amd64 under Wayland.

Requires the same GDK_BACKEND variable to workaround it.

michael-manley commented 2 years ago

I am wondering if this is a wxWidgets + GTK issue in Wayland. Since I don't use Wayland much, I haven't checked much of this.

I am going to reference #101 as it seems Wayland is also part of this. I will change the name to wxWidget Issues under Wayland

lunakittyyy commented 2 years ago

I believe both me and robdaemon were using GNOME. Can someone try KDE Plasma under Wayland as well?

lunakittyyy commented 2 years ago

I'll try to reproduce this on 22.04 LTS when that releases some time next month.

Same results on Ubuntu 22.04 LTS. Workaround still works fine.

daniel-nth commented 2 years ago

Same issue on Chrome OS/Crostini. The workaround fixes it. Adding non-zero x/y coordinates to the window->PopupMenu() call in Frame::OnPopupMenuEvent() also lets the menu appear, but the behavior is not perfect (it might get hidden behind the main window if you click it again).

bartman081523 commented 2 years ago

Thanks @WowItsKaylie , I was not able to open the menu either, and I wondered where I put the iso file in the cd-drive. GDK_BACKEND=x11 pcem fixed the right-cklick menu, and I am able to mount an iso image.

michael-manley commented 2 years ago

Being on Silverblue lately and now with Nvidia cards can use Wayland, i have seen in other apps where GTK gets wonky with geometry on Wayland so it is 100% a GTK/Wayland issue from what I see

fourier commented 1 year ago

Can confirm the issue and a workaround, Ubuntu 22.04.1 LTS

lunakittyyy commented 1 year ago

Being on Silverblue lately and now with Nvidia cards can use Wayland, i have seen in other apps where GTK gets wonky with geometry on Wayland so it is 100% a GTK/Wayland issue from what I see

@michael-manley super late response since someone posted in this thread again, but perhaps we need more testing if its GTK? Needs some more testing in Sway, KDE Wayland, etc.

thp commented 1 year ago

(walls-of-text warning: some logging of looking where the root cause of this bug might be)

Edit: Package versions used locally for the record on Ubuntu 23.04:

Gtk3 3.24.37-1ubuntu1 wxGTK 3.2.2+dfsg-2build1

On my machine, the right-click in the SDL2 window is handled in src/wx-ui/wx-sdl2-display.c:

        while (SDL_PollEvent(&event)) {
                switch (event.type) {
                case SDL_MOUSEBUTTONUP:
                        if (!mousecapture) {
                                if (event.button.button == SDL_BUTTON_LEFT && !pause) {
                                        window_doinputgrab = 1;
                                        if (video_fullscreen)
                                                window_dofullscreen = 1;
                                } else if (event.button.button == SDL_BUTTON_RIGHT)
                                        wx_popupmenu(ghwnd, menu, 0, 0);

It eventually prints the following error messages on the console:

(pcem:112010): Gtk-WARNING **: 21:16:23.631: no trigger event for menu popup

(pcem:112010): Gtk-CRITICAL **: 21:16:23.631: gtk_menu_popup_at_rect: assertion 'GDK_IS_WINDOW (rect_window)' failed

The wx_popupmenu() is defined in src/wx-ui/wx-utils.cc, it just puts a new PopupMenuEvent on the WX event queue. This event type is registered in src/wx-ui/wx-app.cc as Bind(WX_POPUP_MENU_EVENT, &Frame::OnPopupMenuEvent, this);, and the implementation is this:

void Frame::OnPopupMenuEvent(PopupMenuEvent &event) {
        wxWindow *window = event.GetWindow();
        wxMenu *menu = event.GetMenu();
        int *x = event.GetX();
        int *y = event.GetY();

        window->Show();
        window->SetFocus();

        if (x && y)
                window->PopupMenu(menu, *x, *y);
        else
                window->PopupMenu(menu);
}

As wx_popupmenu(...) seems to be only called from the SDL2 window code, it always passed null pointers for x and y, which in turn means window->PopupMenu(menu); is the code path that gets called.

This means that right-clicking eventually calls this later in the event loop:

((wxWindow *)ghwnd)->PopupMenu((wxMenu *)menu);

So, where does ghwnd come from? It's a Frame * in src/wx-ui/wx-app.cc:

        frame = new Frame(this, "null frame", wxPoint(500, 500), wxSize(100, 100));

Then, frame->Start(); is called on it, which does:

void Frame::Start() {
        if (wx_start(this))
                ShowConfigSelection();
        else
                Quit(0);
}

wx_start() then takes its first argument and stores it in ghwnd.

The value of menu is obtained from ghwnd via wx_getmenu(), which is (in src/wx-ui/wx-utils.cc):

void *wx_getmenu(void *window) { return ((Frame *)window)->GetMenu(); }

..this is from the same Frame object as above.

So in the end, because Frame IS the window (aka ghwnd, or this in the scope of Frame::OnPopupMenuEvent) and, it has a member that IS the menu, Frame::OnPopupMenuEvent could at the moment be simplified to:

void Frame::OnPopupMenuEvent(PopupMenuEvent &event) {
        PopupMenu(menu);
}

Now, for the error message:

"no trigger event for menu popup"

This comes from Gtk3:

https://gitlab.gnome.org/GNOME/gtk/-/blob/gtk-3-24/gtk/gtkmenu.c

There are 3 places where this could happen:

If we grep for which functions are called from wxWidgets (using Git master branch here, the installed version might be slightly older):

https://github.com/wxWidgets/wxWidgets/blob/d7a98a44ea6c9f814e35db1d1aa869d3b5bf10d1/src/gtk/window.cpp#L6050

% ag gtk_menu_popup
src/gtk/window.cpp
6079:            gtk_menu_popup_at_pointer(GTK_MENU(menu->m_menu), nullptr);
6083:            gtk_menu_popup_at_rect(GTK_MENU(menu->m_menu),
6092:        gtk_menu_popup(
6105:    // it is possible for gtk_menu_popup() to fail

All the calls turn out to be in the wxWindowGTK::DoPopupMenu() (which I'm freely assuming is the wxGTK implementation of the PopupMenu() function we're calling above).

The problem is, these functions assume that there's a "trigger event" processed at the moment (e.g. if this call comes from within a mouse event handling routine) and always pass nullptr to the gtk_menu_popup_*() functions which means Gtk tries to find a "trigger event" by asking gtk_get_current_event(), and that fails, because:

It seems like with wxGTK, when compiling against GTK < 3.22 (#if GTK_CHECK_VERSION(3,22,0) in src/gtk/window.cpp) or when these checks fail:

    if (wxGTKImpl::IsWayland(window) && wx_is_at_least_gtk3(22))

..then it falls back to gtk_menu_popup(), which seems to work for X11 (but I tried using it on Wayland, and it doesn't seem to work, which the Gtk docs also mention (and why the function is deprected in Gtk 3.22):

Note that this function does not work very well on GDK backends that do not have global coordinates, such as Wayland or Mir. You should probably use one of the gtk_menu_popupat variants, which do not have this problem.

(which is in fact what wxGTK tries to do...)

In any case, that's how far I have come so far.

As a proof-of-concept workaround (but not really that useful, as the GDK_BACKEND=x11 workaround can fix it as well for now, at least as long as some kind of Xwayland is running in the background), one can show the "null frame" that's used as a menu container (and to pop up the menu) and set its menu bar (it will be a separate window, but the menu on it works, because when the menu is clicked, there is an actual Gtk trigger event). The return; in the right click popup handling code is there, because wxGTK complains that you cannot popup a menu that's attached to a menubar, so this just no-ops the right click.

diff --git a/src/wx-ui/wx-app.cc b/src/wx-ui/wx-app.cc
index 685f1d2e..bd2c9264 100644
--- a/src/wx-ui/wx-app.cc
+++ b/src/wx-ui/wx-app.cc
@@ -58,6 +58,10 @@ bool App::OnInit() {
         InitXmlResource();

         frame = new Frame(this, "null frame", wxPoint(500, 500), wxSize(100, 100));
+        wxMenuBar *bar = new wxMenuBar;
+        bar->Append((wxMenu *)wx_getmenu(frame), "Menu");
+        frame->SetMenuBar(bar);
+        frame->Show();
         frame->Start();
         return true;
 }
@@ -67,7 +71,7 @@ int App::OnRun() { return wxApp::OnRun(); }
 #include <sstream>

 Frame::Frame(App *app, const wxString &title, const wxPoint &pos, const wxSize &size)
-        : wxFrame(NULL, wxID_ANY, title, pos, size, 0) // wxDEFAULT_FRAME_STYLE & ~(wxRESIZE_BORDER))
+        : wxFrame(NULL, wxID_ANY, title, pos, size, wxDEFAULT_FRAME_STYLE)
 {
         this->closing = false;
         this->menu = wxXmlResource::Get()->LoadMenu(wxT("main_menu"));
@@ -157,6 +161,8 @@ void Frame::OnShowWindowEvent(wxCommandEvent &event) {
 }

 void Frame::OnPopupMenuEvent(PopupMenuEvent &event) {
+    return;
+
         wxWindow *window = event.GetWindow();
         wxMenu *menu = event.GetMenu();
         int *x = event.GetX();

In summary, the problem seems to be a Gtk popup not opening with a hidden window on Wayland when a non-Gtk window (SDL2 window in this case) has focus and was clicked.

The only Gtk3+SDL2 code I found via a quick Google search assumes X11, so again cannot be used to "fuse" Gtk3/wxGTK and SDL2 together. If it doesn't need to be a popup menu, the "null frame" could just be shown and focused on right click instead as a separate window (not sure how annoying that would be, though).

lgblgblgb commented 1 year ago

I'm not sure at all if my comment is useful:

I had somewhat similar situation with my own project (not related to pcem at all), using SDL2 in general, but using GTK3 for pop-up menus, dialogs etc only. If running on wayland, there can be a situation that SDL still uses the X11 driver, but GTK defaults to native wayland, then the interaction between them goes crazy, no menu can be seen, and other problems. My workaround was something like this (IIRC, I write this now by heart):

if (!strcasecmp(SDL_GetCurrentVideoDriver(), "x11"))
    setenv("GDK_BACKEND", "x11", 1);

Before initializing GTK, (but after initialization SDL2, of course). This will force GTK to use x11 backend as well, if SDL uses that. Of course this scenario may or may not fit this situation with pcem.

davide125 commented 1 year ago

I can confirm this is an issue on Fedora Linux 38 as well. Moreover, unless SDL is also forced to use x11 via SDL_VIDEODRIVER=x11 we get a segfault on machine start (see https://bugzilla.redhat.com/show_bug.cgi?id=2212225 for details). I put up https://github.com/sarah-walker-pcem/pcem/pull/222 as a temporary workaround.

lgblgblgb commented 1 year ago

@davide125 At least with my non-pcem experience with SDL+GTK I only had problem when there is a mismatch between SDL and GTK, one using wayland, and the other does not. If both using wayland, it worked for me just fine (for sure, also, when both using X11 already). So my workaround above, to force only GTK to use X11 if SDL uses that too. Surely my clumsy workaround above can have code for the case when SDL uses wayland, and then GTK should be forced to use wayland as well, just in case to have the same backend for the two libraries.