After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 599885 - [gtk examples] unstable behaviour with recent gtk (post csw merge)
[gtk examples] unstable behaviour with recent gtk (post csw merge)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-gl
git master
Other All
: Normal normal
: 0.10.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
csw
: 573694 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2009-10-28 10:11 UTC by Filippo Argiolas
Modified: 2010-07-14 22:18 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Filippo Argiolas 2009-10-28 10:11:52 UTC
Gtk examples set xoverlay window using direct access to a drawing area native window handle like GDK_WINDOW_XWINDOW(window->window).
Starting from gtk+ 2.17.3 (or .4, after client-side-windows merge) thre is no more warranty that a gdkwindow has a native counterpart. This has the effect of random crashes with fatal X11 IO errors. 

The right way to fix this is to call gdk_window_ensure_native so that we can trust gdk to do everything it can to give us a native handler.

It would be great to also remove flickering from gtk effects while fixing this.
I'd say we should include a function in gstgtk.c that does something like the following and call that in each gtk example. It cannot be done in the set_gtk_window call because that's called in gst threads. Probably the best place is right after the drawingarea has been added to a container so it can be realized.

gtk_widget_realize (screen);
#if GTK_CHECK_VERSION(2.17.3)
GdkWindow win = gtk_widget_get_window (screen);
if (!gdk_window_ensure_native (win))
{
  g_error ("Could not create a native X11 window for the drawing area");
}
#endif
gdk_window_set_back_pixmap (gtk_widget_get_window (screen), NULL, FALSE);
gtk_widget_set_app_paintable (screen, TRUE);
gtk_widget_set_double_buffered (screen, FALSE);
Comment 1 Tim-Philipp Müller 2009-11-03 00:50:22 UTC
Do you happen to know how exactly it fails? And would it fail every single time for a given program/codepath? (Basically, I wondered if this might be related to bug #573694).
Comment 2 Filippo Argiolas 2009-11-03 07:36:12 UTC
(In reply to comment #1)
> Do you happen to know how exactly it fails? And would it fail every single time
> for a given program/codepath? (Basically, I wondered if this might be related
> to bug #573694).

Unfortunately I don't know any way to reproduce it reliably (.e. every single time).
The issue showed in cheese as soon as I upgraded to the new client side gtk+. The guilty code is the one that sets the xoverlay window from the bus sync handler (as per gstxoverlay docs).
We, in cheese, set the xoverlay window to GDK_WINDOW_XWINDOW(win->window) where win is the gdkwindow of a drawing area. This supposes the gdk window has a native counterpart and after the csw merge that's not safe.

The thing that always puzzled me is that sometimes the window is mapped correctly and no crash (BadWindow or a generic fatal X11 IO error) happens, sometimes it happens only when you move the mouse pointer into the overlay area, sometimes it crashes as soon as the sync handler is called.

Anyway I don't have too much interest of understanding gdk windowing internals, I'm pretty sure gdk_window_ensure_native definitely fixed the problem in cheese, it is also a reasonable workaround since we were accessing a native window so it makes perfectly sense.
I'm almost sure empathy devs did the same thing, and after a quick test I'm also sure it fixes gst-gl gtk effects "instability".

Regarding the rhythmbox bug, it really feels like the same error I got in cheese.
Maybe it is worth putting a note in GstXOverlay docs.
Comment 3 Sebastian Dröge (slomo) 2009-11-03 09:49:31 UTC
Note that beginning with GTK 2.17.X, GDK_WINDOW_XWINDOW() calls gdk_x11_drawable_get_xid(), which will ensure that the window is native already. Only problem is, if that macro is called from anothere thread, e.g. from the bus sync handler.

So we should just make sure to not call it from there ;)
Comment 4 Tim-Philipp Müller 2009-11-03 10:53:53 UTC
*** Bug 573694 has been marked as a duplicate of this bug. ***
Comment 5 Filippo Argiolas 2009-11-03 12:29:34 UTC
(In reply to comment #3)
> Note that beginning with GTK 2.17.X, GDK_WINDOW_XWINDOW() calls
> gdk_x11_drawable_get_xid(), which will ensure that the window is native
> already. Only problem is, if that macro is called from anothere thread, e.g.
> from the bus sync handler.

Oh, ok, this explains the random and race-condition-like behaviour :)
 
> So we should just make sure to not call it from there ;)

I'd say the best place is is the GtkDrawingArea "realize" callback, then.
Comment 6 Julien Isorce 2009-11-04 16:19:51 UTC
Actually, I was doing something else and it was finally a good test about this bug.
On win32, in the "realize" callback of a GtkDrawinArea, GDK_WINDOW_HWND(area->window) return the native handle of the parent (here the main window)
Calling gdk_window_ensure_native(area->window) just before it, causes GDK_WINDOW_HWND(area->window) to return the correct native handle of the drawing area.
Sothe "realize" callback is not the correct place to avoid the call of gdk_window_ensure_native (and I do not know if it's really possible to avoid it in our case, and on win32)

(gtk v2.18.3-1)
Comment 7 Filippo Argiolas 2009-11-04 17:24:45 UTC
(In reply to comment #6)
> Actually, I was doing something else and it was finally a good test about this
> bug.
> On win32, in the "realize" callback of a GtkDrawinArea,
> GDK_WINDOW_HWND(area->window) return the native handle of the parent (here the
> main window)
> Calling gdk_window_ensure_native(area->window) just before it, causes
> GDK_WINDOW_HWND(area->window) to return the correct native handle of the
> drawing area.
> Sothe "realize" callback is not the correct place to avoid the call of
> gdk_window_ensure_native (and I do not know if it's really possible to avoid it
> in our case, and on win32)

I never said to avoid the ensure_native, I said to put it in the realize callback (the widget needs to be realized to have a window, so that's probably the best place to call it). Then you can probably just call GDK_WINDOW_HWND from any thread since it will just return the native handle. If you want to stay safe you can save the window handle right after the ensure_native call and give that to gstxoverlay.
Comment 8 Julien Isorce 2009-11-05 16:22:26 UTC
The doc is there: http://library.gnome.org/devel/gtk/2.18/gtk-migrating-ClientSideWindows.html

Well, I just realized that "Calling gdk_x11_drawable_get_xid() (or GDK_WINDOW_XID()) from the X11-specific API on a non-native window will explicitly call gdk_window_ensure_native()"
So my remark #6 had almost no sense. 
Sorry about that.

Except that on win32, we now need to set this env var: GDK_NATIVE_WINDOWS
Because as I said in #6:
"On win32, in the "realize" callback of a GtkDrawinArea,
GDK_WINDOW_HWND(area->window) return the native handle of the parent (here the
main window)"
but it's ok if we now set the env var GDK_NATIVE_WINDOWS.
Comment 9 Julien Isorce 2010-07-12 16:40:03 UTC
commit 1edcd8a2d5335912d119a0544e7d302f760f7d00
Author: Julien Isorce <julien.isorce@gmail.com>
Date:   Mon Jul 12 18:38:59 2010 +0200

    gtk examples: adapt code since the native-window changes from gtk
    
    Fixes bug #599885