Fix errors in keeping track of the stage bounding rectangle

* Add new clutter_geometry_union(), because writing union intersection
  is harder than it looks. Fixes two problems with the inline code in
  clutter_stage_glx_add_redraw_clip().

  1) The ->x and ->y of were reassigned to before using them to
     compute the new width and height.
  2) since ClutterGeometry has unsigned width, x + width is unsigned,
     and comparison goes wrong if either rectangle has a negative
     x + width. (We fixed width for GdkRectangle to be signed for GTK+-2.0,
     this is a potent source of bugs.)

* Use in clutter_stage_glx_add_redraw_clip()

* Account for the case where the incoming rectangle is empty, and don't
  end up with the stage being entirely redrawn.

* Account for the case where the stage already has a degenerate
  width and don't end up with redrawing only the new rectangle and not
  the rest of the stage.

The better fix here for the second two problems is to stop using a 0
width to mean the entire stage, but this should work for now.

http://bugzilla.openedhand.com/show_bug.cgi?id=2040

Signed-off-by: Emmanuele Bassi <ebassi@linux.intel.com>

diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c
index 1bb8f79..cf478a9 100644 (file)
--- a/clutter/clutter-actor.c
+++ b/clutter/clutter-actor.c
@@ -8292,6 +8292,35 @@ clutter_geometry_get_type (void)
   return our_type;
 }
 
+/**
+ * clutter_geometry_union:
+ * @geometry_a: a #ClutterGeometry
+ * @geometry_b: another #ClutterGeometry
+ * @result: (out): location to store the result
+ *
+ * Find the union of two rectangles represented as #ClutterGeometry.
+ *
+ * Since: 1.4
+ */
+void
+clutter_geometry_union (const ClutterGeometry *geometry_a,
+                        const ClutterGeometry *geometry_b,
+                        ClutterGeometry       *result)
+{
+  /* We don't try to handle rectangles that can't be represented
+   * as a signed integer box */
+  gint x1 = MIN (geometry_a->x, geometry_b->x);
+  gint y1 = MIN (geometry_a->y, geometry_b->y);
+  gint x2 = MAX (geometry_a->x + (gint)geometry_a->width,
+                 geometry_b->x + (gint)geometry_b->width);
+  gint y2 = MAX (geometry_a->y + (gint)geometry_a->height,
+                 geometry_b->y + (gint)geometry_b->height);
+  result->x = x1;
+  result->y = y1;
+  result->width = x2 - x1;
+  result->height = y2 - y1;
+}
+
 /*
  * ClutterVertices
  */

diff --git a/clutter/clutter-types.h b/clutter/clutter-types.h
index 119ff1d..72090fa 100644 (file)
--- a/clutter/clutter-types.h
+++ b/clutter/clutter-types.h
@@ -182,6 +182,10 @@ struct _ClutterGeometry
 
 GType clutter_geometry_get_type (void) G_GNUC_CONST;
 
+void  clutter_geometry_union (const ClutterGeometry *geometry_a,
+                              const ClutterGeometry *geometry_b,
+                              ClutterGeometry       *result);
+
 /**
  * ClutterKnot:
  * @x: X coordinate of the knot

diff --git a/clutter/glx/clutter-stage-glx.c b/clutter/glx/clutter-stage-glx.c
index a6218f5..954d347 100644 (file)
--- a/clutter/glx/clutter-stage-glx.c
+++ b/clutter/glx/clutter-stage-glx.c
@@ -377,6 +377,10 @@ clutter_stage_glx_add_redraw_clip (ClutterStageWindow *stage_window,
   if (clutter_stage_glx_ignoring_redraw_clips (stage_window))
     return;
 
+  /* Do nothing on an empty clip, to avoid confusing with the flag degenerate clip */
+  if (stage_clip->width == 0)
+    return;
+
   /* A NULL stage clip means a full stage redraw has been queued and
    * we keep track of this by setting a degenerate
    * stage_glx->bounding_redraw_clip */
@@ -393,26 +397,10 @@ clutter_stage_glx_add_redraw_clip (ClutterStageWindow *stage_window,
       stage_glx->bounding_redraw_clip.width = stage_clip->width;
       stage_glx->bounding_redraw_clip.height = stage_clip->height;
     }
-  else
+  else if (stage_glx->bounding_redraw_clip.width > 0)
     {
-      int x2, y2;
-
-      stage_glx->bounding_redraw_clip.x =
-        MIN (stage_clip->x, stage_glx->bounding_redraw_clip.x);
-      stage_glx->bounding_redraw_clip.y =
-        MIN (stage_clip->y, stage_glx->bounding_redraw_clip.y);
-
-      x2 = MAX (stage_clip->x + stage_clip->width,
-                stage_glx->bounding_redraw_clip.x +
-                stage_glx->bounding_redraw_clip.width);
-      y2 = MAX (stage_clip->y + stage_clip->height,
-                stage_glx->bounding_redraw_clip.y +
-                stage_glx->bounding_redraw_clip.height);
-
-      stage_glx->bounding_redraw_clip.width =
-        x2 - stage_glx->bounding_redraw_clip.x;
-      stage_glx->bounding_redraw_clip.height =
-        y2 - stage_glx->bounding_redraw_clip.y;
+      clutter_geometry_union (&stage_glx->bounding_redraw_clip, stage_clip,
+                             &stage_glx->bounding_redraw_clip);
     }
 
   /* FIXME: This threshold was plucked out of thin air! */