Created attachment 224 [details]
patch which doesn't allow placement of windows on monitor divide

As far as I can tell the only windows that cause problems with being placed
accross monitors are ones that have client->positioned set  so they don't get
placed by openbox.  For these, I wrote a function that checks to see if they
are on a monitor divide and moves them onto a monitor.	It chooses the monitor
based on client_monitor().

Also, because I thought it looked better, moved clients are not placed right on
the monitor edge, but moved 10% of the difference between the client size and
the monitor size away from the edge of the monitor.  It just looks  a little
nicer.

Also, the patch above adds comments in a few places where I had to figure out
what was going on.  I figured it might make it easier for someone else in the
future. Also provided initial values to some variables to stop compiler warnings.

Created attachment 225 [details]
This patch is better than the first

I ran into a particularly ornary java program that was giving my first patch
problems.  This fixes that problem and also seems to fit in better with the
rest of the client placement code.  

Be sure to back out the first patch before applying this one.

Deleting  the patched files and then doing a cvs udpate should do it.

Comment on attachment 225 [details]
This patch is better than the first

>Index: openbox/client.c
>===================================================================
>RCS file: /cvs/cvsroot/openbox/openbox/client.c,v
>retrieving revision 1.313
>diff -u -r1.313 client.c
>--- openbox/client.c	25 Feb 2004 19:07:40 -0000	1.313
>+++ openbox/client.c	16 Mar 2004 01:58:24 -0000
>@@ -355,9 +355,13 @@
> 
>         if (x != ox || y != oy)
>             client_move(self, x, y);
>-    }
> 
>-    client_showhide(self);
>+        /* As far as I can tell, clients that go through the other placement
>+           functions don't get placed on the divide, so we only need to
>+           do this if the client thinks it knows better*/
>+        client_move_onmonitor(self);
>+    }
>+        client_showhide(self);
> 
>     /* use client_focus instead of client_activate cuz client_activate does
>        stuff like switch desktops etc and I'm not interested in all that when
>@@ -645,6 +649,62 @@
> 
>     frame_frame_gravity(self->frame, x, y); /* get where the client
>                                                should be */
>+    return ox != *x || oy != *y;
>+}
>+
>+void client_move_onmonitor (ObClient *self)
>+{
>+    gint x = self->area.x;
>+    gint y = self->area.y;
>+    if (screen_num_monitors > 1 &&
>+        client_find_onmonitor(self, &x, &y,
>+                              self->frame->area.width,
>+                              self->frame->area.height)) {
>+        client_move(self, x, y);
>+    }
>+}
>+
>+gboolean client_find_onmonitor(ObClient *self, gint *x, gint *y, gint w, gint h)
>+{
>+    Rect *a;
>+    Size s;
>+    gint ox = *x, oy = *y;
>+
>+    if (! screen_num_monitors > 1 )
>+        return FALSE;
>+        
>+    frame_client_gravity(self->frame, x, y); /* get where the frame
>+                                                would be */
>+
>+    a = screen_physical_area_monitor(client_monitor(self));
>+
>+    /* Get the difference between monitor size and client size
>+       We use this to not place the client right on the edge
>+       of the monitor as that looks lame.  This way we place it
>+       10% of the difference of the client and monitor sizes away
>+       from the monitor edge.*/
>+    RECT_DIFF(s, *a, self->area);
>+    
>+    /* only move clients onto a monitor if they are not wider than
>+       the monitor itself.  If they are wider, then there's really
>+       not a lot we can do about them being placed on the monitor
>+       divide*/
>+    if (w <= a->width) {
>+        if (*x < a->x)
>+            *x = a->x + (s.width * 0.1);
>+        if (*x + w > a->x + a->width)
>+            *x = a->x + a->width - w - (s.width * 0.1);
>+    }
>+    /* and for all you freaks who stack your monitors vertically */
>+    if (h <= a->height) {
>+        if (*y < a->y)
>+            *y = a->y + (s.height * 0.1);
>+        if (*y + h > a->y + a->height)
>+            *y = a->y + a->height - h  - (s.height * 0.1);
>+    }
>+
>+    frame_frame_gravity(self->frame, x, y); /* get where the self
>+                                               should be */
> 
>     return ox != *x || oy != *y;
> }
>@@ -2979,6 +3039,9 @@
>     }
> }
> 
>+/* Determines which physical monitor a client is on by calculating the
>+   area of the part of the client on each monitor.  The number of the
>+   monitor containing the greatest area of the client is returned.*/
> guint client_monitor(ObClient *self)
> {
>     guint i;
>@@ -3105,7 +3168,7 @@
>  */
> gint client_directional_edge_search(ObClient *c, ObDirection dir)
> {
>-    gint dest;
>+    gint dest = 0;
>     gint my_edge_start, my_edge_end, my_offset;
>     GList *it;
>     Rect *a;
>Index: openbox/client.h
>===================================================================
>RCS file: /cvs/cvsroot/openbox/openbox/client.h,v
>retrieving revision 1.78
>diff -u -r1.78 client.h
>--- openbox/client.h	15 Oct 2003 03:59:35 -0000	1.78
>+++ openbox/client.h	16 Mar 2004 01:58:24 -0000
>@@ -362,6 +362,23 @@
> */
> void client_move_onscreen(ObClient *self, gboolean rude);
> 
>+/*! Finds coordinates to keep a client on one xinerama monitor.
>+  @param self The client
>+  @param x The x coord of the client, may be changed.
>+  @param y The y coord of the client, may be changed.
>+  @param w The width of the client.
>+  @param w The height of the client.
>+  @return true if the client was moved to be on-monitor; false if not.
>+*/
>+gboolean client_find_onmonitor(ObClient *self, gint *x, gint *y, gint w, gint h);
>+
>+
>+/*! Moves a client so that it is on one monitor if it is straddling the monitor
>+  divide in xinerama
>+  @param self The client to move
>+*/
>+void client_move_onmonitor(ObClient *self);
>+
> /*! Fullscreen's or unfullscreen's the client window
>   @param fs true if the window should be made fullscreen; false if it should
>             be returned to normal state.
>Index: openbox/geom.h
>===================================================================
>RCS file: /cvs/cvsroot/openbox/openbox/geom.h,v
>retrieving revision 1.15
>diff -u -r1.15 geom.h
>--- openbox/geom.h	24 Sep 2003 17:16:16 -0000	1.15
>+++ openbox/geom.h	16 Mar 2004 01:58:24 -0000
>@@ -40,6 +40,10 @@
>     int height;
> } Rect;
> 
>+/* defines sz as the differences of the height and width of r and o */
>+#define RECT_DIFF(sz, r, o) ((sz).width = (r).width - (o).width,    \
>+                             (sz).height = (r).height - (o).height) 
>+
> #define RECT_LEFT(r) ((r).x)
> #define RECT_TOP(r) ((r).y)
> #define RECT_RIGHT(r) ((r).x + (r).width - 1)
>@@ -62,10 +66,13 @@
> #define RECT_CONTAINS_RECT(r, o) \
>     ((o).x >= (r).x && (o).x + (o).width <= (r).x + (r).width && \
>      (o).y >= (r).y && (o).y + (o).height <= (r).y + (r).height)
>+
>+/* Returns true if Rect r and o intersect */
> #define RECT_INTERSECTS_RECT(r, o) \
>     ((o).x < (r).x + (r).width && (o).x + (o).width > (r).x && \
>      (o).y < (r).y + (r).height && (o).y + (o).height > (r).y)
> 
>+/* Sets Rect r to be the intersection of Rect a and b. */
> #define RECT_SET_INTERSECTION(r, a, b) \
>     ((r).x = MAX((a).x, (b).x), \
>      (r).y = MAX((a).y, (b).y), \

Created attachment 226 [details]
and this one is better than the second

ok, the ornary java client was actively _moving_ itself into the middle.  If I
try to fix it, then xmms can't get across the monitor divide.  Stupid clients. 
I took out the java client special case.  Columba (mail client) can suck it.

Created attachment 266 [details]
Better than all three and includes feature from #1131

This patch contains the final patch for handling most windows that try to
appear on the monitor divide, as well as the feature I did for #1131.  That
makes the monitor divide an "edge" when using functions like
MoveToEdge<direction>.

This has made openbox a lot more usable for me, thanks!

Little suggestion for this patch: handle menus as well. 
Menus opened from one screen's edge shouldn't open on another screen.

Also, it might be a good idea to have a Send to screen action, or something like
that.

Wouldn't it be just as good to include the stuff from client_find_onmonitor in
client_find_onscreen? You also don't quite follow the pattern here,
client_find_on* is done in client_manage followed by a client_move, and the
client_move_on* is done in screen_resize where you haven't actually placed your
client_move_onmonitor function. Ie, instead of this:

<<<<<<<<<<<<<<

 
         if (x != ox || y != oy)
             client_move(self, x, y);
-    }
 
-    client_showhide(self);
+        /* As far as I can tell, clients that go through the other placement
+           functions don't get placed on the divide, so we only need to
+           do this if the client thinks it knows better*/
+        client_move_onmonitor(self);
+    }
+        client_showhide(self);
 
     /* use client_focus instead of client_activate cuz client_activate does
        stuff like switch desktops etc and I'm not interested in all that when

=====================
we have this (untested)
 
-        if (x != ox || y != oy)
-            client_move(self, x, y);
-    }
 
-    client_showhide(self);
+        if (x != ox || y != oy) {
+        /* As far as I can tell, clients that go through the other placement
+           functions don't get placed on the divide, so we only need to
+           do this if the client thinks it knows better*/
+            client_find_onmonitor(self, &x, &y,
+                                  self->frame->area.width,
+                                  self->frame->area.height);
+            client_move(self, x, y);
+        }
+    }
+        client_showhide(self);
 
     /* use client_focus instead of client_activate cuz client_activate does
        stuff like switch desktops etc and I'm not interested in all that when

--------------------

What do you think?
I also don't agree with the 10% from the side thing, the other place functions
do no such thing.

As long as it works the same way or better, moving the function calls around is
fine with me.  

As for the 10% thing, I know that it is fairly arbitrary, but IMO it goes a long
way to making xinerama easier to look at.  If something gets placed exactly on
the monitor edge I always think, "well that was trying to be put in the middle
but was moved over".  However if I added that little buffer I never even notice
when this code does anything.  It just makes it look more natural.

I think this is fixed in cvs (maybe in rc2 too?), reopen it if it isn't.