Re: [compiz] Patch to wobbly snap for outputs
On Sat, 2006-12-16 at 10:36 -0700, Mike Cook wrote: 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. Yes, I would probably not have accepted some ugly hack that used window geometry to work around gnome-panel not setting struts properly anyhow. - if (y1 pBox-y2 y2 pBox-y1) + /* consider strut if start/end overlaps box, size ends in box, + and edge is outside box or both edge size in same output + (in order to handle struts across multiple outputs) */ + if (y1 pBox-y2 y2 pBox-y1 x2 pBox-x2 + (x1 pBox-x1 || + outputDeviceForPoint(w-screen, x1, y1) == + outputDeviceForPoint(w-screen, x2-1, y2-1))) The x2 pBox-x2 check makes a lost of sense. However I fail to see why this: + (x1 pBox-x1 || + outputDeviceForPoint(w-screen, x1, y1) == + outputDeviceForPoint(w-screen, x2-1, y2-1)) check is necessary. We already know that x2 pBox-x2 so the only struts that are rejected from this check is those that are partially outside the box vertically and why we would want to reject those? This is an obvious fix and it's in head now. @@ -3415,7 +3429,7 @@ outputDeviceForPoint (CompScreen *s, x2 = s-outputDev[i].region.extents.x2; y2 = s-outputDev[i].region.extents.y2; - if (x1 x x2 = x y1 y y2 = y) + if (x1 = x x2 x y1 = y y2 y) return i; } Thanks, -David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Patch to wobbly snap for outputs
On Mon, Dec 18, 2006 at 9:50 AM, David Reveman wrote:
-if (y1 pBox- y2 y2 pBox- y1)
+ /* consider strut if start/end overlaps box, size ends in box,
+ and edge is outside box or both edge size in same output
+ (in order to handle struts across multiple outputs) */
+ if (y1 pBox- y2 y2 pBox- y1 x2 pBox- x2
+ (x1 pBox- x1 ||
+ outputDeviceForPoint(w- screen, x1, y1) ==
+ outputDeviceForPoint(w- screen, x2- 1, y2- 1)))
The x2 pBox- x2 check makes a lost of sense. However I fail to see
why this:
+ (x1 pBox- x1 ||
+ outputDeviceForPoint(w- screen, x1, y1) ==
+ outputDeviceForPoint(w- screen, x2- 1, y2- 1))
check is necessary. We already know that x2 pBox- x2 so the only
struts that are rejected from this check is those that are partially
outside the box vertically and why we would want to reject those?
Since we know the strut ends within the box, that secondary check is
meant to handle the two basic cases of a smaller box (such as for an
output workarea) to allow a strut which starts outside it, and for a larger
box (such as for the screen workarea) to ignore any internal struts which
cross over multiple outputs.
I do see I forgot to set the values in the output point check to within the
box, so it would be better to constrain the non-interesting coords to be
within box coords something like:
+ (x1 pBox- x1 ||
+ outputDeviceForPoint(w- screen, x1, pBox-y1) ==
+ outputDeviceForPoint(w- screen, x2- 1, pBox-y1))
I've attached an updated patch (not including the fix I think you already
have) that should take care of that. Do you see anything I'm missing?
...MC
--- src/screen.c
+++ src/screen.c
@@ -2680,7 +2680,13 @@ computeWorkareaForBox (CompScreen *s,
x2 = x1 + w-struts-left.width;
y2 = y1 + w-struts-left.height;
- if (y1 pBox-y2 y2 pBox-y1)
+ /* consider strut if start/end overlaps box, size ends in box,
+ and edge is outside box or both edge size in same output
+ (in order to handle struts across multiple outputs) */
+ if (y1 pBox-y2 y2 pBox-y1 x2 pBox-x2
+ (x1 pBox-x1 ||
+ outputDeviceForPoint(w-screen, x1, pBox-y1) ==
+ outputDeviceForPoint(w-screen, x2-1, pBox-y1)))
{
if (x2 strutX1)
strutX1 = x2;
@@ -2691,7 +2697,10 @@ computeWorkareaForBox (CompScreen *s,
x2 = x1 + w-struts-right.width;
y2 = y1 + w-struts-right.height;
- if (y1 pBox-y2 y2 pBox-y1)
+ if (y1 pBox-y2 y2 pBox-y1 x1 pBox-x1
+ (x2 pBox-x2 ||
+ outputDeviceForPoint(w-screen, x1, pBox-y1) ==
+ outputDeviceForPoint(w-screen, x2-1, pBox-y1)))
{
if (x1 strutX2)
strutX2 = x1;
@@ -2702,7 +2710,10 @@ computeWorkareaForBox (CompScreen *s,
x2 = x1 + w-struts-top.width;
y2 = y1 + w-struts-top.height;
- if (x1 pBox-x2 x2 pBox-x1)
+ if (x1 pBox-x2 x2 pBox-x1 y2 pBox-y2
+ (y1 pBox-y1 ||
+ outputDeviceForPoint(w-screen, pBox-x1, y1) ==
+ outputDeviceForPoint(w-screen, pBox-x1, y2-1)))
{
if (y2 strutY1)
strutY1 = y2;
@@ -2713,7 +2724,10 @@ computeWorkareaForBox (CompScreen *s,
x2 = x1 + w-struts-bottom.width;
y2 = y1 + w-struts-bottom.height;
- if (x1 pBox-x2 x2 pBox-x1)
+ if (x1 pBox-x2 x2 pBox-x1 y1 pBox-y1
+ (y2 pBox-y2 ||
+ outputDeviceForPoint(w-screen, pBox-x1, y1) ==
+ outputDeviceForPoint(w-screen, pBox-x1, y2-1)))
{
if (y1 strutY2)
strutY2 = y1;
___
compiz mailing list
compiz@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Patch to wobbly snap for outputs
On Mon, 2006-12-18 at 11:03 -0700, Mike Cook wrote: On Mon, Dec 18, 2006 at 9:50 AM, David Reveman wrote: -if (y1 pBox- y2 y2 pBox- y1) + /* consider strut if start/end overlaps box, size ends in box, + and edge is outside box or both edge size in same output + (in order to handle struts across multiple outputs) */ + if (y1 pBox- y2 y2 pBox- y1 x2 pBox- x2 + (x1 pBox- x1 || + outputDeviceForPoint(w- screen, x1, y1) == + outputDeviceForPoint(w- screen, x2- 1, y2- 1))) The x2 pBox- x2 check makes a lost of sense. However I fail to see why this: + (x1 pBox- x1 || + outputDeviceForPoint(w- screen, x1, y1) == + outputDeviceForPoint(w- screen, x2- 1, y2- 1)) check is necessary. We already know that x2 pBox- x2 so the only struts that are rejected from this check is those that are partially outside the box vertically and why we would want to reject those? Since we know the strut ends within the box, that secondary check is meant to handle the two basic cases of a smaller box (such as for an output workarea) to allow a strut which starts outside it, and for a larger box (such as for the screen workarea) to ignore any internal struts which cross over multiple outputs. Sorry, I still don't get it. What struts are a vertical kicker panel setting? +-+-+ |_|_| | | | | | | | | | | | |_|_| | | | | +-+-+ If the screen looks like this and the thing in the middle is a panel covering both outputs I assume that the struts are set to either leftX1 = screenW / 2 - panelW / 2; leftX2 = screenW / 2 + panelW / 2; or rightX1 = screenW / 2 - panelW / 2; rightX2 = screenW / 2 + panelW / 2; or that both those set are set. if (y1 pBox-y2 y2 pBox-y1 x2 pBox-x2) should set the left edge properly for both outputs in all those cases. What am I missing? -David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Patch to wobbly snap for outputs
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 ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Patch to wobbly snap for outputs
On Mon, 2006-12-11 at 22:48 -0700, Mike Cook wrote: 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... :) Don't worry, I have a multi-head setup at my office. I'm just not there right now. You're right, a typo caused it to not work at all. It should be fixed now. While looking at this I also found an issue that caused snapping to normal windows to be a bit incorrect, it should be correct now. 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. OK, lets find out why they are ignored then. What struts are those vertical dock windows using? If those windows are not just setting any strut hints, then it's not much we can do. 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. This shouldn't happen. I'll make sure it gets fixed once I know why it currently doesn't work. 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. Why this behavior? 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). The screen workarea could be the current outputs workarea and be updated as the current output changes. This might confuse desktop windows, though. 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. I would want to snap to each one separately and have windows placed smarter then just in the workarea rectangle. Windows should still be maximized to the workarea rectangle, of course. 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. I agree, snapping to output edges makes sense. I can easily make the current wobbly code do that. 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... ;) There's clearly some more work to be done here and I appreciate your help with getting this right. Thanks, -David ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
Re: [compiz] Patch to wobbly snap for outputs
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 ___ compiz mailing list compiz@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/compiz
[compiz] 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
--- plugins/wobbly.c
+++ plugins/wobbly.c
@@ -510,6 +510,9 @@ findNextWestEdge (CompWindow *w,
int e, end;
int x;
+intoutput;
+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;
+intoutput;
+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;
+intoutput;
+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;
+intoutput;
+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 ||
+
