Accidentally missed this email a week ago. Thanks again for all the reviews!
On 10/31/24 10:32, Akihiko Odaki wrote:
...
>> +# libx11 presents together with SDL or GTK libs on systems that
>> support X11
>> +xlib = dependency('x11', required: false)
>
> There is a line saying:
> x11 = dependency('x11', method: 'pkg-config', required: gtkx11.found())
>
> Please reuse it.
I've seen this option and choose not to use it because despite the brief
'X11' name, it's about X11 support specifically for GTK and not SDL.
Though, we can use this GTK/X11 for now and improve Meson dependencies
later on because in practice GTK is enabled for all distro-built QEMUs.
Will switch to it in v3.
...
>> +void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>> + QemuDmaBuf *dmabuf)
>> +{
>> + struct sdl2_console *scon = container_of(dcl, struct
>> sdl2_console, dcl);
>> +
>> + assert(scon->opengl);
>> + SDL_GL_MakeCurrent(scon->real_window, scon->winctx);
>> +
>> + egl_dmabuf_import_texture(dmabuf);
>> + if (!qemu_dmabuf_get_texture(dmabuf)) {
>> + error_report("%s: failed fd=%d", __func__,
>> qemu_dmabuf_get_fd(dmabuf));
>
> It still continues to call sdl2_gl_scanout_texture() and I doubt that's
> what you meant.
Indeed, better to bail out early on a error.
...
>> +static void sdl2_set_hint_x11_force_egl(void)
>> +{
>> +#if defined(SDL_HINT_VIDEO_X11_FORCE_EGL) && defined(CONFIG_OPENGL) && \
>> + defined(CONFIG_XLIB)
>> + Display *x_disp = XOpenDisplay(NULL);
>> + EGLDisplay egl_display;
>> +
>> + if (!x_disp) {
>> + return;
>> + }
>> +
>> + /* Prefer EGL over GLX to get dma-buf support. */
>> + egl_display = eglGetDisplay((EGLNativeDisplayType)x_disp);
>> +
>> + if (egl_display != EGL_NO_DISPLAY) {
>> + /*
>> + * Setting X11_FORCE_EGL hint doesn't make SDL to prefer 11 over
>
> s/prefer 11 over/prefer X11 over/
>
> Personally, I'm more concerned whether setting that hint will make an
> invalid argument error or something.
There are no known side effects from setting that hint for QEMU and
libsdl code looks sane. AFAICT, EGL is not enabled by default in SDL
only because there are older SDL applications that use GLX features and
they will be broken if SDL will switch to EGL by default.
Technically, should be possible to make SDL use EGL on demand, like only
when QEMU runs with enabled native-context for example. But there is no
point in doing that today since there are no known problems with the EGL
hint, IMO. We will be able to address problems if somebody will report a
bug.
--
Best regards,
Dmitry
