Merge lp:~vanvugt/compiz/fix-928807 into lp:compiz/0.9.9

[Daniel van Vugt]

Disable shadows for maximized windows. They only create annoyance and
confusion on adjacent monitors/workspaces. (LP: #928807)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3477
http://jenkins.qa.ubuntu.com/job/compiz-ci/254/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/254/console

review: Approve (continuous-integration)

[Sam Spilsbury (smspillaz)]

Looks fine implementation wise, just some questions:

8 + /*
9 + * Warning: decor_shadow_create does more than return a decor_shadow_t*
10 + * It also has to be called to populate the context parameter
11 + * (third last parameter). So even if you don't want a shadow
12 + * then you still need to call decor_shadow_create :(
13 + */
14 *shadow_normal = decor_shadow_create (xdisplay,
15 screen,
16 1, 1,

Should we rename decor_shadow_create then? decor_create_shadow_and_pad_context ? Fixing the symbol is probably better than adding a comment

I was going to ask about this:

38 - info, opt_active_shadow, opt_inactive_shadow);
39 + info, opt_active_shadow, &no_shadow);

Then realized that its correct, maybe it might be good to do this:

info,
opt_active_shadow, // for normal windows
&no_shadow, // for maximized windows

Something entirely different to consider would be to clip maximized window shadows on output edges. It still makes sense for them to have shadows in the case of them being transformed etc. Can you try this and see how it works ?

review: Needs Information

[Daniel van Vugt (vanvugt)]

I'd rather not rename decor_shadow_create because that would break the libdecoration API and ABI backward-compatibility. It's not worth it just to rename a function.

Yeah I know we used to clip shadows on the other monitors, but I'm trying to avoid any more shadow clipping. Clipping shadows often causes artefacts of some sort like in bug 979252, bug 1061346.

Finally there is the damage region/clipping problem introduced in 0.9.8 with swapbuffers. It means we no longer have the ability to guarantee every redraw is clipped to the damage region like with regional redraws. Every plugin is now responsible for honouring the damage region and ensuring nothing ever draws outside of that. Doing this with shadows I think is overcomplicated considering the potential pay-off (keeping shadows on transformed maximized windows).

I've been thinking about a nice reusable solution to the damage region problem, and separately, how to layer shadows nicely (and more efficiently). However that's off topic here.

[Sam Spilsbury (smspillaz)]

Okay, lets go with this then.

review: Approve

=== modified file 'gtk/window-decorator/decorator.c'
--- gtk/window-decorator/decorator.c	2012-11-12 07:07:38 +0000
+++ gtk/window-decorator/decorator.c	2012-11-20 08:16:18 +0000
@@ -911,6 +911,12 @@
 	*shadow_normal = NULL;
     }
 
+    /*
+     * Warning: decor_shadow_create does more than return a decor_shadow_t*
+     *          It also has to be called to populate the context parameter
+     *          (third last parameter). So even if you don't want a shadow
+     *          then you still need to call decor_shadow_create :(
+     */
     *shadow_normal = decor_shadow_create (xdisplay,
 						 screen,
 						 1, 1,
@@ -954,7 +960,7 @@
 			     frame->max_win_extents.top + frame->max_titlebar_height -
 			     TRANSLUCENT_CORNER_SIZE,
 			     frame->max_win_extents.bottom - TRANSLUCENT_CORNER_SIZE,
-			     opt_shadow,
+			     opt_no_shadow,  /* No shadow when maximized */
 			     context_max,
 			     draw_border_shape,
 			     (void *) info);
@@ -976,6 +982,7 @@
 		     decor_shadow_options_t *opt_active_shadow,
 		     decor_shadow_options_t *opt_inactive_shadow)
 {
+    static decor_shadow_options_t no_shadow = {0.0, 0.0, {0, 0, 0}, 0, 0};
     gwd_decor_frame_ref (frame);
 
     info->active = TRUE;
@@ -986,7 +993,7 @@
 			     &frame->window_context_active,
 			     &frame->max_border_shadow_active,
 			     &frame->max_window_context_active,
-			     info, opt_active_shadow, opt_inactive_shadow);
+			     info, opt_active_shadow, &no_shadow);
 
     info->active = FALSE;
 
@@ -996,7 +1003,7 @@
                              &frame->window_context_inactive,
                              &frame->max_border_shadow_inactive,
                              &frame->max_window_context_inactive,
-                             info, opt_inactive_shadow, opt_active_shadow);
+                             info, opt_inactive_shadow, &no_shadow);
 
     gwd_decor_frame_unref (frame);
 }