Merge lp:~muktupavels/compiz/gtk-window-decorator-4 into lp:compiz/0.9.12

[Alberts Muktupāvels]

Re-enable metacity theme support

Restore metacity theme support.

[Alberts Muktupāvels (muktupavels)]

I think it is ready for some review. Decorations works, but only with "hack" (added in 3912. revision).

Without it decorations are invisible/transparent. Do you have any idea why?

[Marco Trevisan (Treviño) (3v1n0)]

> I think it is ready for some review. Decorations works, but only with "hack"
> (added in 3912. revision).
>
> Without it decorations are invisible/transparent. Do you have any idea why?

Mh, I've been trying to debug this a little, but I can't either find where the issue is.
It might be related to what meta theme does with the cairo surface, although this seems quite weird (it's like it can't draw in a fully transparent area?).

Anyway, the workaround is not "too ugly" for inclusion, and considering I think we should get the theming back to utopic, I think is fine to accept that.

The only thing I ask you is to document these three lines better, in order to make easier to understand what's going on there.

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Err, I also forgot one thing:

You need to update debian/control:
 - http://pastebin.ubuntu.com/8149755/

review: Needs Fixing

[lp:~muktupavels/compiz/gtk-window-decorator-4]

3914. By Alberts Muktupāvels on 2014-08-26

update build-depends

3915. By Alberts Muktupāvels on 2014-08-26

Add comment to workaround to fix invisible decorations.

[Alberts Muktupāvels (muktupavels)]

Added comment and fixed build-depends. Will it be ok now?

I will try to find if invisible decorations is bug with metacity.

[Alberts Muktupāvels (muktupavels)]

> I will try to find if invisible decorations is bug with metacity.

Does not look like metacity bug. After meta_theme_draw_frame I added following lines:

cairo_status_t status = cairo_surface_write_to_png (cairo_get_target (cr), "gwd.png");
if (status) {
 g_warning ("Failed to write \"%s\": %s\n", "gwd.png", cairo_status_to_string (status));
}

I don't see decorations on windows, but they are saved in ~/gwd.png file.

[Marco Trevisan (Treviño) (3v1n0)]

Ok, that's fine for now.

I was curious to try to do the same you did, but it seems then that the drawing happens correctly.

I'm not sure if that might be caused the decor plugin, expecting something else to be in the texture, but that would be strange, as it should not care about the texture content.

Anyway, the hack is not the nicest thing to see, but it causes no troubles to have it in. So I approve this.

review: Approve

[Alberts Muktupāvels (muktupavels)]

Maybe decor_update_meta_window_property function needs some adjustments? If XChangePropery is not called then decorations does not disappear.

[Alberts Muktupāvels (muktupavels)]

Something is wrong with d->surface. Added cairo_surface_write_to_png (d->surface, "test.png"); in decor_update_meta_window_property.

With hack decorations are visible in test.png, but without hack it is empty/transparent.

[Marco Trevisan (Treviño) (3v1n0)]

> Something is wrong with d->surface. Added cairo_surface_write_to_png
> (d->surface, "test.png"); in decor_update_meta_window_property.
>
> With hack decorations are visible in test.png, but without hack it is
> empty/transparent.

Yeah, that's strange. I've also tried to do something with this, but I wasn't able to find where the problem happens.
It's like that a fully transparent surface create troubles. While not calling the XChangeProperty function seems the decorator not to redraw the surface properly.

Not sure if there's instead a problem caused by the surface not being reinitialized correctly... By the way, thanks for your further researches and it would be nice if you could continue. Although, I'm more concerned with crashes you found (and I didn't) due to BadWindow arguments.

[Alberts Muktupāvels (muktupavels)]

It might not be ready for merging. :(

Did you tried to resize windows with metacity theme? I am getting black decorations:
https://www.dropbox.com/s/on1hianmmphqsrd/black-decorations.png?dl=0

I am getting crashes too often. Does this crash file is useful for you:
https://www.dropbox.com/s/bklaya9phs6dl9y/_usr_bin_gtk-window-decorator.1002.crash?dl=0

Also I created one more merge proposal:
https://code.launchpad.net/~albertsmuktupavels/compiz/gtk-window-decorator-6/+merge/232477

[Marco Trevisan (Treviño) (3v1n0)]

> It might not be ready for merging. :(
>
> Did you tried to resize windows with metacity theme? I am getting black
> decorations:
> https://www.dropbox.com/s/on1hianmmphqsrd/black-decorations.png?dl=0

Mh, no I'm not experiencing that issue here.

> I am getting crashes too often. Does this crash file is useful for you:
> https://www.dropbox.com/s/bklaya9phs6dl9y/_usr_bin_gtk-window-
> decorator.1002.crash?dl=0

Mh, that would be helpful but my binary doesn't match.
So in order to give me a proper crash file, copy to /usr/bin/gtk-window-decorator a binary with debugging symbols, then once you've the crash file send me both the binary and the core dump (crash file).

(otherwise you can examine it getting the core file with apport-extract and then open it with -core option of gdb and the binary file).
Thanks.

> Also I created one more merge proposal:
> https://code.launchpad.net/~albertsmuktupavels/compiz/gtk-window-
> decorator-6/+merge/232477

Fine, I'll check that later, once we've found these issues

[Alberts Muktupāvels (muktupavels)]

So I have two problems that you don't have. Maybe I have problems because I am using nvidia proprietary drivers? Actualy I have third problem.

Videos:
https://www.dropbox.com/s/wjgmx2txye806th/VIDEO0005.mp4?dl=0
https://www.dropbox.com/s/ygmf6eoa1wyw7t1/VIDEO0006.mp4?dl=0
https://www.dropbox.com/s/xu118nuphe1dtto/VIDEO0007.mp4?dl=0

Is this bug with compiz or it is bug in nvidia driver? If it is nvidia than could it cause my crashes too?

I don't have above problems with metacity.

[Marco Trevisan (Treviño) (3v1n0)]

Mh, no I don't thin I've them.

As for the BadWindow errors, using GDK_SYNCRNIZE you should be able to get where the error is more easily. Otherwise try to cast all the XLib calls inside gtk-window-decorator inside error traps (or just do that one for once when starting).

The 2nd video is quite strange, and weird that it's caused by decorator.
As for the last video: do you get the crash when running the decorator in an unity session?

Anyway, indeed the nvidia drivers often cause weird issues, but not sure we can only stop on this assumption.

[Alberts Muktupāvels (muktupavels)]

How about adding 'gdk_x11_display_error_trap_push (gdkdisplay);' in gtk-window-decorator.c? After short period of testing it seem that I am not getting crashes. I think it should be added in part 3 if you agree.

Is it possible to run decorator in unity? If I try to enable it it will disable unity.

So now I have only problem with black decorations, but that might be nvidia problem. Video with it:
https://www.dropbox.com/s/2ba27yvyfb8gjsi/VIDEO0009.mp4?dl=0

[Marco Trevisan (Treviño) (3v1n0)]

> How about adding 'gdk_x11_display_error_trap_push (gdkdisplay);' in gtk-
> window-decorator.c? After short period of testing it seem that I am not
> getting crashes. I think it should be added in part 3 if you agree.

Ok, but and pop it also. At this point you should not get errors.
And you can also remove the other instances.

> Is it possible to run decorator in unity? If I try to enable it it will
> disable unity.

No currently it's not possible.

> So now I have only problem with black decorations, but that might be nvidia
> problem. Video with it:
> https://www.dropbox.com/s/2ba27yvyfb8gjsi/VIDEO0009.mp4?dl=0

Mh, no I've not found that issue definitely.

[Alberts Muktupāvels (muktupavels)]

I decided to not use error trap for whole display...

Added XSynchronize (x, True) in gtk-window-decorator.c. After that I found that XFreePixmap sometimes generate BadPixmap error in pixmap_destroy_cb function you suggested to add in gdk.c. Also seems that it is causing black decorations for me. So now my question is - are you sure that it was needed?

This is what I get from gdb when decorator crashes with BadWindow:
http://pastebin.ubuntu.com/8188141/

[Alberts Muktupāvels (muktupavels)]

I tested this in VirtualBox. It is not crashing there.

Also today Dmitry tested this branch. GWD was not crashing for him too. Radeon HD 6400M/7400M Series with the default open source driver.

[Marco Trevisan (Treviño) (3v1n0)]

Yeah, I'm using a similar configuration... And in fact it works well here.

Wondering why pixmap_destroy_cb function cause problems to you when using metacity, while it doesn't when using standard gtk decorations.

I'm wondering weather if the metacity code in some part tries to delete these textures in a different way (not checked the full code yet for that). But it sounds like it's that.

Anyway, disabling the pixmap_destroy_cb function fixes all the problems you have?

[Alberts Muktupāvels (muktupavels)]

No. It only fix resizing problem.

Also it crashed for Dmitry too. He send me some stack traces. I have not looked at them yet. If you have time maybe you could look:
1) https://www.dropbox.com/s/utvqkcj7n5anhnq/trace1.txt?dl=0
2) https://www.dropbox.com/s/1zupkjtszsi44ym/trace2.txt?dl=0
3) https://www.dropbox.com/s/os5nin5jlis9yz9/trace3.txt?dl=0

[Marco Trevisan (Treviño) (3v1n0)]

Was he launching gdk-window-decorator using GDK_SYNCHRONIZE or calling XSynchronize (x, True) somewhere?

Anyway, all these problems seems to be Bad{Window,Pixmap} issues, that - even if it would be better to avoid - it's not a big problem to ignore them using gdk traps (at the end these can be considered only warnings).

I'm more concerned about the black textures issue you get, and it would be nice to find why that happens if we clear them.
If I'm not wrong these aren't destroyed at decor plugin level, right?

[lp:~muktupavels/compiz/gtk-window-decorator-4]

3916. By Alberts Muktupāvels on 2014-09-07

Merge with gtk-window-decorator-3

[Alberts Muktupāvels (muktupavels)]

> Was he launching gdk-window-decorator using GDK_SYNCHRONIZE or calling
> XSynchronize (x, True) somewhere?

GDK_SYNCHRONIZE

> Anyway, all these problems seems to be Bad{Window,Pixmap} issues, that - even
> if it would be better to avoid - it's not a big problem to ignore them using
> gdk traps (at the end these can be considered only warnings).

I added one more error trap. See gtk-window-decorator-3 part.

> I'm more concerned about the black textures issue you get, and it would be
> nice to find why that happens if we clear them.
> If I'm not wrong these aren't destroyed at decor plugin level, right?

I am not good at reading stack traces and/or debuging, but it looks like crash is happending in cairo. Crash is happening when calling cairo_destroy (d->cr) in decorator.c (692 line). If I delete it then it is not crashing, also black decoration problem dissapears. Any ideas? This looks like last problem.

[lp:~muktupavels/compiz/gtk-window-decorator-4]

3917. By Alberts Muktupāvels on 2014-09-10

Merge with gtk-window-decorator-3

=== modified file 'debian/control'
--- debian/control	2014-09-10 12:52:10 +0000
+++ debian/control	2014-09-10 12:52:11 +0000
@@ -22,7 +22,7 @@
                libglibmm-2.4-dev,
                libgtk-3-dev,
                libice-dev (>= 1:1.0.1), 
-               libmetacity-dev (>= 1:2.34.0),
+               libmetacity-dev (>= 1:3.12.0),
                libpango1.0-dev,
                libpng12-dev | libpng-dev,
                librsvg2-dev, 

=== modified file 'gtk/CMakeLists.txt'
--- gtk/CMakeLists.txt	2014-09-10 12:52:10 +0000
+++ gtk/CMakeLists.txt	2014-09-10 12:52:11 +0000
@@ -25,7 +25,10 @@
         set (CMAKE_REQUIRED_LIBRARIES "")
 
         if (USE_METACITY)
-		compiz_set (USE_METACITY 0)
+            pkg_check_modules (METACITY libmetacity-private>=3.12.0)
+            if (NOT METACITY_FOUND)
+                compiz_set (USE_METACITY 0)
+            endif (NOT METACITY_FOUND)
         endif (USE_METACITY)
 
 	if (USE_GNOME)

=== modified file 'gtk/window-decorator/metacity.c'
--- gtk/window-decorator/metacity.c	2014-09-10 12:52:10 +0000
+++ gtk/window-decorator/metacity.c	2014-09-10 12:52:11 +0000
@@ -122,7 +122,7 @@
     else
     {
 	data = decor_alloc_property (nOffset, WINDOW_DECORATION_TYPE_PIXMAP);
-	decor_quads_to_property (data, nOffset - 1, GDK_PIXMAP_XID (d->pixmap),
+	decor_quads_to_property (data, nOffset - 1, cairo_xlib_surface_get_drawable (d->surface),
 				 &frame_win_extents, &win_extents,
 				 &frame_max_win_extents, &max_win_extents,
 				 ICON_SPACE + d->button_width,
@@ -563,7 +563,7 @@
 {
     Display	      *xdisplay =
 	GDK_DISPLAY_XDISPLAY (gdk_display_get_default ());
-    GdkPixmap	      *pixmap;
+    cairo_surface_t *surface;
     Picture	      src;
     MetaButtonState   button_states [META_BUTTON_TYPE_LAST];
     MetaButtonLayout  button_layout;
@@ -573,9 +573,8 @@
     MetaTheme	      *theme;
     GtkStyle	      *style;
     cairo_t	      *cr;
-    gint	      size, i;
-    GdkRectangle      clip, rect;
-    GdkDrawable       *drawable;
+    gint	      i;
+    GdkRectangle      clip;
     Region	      top_region = NULL;
     Region	      bottom_region = NULL;
     Region	      left_region = NULL;
@@ -593,16 +592,17 @@
 						  meta_inactive_shade_opacity;
     MetaFrameStyle    *frame_style;
     GtkWidget	      *style_window;
+    GdkRGBA           bg_rgba;
     GdkColor	      bg_color;
     double	      bg_alpha;
 
-    if (!d->pixmap || !d->picture)
+    if (!d->surface || !d->picture)
 	return;
 
     if (decoration_alpha == 1.0)
 	alpha = 1.0;
 
-    if (gdk_drawable_get_depth (GDK_DRAWABLE (d->pixmap)) == 32)
+    if (cairo_xlib_surface_get_depth (d->surface) == 32)
     {
 	style = gtk_widget_get_style (d->frame->style_window_rgba);
 	style_window = d->frame->style_window_rgba;
@@ -613,9 +613,7 @@
 	style_window = d->frame->style_window_rgb;
     }
 
-    drawable = d->buffer_pixmap ? d->buffer_pixmap : d->pixmap;
-
-    cr = gdk_cairo_create (GDK_DRAWABLE (drawable));
+    cr = cairo_create (d->buffer_surface ? d->buffer_surface : d->surface);
 
     cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
 
@@ -629,7 +627,7 @@
     meta_get_decoration_geometry (d, theme, &flags, &fgeom, &button_layout,
 				  frame_type, &clip);
 
-    if ((d->prop_xid || !d->buffer_pixmap) && !d->frame_window)
+    if ((d->prop_xid || !d->buffer_surface) && !d->frame_window)
 	draw_shadow_background (d, cr, d->shadow, d->context);
 
     for (i = 0; i < META_BUTTON_TYPE_LAST; ++i)
@@ -645,61 +643,54 @@
     if (frame_style->window_background_color)
     {
 	meta_color_spec_render (frame_style->window_background_color,
-				GTK_WIDGET (style_window),
-				&bg_color);
+				gtk_widget_get_style_context (style_window),
+				&bg_rgba);
+
+	bg_color.red = bg_rgba.red * 65535.0;
+	bg_color.green = bg_rgba.green * 65535.0;
+	bg_color.blue = bg_rgba.blue * 65535.0;
 
 	bg_alpha = frame_style->window_background_alpha / 255.0;
     }
 
+    /* Draw something that will be almost invisible to user. This is hacky way
+     * to fix invisible decorations. */
+    cairo_set_source_rgba (cr, 0, 0, 0, 0.01);
+    cairo_rectangle (cr, 0, 0, 1, 1);
+    cairo_fill (cr);
+    /* ------------ */
+
     cairo_destroy (cr);
 
-    rect.x     = 0;
-    rect.y     = 0;
-    rect.width = clip.width;
-
-    size = MAX (fgeom.top_height, fgeom.bottom_height);
-
-    if (rect.width && size)
-    {
-	XRenderPictFormat *format;
-
 	if (d->frame_window)
-	    pixmap = create_pixmap (rect.width, size, d->frame->style_window_rgb);
+	    surface = create_surface (clip.width, clip.height, d->frame->style_window_rgb);
 	else
-	    pixmap = create_pixmap (rect.width, size, d->frame->style_window_rgba);
+	    surface = create_surface (clip.width, clip.height, d->frame->style_window_rgba);
 
-	cr = gdk_cairo_create (GDK_DRAWABLE (pixmap));
+	cr = cairo_create (surface);
 	gdk_cairo_set_source_color_alpha (cr, &bg_color, bg_alpha);
-	cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
-
-	format = get_format_for_drawable (d, GDK_DRAWABLE (pixmap));
-	src = XRenderCreatePicture (xdisplay, GDK_PIXMAP_XID (pixmap),
-				    format, 0, NULL);
-
-	if (fgeom.top_height)
+	cairo_set_operator (cr, CAIRO_OPERATOR_OVER);
+
+	src = XRenderCreatePicture (xdisplay, cairo_xlib_surface_get_drawable (surface),
+                                get_format_for_surface (d, surface), 0, NULL);
+
+    cairo_paint (cr);
+    meta_theme_draw_frame (theme,
+                           style_window,
+                           cr,
+                           frame_type,
+                           flags,
+                           clip.width - fgeom.left_width - fgeom.right_width,
+                           clip.height - fgeom.top_height - fgeom.bottom_height,
+                           d->layout,
+                           d->frame->text_height,
+                           &button_layout,
+                           button_states,
+                           d->icon_pixbuf,
+                           NULL);
+
+    if (fgeom.top_height)
 	{
-	    rect.height = fgeom.top_height;
-
-	    cairo_paint (cr);
-
-	    meta_theme_draw_frame (theme,
-				   style_window,
-				   pixmap,
-				   &rect,
-				   0, 0,
-				   frame_type,
-				   flags,
-				   clip.width - fgeom.left_width -
-				   fgeom.right_width,
-				   clip.height - fgeom.top_height -
-				   fgeom.bottom_height,
-				   d->layout,
-				   d->frame->text_height,
-				   &button_layout,
-				   button_states,
-				   d->icon_pixbuf,
-				   NULL);
-
 	    top_region = meta_get_top_border_region (&fgeom, clip.width);
 
 	    decor_blend_border_picture (xdisplay,
@@ -717,35 +708,12 @@
 
 	if (fgeom.bottom_height)
 	{
-	    rect.height = fgeom.bottom_height;
-
-	    cairo_paint (cr);
-
-	    meta_theme_draw_frame (theme,
-				   style_window,
-				   pixmap,
-				   &rect,
-				   0,
-				   -(clip.height - fgeom.bottom_height),
-				   frame_type,
-				   flags,
-				   clip.width - fgeom.left_width -
-				   fgeom.right_width,
-				   clip.height - fgeom.top_height -
-				   fgeom.bottom_height,
-				   d->layout,
-				   d->frame->text_height,
-				   &button_layout,
-				   button_states,
-				   d->icon_pixbuf,
-				   NULL);
-
 	    bottom_region = meta_get_bottom_border_region (&fgeom, clip.width);
 
 	    decor_blend_border_picture (xdisplay,
 					d->context,
 					src,
-					0, 0,
+					0, clip.height - fgeom.bottom_height,
 					d->picture,
 					&d->border_layout,
 					BORDER_BOTTOM,
@@ -756,67 +724,14 @@
 
 	}
 
-	cairo_destroy (cr);
-
-	g_object_unref (G_OBJECT (pixmap));
-
-	XRenderFreePicture (xdisplay, src);
-    }
-
-    rect.height = clip.height - fgeom.top_height - fgeom.bottom_height;
-
-    size = MAX (fgeom.left_width, fgeom.right_width);
-
-    if (size && rect.height)
-    {
-	XRenderPictFormat *format;
-
-	if (d->frame_window)
-	{
-	    pixmap = create_pixmap (size, rect.height, d->frame->style_window_rgb);
-	}
-	else
-	    pixmap = create_pixmap (size, rect.height, d->frame->style_window_rgba);
-
-	cr = gdk_cairo_create (GDK_DRAWABLE (pixmap));
-	gdk_cairo_set_source_color_alpha (cr, &bg_color, bg_alpha);
-	cairo_set_operator (cr, CAIRO_OPERATOR_SOURCE);
-
-	format = get_format_for_drawable (d, GDK_DRAWABLE (pixmap));
-	src = XRenderCreatePicture (xdisplay, GDK_PIXMAP_XID (pixmap),
-				    format, 0, NULL);
-
 	if (fgeom.left_width)
 	{
-	    rect.width = fgeom.left_width;
-
-	    cairo_paint (cr);
-
-	    meta_theme_draw_frame (theme,
-				   style_window,
-				   pixmap,
-				   &rect,
-				   0,
-				   -fgeom.top_height,
-				   frame_type,
-				   flags,
-				   clip.width - fgeom.left_width -
-				   fgeom.right_width,
-				   clip.height - fgeom.top_height -
-				   fgeom.bottom_height,
-				   d->layout,
-				   d->frame->text_height,
-				   &button_layout,
-				   button_states,
-				   d->icon_pixbuf,
-				   NULL);
-
 	    left_region = meta_get_left_border_region (&fgeom, clip.height);
 
 	    decor_blend_border_picture (xdisplay,
 					d->context,
 					src,
-					0, 0,
+					0, fgeom.top_height,
 					d->picture,
 					&d->border_layout,
 					BORDER_LEFT,
@@ -828,35 +743,12 @@
 
 	if (fgeom.right_width)
 	{
-	    rect.width = fgeom.right_width;
-
-	    cairo_paint (cr);
-
-	    meta_theme_draw_frame (theme,
-				   style_window,
-				   pixmap,
-				   &rect,
-				   -(clip.width - fgeom.right_width),
-				   -fgeom.top_height,
-				   frame_type,
-				   flags,
-				   clip.width - fgeom.left_width -
-				   fgeom.right_width,
-				   clip.height - fgeom.top_height -
-				   fgeom.bottom_height,
-				   d->layout,
-				   d->frame->text_height,
-				   &button_layout,
-				   button_states,
-				   d->icon_pixbuf,
-				   NULL);
-
 	    right_region = meta_get_right_border_region (&fgeom, clip.height);
 
 	    decor_blend_border_picture (xdisplay,
 					d->context,
 					src,
-					0, 0,
+					clip.width - fgeom.right_width, fgeom.top_height,
 					d->picture,
 					&d->border_layout,
 					BORDER_RIGHT,
@@ -867,23 +759,19 @@
 	}
 
 	cairo_destroy (cr);
-
-	g_object_unref (G_OBJECT (pixmap));
-
+	cairo_surface_destroy (surface);
 	XRenderFreePicture (xdisplay, src);
-    }
 
     copy_to_front_buffer (d);
 
     if (d->frame_window)
     {
 	GdkWindow *gdk_frame_window = gtk_widget_get_window (d->decor_window);
-
-	/*
-	 * FIXME: What is '4' supposed to be for here...
-	 */
-
-	gtk_image_set_from_pixmap (GTK_IMAGE (d->decor_image), d->pixmap, NULL);
+	GdkPixbuf *pixbuf = gdk_pixbuf_get_from_surface (d->surface, 0, 0, d->width, d->height);
+
+	gtk_image_set_from_pixbuf (GTK_IMAGE (d->decor_image), pixbuf);
+	g_object_unref (pixbuf);
+
 	gtk_window_resize (GTK_WINDOW (d->decor_window), d->width, d->height);
 	gdk_window_move (gdk_frame_window,
 			 d->context->left_corner_space - 1,