Add EGL-based backend for wxGLCanvas #2038

swt2c · 2020-08-28T15:01:14Z

Among other things, this enables wxGLCanvas to be used natively on Wayland.

Among other things, this enables wxGLCanvas to be used natively on Wayland. Fixes wxWidgets#17702.

swt2c · 2020-08-28T16:00:33Z

Updated to (hopefully) fix some CI errors for GTK2, CMake.

vadz

Thanks a lot, it's great that you could have finished it!

I don't know anything about neither EGL nor Wayland, so I can't meaningfully review it, and I'm not sure if anybody else can, so let's merge it and see how it works in practice.

If you can fix the various minor comments, it would be great, but if not I could do it myself a bit later (once I deal with the other "in progress" PRs).

Thanks again!

include/wx/gtk/glcanvas.h

include/wx/setup_inc.h

include/wx/unix/glegl.h

include/wx/utils.h

src/unix/glegl.cpp

include/wx/unix/glegl.h

src/unix/glegl.cpp

paulcor

Yes, thanks, we really need this. I've taken a quick look and made a few comments.

src/unix/glegl.cpp

paulcor · 2020-08-28T16:45:35Z

src/unix/glegl.cpp

+#ifdef GDK_WINDOWING_WAYLAND
+    if ( GDK_IS_WAYLAND_WINDOW(window) ) {
+        int x, y;
+        gdk_window_get_origin(window, &x, &y);


gdk_window_get_origin() is documented as returning root window coordinates, but Wayland won't give you those, so aren't x and y always zero here?

Actually, I was hoping to discuss that with you. I was originally using gdk_window_get_geometry() which did always return zero for x and y. However, that resulted in the Wayland surface being incorrectly positioned by about 26x70 pixels (can't remember the exact value). gdk_window_get_origin() does not return zero on Wayland and it seems to return the correct offset that is needed. The only case where it seems not to work is if you maximize the window.

All of the examples of doing Wayland EGL on top of a GTK window I could find always use gdk_window_get_geometry() for positioning the wl_subsurface, though, so I wonder if something isn't right with the positioning of the underlying GdkWindow on Wayland?

include/wx/unix/glegl.h

swt2c · 2020-08-28T18:32:03Z

Thank you both for your comments. I'll send some fixes, but do you have a preference about whether you'd rather see fixes in new commits (with a squash later) vice just amending the original commit? My personal preference is to just keep amending the original commit, but I don't know if that makes it harder to see what I fixed.

vadz · 2020-08-28T20:31:12Z

Please don't amend the existing commits if possible, this makes it difficult/impossible to see the changes in the latest versions. Rather make your commits with git commit --fixup (or --squash) and then rebase them once everything is finalized (or I could rebase them before merging if you prefer). TIA!

swt2c · 2020-08-29T00:43:31Z

Okay, I think all initial comments are either fixed or at least responded to.

vadz · 2020-08-29T00:46:20Z

Thanks a lot for the fixes! I'm still not totally sure about the error message (should we really mention Wayland when not running under it?), but this can be tweaked later and I'm fine with merging this right now. I don't know about what's going on with gdk_window_get_origin(), unfortunately. @paulcor please let us know if you'd like to change anything here before merging.

swt2c · 2020-08-29T00:58:56Z

Thanks a lot for the fixes! I'm still not totally sure about the error message (should we really mention Wayland when not running under it?), but this can be tweaked later and I'm fine with merging this right now. I don't know about what's going on with gdk_window_get_origin(), unfortunately. @paulcor please let us know if you'd like to change anything here before merging.

We mention X11 when not running under it too?

vadz · 2020-08-29T01:02:58Z

We mention X11 when not running under it too?

Yes, but this makes sense because we advise to switch to it to make OpenGL work. Switching to Wayland may not be possible and, worse, even if it is, OpenGL might still not work there if EGL is not available in this build. Which is why I think this might be misleading/aggravating to the user, if we send them on a wild goose chase...

paulcor · 2020-08-29T02:49:30Z

Scott Talbert wrote:

@swt2c commented on this pull request. > +} + +/* static */ +EGLDisplay wxGLCanvasEGL::GetDisplay() +{ + wxDisplayType type = wxDisplayNone; + void *display = wxGetDisplayAndType(&type); + EGLenum platform; + if (type == wxDisplayX11) + platform = EGL_PLATFORM_X11_EXT; + else if (type == wxDisplayWayland) + platform = EGL_PLATFORM_WAYLAND_EXT; + else + return EGL_NO_DISPLAY; + + return eglGetPlatformDisplay(platform, display, NULL); Actually, it isn't. :-)

Oops. Sorry. Well, I did say it was a quick look...

vadz · 2020-08-31T13:03:49Z

@paulcor Do you have any objections to merging it? If not, I'll do it soon.

paulcor · 2020-08-31T14:47:19Z

No objections.

paulcor · 2020-09-01T15:22:32Z

I'm getting link errors building the opengl samples:
libwx_gtk3u_gl-3.1.so: error: undefined reference to 'wl_registry_interface'
and 15 more similar.

swt2c · 2020-09-01T15:30:18Z

I'm getting link errors building the opengl samples:
libwx_gtk3u_gl-3.1.so: error: undefined reference to 'wl_registry_interface'
and 15 more similar.

Is that fixed by your 05a134d?

paulcor · 2020-09-01T15:32:18Z

Scott Talbert wrote:

Is that fixed by your 05a134d?

No.

swt2c · 2020-09-01T15:36:56Z

Okay, I'll look into it. I guess we need to link the gl library against the wayland libraries, but detecting when to do so may be tricky.

swt2c · 2020-09-01T16:40:49Z

I'm getting link errors building the opengl samples:
libwx_gtk3u_gl-3.1.so: error: undefined reference to 'wl_registry_interface'
and 15 more similar.

What environment do you use, BTW?

paulcor · 2020-09-01T17:41:07Z

Scott Talbert wrote:

What environment do you use, BTW?

My DE is XFCE, using X11. But I have Wayland devel packages installed, and can run clients under weston.

swt2c · 2020-09-01T17:57:58Z

On Tue, 1 Sep 2020, paulcor wrote: My DE is XFCE, using X11. But I have Wayland devel packages installed, and can run clients under weston.

What distribution?

paulcor · 2020-09-01T18:25:19Z

Scott Talbert wrote:

What distribution?

Fedora 32

paulcor · 2020-09-02T16:00:44Z

This works for me:

diff --git a/configure.in b/configure.in
index fd7d7d9caf..61b6bf266b 100644
--- a/configure.in
+++ b/configure.in
@@ -3795,7 +3795,7 @@ if test "$wxUSE_OPENGL" = "yes" -o "$wxUSE_OPENGL" = "auto"; then
                         OPENGL_LIBS="-lGL -lGLU"
 
                         if test "$WXGTK3" = 1; then
-                            PKG_CHECK_MODULES(EGL, [egl >= 1.5],
+                            PKG_CHECK_MODULES(EGL, [egl >= 1.5 wayland-egl],
                                 [
                                     OPENGL_LIBS="$OPENGL_LIBS $EGL_LIBS"
                                     AC_DEFINE(wxUSE_GLCANVAS_EGL)

swt2c · 2020-09-02T16:21:40Z

This works for me:

diff --git a/configure.in b/configure.in
index fd7d7d9caf..61b6bf266b 100644
--- a/configure.in
+++ b/configure.in
@@ -3795,7 +3795,7 @@ if test "$wxUSE_OPENGL" = "yes" -o "$wxUSE_OPENGL" = "auto"; then
                         OPENGL_LIBS="-lGL -lGLU"
 
                         if test "$WXGTK3" = 1; then
-                            PKG_CHECK_MODULES(EGL, [egl >= 1.5],
+                            PKG_CHECK_MODULES(EGL, [egl >= 1.5 wayland-egl],
                                 [
                                     OPENGL_LIBS="$OPENGL_LIBS $EGL_LIBS"
                                     AC_DEFINE(wxUSE_GLCANVAS_EGL)

A couple things with that:

I think we need to ensure that the GTK we're using was built with Wayland support. Although I guess it won't actually hurt to link with wayland-egl even if GTK doesn't have Wayland support?
This will require wayland-egl to get wxUSE_GLCANVAS_EGL, right? I think we should still prefer the EGL backend even without Wayland?

swt2c · 2020-09-02T18:47:26Z

What do you think about the following instead?

--- a/configure.in
+++ b/configure.in
@@ -3799,6 +3799,14 @@ if test "$wxUSE_OPENGL" = "yes" -o "$wxUSE_OPENGL" = "auto"; then
                                 [
                                     OPENGL_LIBS="$OPENGL_LIBS $EGL_LIBS"
                                     AC_DEFINE(wxUSE_GLCANVAS_EGL)
+                                    PKG_CHECK_MODULES(WAYLAND_EGL, [gdk-wayland-3.0 wayland-egl],
+                                        [
+                                            OPENGL_LIBS="$OPENGL_LIBS $WAYLAND_EGL_LIBS"
+                                        ],
+                                        [
+                                            AC_MSG_NOTICE([wayland-egl not found.  wxGLCanvas will not have native Wayland support.])
+                                        ]
+                                    )
                                 ],
                                 [
                                     AC_MSG_NOTICE([EGL 1.5+ not available. Will use GLX.])

paulcor · 2020-09-02T19:35:40Z

I'm wondering if EGL should be used when the GTK Wayland backend is not available at compile time. Is there an advantage to using EGL instead of GLX?

swt2c · 2020-09-02T19:41:40Z

I'm wondering if EGL should be used when the GTK Wayland backend is not available at compile time. Is there an advantage to using EGL instead of GLX?

AFAIK GLX is basically deprecated so IMO we should use EGL even for X11-only.

paulcor · 2020-09-04T18:15:39Z

Although it's uglier, I'd rather check for the Wayland backend with a compile test. I have multiple custom builds of GTK, in addition to the distribution packages, for testing purposes. If I'm building against one that doesn't have gdk-wayland-3.0, pkg-config will still pick up the system one. I propose something like this:

diff --git a/configure.in b/configure.in
index fd7d7d9caf..4905283d4e 100644
--- a/configure.in
+++ b/configure.in
@@ -3799,11 +3799,38 @@ if test "$wxUSE_OPENGL" = "yes" -o "$wxUSE_OPENGL" = "auto"; then
                                 [
                                     OPENGL_LIBS="$OPENGL_LIBS $EGL_LIBS"
                                     AC_DEFINE(wxUSE_GLCANVAS_EGL)
+                                    PKG_CHECK_MODULES(WAYLAND_EGL, [wayland-egl],
+                                        [
+                                            AC_CACHE_CHECK([for GDK Wayland backend], wx_cv_wayland_backend,
+                                                [
+                                                    save_CFLAGS=$CFLAGS
+                                                    CFLAGS="$CFLAGS $TOOLKIT_INCLUDE"
+                                                    AC_TRY_COMPILE([#include <gdk/gdk.h>],
+                                                        [
+                                                            #ifndef GDK_WINDOWING_WAYLAND
+                                                                choke me
+                                                            #endif
+                                                        ],
+                                                        wx_cv_wayland_backend=yes,
+                                                        wx_cv_wayland_backend=no)
+                                                    CFLAGS=$save_CFLAGS
+                                                ]
+                                            )
+                                            if test $wx_cv_wayland_backend = "yes"; then
+                                                OPENGL_LIBS="$OPENGL_LIBS $WAYLAND_EGL_LIBS"
+                                                have_wayland=1
+                                            fi
+                                        ],
+                                        [:]
+                                    )
                                 ],
                                 [
                                     AC_MSG_NOTICE([EGL 1.5+ not available. Will use GLX.])
                                 ]
                             )
+                            if test "$have_wayland" != 1; then
+                                AC_MSG_NOTICE([wxGLCanvas will not have Wayland support])
+                            fi
                         fi
                     fi
                 fi

Add EGL-based backend for wxGLCanvas

6ae8e97

Among other things, this enables wxGLCanvas to be used natively on Wayland. Fixes wxWidgets#17702.

swt2c force-pushed the glcanvas_egl branch from 9e7bf8e to 6ae8e97 Compare August 28, 2020 16:00

vadz approved these changes Aug 28, 2020

View reviewed changes

paulcor reviewed Aug 28, 2020

View reviewed changes

fixup! Add EGL-based backend for wxGLCanvas

c37ddb0

vadz added this to the 3.1.5 milestone Aug 29, 2020

vadz closed this in 7cd12a2 Sep 1, 2020

pauldmccarthy mentioned this pull request Feb 16, 2021

Clarification on X11/GLX versus Wayland/EGL wxWidgets/Phoenix#1923

Open

cebtenzzre mentioned this pull request May 24, 2021

OpenGL - Attempt to retrieve context when no valid context Amulet-Team/Amulet-Map-Editor#247

Closed

badshah400 mentioned this pull request Aug 18, 2021

Built with SYSTEM=linux-egl, glew apps crash on NVIDIA nigels-com/glew#315

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EGL-based backend for wxGLCanvas #2038

Add EGL-based backend for wxGLCanvas #2038

swt2c commented Aug 28, 2020

swt2c commented Aug 28, 2020

vadz left a comment

paulcor left a comment

paulcor Aug 28, 2020

swt2c Aug 28, 2020

swt2c commented Aug 28, 2020

vadz commented Aug 28, 2020

swt2c commented Aug 29, 2020

vadz commented Aug 29, 2020

swt2c commented Aug 29, 2020

vadz commented Aug 29, 2020

paulcor commented Aug 29, 2020 via email

vadz commented Aug 31, 2020

paulcor commented Aug 31, 2020

paulcor commented Sep 1, 2020

swt2c commented Sep 1, 2020

paulcor commented Sep 1, 2020 via email

swt2c commented Sep 1, 2020

swt2c commented Sep 1, 2020

paulcor commented Sep 1, 2020 via email

swt2c commented Sep 1, 2020 via email

paulcor commented Sep 1, 2020 via email

paulcor commented Sep 2, 2020

swt2c commented Sep 2, 2020

swt2c commented Sep 2, 2020

paulcor commented Sep 2, 2020

swt2c commented Sep 2, 2020

paulcor commented Sep 4, 2020

Add EGL-based backend for wxGLCanvas #2038

Add EGL-based backend for wxGLCanvas #2038

Conversation

swt2c commented Aug 28, 2020

swt2c commented Aug 28, 2020

vadz left a comment

Choose a reason for hiding this comment

paulcor left a comment

Choose a reason for hiding this comment

paulcor Aug 28, 2020

Choose a reason for hiding this comment

swt2c Aug 28, 2020

Choose a reason for hiding this comment

swt2c commented Aug 28, 2020

vadz commented Aug 28, 2020

swt2c commented Aug 29, 2020

vadz commented Aug 29, 2020

swt2c commented Aug 29, 2020

vadz commented Aug 29, 2020

paulcor commented Aug 29, 2020 via email

vadz commented Aug 31, 2020

paulcor commented Aug 31, 2020

paulcor commented Sep 1, 2020

swt2c commented Sep 1, 2020

paulcor commented Sep 1, 2020 via email

swt2c commented Sep 1, 2020

swt2c commented Sep 1, 2020

paulcor commented Sep 1, 2020 via email

swt2c commented Sep 1, 2020 via email

paulcor commented Sep 1, 2020 via email

paulcor commented Sep 2, 2020

swt2c commented Sep 2, 2020

swt2c commented Sep 2, 2020

paulcor commented Sep 2, 2020

swt2c commented Sep 2, 2020

paulcor commented Sep 4, 2020