wxGLCanvas inside wxPopupWindow gets positioned incorrectly on EGL+Wayland. (Issue #26250)
dsa-t
dsa-t created an issue (wxWidgets/wxWidgets#26250)In KiCad we have a wxGLCanvas inside a sizer in a wxPopupWindow.
On Wayland+EGL the canvas gets positioned incorrectly:
image.png (view on web)Also messages like this appear:
Trace: (glegl) Window 0x55555a048df0 is not not ready to draw yet
Trace: (glegl) In frame callback handler for 0x55555a048df0
This worked fine at some point. Also works fine with GDK_BACKEND=x11.
Single display, no scaling.
Platform and version information
- wxWidgets version you use: master (3.3)
- wxWidgets port you use: wxGTK
- OS and its version: Manjaro
- Which GDK backend is used: Wayland
- Desktop environment : KDE
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)I think the messages are fine/expected and not related to the wrong origin problem.
I'll try to look at this, starting by making a reproducer for it, but I suspect this might be another instance when we do something in wxTLW in wxGTK that ought to be done in wxNonOwnedWindow.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Jiří Pinkava
pinkavaj left a comment (wxWidgets/wxWidgets#26250)FYI: https://gitlab.com/kicad/code/kicad/-/work_items/23460
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)Here's a basic repro case:
The menu doesn't open on Wayland either with message Tried to map a popup with a non-top most parent
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)Thanks for the reproducer, the fix is actually embarrassingly simple:
diff --git a/src/unix/glegl.cpp b/src/unix/glegl.cpp index 02e3374143..daf5eb4a4c 100644 --- a/src/unix/glegl.cpp +++ b/src/unix/glegl.cpp @@ -449,7 +449,7 @@ void wxGLCanvasEGL::UpdateSubsurfacePosition() } int x, y; - gdk_window_get_origin(m_canvas->GTKGetDrawingWindow(), &x, &y); + gdk_window_get_position(m_canvas->GTKGetDrawingWindow(), &x, &y); wl_subsurface_set_position(m_wlSubsurface, x, y); }
Apparently this only worked before because for normal TLWs (0,0) of the root coordinate system is the top left corner of the window under Wayland — but for popups this is not the case. Using position instead of origin works fine in both cases however, so I'm going to push it, please retest with KiCad when you can.
Note that I'm still not sure what's going on with hiding/showing: I was wondering why did you do it like this and tried without these calls, but then the popup was never shown at all. I didn't try to debug this further, as I guess this is not a problem in real use, but please open another issue if it is. Thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)Note that I'm still not sure what's going on with hiding/showing: I was wondering why did you do it like this
Wayland doesn't allow moving shown windows, so we hide them before moving.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)That's how KiCad PCB editor looks now:
image.png (view on web)—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)Is this also with Wayland or with X11?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)logging origin/position:
origin=(0,0) position=(0,0)
origin=(237,285) position=(-1,-1)
origin=(241,310) position=(0,0)
origin=(241,307) position=(0,0)
origin=(241,253) position=(0,0)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)It is all fine when using GDK_BACKEND=x11
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)But does the sample with your patch work for you? I.e. is it the difference between the compositors (the patched sample works fine for me under Sway, I didn't test with anything else yet) or between the sample and the actual KiCad code?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)FWIW I've put the following in MyFrame::UpdatePopupPosition() (after Move() and Show()):
int x0, y0; gdk_window_get_origin(m_canvas->GTKGetDrawingWindow(), &x0, &y0); int x, y; gdk_window_get_position(m_canvas->GTKGetDrawingWindow(), &x, &y); wxLogDebug("Popup position=%d,%d, origin=%d,%d, pos=%d,%d", popupPos.x, popupPos.y, x0, y0, x, y);
and I get this kind of output with Wayland/Sway:
18:41:57: Debug: Popup position=149,103, origin=157,219, pos=8,116
18:41:57: Debug: Popup position=148,102, origin=156,218, pos=8,116
18:41:57: Debug: Popup position=147,101, origin=155,217, pos=8,116
In comparison, with X11 it's something like this:
18:43:51: Debug: Popup position=2835,475, origin=2843,591, pos=8,116
18:43:51: Debug: Popup position=2835,476, origin=2843,592, pos=8,116
18:43:51: Debug: Popup position=2835,478, origin=2843,594, pos=8,116
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)I get similar values. It fixes the sample, but not kicad.
As seen here, the "Text before preview" is not visible:
image.png (view on web)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)Wrapping the canvas in a panel breaks the sample:
CubePopupWindow(wxWindow* parent, bool stereoWindow, wxGLCanvas** canvasOut) : wxPopupWindow(parent, wxBORDER_SIMPLE) { auto* topLabel = new wxStaticText(this, wxID_ANY, "Before GL canvas"); auto* canvasPanel = new wxPanel(this, wxID_ANY); auto* canvas = new TestGLCanvas(canvasPanel, stereoWindow); canvas->SetMinSize(wxSize(320, 320)); auto* canvasSizer = new wxBoxSizer(wxVERTICAL); canvasSizer->Add(canvas, 1, wxEXPAND); canvasPanel->SetSizer(canvasSizer); auto* bottomLabel = new wxStaticText(this, wxID_ANY, "After GL canvas"); auto* sizer = new wxBoxSizer(wxVERTICAL); sizer->Add(topLabel, 0, wxALL | wxALIGN_CENTER_HORIZONTAL, 8); sizer->Add(canvasPanel, 1, wxLEFT | wxRIGHT | wxEXPAND, 8); sizer->Add(bottomLabel, 0, wxALL | wxALIGN_CENTER_HORIZONTAL, 8); SetSizerAndFit(sizer); if ( canvasOut ) *canvasOut = canvas; }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
dsa-t
dsa-t left a comment (wxWidgets/wxWidgets#26250)Something like this seems to do the trick:
void wxGLCanvasEGL::UpdateSubsurfacePosition() { if ( !m_wlSubsurface ) { // In some circumstances such as when reparenting a canvas between two hidden // toplevel windows, GTK will call size-allocate before mapping the canvas // Ignore the call, the position will be fixed when it is mapped return; } GtkWidget* toplevel = gtk_widget_get_toplevel(m_canvas->m_widget); GdkWindow* toplevelWindow = gtk_widget_get_window(toplevel); int tlwx, tlwy, ox, oy; gdk_window_get_origin(toplevelWindow, &tlwx, &tlwy); gdk_window_get_origin(m_canvas->GTKGetDrawingWindow(), &ox, &oy); wl_subsurface_set_position(m_wlSubsurface, ox - tlwx, oy - tlwy); std::cout << "origin=(" << ox << "," << oy << ") toplevel origin=(" << tlwx << "," << tlwy << ")" << std::endl; }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)Yes, this makes sense, thanks. The funny (sad?) thing is that I had started with something very similar, if not identical, but then simplified it to the fix which I pushed because I couldn't see any difference.
I'll test this a bit more and push later, thanks!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#26250)There is also gtk_widget_translate_coordinates() which could be used for this, but I'm not sure if there is really any difference between what it does and your patch in this case.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.