Merge lp:~smspillaz/compiz-core/compiz-core.fix_924736.2 into lp:compiz-core

[Sam Spilsbury]

Only clip shadows based on windows underneath and only clip if the window is adjacent
and not overlapping (this way you can have two semi maximized windows right next to each
other and not have them cast shadows)

Also update the window decoration state and actions when the window is redecorated

Fix typo that would cause active maximized windows to not get shadows

[Daniel van Vugt (vanvugt)]

The shadows seem fixed. However the window borders are still lost when semi-maximized as described in the bug;
"Semi-maximized windows also seem to not have any border (as if they think they are fully maximized)."

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

> The shadows seem fixed. However the window borders are still lost when semi-
> maximized as described in the bug;
> "Semi-maximized windows also seem to not have any border (as if they think
> they are fully maximized)."

Does this only happen with the unity plugin enabled or does it happen without? I have an impression
that this might be a recent regression in the unity plugin. (Can't test right now, am out)

[Daniel van Vugt (vanvugt)]

I'm not testing with Unity at all. It happens with just plain old lp:compiz-core.

[Sam Spilsbury (smspillaz)]

hmmm....

Are you running a custom theme or something? What happens when you run ./gtk-window-decorator --metacity-theme (some-garbage) to force the cairo mode?

[Daniel van Vugt (vanvugt)]

I'm using the standard theme (Ambiance) in a standard oneiric installation. I haven't changed any defaults.

Sorry, I can't test --metacity-theme right now. Too much other building/testing to do.

[Sam Spilsbury (smspillaz)]

ok.

[lp:~smspillaz/compiz-core/compiz-core.fix_924736.2]

2999. By Sam Spilsbury on 2012-02-16

Use a default value for the draggable border width if the mutter key doesn't exist

[Sam Spilsbury (smspillaz)]

Took me ages to figure out what people meant when they talked about "borders". I usually think in terms of "invisible grab area" or the like.

The draggable border seems to work fine, actually, on semi maximized windows, but it would have been disabled by default if you didn't have mutter installed since we integrate with those keys. I've detected that case and added a default for now. Please test.

[Daniel van Vugt (vanvugt)]

Sorry I didn't notice the new revision 3 hours ago. Probably because it wasn't resubmitted formally and I didn't get a review-request email at all. Now testing...

[Daniel van Vugt (vanvugt)]

Still not fixed in r2999. The 1-pixel border is still missing when semi-maximized:
https://launchpadlibrarian.net/92797648/semi-maximized.png

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Have a look at this instead:
https://code.launchpad.net/~vanvugt/compiz-core/fix-924736/+merge/93342

[Sam Spilsbury (smspillaz)]

Hmm, hang on. You're talking about the 1px border for semi-maximized windows.

They're supposed to use the maximized window decorations iirc. I'll double check with John today. In that case the lack of the 1px border would be a light-themes thing.

[Sam Spilsbury (smspillaz)]

(Incidentally, we actually tried a while ago to remove all visible side borders from all windows, but this obviously didn't fly in the case of, eg, metacity, which doesn't support this feature)

[Daniel van Vugt (vanvugt)]

I understand why design might want to remove visible borders (like OSX?). But to only do so for semi-maximized windows is certainly a bug. That's what is fixed here:
https://code.launchpad.net/~vanvugt/compiz-core/fix-924736/+merge/93342

[Sam Spilsbury (smspillaz)]

> I understand why design might want to remove visible borders (like OSX?). But
> to only do so for semi-maximized windows is certainly a bug. That's what is
> fixed here:
> https://code.launchpad.net/~vanvugt/compiz-core/fix-924736/+merge/93342

It is possible to re-size the window still using the invisible border :)

[Daniel van Vugt (vanvugt)]

Well if it's not a bug then that should be stated here --> bug 924736
and this branch should have been merged already :)

If you want it to look like oneiric however (which I think is right) then use this instead:
https://code.launchpad.net/~vanvugt/compiz-core/fix-924736/+merge/93342

[Sam Spilsbury (smspillaz)]

Alright the verdict is in.

1. Should show rounded corners (but ideally square corners on monitor edges and rounded corners in the middle)
2. Should show restore button on semi maximized windows
3. Should not cast shadows on to each other

So needs work for now. I'll roll your changes into mine and that will tackle item 1, I have a rough idea about item 2, item 3 is done.

[Daniel van Vugt (vanvugt)]

No shadows for a window that is not fully maximized and can be stacked any-which-way? Sounds like a misunderstanding.

[Daniel van Vugt (vanvugt)]

How about we fix the simple bug, and make it look like oneiric with this:
   https://code.launchpad.net/~vanvugt/compiz-core/fix-924736/+merge/93342
then worry about the new look as dictated by design separately?

Anyway, I'm logging off now. Trying to avoid working 4 nights in a row.

[Daniel van Vugt (vanvugt)]

Do we still need this branch given bug 924736 is already fixed?

If so, could you please provide a pointer to the new design spec/bug (other than #924736) that this change is related to?

[Daniel van Vugt (vanvugt)]

review: Needs Information

[Sam Spilsbury (smspillaz)]

<JohnLea> to answer the 2nd question first, no, semi-maximised windows should not cast shadows
                on each other
<JohnLea> also both maximised and semi-maximised windows should not cast shadows on to
                adjacent monitors

As for the other stuff -

8 - &frame->max_border_shadow_inactive,
9 + &frame->max_border_shadow_active,

That is a typo that needs to be fixed

21 + update_window_decoration_state (win);
22 + update_window_decoration_actions (win);
23 + update_window_decoration_icon (win);

That was causing quickly demaximized windows to retain maximized decorations

40 + if (gconf_client_dir_exists (client, "/apps/mutter/general", NULL))
41 + {
42 + new_width = gconf_client_get_int (client,
43 + MUTTER_DRAGGABLE_BORDER_WIDTH_KEY,
44 + NULL);
45 + }
46 + else
47 + {
48 + new_width = 7;
49 + }

That was causing no draggable border

[Daniel van Vugt (vanvugt)]

OK, I have re-tested this branch and compared it to lp:compiz-core. There are a few problems however, with at least 2 regressions introduced by this branch...

1. The fix to make semi-maximized windows not cast shadows on each other mostly works, but causes shadow-like artifacts appear when you slowly hover the mouse over the common border between the windows.

2. Using lp:compiz-core, maximized windows do not cast shadows on adjacent monitors at all. That bug has seemingly already been fixed since oneiric. However this branch reintroduces that bug and causes adjacent monitors to show the shadow of maximized windows.

3. The branch name is misleading because it actually has nothing to do with "924736" any more.

Please log a bug for #1, fix #1 & #2, and resubmit using a more accurate branch name to resolve #3.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

On Mon, 20 Feb 2012, Daniel van Vugt wrote:

> Review: Needs Fixing
>
> OK, I have re-tested this branch and compared it to lp:compiz-core. There are a few problems however, with at least 2 regressions introduced by this branch...
>
> 1. The fix to make semi-maximized windows not cast shadows on each other mostly works, but causes shadow-like artifacts appear when you slowly hover the mouse over the common border between the windows.
>

ack

> 2. Using lp:compiz-core, maximized windows do not cast shadows on adjacent monitors at all. That bug has seemingly already been fixed since oneiric. However this branch reintroduces that bug and causes adjacent monitors to show the shadow of maximized windows.
>

The problem was that maximized windows were not casting shadows /at all/.
This in itself was a bug[tm]

> 3. The branch name is misleading because it actually has nothing to do with "924736" any more.
>

ack

> Please log a bug for #1, fix #1 & #2, and resubmit using a more accurate branch name to resolve #3.
>
>
> --
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_924736.2/+merge/92727
> You are the owner of lp:~smspillaz/compiz-core/compiz-core.fix_924736.2.
>

Unmerged revisions

2999. By Sam Spilsbury on 2012-02-16

Use a default value for the draggable border width if the mutter key doesn't exist

2998. By Sam Spilsbury on 2012-02-13

  Only clip shadows based on windows underneath and only clip if the window is adjacent
  and not overlapping (this way you can have two semi maximized windows right next to each
  other and not have them cast shadows)

  Also update the window decoration state and actions when the
  window is redecorated

  Fix typo that would cause active maximized windows to not get shadows

  Fixes LP #924736

=== modified file 'gtk/window-decorator/decorator.c'
--- gtk/window-decorator/decorator.c	2011-10-13 11:31:37 +0000
+++ gtk/window-decorator/decorator.c	2012-02-16 02:06:18 +0000
@@ -936,7 +936,7 @@
 			     gdk_x11_screen_get_xscreen (gdk_screen_get_default ()),
 			     frame, &frame->border_shadow_active,
 			     &frame->window_context_active,
-			     &frame->max_border_shadow_inactive,
+			     &frame->max_border_shadow_active,
 			     &frame->max_window_context_active,
 			     info, opt_active_shadow, opt_inactive_shadow);
 

=== modified file 'gtk/window-decorator/events.c'
--- gtk/window-decorator/events.c	2012-02-10 10:23:27 +0000
+++ gtk/window-decorator/events.c	2012-02-16 02:06:18 +0000
@@ -1011,7 +1011,11 @@
 
 			d->frame = gwd_get_decor_frame (get_frame_type (win));
 
+			update_window_decoration_state (win);
+			update_window_decoration_actions (win);
+			update_window_decoration_icon (win);
 			update_window_decoration_size (win);
+
 			update_event_windows (win);
 		    }
 		    else

=== modified file 'gtk/window-decorator/settings.c'
--- gtk/window-decorator/settings.c	2011-10-13 12:22:14 +0000
+++ gtk/window-decorator/settings.c	2012-02-16 02:06:18 +0000
@@ -154,9 +154,16 @@
     int      new_width;
     int      width = settings->mutter_draggable_border_width;
 
-    new_width = gconf_client_get_int (client,
-				      MUTTER_DRAGGABLE_BORDER_WIDTH_KEY,
-				      NULL);
+    if (gconf_client_dir_exists (client, "/apps/mutter/general", NULL))
+    {
+	new_width = gconf_client_get_int (client,
+					  MUTTER_DRAGGABLE_BORDER_WIDTH_KEY,
+					  NULL);
+    }
+    else
+    {
+	new_width = 7;
+    }
 
     if (new_width != width)
     {

=== modified file 'plugins/decor/src/decor.cpp'
--- plugins/decor/src/decor.cpp	2012-02-09 07:48:57 +0000
+++ plugins/decor/src/decor.cpp	2012-02-16 02:06:18 +0000
@@ -107,8 +107,6 @@
 
         for (it--; it != screen->windows ().end (); it--)
         {
-            CompRegion inter;
-
             if (!(*it)->isViewable ())
                 continue;
 
@@ -121,7 +119,7 @@
             if (!isAncestorTo (window, (*it)))
                 continue;
 
-	    inter = shadowRegion.intersected ((*it)->borderRect ());
+	    CompRegion inter = shadowRegion.intersected ((*it)->borderRect ());
 
             if (!inter.isEmpty ())
 		shadowRegion = shadowRegion.subtracted (inter);
@@ -151,6 +149,32 @@
 	    shadowRegion = shadowRegion.subtracted (area);
         }
     }
+    else if (window->state () & MAXIMIZE_STATE)
+    {
+	shadowRegion = shadowRegion.intersected (screen->region ());
+
+	CompWindowVector::const_iterator it = std::find (screen->clientList ().begin (),
+							 screen->clientList ().end (),
+							 window);
+	CompWindowVector::const_reverse_iterator rit (it);
+
+	for (; rit != screen->clientList ().rend (); rit++)
+	{
+	    if (!((*rit)->state () & MAXIMIZE_STATE))
+		continue;
+
+	    /* Only care about windows that logically overlap
+	     * the shadow, but aren't actually obscured by this window */
+	    if (window->borderRect ().intersects ((*rit)->borderRect ()))
+		continue;
+
+	    CompRegion clippable = shadowRegion - CompRegion (window->borderRect ());
+	    CompRegion inter = clippable.intersected ((*rit)->borderRect ());
+
+	    if (!inter.isEmpty ())
+		shadowRegion = shadowRegion.subtracted (inter);
+	}
+    }
 }
 
 /*
@@ -2229,6 +2253,14 @@
 	    unshading = true;
 	    shading = false;
 	    break;
+	case CompWindowNotifyRestack:
+	    if (dScreen->cmActive)
+	    {
+		foreach (CompWindow *cw, DecorScreen::get (screen)->cScreen->getWindowPaintList ())
+		{
+		    DecorWindow::get (cw)->computeShadowRegion ();
+		}
+	    }
 	default:
 	    break;
     }