[Wayland-bugs] [Bug 769835] On Wayland, application containing GtkGLArea stops responding if it's not on current workspace

[gtk+ (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=769835

--- Comment #18 from Carlos Garnacho <carlosg at gnome.org> ---
(In reply to Emmanuele Bassi (:ebassi) from comment #17)
> Review of attachment 342410 [details] [review]:
> 
> ::: gdk/wayland/gdkglcontext-wayland.c
> @@ +441,3 @@
> +static gboolean
> +egl_config_can_disable_swap_interval (GdkDisplay      *display,
> +                                      const EGLConfig *config)
> 
> EGLConfig is already a pointer; I don't think you need to pass it as a
> pointer…

Hmm, I see. I was confused by the weird memory management at:

https://git.gnome.org//browse/gtk+/tree/gdk/wayland/gdkglcontext-wayland.c#n410

where it seems to return stuff from a freed block, knowing that EGLConfig is a
pointer itself makes sense then.

> 
> @@ +446,3 @@
> +  EGLint interval;
> +
> +  if (!eglGetConfigAttrib (display_wayland->egl_display, *config,
> 
> … especially since you're just dereferencing it here.
> 
> @@ +483,3 @@
>      return NULL;
>  
> +  if (!egl_config_can_disable_swap_interval (display, &config))
> 
> I'm not convinced this is an appropriate action to take.
> 
> If swap interval cannot be disabled then we should fall back to the existing
> behaviour; it's not optimal, but saying "you can't have GL because your
> driver does not allow disabling swap interval" seems to me far more drastic
> than necessary.
> 
> Just save the return value inside the GdkGLContext and emit a warning or a
> debug message.

Sure, question is when it's more appropriate to warn then. Before it locks
presumably...

> 
> @@ +543,3 @@
>        eglMakeCurrent(display_wayland->egl_display, EGL_NO_SURFACE,
> EGL_NO_SURFACE,
>                       EGL_NO_CONTEXT);
> +      eglSwapInterval (dpy, 0);
> 
> Not sure it's needed, here; we're unbinding everything, which means there's
> no context and no surface to swap.

Right, I was unsure about this one.

Thanks for the review!

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-bugs/attachments/20161223/c91f2f36/attachment.html>