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

wxGLCanvasEGL::IsShownOnScreen() regression under Wayland #23899

Closed
vadz opened this issue Sep 24, 2023 · 1 comment · Fixed by #23900
Closed

wxGLCanvasEGL::IsShownOnScreen() regression under Wayland #23899

vadz opened this issue Sep 24, 2023 · 1 comment · Fixed by #23900

Comments

@vadz
Copy link
Contributor

vadz commented Sep 24, 2023

After thinking about this for a while I think I may have found a small oversight.
IsShownOnScreen() uses m_readyToDraw as one of the checks under Wayland. Before this PR, m_readyToDraw was set to true once after first wl_frame_callback_handler. After this PR now the m_readyToDraw is toggled after each frame.

IsShownOnScreen() probably shouldn't flap at 60hz?

I hacked it use gtk_window_is_visible() for IsShownOnScreen check and in my limited testing Kicad renders a bit better.

diff --git a/src/unix/glegl.cpp b/src/unix/glegl.cpp
index c35273caddaa..0f009e3f4fb8 100644
--- a/src/unix/glegl.cpp
+++ b/src/unix/glegl.cpp
@@ -697,12 +697,13 @@ bool wxGLCanvasEGL::SwapBuffers()
 bool wxGLCanvasEGL::IsShownOnScreen() const
 {
     wxDisplayInfo info = wxGetDisplayInfo();
+    GdkWindow* const window = GTKGetDrawingWindow();
     switch ( info.type )
     {
         case wxDisplayX11:
             return GetXWindow() && wxGLCanvasBase::IsShownOnScreen();
         case wxDisplayWayland:
-            return m_readyToDraw && wxGLCanvasBase::IsShownOnScreen();
+            return gtk_window_is_visible(window) && wxGLCanvasBase::IsShownOnScreen();
         default:
             return false;
     }

Originally posted by @artizirk in #23554 (comment)

@artizirk
Copy link

Sorry for being absent for quite a long time, life got too busy.

Thanks @artizirk, I think you're right and we shouldn't be using m_readyToDraw here, but I'm a bit confused as to how does it actually change anything for redrawing: we don't seem to use IsShownOnScreen() anywhere under Wayland (under X11 we use it in SwapBuffers()).

People are working on trying to get KiCad running on Wayland which is the reason why bunch of interesting bugs are cropping up when testing things (me included).

So, as far as I understand as a side observer. Most of the canvas rendering in Kicad is done via their Graphics Abstraction Layer (include/gal/graphics_abstraction_layer.h). Both Cairo and OpenGL GAL backends use IsShownOnScreen() inside their IsVisible() implementation. IsVisible() in turn is used in a whole lot of places to gate if things should be updated on the screen. (grep -r 'IsVisible()' | wc -l: 159) And thats why I think broken IsShownOnScreen affects KiCad rendering but only under Wayland. (There is also a chance that I misunderstood how wxWidgets and KiCad rendering works...)

@vadz vadz closed this as completed in cf44a87 Sep 30, 2023
vadz added a commit to vadz/wxWidgets that referenced this issue Oct 29, 2023
Don't use m_readyToDraw in it, as the canvas is still shown even when
it's not ready to draw yet, and use m_wlSubsurface instead as it is
destroyed when the window is unmapped.

See wxWidgets#23899.

(cherry picked from commit cf44a87)
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 a pull request may close this issue.

2 participants