slint-ui / slint

Slint is a declarative GUI toolkit to build native user interfaces for Rust, C++, or JavaScript apps.
https://slint.dev
Other
17.54k stars 600 forks source link

[experimental c++ api] Add support for GLX with Xlib NativeWindowHandle #2978

Closed the-argus closed 1 year ago

the-argus commented 1 year ago

Edited by @tronical:

The NativeWindowHandle API allows creation from xcb window handles/connection. When that happens we should not try to create a GLX context, because that can't work.

In addition, we should add support for creating NativeWindowHandle from xlib (where we can support GLX):

static NativeWindowHandle from_xlib(Display* dpy, XVisualInfo* vi, Window win);

Original description:

Disclaimer: I haven't tested this on wayland, so it's possible that the title of this should be amended to be "on X11."

The Skia renderer is created (on X11) using slint_new_raw_window_handle_x11, which creates an Xcb display handle:

        RawDisplayHandle::Xcb(init_raw!(XcbDisplayHandle { connection, screen })),

The glutin crate only allows you to use a RawDisplayHandle::XLib to interface with GLX. So if you try to use a skia renderer attached to a native X11 window on linux, you will get the following error:

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: Other("Skia OpenGL Renderer: Failed to create OpenGL Window Surface: provided native display isn't supported")', api/cpp/platform.rs:316:10
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::unwrap
             at /build/rustc-1.69.0-src/library/core/src/result.rs:1090:23
   4: slint_skia_renderer_new
             at ./_deps/slint-src/api/cpp/platform.rs:311:9
   5: _ZN5slint12experimental8platform12SkiaRendererC4ERKNS1_18NativeWindowHandleENS_12PhysicalSizeE
             at ./_deps/slint-src/api/cpp/include/slint_platform.h:323:58
   6: _ZSt10_ConstructIN5slint12experimental8platform12SkiaRendererEJNS2_18NativeWindowHandleERNS0_12PhysicalSizeEEEvPT_DpOT0_
             at /nix/store/dcd1zhv56rk0d2z7akzfjgzr076c4jl9-gcc-12.2.0/include/c++/12.2.0/bits/stl_construct.h:119:7
   7: _ZNSt22_Optional_payload_baseIN5slint12experimental8platform12SkiaRendererEE12_M_constructIJNS2_18NativeWindowHandleERNS0_12PhysicalSizeEEEEvDpOT_
             at /nix/store/dcd1zhv56rk0d2z7akzfjgzr076c4jl9-gcc-12.2.0/include/c++/12.2.0/optional:278:19
   8: _ZNSt19_Optional_base_implIN5slint12experimental8platform12SkiaRendererESt14_Optional_baseIS3_Lb0ELb0EEE12_M_constructIJNS2_18NativeWindowHandleERNS0_12PhysicalSizeEEEEvDpOT_
             at /nix/store/dcd1zhv56rk0d2z7akzfjgzr076c4jl9-gcc-12.2.0/include/c++/12.2.0/optional:457:52
   9: _ZNSt8optionalIN5slint12experimental8platform12SkiaRendererEE7emplaceIJNS2_18NativeWindowHandleERNS0_12PhysicalSizeEEEENSt9enable_ifIX18is_constructible_vIS3_DpT_EERS3_E4typeEDpOSA_
             at /nix/store/dcd1zhv56rk0d2z7akzfjgzr076c4jl9-gcc-12.2.0/include/c++/12.2.0/optional:918:22
  10: _ZN6mecaps15KDWindowAdapterC2Ev
             at /home/argus/Programming/KDAB/mecaps/src/src/window_adapter.h:87:22
  11: _ZSt11make_uniqueIN6mecaps15KDWindowAdapterEJEENSt8__detail9_MakeUniqIT_E15__single_objectEDpOT0_
             at /nix/store/dcd1zhv56rk0d2z7akzfjgzr076c4jl9-gcc-12.2.0/include/c++/12.2.0/bits/unique_ptr.h:1065:69
  12: _Z17connectSlintToAppv
             at /home/argus/Programming/KDAB/mecaps/src/src/main.cpp:16:65
  13: main
             at /home/argus/Programming/KDAB/mecaps/src/src/main.cpp:53:19
  14: __libc_start_call_main
  15: __libc_start_main@@GLIBC_2.34
  16: _start
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

I think this could be solved by using glutin egl api instead of glx. Alternatively/in addition, the femotvg renderer could be exposed to the C++ api.

I don't know much about GLX, but maybe xcb handles can be used with it, and this should be a glutin issue?

the-argus commented 1 year ago

Tested on wayland. Produced an identical error and stack trace. Still just creates a window handle which is not Xlib and then slint tries to get the gl Display from glutin/glx, which fails. Not really sure why glutin GLX is being selected in this case and not EGL.

Also, there definitely is a GLX XCB API so I think I might make an upstream issue, provided this isn't easily solved here.

tronical commented 1 year ago

I think you're right. In addition to xlib vs. xcb there is a bit of a conceptual problem: AFAIR we need to choose an glx config first, then see what visual glx provides for us (GetVisualFromFBConfig) and use that visual to create the window.

We might need to introduce new API with a builder pattern here, where first say a "SkiaRendererConfigBuilder" is created (that will for just for glx encapsulate the config), then that can provide a visual that needs to be passed to whoever creates the window, and then continue with the builder to build the skia renderer itself.

In what kind of environment would you use this API? (how do you create the window?)

the-argus commented 1 year ago

The window is created using KDGui, which is part of kdutils. The windowing is meant to be cross platform so the actual xcb calls are not public.

I believe its possible to avoid GLX entirely by using glutin EGL. Additionally, wayland needs EGL or some other solution since it's clearly a problem that attaching to a window with a NativeWindowHandle::from_wayland should make a call to glutin GLX.

Also, the way native window handles work in the API right now is great. If you plan to turn the skia renderer into a builder to solve the GLX problem, consider leaving the original API as an option for everyone on the good platforms.

tronical commented 1 year ago

Yes, in theory GLX is not needed at all. In practice according to https://github.com/emilk/egui/pull/2541#issuecomment-1370767582 and bug reports we received GLX works better in some situations (iirc Nvidia driver) and EGL aborts.

But it's okay, we can make it so that the C++ api only supports EGL on x11 (and Wayland anyway) for now.

So some gymnastics in https://github.com/slint-ui/slint/blob/a709402bf967f2e3a136c510c32e40255759ab5b/api/cpp/platform.rs#L293 are needed to construct the Skia rendere from an opengl surface and add the OpenGL surface constructors in the rust Skia renderer on our side to allow passing Egl instead of GlxThenEgl to glutin.

tronical commented 1 year ago

I can try to set up an example to hack on this, but it'll take me some time. So if you already have a git repo somewhere let me know and I can probably do the Slint bits and test it against it.

the-argus commented 1 year ago

Here's an example of GLX backend failure @tronical : https://github.com/the-argus/slint-kdfoundation-glx-example Please let me know if you have any luck using EGL instead.

If you've resolved to go full EGL then I believe no code changes should be necessary outside of slint rust, which is pretty nice. Maybe you'll fix it in ten minutes and then I can pull and everything in my project will be magically fixed :)

tronical commented 1 year ago

I tried to reproduce this now, but I haven't had any luck :(

I needed https://github.com/the-argus/slint-kdfoundation-glx-example/pull/1 to get it to start up, and I needed https://gist.github.com/tronical/3a42fdd100f9a246b5f21f4b8e3bdb09 to get something onto the screen. But other than that it works AFAICS.

I've tried this on X11, as the code in window_adapter.cpp assumes X11 and not wayland. The code here will in fact first try to create an EGL context and only if that fails, it will try GLX. The GLX part will fail though, exactly as in your case.

So that makes me wonder: Even if we didn't try GLX, I'd expect the EGL connection to fail for you, too.

Can you describe your windowing system setup a little? Maybe I'm missing something here.

tronical commented 1 year ago

Ah, do you have an Nvidia driver by chance? I think it's possible that that one doesn't support EGL :(.

So we do somehow need to fix GLX support...

tronical commented 1 year ago

So either we find a way to support GLX with XCB and then that would have to be implemented in glutin. I can't find any example code out there though how to do that. The other path that I see taken is that the connection to the display is established using XLib (XOpenDisplay) and then the xcb_connection is retrieved using XGetXCBConnection.

If you can do that bit in KDFoundation, then I can add the APIs in Slint to not only create a NativeWindowHandle from xcb but also from Xlib.

Otherwise, I'm open to ideas how to get GL working on Nvidia with x11 :).

In a way, this issue is basically the same as #2269.

tronical commented 1 year ago

Alternatively, would you rather we make the Skia Vulkan renderer available via the C++ API for an explicit opt-in, as a way to avoid the entire EGL/GLX dance?

the-argus commented 1 year ago

I tried to reproduce this now, but I haven't had any luck :(

I am not a reliable source, it seems. I was convinced that the ApiPreference was GlxThenEgl. Guess I just misread it? But my coworker was also able to reproduce the problem at one point, and now it works fine for him... Well regardless, seems that EGL is in fact failing for me.

I needed the-argus/slint-kdfoundation-glx-example#1 to get it to start up, and I needed https://gist.github.com/tronical/3a42fdd100f9a246b5f21f4b8e3bdb09 to get something onto the screen. But other than that it works AFAICS.

Thanks for solving the problems in my code! I didn't even get as far as the memory corruption, slint_skia_renderer_new was where the error unwrap happened for me. I'll accept your PR, although I'm not sure whether my example repository will stay up/public.

I'd expect the EGL connection to fail for you, too.

Can you describe your windowing system setup a little? Maybe I'm missing something here.

I'm on NixOS. I think that's the problem. Libraries aren't located at /lib. I bet if I added things to my LD_LIBRARY_PRELOAD glutin would be able to load EGL properly. I don't have time to test it at the moment but I will tomorrow.

So we do somehow need to fix GLX support...

I think the best solution is to deepen the C++ api, so that C++ users can provide an existing surface for slint to draw to. Maybe just passing in the GL_INT handles would be enough? I'd have to look into the glutin API to get a better idea of what's possible.

So either we find a way to support GLX with XCB and then that would have to be implemented in glutin. I can't find any example code out there though how to do that. The other path that I see taken is that the connection to the display is established using XLib (XOpenDisplay) and then the xcb_connection is retrieved using XGetXCBConnection.

Well, glutin has glutin_glx_sys, so it just needs glutin_glx_xcb_sys over the xcb glx api. I went ahead and made an issue over there. I think if that can be implemented then Xcb could be an accepted handle for GLX displays.

If you can do that bit in KDFoundation, then I can add the APIs in Slint to not only create a NativeWindowHandle from xcb but also from Xlib.

This is definitely possible. If the issue I made isn't recieved well/my idea wouldn't work, we can explore this.

Otherwise, I'm open to ideas how to get GL working on Nvidia with x11 :).

I'm actually using Intel graphics haha, I think it's just my OS.

tronical commented 1 year ago

So we do somehow need to fix GLX support...

I think the best solution is to deepen the C++ api, so that C++ users can provide an existing surface for slint to draw to. Maybe just passing in the GL_INT handles would be enough? I'd have to look into the glutin API to get a better idea of what's possible.

I think that "existing surface" is exactly the NativeWindowHandle type that we need to extend to also support xlib. I'm still unsure if it's really going to work in terms of compatibility of the x11 visuals, but either way it's missing API.

If you can do that bit in KDFoundation, then I can add the APIs in Slint to not only create a NativeWindowHandle from xcb but also from Xlib.

This is definitely possible. If the issue I made isn't recieved well/my idea wouldn't work, we can explore this.

It looks like this is what we need to do, i.e. we can only support glx via xlib.

Otherwise, I'm open to ideas how to get GL working on Nvidia with x11 :).

I'm actually using Intel graphics haha, I think it's just my OS.

Hehe.

I'll amend the description of this issue to make it clear that what remains to be done for this issue is to add support for xlib window handles with the C++ platform API (and testing that GLX works).

the-argus commented 1 year ago

I think that "existing surface" is exactly the NativeWindowHandle type that we need to extend to also support xlib. I'm still unsure if it's really going to work in terms of compatibility of the x11 visuals, but either way it's missing API.

If you can do that bit in KDFoundation, then I can add the APIs in Slint to not only create a NativeWindowHandle from xcb but also from Xlib.

This is definitely possible. If the issue I made isn't recieved well/my idea wouldn't work, we can explore this.

It looks like this is what we need to do, i.e. we can only support glx via xlib.

Heh... yeah, maybe that glutin issue I made was a bit hasty... Xlib it is!

I think to aquire an existing window and create a window surface on it with GLX, you'd need something like this:

static NativeWindowHandle from_xlib(Display* dpy, XVisualInfo* vi, Window win);

But glutin only supports recieving display handles with the display and screen and then calling their create_window_surface on them.

I think it could be done within slint, though. You could probably do some messy stuff to use the C types to manually create a glutin Surface<WindowSurface>, to replace calling create_window_surface. A glutin Surface consists of the display, framebuffer config, and window raw types.

tronical commented 1 year ago

I agree, that looks like the kind if API we want. And in addition, when the NativeWindowHandle is created from xcb, we shouldn't even try GLX but report an error (instead of letting glutin panic).

tronical commented 1 year ago

FWIW, I can reproduce this now also with xcb with my laptop and intel graphics card - previously I had only tried inside VMware. Mysteriously EGL initialisation fails.

the-argus commented 1 year ago

Oh, you can just create a window handle with the visual. Nice.

FWIW, I can reproduce this now also with xcb with my laptop and intel graphics card - previously I had only tried inside VMware. Mysteriously EGL initialisation fails.

Now this is funny, I've fixed EGL on my end. It was indeed that glutin couldn't dynamically load libGL and libwayland-egl. But regardless, you now support GLX as a fallback for whenever glutin and/or the hardware fails with EGL. Thanks for the fix :tada:

tronical commented 1 year ago

Not sure this is still the right place, but anyway:

I tried to also make this work against wayland, so I created https://github.com/the-argus/slint-kdfoundation-glx-example/pull/2 .

Unfortunately it crashes when setting the window title:

[2023-07-14 15:34:28.510] [wayland] [info] [linux_wayland_platform_integration.cpp:53] Wayland display is: wayland-1
[2023-07-14 15:34:28.552] [wayland] [info] [linux_wayland_platform_integration.cpp:60] Connected to the Wayland server
==106814== Invalid read of size 4
==106814==    at 0x8A44E84: wl_proxy_get_version (wayland-client.c:2212)
==106814==    by 0x49D8639: xdg_toplevel_set_title (wayland-xdg-shell-client-protocol.h:1675)
==106814==    by 0x49D8CFF: KDGui::LinuxWaylandPlatformWindow::setTitle(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (linux_wayland_platform_window.cpp:137)
==106814==    by 0x4980AA0: void std::__invoke_impl<void, void (KDGui::AbstractPlatformWindow::*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), KDGui::AbstractPlatformWindow*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(std::__invoke_memfun_deref, void (KDGui::AbstractPlatformWindow::*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), KDGui::AbstractPlatformWindow*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (invoke.h:74)
==106814==    by 0x49802C0: std::__invoke_result<void (KDGui::AbstractPlatformWindow::*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), KDGui::AbstractPlatformWindow*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>::type std::__invoke<void (KDGui::AbstractPlatformWindow::*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), KDGui::AbstractPlatformWindow*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(void (KDGui::AbstractPlatformWindow::*&)(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), KDGui::AbstractPlatformWindow*&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (invoke.h:96)
==106814==    by 0x497FAE1: void std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::__call<void, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, 0ul, 1ul>(std::tuple<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>&&, std::_Index_tuple<0ul, 1ul>) (functional:484)
==106814==    by 0x497F4EF: void std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, void>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (functional:567)
==106814==    by 0x497ECF2: void std::__invoke_impl<void, std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(std::__invoke_other, std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (invoke.h:61)
==106814==    by 0x497E652: std::enable_if<is_invocable_r_v<void, std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>, void>::type std::__invoke_r<void, std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (invoke.h:111)
==106814==    by 0x497DD70: std::_Function_handler<void (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&), std::_Bind<void (KDGui::AbstractPlatformWindow::*(KDGui::AbstractPlatformWindow*, KDBindings::Private::placeholder<1>))(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)> >::_M_invoke(std::_Any_data const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (std_function.h:290)
==106814==    by 0x17DF68: std::function<void (std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (std_function.h:591)
==106814==    by 0x17A618: KDBindings::Signal<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>::Impl::emit(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (signal.h:301)
==106814==  Address 0x40 is not stack'd, malloc'd or (recently) free'd
==106814==
==106814==

If I comment out the setting of the window title, it runs, but nothing is shown on the screen.

the-argus commented 1 year ago

@tronical I ran into the same issue, it's still unsolved but the quick fix is to call app.processEvents() before changing the title. The title onValueChanged makes a wayland call to change the title, but the window resources don't get created until onVisibleChanged is called from the event loop.

tronical commented 1 year ago

Ah, with https://gist.github.com/tronical/3a42fdd100f9a246b5f21f4b8e3bdb09 applied I also see something on Wayland. Yay.