On 11/9/24 13:07, Akihiko Odaki wrote:
> On 2024/11/09 15:52, Dmitry Osipenko wrote:
>> 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.
>
> I think you can just remove "if gtkx11.found()" to use it for SDL.
Right, thanks.
>> ...
>>>> +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.
>>
>
> I was thinking of scenarios where X11 is not used. A convincing scenario
> of failure is that SDL emits an error for the flag and stops working.
> The fact that this code just works implies it is not the case, but it's
> worth noting if you leave a comment anyway.
We check X11 presence using XOpenDisplay() and hint won't be set if Xorg
is unavailable.
For the case of XWayland, there is already a comment in this patch
telling that flag has no effect on Wayland. Will extend this comment,
thanks.
--
Best regards,
Dmitry
