One more testcase, a few more bug fixes (yaay!), and more notes

2005-10-11  Elijah Newren  <newren@gmail.com>

	One more testcase, a few more bug fixes (yaay!), and more notes

	* src/constraints-ideas.txt: lots more stuff on the short term
	todo list so I don't forget it

	* src/boxes.c:

	(meta_rectangle_overlap):
	(meta_rectangle_vert_overlap):
	(meta_rectangle_horiz_overlap):
	fix off by one errors

	(meta_rectangle_clip_to_region):
	fix the warning message to refer to this function (stupid
	cut-and-paste-and-forget-to-modify errors...), actually do the
	clipping correctly

	* src/constraints.c:

	(do_screen_and_xinerama_relative_constraints):
	compilation fix--meta_rectangle_could_be_contained_in_region() has
	been renamed, add a HUGE FIXME COMMENT

	* src/testboxes.c:

	increase the number of random runs by a couple orders of
	magnitude; 100 runs would only catch the off-by-one errors very
	rarely

	(get_random_rect):
	rectangle width should be 1..1600 not 0..1599.  Similarly for
	height.

	(test_region_fitting, test_clamping_to_region)
	fix a comment typo

	(rect_overlaps_region):
	new function that perhaps could be moved to boxes.[ch]

	(test_clipping_to_region):
	implement this

5 files changed, 174 insertions, 31 deletions

diff --git a/ChangeLog b/ChangeLog
index 8ca04def..b95d7730 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,49 @@
 2005-10-11  Elijah Newren  <newren@gmail.com>
 
+	One more testcase, a few more bug fixes (yaay!), and more notes
+
+	* src/constraints-ideas.txt: lots more stuff on the short term
+	todo list so I don't forget it
+
+	* src/boxes.c:
+
+	(meta_rectangle_overlap):
+	(meta_rectangle_vert_overlap):
+	(meta_rectangle_horiz_overlap):
+	fix off by one errors
+
+	(meta_rectangle_clip_to_region):
+	fix the warning message to refer to this function (stupid
+	cut-and-paste-and-forget-to-modify errors...), actually do the
+	clipping correctly
+
+	* src/constraints.c:
+
+	(do_screen_and_xinerama_relative_constraints):
+	compilation fix--meta_rectangle_could_be_contained_in_region() has
+	been renamed, add a HUGE FIXME COMMENT
+
+	* src/testboxes.c:
+
+	increase the number of random runs by a couple orders of
+	magnitude; 100 runs would only catch the off-by-one errors very
+	rarely
+
+	(get_random_rect):
+	rectangle width should be 1..1600 not 0..1599.  Similarly for
+	height.
+
+	(test_region_fitting, test_clamping_to_region)
+	fix a comment typo
+
+	(rect_overlaps_region):
+	new function that perhaps could be moved to boxes.[ch]
+
+	(test_clipping_to_region):
+	implement this
+
+2005-10-11  Elijah Newren  <newren@gmail.com>
+
 	More testcases and a couple more bugs squashed
 
 	* src/boxes.[ch]: 
diff --git a/constraints-ideas.txt b/constraints-ideas.txt
index 9f05edc0..6dc24057 100644
--- a/constraints-ideas.txt
+++ b/constraints-ideas.txt
@@ -1,8 +1,25 @@
 Short-term TODO list/reminders:
 
+  - Might be good to optimize by storing spanning rects in MetaWorkspace
+
+  - Need to nuke old #ifdef'd out code
+
+  - Need to replace xinerama and screen structures (screen.h?) and
+    maybe tabpopup stuff (see bug 98340, IIRC) with MetaRectangle's
+
+  - Need to make shove_into_rect use shortest distance instead of
+    maximal overlap (especially for totally offscreen case)
+
+  - Need to clean out lots of FIXMEs in the code.
+
+  - Need to do the titlebar offscreen checking, partial maximization,
+    etc.
+
+  - Need to add testboxes stuff to appropriate Makefile.am thingies
+
   - It looks like I can nuke work_area_screen in ConstraintInfo
 
-  - Okay the on-single-xinerama/fully-onscreen/partially-onscreen
+  X Okay the on-single-xinerama/fully-onscreen/partially-onscreen
     constraints aren't as simple as I thought, the boxes.[ch] is still the
     wrong way to go about it and doesn't cover everything needed, and I
     need something else big.  Ideas:
@@ -107,7 +124,7 @@ Short-term TODO list/reminders:
 
   - Do I need the include_frame parameter to get_size_limits() anymore?
 
-  - Need to add the require_fully_onscreen and require_on_single_xinerama
+  X Need to add the require_fully_onscreen and require_on_single_xinerama
     flags and get them all initialized and everything
 
 Extra window.h flags:
@@ -177,7 +194,7 @@ Some ideas:
     <All three below can be handled via the resize algorithm; extras are no-ops>
     - app resize, move, move&resize: same but with bigger region
 
-Constraints in constraints.c:
+Constraints in old constraints.c:
   - place the window (huh?), including maximization after placement
   - maximization constraints (both horiz & vert; window-size == workarea size)
   - fullscreen constraints (window-size == xinerama size)
diff --git a/src/boxes.c b/src/boxes.c
index 4bae069a..133b6d54 100644
--- a/src/boxes.c
+++ b/src/boxes.c
@@ -122,26 +122,26 @@ meta_rectangle_overlap (const MetaRectangle *rect1,
   g_return_val_if_fail (rect1 != NULL, FALSE);
   g_return_val_if_fail (rect2 != NULL, FALSE);
 
-  return !((rect1->x + rect1->width  < rect2->x) ||
-           (rect2->x + rect2->width  < rect1->x) ||
-           (rect1->y + rect1->height < rect2->y) ||
-           (rect2->y + rect2->height < rect1->y));
+  return !((rect1->x + rect1->width  <= rect2->x) ||
+           (rect2->x + rect2->width  <= rect1->x) ||
+           (rect1->y + rect1->height <= rect2->y) ||
+           (rect2->y + rect2->height <= rect1->y));
 }
 
 gboolean
 meta_rectangle_vert_overlap (const MetaRectangle *rect1,
                              const MetaRectangle *rect2)
 {
-  return (rect1->y <= rect2->y + rect2->height &&
-          rect2->y <= rect1->y + rect1->height);
+  return (rect1->y < rect2->y + rect2->height &&
+          rect2->y < rect1->y + rect1->height);
 }
 
 gboolean
 meta_rectangle_horiz_overlap (const MetaRectangle *rect1,
                               const MetaRectangle *rect2)
 {
-  return (rect1->x <= rect2->x + rect2->width &&
-          rect2->x <= rect1->x + rect1->width);
+  return (rect1->x < rect2->x + rect2->width &&
+          rect2->x < rect1->x + rect1->width);
 }
 
 gboolean
@@ -722,7 +722,7 @@ meta_rectangle_clip_to_region (const GList         *spanning_rects,
 
   /* Clip rect appropriately */
   if (best_rect == NULL)
-    meta_warning ("No rect to shove into found!\n");
+    meta_warning ("No rect to clip to found!\n");
   else
     {
       /* Extra precaution with checking fixed direction shouldn't be needed
@@ -730,12 +730,11 @@ meta_rectangle_clip_to_region (const GList         *spanning_rects,
        */
       if (!(fixed_directions & FIXED_DIRECTION_X))
         {
-          /* Clip the left, if needed */
-          rect->x = MAX (rect->x, best_rect->x);
-
-          /* Clip the right, if needed */
-          rect->width = MIN (rect->width, 
-                             (best_rect->x + best_rect->width) - rect->x);
+          /* Find the new left and right */
+          int new_x = MAX (rect->x, best_rect->x);
+          rect->width = MIN ((rect->x + rect->width)           - new_x,
+                             (best_rect->x + best_rect->width) - new_x);
+          rect->x = new_x;
         }
 
       /* Extra precaution with checking fixed direction shouldn't be needed
@@ -744,11 +743,10 @@ meta_rectangle_clip_to_region (const GList         *spanning_rects,
       if (!(fixed_directions & FIXED_DIRECTION_Y))
         {
           /* Clip the top, if needed */
-          rect->y = MAX (rect->y, best_rect->y);
-
-          /* Clip the bottom, if needed */
-          rect->height = MIN (rect->height, 
-                              (best_rect->y + best_rect->height) - rect->y);
+          int new_y = MAX (rect->y, best_rect->y);
+          rect->height = MIN ((rect->y + rect->height)           - new_y,
+                              (best_rect->y + best_rect->height) - new_y);
+          rect->y = new_y;
         }
     }
 }
diff --git a/src/constraints.c b/src/constraints.c
index 0291932b..6353d951 100644
--- a/src/constraints.c
+++ b/src/constraints.c
@@ -1117,8 +1117,8 @@ do_screen_and_xinerama_relative_constraints (
       if (!(info->fixed_directions & FIXED_DIRECTION_Y))
         how_far_it_can_be_smushed.height = min_size.height;
     }
-  if (!meta_rectangle_could_be_contained_in_region (region_spanning_rectangles,
-                                                    &how_far_it_can_be_smushed))
+  if (!meta_rectangle_could_fit_in_region (region_spanning_rectangles,
+                                           &how_far_it_can_be_smushed))
     return TRUE;
 
   /* Determine whether constraint is already satisfied; exit if it is */
@@ -1131,7 +1131,12 @@ do_screen_and_xinerama_relative_constraints (
   /* Enforce constraint */
 
   /* Clamp rectangle size for user move+resize, app move+resize, and
-   * app resize
+   * app resize; FIXME FIXME FIXME: Is this really right?!?  Why not
+   * clamp for user resize if clamping for user move+resize?!?  Just
+   * because it'll be clipped below anyway?  Also, why doesn't the
+   * comment match the code?!??
+   *
+   * QUIT WRITING SUCH STINKING BUGGY CODE AND COMMENTS!!!!!
    */
   if (info->action_type == ACTION_MOVE_AND_RESIZE ||
       (info->is_user_action && info->action_type == ACTION_RESIZE))
diff --git a/src/testboxes.c b/src/testboxes.c
index 4a4707eb..1658f45b 100644
--- a/src/testboxes.c
+++ b/src/testboxes.c
@@ -31,7 +31,7 @@
 #include <glib.h>
 #include "boxes.h"
 
-#define NUM_RANDOM_RUNS 100
+#define NUM_RANDOM_RUNS 10000
 
 static void
 init_random_ness ()
@@ -44,8 +44,8 @@ get_random_rect (MetaRectangle *rect)
 {
   rect->x = rand () % 1600;
   rect->y = rand () % 1200;
-  rect->width  = rand () % 1600;
-  rect->height = rand () % 1200;
+  rect->width  = rand () % 1600 + 1;
+  rect->height = rand () % 1200 + 1;
 }
 
 static void
@@ -601,7 +601,7 @@ test_region_fitting ()
     }
   meta_rectangle_free_spanning_set (region);
 
-  /* Do so manual tests too */
+  /* Do some manual tests too */
   region = get_screen_region (1);
 
   rect = meta_rect (50, 50, 400, 400);
@@ -655,7 +655,7 @@ test_clamping_to_region ()
     }
   meta_rectangle_free_spanning_set (region);
 
-  /* Do so manual tests too */
+  /* Do some manual tests too */
   region = get_screen_region (1);
 
   rect = meta_rect (50, 50, 10000, 10000);
@@ -718,9 +718,88 @@ test_clamping_to_region ()
   printf ("%s passed.\n", __PRETTY_FUNCTION__);
 }
 
+static gboolean
+rect_overlaps_region (const GList         *spanning_rects,
+                      const MetaRectangle *rect)
+{
+  /* FIXME: Should I move this to boxes.[ch]? */
+  const GList *temp;
+  gboolean     overlaps;
+
+  temp = spanning_rects;
+  overlaps = FALSE;
+  while (!overlaps && temp != NULL)
+    {
+      overlaps = overlaps || meta_rectangle_overlap (temp->data, rect);
+      temp = temp->next;
+    }
+
+  return overlaps;
+}
+
+gboolean time_to_print = FALSE;
+
 void
 test_clipping_to_region ()
 {
+  GList* region;
+  MetaRectangle rect, temp;
+  MetaRectangle min_size;
+  FixedDirections fixed_directions;
+
+  min_size.height = min_size.width = 1;
+  fixed_directions = 0;
+
+  int i;
+  region = get_screen_region (3);
+  for (i = 0; i < NUM_RANDOM_RUNS; i++)
+    {
+      get_random_rect (&rect);
+      if (rect_overlaps_region (region, &rect))
+        {
+          meta_rectangle_clip_to_region (region, 0, &rect);
+          g_assert (meta_rectangle_contained_in_region (region, &rect) == TRUE);
+        }
+    }
+  meta_rectangle_free_spanning_set (region);
+
+  /* Do some manual tests too */
+  region = get_screen_region (2);
+
+  rect = meta_rect (-50, -10, 10000, 10000);
+  meta_rectangle_clip_to_region (region,
+                                 fixed_directions,
+                                 &rect);
+  g_assert (meta_rectangle_equal (region->data, &rect));
+
+  rect = meta_rect (300, 1000, 400, 200);
+  temp = meta_rect (300, 1000, 400, 150);
+  meta_rectangle_clip_to_region (region,
+                                 fixed_directions,
+                                 &rect);
+  g_assert (meta_rectangle_equal (&rect, &temp));
+
+  rect = meta_rect (400, 1000, 300, 200);
+  temp = meta_rect (450, 1000, 250, 200);
+  meta_rectangle_clip_to_region (region,
+                                 fixed_directions,
+                                 &rect);
+  g_assert (meta_rectangle_equal (&rect, &temp));
+
+  rect = meta_rect (400, 1000, 300, 200);
+  temp = meta_rect (400, 1000, 300, 150);
+  meta_rectangle_clip_to_region (region,
+                                 FIXED_DIRECTION_X,
+                                 &rect);
+  g_assert (meta_rectangle_equal (&rect, &temp));
+
+  rect = meta_rect (400, 1000, 300, 200);
+  temp = meta_rect (400, 1000, 300, 150);
+  meta_rectangle_clip_to_region (region,
+                                 FIXED_DIRECTION_X,
+                                 &rect);
+  g_assert (meta_rectangle_equal (&rect, &temp));
+
   printf ("%s passed.\n", __PRETTY_FUNCTION__);
 }