Merge lp:~smspillaz/compiz-core/compiz-core.decor_synchronization into lp:compiz-core/0.9.8

[Sam Spilsbury]

Add synchronization primitives to the decoration protocol so that there isn't a race where we bind a texture that's being freed.

[Daniel van Vugt (vanvugt)]

I haven't reviewed this, but please fix and/or resubmit to resolve the conflicts.

review: Needs Resubmitting

[lp:~smspillaz/compiz-core/compiz-core.decor_synchronization]

3016. By Sam Spilsbury on 2012-04-20

Merge lp:compiz-core

3017. By Sam Spilsbury on 2012-04-20

Merge lp:~smspillaz/compiz-core/compiz-core.work_923683

3018. By Sam Spilsbury on 2012-04-25

Merge

3019. By Sam Spilsbury on 2012-04-25

We don't need a pixmap to determine whether or not the window is active or not

We only need the pixmap in order to determine whether or not the window
should be redrawin in place

3020. By Sam Spilsbury on 2012-04-25

Refactored the pixmap request / release interface so we can test it

3021. By Sam Spilsbury on 2012-04-25

Put it all in separate files

3022. By Sam Spilsbury on 2012-04-25

Test destroying a pixmap also deletes it

3023. By Sam Spilsbury on 2012-04-25

Added some more mocks

3024. By Sam Spilsbury on 2012-04-25

Tests

Unmerged revisions

=== modified file 'gtk/window-decorator/decorator.c'
--- gtk/window-decorator/decorator.c	2012-03-30 07:01:48 +0000
+++ gtk/window-decorator/decorator.c	2012-04-20 07:34:19 +0000
@@ -564,6 +564,49 @@
     }
 }
 
+/*
+ * request_update_window_decoration_size
+ * Description: asks the rendering process to allow a size update
+ * for pixmap synchronization */
+
+
+gboolean
+request_update_window_decoration_size (WnckWindow *win)
+{
+    decor_t           *d;
+    gint              width, height;
+    gint              x, y, w, h, name_width;
+
+    if (win == NULL)
+	return FALSE;
+
+    d = g_object_get_data (G_OBJECT (win), "decor");
+
+    /* Get the geometry of the window, we'll need it later */
+    wnck_window_get_client_window_geometry (win, &x, &y, &w, &h);
+
+    /* Get the width of the name */
+    name_width = max_window_name_width (win);
+
+    /* Ask the theme to tell us how much space it needs. If this is not successful
+     * update the decoration name and return false */
+    if (!(*theme_calc_decoration_size) (d, w, h, name_width, &width, &height))
+    {
+	update_window_decoration_name (win);
+	return FALSE;
+    }
+
+    d->width  = width;
+    d->height = height;
+
+    decor_post_pending (gdk_x11_display_get_xdisplay (gdk_display_get_default ()),
+			wnck_window_get_xid (win),
+			populate_frame_type (d),
+			populate_frame_state (d),
+			populate_frame_actions (d));
+
+    return TRUE;
+}
 
 /*
  * update_window_decoration_size
@@ -581,8 +624,6 @@
     decor_t           *d;
     GdkPixmap         *pixmap, *buffer_pixmap = NULL;
     Picture           picture;
-    gint              width, height;
-    gint              x, y, w, h, name_width;
     Display           *xdisplay;
     XRenderPictFormat *format;
 
@@ -593,28 +634,14 @@
 
     xdisplay = GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
 
-    /* Get the geometry of the window, we'll need it later */
-    wnck_window_get_client_window_geometry (win, &x, &y, &w, &h);
-
-    /* Get the width of the name */
-    name_width = max_window_name_width (win);
-
-    /* Ask the theme to tell us how much space it needs. If this is not successful
-     * update the decoration name and return false */
-    if (!(*theme_calc_decoration_size) (d, w, h, name_width, &width, &height))
-    {
-	update_window_decoration_name (win);
-	return FALSE;
-    }
-
     gdk_error_trap_push ();
 
     /* Get the correct depth for the frame window in reparenting mode, otherwise
      * enforce 32 */
     if (d->frame_window)
-	pixmap = create_pixmap (width, height, d->frame->style_window_rgb);
+	pixmap = create_pixmap (d->width, d->height, d->frame->style_window_rgb);
     else
-	pixmap = create_pixmap (width, height, d->frame->style_window_rgba);
+	pixmap = create_pixmap (d->width, d->height, d->frame->style_window_rgba);
 
     gdk_flush ();
 
@@ -628,9 +655,9 @@
     gdk_error_trap_push ();
 
     if (d->frame_window)
-	buffer_pixmap = create_pixmap (width, height, d->frame->style_window_rgb);
+	buffer_pixmap = create_pixmap (d->width, d->height, d->frame->style_window_rgb);
     else
-	buffer_pixmap = create_pixmap (width, height, d->frame->style_window_rgba);
+	buffer_pixmap = create_pixmap (d->width, d->height, d->frame->style_window_rgba);
 
     gdk_flush ();
 
@@ -649,7 +676,7 @@
 
     /* Destroy the old pixmaps and pictures */
     if (d->pixmap)
-	g_object_unref (G_OBJECT (d->pixmap));
+	g_hash_table_insert (destroyed_pixmaps_table, GINT_TO_POINTER (GDK_PIXMAP_XID (d->pixmap)), d->pixmap);
 
     if (d->buffer_pixmap)
 	g_object_unref (G_OBJECT (d->buffer_pixmap));
@@ -667,9 +694,6 @@
 
     d->picture = picture;
 
-    d->width  = width;
-    d->height = height;
-
     d->prop_xid = wnck_window_get_xid (win);
 
     update_window_decoration_name (win);

=== modified file 'gtk/window-decorator/events.c'
--- gtk/window-decorator/events.c	2012-03-30 07:01:48 +0000
+++ gtk/window-decorator/events.c	2012-04-20 07:34:19 +0000
@@ -1014,7 +1014,7 @@
 			update_window_decoration_state (win);
 			update_window_decoration_actions (win);
 			update_window_decoration_icon (win);
-			update_window_decoration_size (win);
+			request_update_window_decoration_size (win);
 			update_event_windows (win);
 		    }
 		    else
@@ -1069,6 +1069,17 @@
 		}
 	    }
 	}
+	else if (xevent->xclient.message_type == decor_request_atom)
+	{
+	    WnckWindow *win = wnck_window_get (xevent->xclient.window);
+
+	    if (win)
+		update_window_decoration_size (win);
+	}
+	else if (xevent->xclient.message_type == decor_delete_pixmap_atom)
+	{
+	    g_hash_table_remove (destroyed_pixmaps_table, GINT_TO_POINTER (xevent->xclient.data.l[0]));
+	}
     default:
 	break;
     }

=== modified file 'gtk/window-decorator/gtk-window-decorator.c'
--- gtk/window-decorator/gtk-window-decorator.c	2012-03-30 07:01:48 +0000
+++ gtk/window-decorator/gtk-window-decorator.c	2012-04-20 07:34:19 +0000
@@ -48,6 +48,9 @@
 Atom toolkit_action_atom;
 Atom toolkit_action_window_menu_atom;
 Atom toolkit_action_force_quit_dialog_atom;
+Atom decor_request_atom;
+Atom decor_pending_atom;
+Atom decor_delete_pixmap_atom;
 
 Atom net_wm_state_atom;
 Atom net_wm_state_modal_atom;
@@ -103,6 +106,7 @@
 GtkWidget     *switcher_label;
 
 GHashTable    *frame_table;
+GHashTable    *destroyed_pixmaps_table;
 GtkWidget     *action_menu = NULL;
 gboolean      action_menu_mapped = FALSE;
 decor_color_t _title_color[2];
@@ -305,6 +309,10 @@
     net_wm_state_atom = XInternAtom (xdisplay,"_NET_WM_STATE", 0);
     net_wm_state_modal_atom = XInternAtom (xdisplay, "_NET_WM_STATE_MODAL", 0);
 
+    decor_request_atom = XInternAtom (xdisplay, "_COMPIZ_DECOR_REQUEST", 0);
+    decor_pending_atom = XInternAtom (xdisplay, "_COMPIZ_DECOR_PENDING", 0);
+    decor_delete_pixmap_atom = XInternAtom (xdisplay, "_COMPIZ_DECOR_DELETE_PIXMAP", 0);
+
     status = decor_acquire_dm_session (xdisplay,
 				       gdk_screen_get_number (gdkscreen),
 				       "gwd", replace, &dm_sn_timestamp);
@@ -390,6 +398,7 @@
     xformat_rgb  = XRenderFindStandardFormat (xdisplay, PictStandardRGB24);
 
     frame_table = g_hash_table_new (NULL, NULL);
+    destroyed_pixmaps_table = g_hash_table_new_full (NULL, NULL, NULL, g_object_unref);
 
     if (!create_tooltip_window ())
     {

=== modified file 'gtk/window-decorator/gtk-window-decorator.h'
--- gtk/window-decorator/gtk-window-decorator.h	2012-03-30 07:01:48 +0000
+++ gtk/window-decorator/gtk-window-decorator.h	2012-04-20 07:34:19 +0000
@@ -306,6 +306,9 @@
 extern Atom toolkit_action_force_quit_dialog_atom;
 extern Atom net_wm_state_atom;
 extern Atom net_wm_state_modal_atom;
+extern Atom decor_request_atom;
+extern Atom decor_pending_atom;
+extern Atom decor_delete_pixmap_atom;
 
 extern Time dm_sn_timestamp;
 
@@ -511,6 +514,7 @@
 
 /* list of all decorations */
 extern GHashTable    *frame_table;
+extern GHashTable    *destroyed_pixmaps_table;
 
 /* action menu */
 extern GtkWidget     *action_menu;
@@ -628,9 +632,13 @@
 void
 destroy_bare_frame ();
 
+/* Don't use directly */
 gboolean
 update_window_decoration_size (WnckWindow *win);
 
+gboolean
+request_update_window_decoration_size (WnckWindow *win);
+
 void
 update_window_decoration_name (WnckWindow *win);
 

=== modified file 'gtk/window-decorator/wnck.c'
--- gtk/window-decorator/wnck.c	2012-03-30 07:01:48 +0000
+++ gtk/window-decorator/wnck.c	2012-04-20 07:34:19 +0000
@@ -80,7 +80,7 @@
 
     if (d->decorated)
     {
-	if (!update_window_decoration_size (win))
+	if (!request_update_window_decoration_size (win))
 	    queue_decor_draw (d);
     }
 }
@@ -102,7 +102,7 @@
 	    d->client_width  = width;
 	    d->client_height = height;
 
-	    update_window_decoration_size (win);
+	    request_update_window_decoration_size (win);
 	    update_event_windows (win);
 	}
     }
@@ -128,7 +128,7 @@
     if (d->decorated)
     {
 	update_window_decoration_state (win);
-	if (!update_window_decoration_size (win))
+	if (!request_update_window_decoration_size (win))
 	    queue_decor_draw (d);
 
 	update_event_windows (win);
@@ -143,7 +143,7 @@
     if (d->decorated)
     {
 	update_window_decoration_actions (win);
-	if (!update_window_decoration_size (win))
+	if (!request_update_window_decoration_size (win))
 	    queue_decor_draw (d);
 
 	update_event_windows (win);
@@ -422,7 +422,7 @@
 	update_window_decoration_state (win);
 	update_window_decoration_actions (win);
 	update_window_decoration_icon (win);
-	update_window_decoration_size (win);
+	request_update_window_decoration_size (win);
 
 	update_event_windows (win);
     }
@@ -642,7 +642,7 @@
 	    * then we need to redraw the decoration anyways
 	    * since the image would have changed */
 	    if (d->win != NULL &&
-		!update_window_decoration_size (d->win) &&
+		!request_update_window_decoration_size (d->win) &&
 		d->decorated)
 		queue_decor_draw (d);
 
@@ -710,7 +710,7 @@
 	    * then we need to redraw the decoration anyways
 	    * since the image would have changed */
 	    if (d->win != NULL &&
-		!update_window_decoration_size (d->win) &&
+		!request_update_window_decoration_size (d->win) &&
 		d->decorated)
 		queue_decor_draw (d);
 

=== modified file 'include/decoration.h'
--- include/decoration.h	2012-02-09 07:48:57 +0000
+++ include/decoration.h	2012-04-20 07:34:19 +0000
@@ -492,6 +492,24 @@
 				XEvent  *event,
 				Time    dm_sn_timestamp);
 
+int
+decor_post_pending (Display *xdisplay,
+		    Window  client,
+		    unsigned int frame_state,
+		    unsigned int frame_type,
+		    unsigned int frame_actions);
+
+int
+decor_post_delete_pixmap (Display *xdisplay,
+			  Pixmap  pixmap);
+
+int
+decor_post_generate_request (Display *xdisplay,
+			     Window  client,
+			     unsigned int frame_type,
+			     unsigned int frame_state,
+			     unsigned int frame_actions);
+
 #ifdef  __cplusplus
 }
 #endif

=== modified file 'libdecoration/decoration.c'
--- libdecoration/decoration.c	2012-02-27 06:43:04 +0000
+++ libdecoration/decoration.c	2012-04-20 07:34:19 +0000
@@ -2877,6 +2877,92 @@
 }
 
 int
+decor_post_pending (Display *xdisplay,
+		    Window  client,
+		    unsigned int frame_state,
+		    unsigned int frame_type,
+		    unsigned int frame_actions)
+{
+    XEvent event;
+
+    Atom   decor_pending  = XInternAtom (xdisplay, "_COMPIZ_DECOR_PENDING", FALSE);
+
+    /* Send a client message indicating that a new
+     * decoration can be generated for this window
+     */
+    event.xclient.type	       = ClientMessage;
+    event.xclient.window       = client;
+    event.xclient.message_type = decor_pending;
+    event.xclient.format       = 32;
+    event.xclient.data.l[0]    = frame_type;
+    event.xclient.data.l[1]    = frame_state;
+    event.xclient.data.l[2]    = frame_actions;
+    event.xclient.data.l[3]    = 0;
+    event.xclient.data.l[4]    = 0;
+
+    XSendEvent (xdisplay, DefaultRootWindow (xdisplay), 0,
+		StructureNotifyMask, &event);
+
+    return 1;
+}
+
+int
+decor_post_generate_request (Display *xdisplay,
+			     Window  client,
+			     unsigned int frame_type,
+			     unsigned int frame_state,
+			     unsigned int frame_actions)
+{
+    XEvent event;
+
+    Atom   decor_request  = XInternAtom (xdisplay, "_COMPIZ_DECOR_REQUEST", FALSE);
+
+    /* Send a client message indicating that a new
+     * decoration can be generated for this window
+     */
+    event.xclient.type	       = ClientMessage;
+    event.xclient.window       = client;
+    event.xclient.message_type = decor_request;
+    event.xclient.format       = 32;
+    event.xclient.data.l[0]    = frame_type;
+    event.xclient.data.l[1]    = frame_state;
+    event.xclient.data.l[2]    = frame_actions;
+    event.xclient.data.l[3]    = 0;
+    event.xclient.data.l[4]    = 0;
+
+    XSendEvent (xdisplay, DefaultRootWindow (xdisplay), 0,
+		StructureNotifyMask, &event);
+
+    return 1;
+}
+
+int
+decor_post_delete_pixmap (Display *xdisplay,
+			  Pixmap  pixmap)
+{
+    XEvent event;
+
+    Atom   decor_delete_pixmap  = XInternAtom (xdisplay, "_COMPIZ_DECOR_DELETE_PIXMAP", FALSE);
+
+    /* Send a client message indicating that a new
+     * decoration can be generated for this window
+     */
+    event.xclient.type	       = ClientMessage;
+    event.xclient.window       = DefaultRootWindow (xdisplay);
+    event.xclient.message_type = decor_delete_pixmap;
+    event.xclient.format       = 32;
+    event.xclient.data.l[0]    = pixmap;
+    event.xclient.data.l[1]    = 0;
+    event.xclient.data.l[2]    = 0;
+    event.xclient.data.l[3]    = 0;
+    event.xclient.data.l[4]    = 0;
+
+    XSendEvent (xdisplay, DefaultRootWindow (xdisplay), 0,
+		StructureNotifyMask, &event);
+
+    return 1;
+}
+int
 decor_acquire_dm_session (Display    *xdisplay,
 			  int	     screen,
 			  const char *name,

=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2012-04-20 07:34:18 +0000
+++ plugins/decor/src/decor.cpp	2012-04-20 07:34:19 +0000
@@ -347,6 +347,8 @@
 
 DecorTexture::~DecorTexture ()
 {
+     decor_post_delete_pixmap (screen->dpy (), pixmap);
+
     if (damage)
 	XDamageDestroy (screen->dpy (), damage);
 }
@@ -366,6 +368,9 @@
     Window	 root;
     int		 i;
 
+    if (pixmap)
+	decor_post_delete_pixmap (screen->dpy (), pixmap);
+
     pixmap = src;
 
     if (!XGetGeometry (screen->dpy (), pixmap, &root,
@@ -390,6 +395,12 @@
     if (!DecorScreen::get (screen)->optionGetMipmap ())
 	textures[0]->setMipmap (false);
 
+    if (damage)
+	XDamageDestroy (screen->dpy (), damage);
+
+    damage = XDamageCreate (screen->dpy (), pixmap,
+			     XDamageReportRawRectangles);
+
     return true;
 }
 
@@ -629,7 +640,8 @@
     frameActions (frameActions),
     quad (quad),
     nQuad (nQuad),
-    type (type)
+    type (type),
+    updateState (0)
 {
     int		    left, right, top, bottom;
     int		    x1, y1, x2, y2;
@@ -734,6 +746,19 @@
     int		    result, format;
     unsigned int    type;
 
+    /* Dispatch any new updates */
+    foreach (const Decoration::Ptr &d, mList)
+    {
+	if (d->updateState & Decoration::UpdatesPending)
+	    decor_post_generate_request (screen->dpy (),
+					 id,
+					 d->frameType,
+					 d->frameState,
+					 d->frameActions);
+
+	d->updateState = 0;
+    }
+
     result = XGetWindowProperty (screen->dpy (), id,
                                  decorAtom, 0L,
                                  PROP_HEADER_SIZE + 6 * (BASE_PROP_SIZE +
@@ -2345,6 +2370,48 @@
 		if (w)
 		    DecorWindow::get (w)->update (true);
 	    }
+	    /* A decoration is pending creation, allow it to be created */
+	    if (event->xclient.message_type == decorPendingAtom)
+	    {
+		CompWindow *w = screen->findWindow (event->xclient.window);
+
+		if (w)
+		{
+		    DecorWindow *dw = DecorWindow::get (w);
+		    bool found = false;
+		    
+		    foreach (const Decoration::Ptr &d, dw->decor.mList)
+		    {
+			if (d->frameType == static_cast <unsigned int> (event->xclient.data.l[0]) &&
+			    d->frameState == static_cast <unsigned int> (event->xclient.data.l[1]) &&
+			    d->frameActions == static_cast <unsigned int> (event->xclient.data.l[2]))
+			{
+			    if (d->updateState & Decoration::UpdateRequested)
+				d->updateState |= Decoration::UpdatesPending;
+			    else
+			    {
+				d->updateState |= Decoration::UpdateRequested;
+
+				decor_post_generate_request (screen->dpy (),
+							     event->xclient.window,
+							     event->xclient.data.l[0],
+							     event->xclient.data.l[1],
+							     event->xclient.data.l[2]);
+			    }
+
+			    found = true;
+			    break;
+			}
+		    }
+
+		    if (!found)
+		        decor_post_generate_request (screen->dpy (),
+						     event->xclient.window,
+						     event->xclient.data.l[0],
+						     event->xclient.data.l[1],
+						     event->xclient.data.l[2]);
+		}
+	    }
 	default:
 	    /* Check for damage events. If the output or input window
 	     * or a texture is updated then damage output extents.
@@ -2419,7 +2486,6 @@
 		    DECOR_WINDOW (w);
 
 		    dw->updateDecoration ();
-
 		    dw->update (true);
 		}
 	    }
@@ -2992,6 +3058,10 @@
 	XInternAtom (s->dpy (), DECOR_TYPE_WINDOW_ATOM_NAME, 0);
     decorSwitchWindowAtom =
 	XInternAtom (s->dpy (), DECOR_SWITCH_WINDOW_ATOM_NAME, 0);
+    decorPendingAtom =
+	XInternAtom (s->dpy (), "_COMPIZ_DECOR_PENDING", 0);
+    decorRequestAtom =
+	XInternAtom (s->dpy (), "_COMPIZ_DECOR_REQUEST", 0);
     requestFrameExtentsAtom =
         XInternAtom (s->dpy (), "_NET_REQUEST_FRAME_EXTENTS", 0);
     shadowColorAtom =

=== modified file 'plugins/decor/src/decor.h'
--- plugins/decor/src/decor.h	2012-04-20 07:34:18 +0000
+++ plugins/decor/src/decor.h	2012-04-20 07:34:19 +0000
@@ -97,6 +97,9 @@
 
 	typedef boost::shared_ptr <Decoration> Ptr;
 
+	static const unsigned int UpdateRequested = 1 << 0;
+	static const unsigned int UpdatesPending = 1 << 1;
+
 	static Decoration::Ptr create (Window        id,
 				       long          *prop,
 				       unsigned int  size,
@@ -135,6 +138,8 @@
 	boost::shared_array <decor_quad_t> quad;
 	int                       nQuad;
 	int                       type;
+
+	unsigned int              updateState;
 };
 
 class DecorationList
@@ -217,6 +222,8 @@
 	Atom shadowColorAtom;
 	Atom shadowInfoAtom;
 	Atom decorSwitchWindowAtom;
+	Atom decorPendingAtom;
+	Atom decorRequestAtom;
 
 	Window dmWin;
 	int    dmSupports;