compiz - Dec 2006 - Patch to wobbly snap for outputs

Here's a patch to wobbly.c to handle edge snapping with multiple outputs...

Also, I tweaked the window edge snapping to include dock window types, to
support the case where dock windows may be on the inner edges of multiple
monitors (and thus currently ignored as struts in the output workarea setup).

I personally think we should include these "inner" struts when
calculating the
workarea for each output (which also helps window maximizing), and only
ignore them in the screen workarea case (for the _NET_WORKAREA hint).

Any reason that wouldn't make sense?

...MC
-------------- next part --------------
--- plugins/wobbly.c
+++ plugins/wobbly.c
@@ -510,6 +510,9 @@ findNextWestEdge (CompWindow *w,
     int e, end;
     int x;
 
+    int        output;
+    XRectangle workArea;
+
     start = -65535.0f;
     end   =  65535.0f;
 
@@ -518,15 +521,20 @@ findNextWestEdge (CompWindow *w,
 
     x = object->position.x + w->output.left - w->input.left;
 
-    if (x >= w->screen->workArea.x)
+    output = outputDeviceForPoint (w->screen, x, object->position.y);
+    workArea = w->screen->outputDev[output].workArea;
+
+    if (x >= workArea.x)
     {
 	CompWindow *p;
 
-	v1 = w->screen->workArea.x;
+	v1 = workArea.x;
 
 	for (p = w->screen->windows; p; p = p->next)
 	{
-	    if (p->invisible || w == p || p->type != CompWindowTypeNormalMask)
+	    if (p->invisible || w == p ||
+		    (p->type != CompWindowTypeNormalMask &&
+		     p->type != CompWindowTypeDockMask))
 		continue;
 
 	    s = p->attrib.y - p->output.top;
@@ -566,7 +574,7 @@ findNextWestEdge (CompWindow *w,
     }
     else
     {
-	v2 = w->screen->workArea.x;
+	v2 = workArea.x;
     }
 
     v1 = v1 - w->output.left + w->input.left;
@@ -594,6 +602,9 @@ findNextEastEdge (CompWindow *w,
     int e, end;
     int x;
 
+    int        output;
+    XRectangle workArea; 
+
     start = -65535.0f;
     end   =  65535.0f;
 
@@ -602,15 +613,20 @@ findNextEastEdge (CompWindow *w,
 
     x = object->position.x - w->output.right + w->input.right;
 
-    if (x <= w->screen->workArea.x + w->screen->workArea.width)
+    output = outputDeviceForPoint (w->screen, x, object->position.y);
+    workArea = w->screen->outputDev[output].workArea;
+
+    if (x <= workArea.x + workArea.width)
     {
 	CompWindow *p;
 
-	v1 = w->screen->workArea.x + w->screen->workArea.width;
+	v1 = workArea.x + workArea.width;
 
 	for (p = w->screen->windows; p; p = p->next)
 	{
-	    if (p->invisible || w == p || p->type != CompWindowTypeNormalMask)
+	    if (p->invisible || w == p ||
+		    (p->type != CompWindowTypeNormalMask &&
+		     p->type != CompWindowTypeDockMask))
 		continue;
 
 	    s = p->attrib.y - p->output.top;
@@ -650,7 +666,7 @@ findNextEastEdge (CompWindow *w,
     }
     else
     {
-	v2 = w->screen->workArea.x + w->screen->workArea.width;
+	v2 = workArea.x + workArea.width;
     }
 
     v1 = v1 + w->output.right - w->input.right;
@@ -678,6 +694,9 @@ findNextNorthEdge (CompWindow *w,
     int e, end;
     int y;
 
+    int        output;
+    XRectangle workArea;
+
     start = -65535.0f;
     end   =  65535.0f;
 
@@ -686,15 +705,20 @@ findNextNorthEdge (CompWindow *w,
 
     y = object->position.y + w->output.top - w->input.top;
 
-    if (y >= w->screen->workArea.y)
+    output = outputDeviceForPoint (w->screen, object->position.x, y);
+    workArea = w->screen->outputDev[output].workArea;
+
+    if (y >= workArea.y)
     {
 	CompWindow *p;
 
-	v1 = w->screen->workArea.y;
+	v1 = workArea.y;
 
 	for (p = w->screen->windows; p; p = p->next)
 	{
-	    if (p->invisible || w == p || p->type != CompWindowTypeNormalMask)
+	    if (p->invisible || w == p ||
+		    (p->type != CompWindowTypeNormalMask &&
+		     p->type != CompWindowTypeDockMask))
 		continue;
 
 	    s = p->attrib.x - p->output.left;
@@ -734,7 +758,7 @@ findNextNorthEdge (CompWindow *w,
     }
     else
     {
-	v2 = w->screen->workArea.y;
+	v2 = workArea.y;
     }
 
     v1 = v1 - w->output.top + w->input.top;
@@ -762,6 +786,9 @@ findNextSouthEdge (CompWindow *w,
     int e, end;
     int y;
 
+    int        output;
+    XRectangle workArea;
+
     start = -65535.0f;
     end   =  65535.0f;
 
@@ -770,15 +797,20 @@ findNextSouthEdge (CompWindow *w,
 
     y = object->position.y - w->output.bottom + w->input.bottom;
 
-    if (y <= w->screen->workArea.y + w->screen->workArea.height)
+    output = outputDeviceForPoint (w->screen, object->position.x, y);
+    workArea = w->screen->outputDev[output].workArea;
+
+    if (y <= workArea.y + workArea.height)
     {
 	CompWindow *p;
 
-	v1 = w->screen->workArea.y + w->screen->workArea.height;
+	v1 = workArea.y + workArea.height;
 
 	for (p = w->screen->windows; p; p = p->next)
 	{
-	    if (p->invisible || w == p || p->type != CompWindowTypeNormalMask)
+	    if (p->invisible || w == p ||
+		    (p->type != CompWindowTypeNormalMask &&
+		     p->type != CompWindowTypeDockMask))
 		continue;
 
 	    s = p->attrib.x - p->output.left;
@@ -818,7 +850,7 @@ findNextSouthEdge (CompWindow *w,
     }
     else
     {
-	v2 = w->screen->workArea.y + w->screen->workArea.height;
+	v2 = workArea.y + workArea.height;
     }
 
     v1 = v1 + w->output.bottom - w->input.bottom;

On Fri, 2006-12-08 at 12:09 -0700, Mike Cook wrote:
> Here's a patch to wobbly.c to handle edge snapping with multiple
outputs...
After looking at this I found the correct solution to be to just snap to
window struts instead of any workarea. I've pushed out changes that
should take care of this. I don't have access to a multi-head setup
right now so I can't verify that it works OK. Please give it a try.
> 
> Also, I tweaked the window edge snapping to include dock window types, to
> support the case where dock windows may be on the inner edges of multiple
> monitors (and thus currently ignored as struts in the output workarea
setup).
I've included a few more window types in snapping. However, including
dock windows is probably not a good idea. Sometime windows with dock
type and below state are used for windows that shouldn't have any
decorations and stick to the desktop. We don't want to snap to those.
> 
> I personally think we should include these "inner" struts when
calculating the
> workarea for each output (which also helps window maximizing), and only
> ignore them in the screen workarea case (for the _NET_WORKAREA hint).
What do you mean by "inner" struts? Each workarea rectangle should be
the maximum rectangle that doesn't intersect any struts. If that's
currently not the case, it should be fixed.

If the workarea isn't good enough for a plugin it should instead look at
the strut hints for each window like the wobbly plugin is now doing. We
can add a region to the output struct that is the output area minus all
window struts if that turns out useful for plugins.

-David

On Fri, 2006-12-11 at 21:32 -0700, David Reveman wrote:
> After looking at this I found the correct solution to be to just snap to
> window struts instead of any workarea. I've pushed out changes that
> should take care of this. I don't have access to a multi-head setup
> right now so I can't verify that it works OK. Please give it a try.
Hm, somebody will have to donate a couple extra monitors...  ;)  After a quick
first test on a single head machine I think there's a typo, as it seems to
be
ignoring the struts and just snapping to screen edges for me.  Also, I'm not
sure if the struts are the best option-- more on that below...
> I've included a few more window types in snapping. However, including
> dock windows is probably not a good idea. Sometime windows with dock
> type and below state are used for windows that shouldn't have any
> decorations and stick to the desktop. We don't want to snap to those.
Ah, yeah, I wasn't sure if dock type might be used like that.
> > I personally think we should include these "inner" struts
when calculating the
> > workarea for each output (which also helps window maximizing), and
only
> > ignore them in the screen workarea case (for the _NET_WORKAREA hint).
> 
> What do you mean by "inner" struts? Each workarea rectangle
should be
> the maximum rectangle that doesn't intersect any struts. If that's
> currently not the case, it should be fixed.
Sorry, by "inner struts" I was trying to refer to those on the inside
edge between
two outputs.  Say I have 2 monitors, and I have a vertical dock covering the
right edge of the left monitor, or the left edge of the right monitor--
basically
down the middle of the screen.  Those type seem to be currently ignored.
> If the workarea isn't good enough for a plugin it should instead look
at
> the strut hints for each window like the wobbly plugin is now doing. We
> can add a region to the output struct that is the output area minus all
> window struts if that turns out useful for plugins.
I can't think of any time the output's workarea wouldn't work, as
however that is
calculated we should probably use it to be consistent.  Here are the issues I
ran into as I was trying to work on this:
1- Struts that are on those "inner" edges like I described before are
currently
   ignored in the output workarea calculation.  So, for instance, a window will
   maximize to the output edge under that dock.
2- A dock may cover the top or bottom of only one output.  That strut should
   not be considered when snapping or maximizing on a different output.
3- In the case of the total screen's workarea we should ignore those in #1
and
   clip to those in #2 to have the largest total area (at least until the
workarea
   hint can handle multiple regions, or whatever other solution the spec comes
   up with).
4- Also, say you have two or three panels across the top of an output.  You
   wouldn't want to snap to each one separately, but just to the lowest one,
   which should be what the output workarea calculation would give you.
5- And finally, in the patch I was intending that a window also snaps to the
   edges of each output, which is the current behavior that I get with metacity,
   I believe.

So, in the end, that's why I was thinking that each output workarea would be
the best region to snap to, if they include calculations for all the struts at
any
edge within that output.  That then also covers snapping to the output edges
(strut or not).  Sorry for the long response... ;)

...MC

On Tue, Dec 13, 2006 at 11:00 AM, Mike Cook wrote: 
> Whoa!  There are definitely some issues to address here...  Kicker actually
> does set the struts right on those "inner" edges, which really
throws things
> off in compiz.  I'll see if I can't put together a couple patches
to try to do a
> better job of handling the struts when they're set, and hacking around
the
> fact that gnome-panel doesn't set them (currently).
Alright, this patch should handle "inner" struts when they're
properly set.  I
toyed with hacking around the fact that gnome-panel doesn't set the strut
size properly, but the window geometry doesn't seem reliable at the time
the struts are set, so we'll just have to get that fixed in gnome-panel
(which
would then require some fixes in metacity to handle it, I expect).  Kicker
(and probably others) does seem to set them and so this should handle it.

...MC
-------------- next part --------------
A non-text attachment was scrubbed...
Name: innerstrutworkarea.patch
Type: application/octet-stream
Size: 2206 bytes
Desc: not available
Url :
http://lists.freedesktop.org/archives/compiz/attachments/20061216/9047f76a/innerstrutworkarea.obj

On Mon, Dec 18, 2006 at 18:18, David Reveman wrote:
> I have now pushed out some code to handle this properly. Here's what I
> think is the correct way to include each strut:
> 
> (a window strut edge is valid if it's within the destination box)
> 
> 1. left_strut = 0.
> 2. right_strut = box_max_x.
> 3. update left_strut if left window strut edge is valid.
> 4. update right_strut if right window strut edge is valid.
> 5. horizontal struts are only valid if they are not overlapping,
> left_strut must be less than right_strut. if they are not valid go to 6.
> 5.1. update max_left_strut if left_strut is greater.
> 5.2. update min_right_strut if right_strut is less.
> 
> 6. top_strut = 0.
> 7. bottom_strut = box_max_y.
> 8. update top_strut if top window strut edge is valid.
> 9. update bottom_strut if bottom window strut edge is valid.
> 10. vertical struts are only valid if they are not overlapping,
> top_strut must be less than bottom_strut. if they are not valid go to
> 11.
> 10.1. update max_top_strut if top_strut is greater.
> 10.2. update min_bottom_strut if bottom_strut is less.
> 
> 11. done.
> 
> 
> This will allow floating struts while still handling overlapping struts
> like what kicker sets properly.
> 
> Have a look at the code I put in screen.c, it's simple and hopefully
> very easy to understand for anyone who wants to know what's going on
> there.
> 
> -David
>From what I've seen a window's struts are only set for a single
side,
depending on which edge it's on (regardless if it's the screen edge or
an
output edge).  So, a single window wouldn't overlap itself, as they would
not set both left and right struts on the same window, not currently.
You could have two separate panels that overlap in that way, which was
what I was trying to describe with the diagrams a few replies back.
Sorry if I didn't explain what I was seeing clearly...

So, I think this latest update lends itself to a better solution for floating
windows (if they set struts for more than one side), but I don't think it
will handle the current case of a single kicker panel on the center edge
between the two outputs (i.e., the screen workarea gets shifted entirely
to the non-strut side of the panel).

I'll test some more once I get back to my multi-monitor machine, but
that's what I'm thinking so far...  Cheers!

...MC