Optimize culling by doing culling in eye-coordinates

This implements a variation of frustum culling whereby we convert screen
space clip rectangles into eye space mini-frustums so that we don't have
to repeatedly transform actor paint-volumes all the way into screen
coordinates to perform culling, we just have to apply the modelview
transform and then determine each points distance from the planes that
make up the clip frustum.

By avoiding the projective transform, perspective divide and viewport
scale for each point culled this makes culling much cheaper.

clutter/clutter-actor-private.h

@@ -147,9 +147,9 @@ void _clutter_actor_set_has_pointer (ClutterActor *self,
 void _clutter_actor_queue_redraw_with_clip   (ClutterActor              *self,
                                               ClutterRedrawFlags         flags,
                                               ClutterPaintVolume        *clip_volume);
-const ClutterPaintVolume *_clutter_actor_get_queue_redraw_clip (ClutterActor *self);
+ClutterPaintVolume *_clutter_actor_get_queue_redraw_clip (ClutterActor *self);
 void _clutter_actor_set_queue_redraw_clip     (ClutterActor             *self,
-                                               const ClutterPaintVolume *clip_volume);
+                                               ClutterPaintVolume *clip_volume);
 void _clutter_actor_finish_queue_redraw       (ClutterActor             *self,
                                                ClutterPaintVolume       *clip);
 

clutter/clutter-actor.c

@@ -411,7 +411,7 @@ struct _ClutterActorPrivate
   guint has_pointer                 : 1;
   guint propagated_one_redraw       : 1;
   guint paint_volume_valid          : 1;
-  guint last_paint_box_valid        : 1;
+  guint last_paint_volume_valid     : 1;
   guint in_clone_paint              : 1;
 
   gfloat clip[4];
@@ -460,7 +460,7 @@ struct _ClutterActorPrivate
    * of the QUEUE_REDRAW signal. It's an out-of-band argument.
    * See clutter_actor_queue_clipped_redraw() for details.
    */
-  const ClutterPaintVolume *oob_queue_redraw_clip;
+  ClutterPaintVolume *oob_queue_redraw_clip;
 
   ClutterMetaGroup *actions;
   ClutterMetaGroup *constraints;
@@ -471,7 +471,10 @@ struct _ClutterActorPrivate
 
   ClutterPaintVolume paint_volume;
 
-  ClutterActorBox last_paint_box;
+  /* NB: This volume isn't relative to this actor, it is in eye
+   * coordinates so that it can remain valid after the actor changes.
+   */
+  ClutterPaintVolume last_paint_volume;
 
   ClutterStageQueueRedrawEntry *queue_redraw_entry;
 };
@@ -1092,11 +1095,11 @@ clutter_actor_real_unmap (ClutterActor *self)
 
   CLUTTER_ACTOR_UNSET_FLAGS (self, CLUTTER_ACTOR_MAPPED);
 
-  /* unset the contents of the last paint box, so that hiding + moving +
+  /* clear the contents of the last paint volume, so that hiding + moving +
    * showing will not result in the wrong area being repainted
    */
-  memset (&self->priv->last_paint_box, 0, sizeof (ClutterActorBox));
+  _clutter_paint_volume_init_static (&self->priv->last_paint_volume, NULL);
-  self->priv->last_paint_box_valid = TRUE;
+  self->priv->last_paint_volume_valid = TRUE;
 
   /* notify on parent mapped after potentially unmapping
    * children, so apps see a bottom-up notification.
@@ -1974,7 +1977,7 @@ _clutter_actor_fully_transform_vertices (ClutterActor *self,
 
   /* NB: _clutter_actor_apply_modelview_transform_recursive will never
    * include the transformation between stage coordinates and OpenGL
-   * window coordinates, we have to explicitly use the
+   * eye coordinates, we have to explicitly use the
    * stage->apply_transform to get that... */
   stage = _clutter_actor_get_stage_internal (self);
 
@@ -2402,7 +2405,7 @@ _clutter_actor_draw_paint_volume (ClutterActor *self)
     {
       gfloat width, height;
       ClutterActor *stage = _clutter_actor_get_stage_internal (self);
-      _clutter_paint_volume_init_static (stage, &fake_pv);
+      _clutter_paint_volume_init_static (&fake_pv, stage);
       free_fake_pv = TRUE;
 
       clutter_actor_get_size (self, &width, &height);
@@ -2492,16 +2495,19 @@ in_clone_paint (void)
 }
 
 /* Returns TRUE if the actor can be ignored */
+/* FIXME: we should return a ClutterCullResult, and
+ * clutter_actor_paint should understand that a CLUTTER_CULL_RESULT_IN
+ * means there's no point in trying to cull descendants of the current
+ * node. */
 static gboolean
 cull_actor (ClutterActor *self)
 {
   ClutterActorPrivate *priv = self->priv;
   ClutterActor *stage;
-  const ClutterGeometry *stage_clip;
+  const ClutterPlane *stage_clip;
-  ClutterActorBox *box;
+  ClutterCullResult result;
-  ClutterGeometry paint_geom;
 
-  if (G_UNLIKELY (priv->last_paint_box_valid == FALSE))
+  if (!priv->last_paint_volume_valid)
     return FALSE;
 
   if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_DISABLE_CULLING))
@@ -2512,18 +2518,36 @@ cull_actor (ClutterActor *self)
   if (G_UNLIKELY (!stage_clip))
     return FALSE;
 
-  /* XXX: It might be better if _get_paint_box returned a
+  result = _clutter_paint_volume_cull (&priv->last_paint_volume, stage_clip);
-   * ClutterGeometry instead. */
+  if (result == CLUTTER_CULL_RESULT_IN ||
-  box = &priv->last_paint_box;
+      result == CLUTTER_CULL_RESULT_PARTIAL)
-  paint_geom.x = box->x1;
-  paint_geom.y = box->y1;
-  paint_geom.width = box->x2 - box->x1;
-  paint_geom.height = box->y2 - box->y1;
-
-  if (!clutter_geometry_intersects (stage_clip, &paint_geom))
-    return TRUE;
-  else
     return FALSE;
+  else
+    return TRUE;
+}
+
+static void
+_clutter_actor_update_last_paint_volume (ClutterActor *self)
+{
+  ClutterActorPrivate *priv = self->priv;
+  const ClutterPaintVolume *pv;
+
+  if (priv->last_paint_volume_valid)
+    {
+      clutter_paint_volume_free (&priv->last_paint_volume);
+      priv->last_paint_volume_valid = FALSE;
+    }
+
+  pv = clutter_actor_get_paint_volume (self);
+  if (!pv)
+    return;
+
+  _clutter_paint_volume_copy_static (pv, &priv->last_paint_volume);
+
+  _clutter_paint_volume_transform_relative (&priv->last_paint_volume,
+                                            NULL); /* eye coordinates */
+
+  priv->last_paint_volume_valid = TRUE;
 }
 
 static inline gboolean
@@ -2632,58 +2656,36 @@ clutter_actor_paint (ClutterActor *self)
   if (pick_mode == CLUTTER_PICK_NONE)
     {
       gboolean effect_painted = FALSE;
-      gboolean need_paint_box;
 
       CLUTTER_COUNTER_INC (_clutter_uprof_context, actor_paint_counter);
 
-      if (G_UNLIKELY (clutter_paint_debug_flags &
+      /* We save the current paint volume so that the next time the
-                      CLUTTER_DEBUG_DISABLE_CULLING &&
-                      clutter_paint_debug_flags &
-                      CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS))
-        need_paint_box = FALSE;
-      else
-        need_paint_box = TRUE;
-
-      /* We save the current paint box so that the next time the
        * actor queues a redraw we can constrain the redraw to just
        * cover the union of the new bounding box and the old.
        *
-       * We also fetch the current paint box to perform culling so we
+       * We also fetch the current paint volume to perform culling so
-       * can avoid painting actors outside the current clip region.
+       * we can avoid painting actors outside the current clip region.
        *
        * If we are painting inside a clone, we should neither update
-       * the paint box or use it to cull painting, since the paint
+       * the paint volume or use it to cull painting, since the paint
        * box represents the location of the source actor on the
        * screen.
        *
        * XXX: We are starting to do a lot of vertex transforms on
        * the CPU in a typical paint, so at some point we should
        * audit these and consider caching some things.
-       *
-       * XXX: We should consider doing all our culling in the
-       * stage's model space using PaintVolumes so we don't have
-       * to project actor paint volumes all the way into window
-       * coordinates!
-       *   XXX: To do this we also need a way to store an
-       *   "absolute paint volume" in some way. Currently the
-       *   paint volumes are defined relative to a referenced
-       *   actor's coordinates, but we'd need to be able to cache
-       *   the last paint volume used with the actor's *current*
-       *   modelview and either with a specific projection matrix
-       *   or we'd need to be able to invalidate paint-volumes on
-       *   projection changes.
        */
       if (!in_clone_paint ())
-	{
+        {
-          if (G_LIKELY (need_paint_box) &&
+          if (G_LIKELY (!(clutter_paint_debug_flags &
-              clutter_actor_get_paint_box (self, &priv->last_paint_box))
+                          CLUTTER_DEBUG_DISABLE_CULLING) &&
-            priv->last_paint_box_valid = TRUE;
+                        !(clutter_paint_debug_flags &
-          else
+                          CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)))
-            priv->last_paint_box_valid = FALSE;
+            _clutter_actor_update_last_paint_volume (self);
 
-	  if (cull_actor (self))
+          if (cull_actor (self))
-	    goto done;
+            goto done;
-	}
+        }
 
       if (priv->effects != NULL)
         effect_painted = _clutter_actor_effects_pre_paint (self);
@@ -4903,9 +4905,9 @@ clutter_actor_init (ClutterActor *self)
   priv->opacity_override = -1;
   priv->enable_model_view_transform = TRUE;
 
-  /* Initialize an empty paint box to start with */
+  /* Initialize an empty paint volume to start with */
-  memset (&priv->last_paint_box, 0, sizeof (ClutterActorBox));
+  _clutter_paint_volume_init_static (&priv->last_paint_volume, NULL);
-  priv->last_paint_box_valid = TRUE;
+  priv->last_paint_volume_valid = TRUE;
 
   memset (priv->clip, 0, sizeof (gfloat) * 4);
 }
@@ -4954,18 +4956,19 @@ _clutter_actor_finish_queue_redraw (ClutterActor *self,
                                     ClutterPaintVolume *clip)
 {
   ClutterActorPrivate *priv = self->priv;
-  const ClutterPaintVolume *pv;
+  ClutterPaintVolume *pv;
   gboolean clipped;
 
   /* If we've been explicitly passed a clip volume then there's
-   * nothing more to calculate, but otherwhise the only thing we know
+   * nothing more to calculate, but otherwise the only thing we know
    * is that the change is constrained to the given actor.
    *
-   * The idea is that if we know the paint box for where the actor was
+   * The idea is that if we know the paint volume for where the actor
-   * last drawn and we also have the paint volume for where it will be
+   * was last drawn (in eye coordinates) and we also have the paint
-   * drawn next then if we queue a redraw for both these regions that
+   * volume for where it will be drawn next (in actor coordinates)
-   * will cover everything that needs to be redrawn to clear the old
+   * then if we queue a redraw for both these volumes that will cover
-   * view and show the latest view of the actor.
+   * everything that needs to be redrawn to clear the old view and
+   * show the latest view of the actor.
    *
    * Don't clip this redraw if we don't know what position we had for
    * the previous redraw since we don't know where to set the clip so
@@ -4976,32 +4979,19 @@ _clutter_actor_finish_queue_redraw (ClutterActor *self,
       _clutter_actor_set_queue_redraw_clip (self, clip);
       clipped = TRUE;
     }
-  else if (G_LIKELY (priv->last_paint_box_valid))
+  else if (G_LIKELY (priv->last_paint_volume_valid))
     {
-      pv = clutter_actor_get_paint_volume (self);
+      pv = _clutter_actor_get_paint_volume_mutable (self);
       if (pv)
         {
           ClutterActor *stage = _clutter_actor_get_stage_internal (self);
-          ClutterPaintVolume stage_pv;
-          ClutterActorBox *box = &priv->last_paint_box;
-          ClutterVertex origin;
-
-          _clutter_paint_volume_init_static (stage, &stage_pv);
-
-          origin.x = box->x1;
-          origin.y = box->y1;
-          origin.z = 0;
-          clutter_paint_volume_set_origin (&stage_pv, &origin);
-          clutter_paint_volume_set_width (&stage_pv, box->x2 - box->x1);
-          clutter_paint_volume_set_height (&stage_pv, box->y2 - box->y1);
 
           /* make sure we redraw the actors old position... */
-          _clutter_actor_set_queue_redraw_clip (stage, &stage_pv);
+          _clutter_actor_set_queue_redraw_clip (stage,
+                                                &priv->last_paint_volume);
           _clutter_actor_signal_queue_redraw (stage, stage);
           _clutter_actor_set_queue_redraw_clip (stage, NULL);
 
-          clutter_paint_volume_free (&stage_pv);
-
           /* XXX: Ideally the redraw signal would take a clip volume
            * argument, but that would be an ABI break. Until we can
            * break the ABI we pass the argument out-of-band via an
@@ -5226,7 +5216,7 @@ _clutter_actor_queue_redraw_with_clip (ClutterActor       *self,
           return;
         }
 
-      _clutter_paint_volume_init_static (self, &allocation_pv);
+      _clutter_paint_volume_init_static (&allocation_pv, self);
       pv = &allocation_pv;
 
       _clutter_actor_get_allocation_clip (self, &allocation_clip);
@@ -10812,7 +10802,7 @@ clutter_actor_has_pointer (ClutterActor *self)
  * the QUEUE_REDRAW signal. It is an out-of-band argument.  See
  * clutter_actor_queue_clipped_redraw() for details.
  */
-const ClutterPaintVolume *
+ClutterPaintVolume *
 _clutter_actor_get_queue_redraw_clip (ClutterActor *self)
 {
   return self->priv->oob_queue_redraw_clip;
@@ -10820,7 +10810,7 @@ _clutter_actor_get_queue_redraw_clip (ClutterActor *self)
 
 void
 _clutter_actor_set_queue_redraw_clip (ClutterActor *self,
-                                      const ClutterPaintVolume *clip)
+                                      ClutterPaintVolume *clip)
 {
   self->priv->oob_queue_redraw_clip = clip;
 }
@@ -11555,31 +11545,21 @@ clutter_actor_has_key_focus (ClutterActor *self)
   return clutter_stage_get_key_focus (CLUTTER_STAGE (stage)) == self;
 }
 
-/* The public clutter_actor_get_paint_volume API returns a const
+static gboolean
- * pointer since we return a pointer directly to the cached
+_clutter_actor_get_paint_volume_real (ClutterActor *self,
- * PaintVolume associated with the actor and don't want the user to
+                                      ClutterPaintVolume *pv)
- * inadvertently modify it, but for internal uses we sometimes need
- * access to the same PaintVolume but need to apply some book-keeping
- * modifications to it so we don't want a const pointer.
- */
-static ClutterPaintVolume *
-_clutter_actor_get_paint_volume_mutable (ClutterActor *self)
 {
-  ClutterActorPrivate *priv;
+  ClutterActorPrivate *priv = self->priv;
-  ClutterPaintVolume *pv;
-
-  priv = self->priv;
-
-  if (priv->paint_volume_valid)
-    {
-      clutter_paint_volume_free (&priv->paint_volume);
-      priv->paint_volume_valid = FALSE;
-    }
 
   /* Actors are only expected to report a valid paint volume
    * while they have a valid allocation. */
   if (G_UNLIKELY (priv->needs_allocation))
-    return NULL;
+    {
+      CLUTTER_NOTE (CLIPPING, "Bail from get_paint_volume (%s): "
+                    "Actor needs allocation",
+                    G_OBJECT_TYPE_NAME (self));
+      return FALSE;
+    }
 
   /* Check if there are any handlers connected to the paint
    * signal. If there are then all bets are off for what the paint
@@ -11612,15 +11592,22 @@ _clutter_actor_get_paint_volume_mutable (ClutterActor *self)
                                     actor_signals[PAINT],
                                     0,
                                     TRUE))
-    return NULL;
+    {
+      CLUTTER_NOTE (CLIPPING, "Bail from get_paint_volume (%s): "
+                    "Actor has \"paint\" signal handlers",
+                    G_OBJECT_TYPE_NAME (self));
+      return FALSE;
+    }
 
-  pv = &priv->paint_volume;
+  _clutter_paint_volume_init_static (pv, self);
-  _clutter_paint_volume_init_static (self, pv);
 
   if (!CLUTTER_ACTOR_GET_CLASS (self)->get_paint_volume (self, pv))
     {
       clutter_paint_volume_free (pv);
-      return NULL;
+      CLUTTER_NOTE (CLIPPING, "Bail from get_paint_volume (%s): "
+                    "Actor failed to report a volume",
+                    G_OBJECT_TYPE_NAME (self));
+      return FALSE;
     }
 
   /* since effects can modify the paint volume, we allow them to actually
@@ -11643,7 +11630,11 @@ _clutter_actor_get_paint_volume_mutable (ClutterActor *self)
               if (!_clutter_effect_get_paint_volume (l->data, pv))
                 {
                   clutter_paint_volume_free (pv);
-                  return NULL;
+                  CLUTTER_NOTE (CLIPPING, "Bail from get_paint_volume (%s): "
+                                "Effect (%s) failed to report a volume",
+                                G_OBJECT_TYPE_NAME (self),
+                                G_OBJECT_TYPE_NAME (l->data));
+                  return FALSE;
                 }
             }
         }
@@ -11657,13 +11648,45 @@ _clutter_actor_get_paint_volume_mutable (ClutterActor *self)
             if (!_clutter_effect_get_paint_volume (l->data, pv))
               {
                 clutter_paint_volume_free (pv);
-                return NULL;
+                CLUTTER_NOTE (CLIPPING, "Bail from get_paint_volume (%s): "
+                              "Effect (%s) failed to report a volume",
+                              G_OBJECT_TYPE_NAME (self),
+                              G_OBJECT_TYPE_NAME (l->data));
+                return FALSE;
               }
         }
     }
 
-  priv->paint_volume_valid = TRUE;
+  return TRUE;
-  return pv;
+}
+
+/* The public clutter_actor_get_paint_volume API returns a const
+ * pointer since we return a pointer directly to the cached
+ * PaintVolume associated with the actor and don't want the user to
+ * inadvertently modify it, but for internal uses we sometimes need
+ * access to the same PaintVolume but need to apply some book-keeping
+ * modifications to it so we don't want a const pointer.
+ */
+static ClutterPaintVolume *
+_clutter_actor_get_paint_volume_mutable (ClutterActor *self)
+{
+  ClutterActorPrivate *priv;
+
+  priv = self->priv;
+
+  if (priv->paint_volume_valid)
+    clutter_paint_volume_free (&priv->paint_volume);
+
+  if (_clutter_actor_get_paint_volume_real (self, &priv->paint_volume))
+    {
+      priv->paint_volume_valid = TRUE;
+      return &priv->paint_volume;
+    }
+  else
+    {
+      priv->paint_volume_valid = FALSE;
+      return NULL;
+    }
 }
 
 /**
@@ -11690,7 +11713,7 @@ _clutter_actor_get_paint_volume_mutable (ClutterActor *self)
  * Return value: (transfer none): a pointer to a #ClutterPaintVolume
  *   or %NULL if no volume could be determined.
  *
- * Since: 1.4
+ * Since: 1.6
  */
 const ClutterPaintVolume *
 clutter_actor_get_paint_volume (ClutterActor *self)
@@ -11721,36 +11744,34 @@ clutter_actor_get_paint_volume (ClutterActor *self)
  * Return value: (transfer none): a pointer to a #ClutterPaintVolume
  *   or %NULL if no volume could be determined.
  *
- * Since: 1.4
+ * Since: 1.6
  */
 const ClutterPaintVolume *
 clutter_actor_get_transformed_paint_volume (ClutterActor *self,
                                             ClutterActor *relative_to_ancestor)
 {
-  CoglMatrix matrix;
   const ClutterPaintVolume *volume;
-  ClutterStage *stage;
+  ClutterActor *stage;
   ClutterPaintVolume *transformed_volume;
 
-  if (relative_to_ancestor == NULL)
+  stage = _clutter_actor_get_stage_internal (self);
-    relative_to_ancestor = _clutter_actor_get_stage_internal (self);
+  if (G_UNLIKELY (stage == NULL))
+    return NULL;
 
   if (relative_to_ancestor == NULL)
-    return NULL;
+    relative_to_ancestor = stage;
 
   volume = clutter_actor_get_paint_volume (self);
   if (volume == NULL)
     return NULL;
 
-  _clutter_actor_get_relative_modelview (self, relative_to_ancestor, &matrix);
+  transformed_volume =
+    _clutter_stage_paint_volume_stack_allocate (CLUTTER_STAGE (stage));
 
-  stage = CLUTTER_STAGE (_clutter_actor_get_stage_internal (self));
-  transformed_volume = _clutter_stage_paint_volume_stack_allocate (stage);
   _clutter_paint_volume_copy_static (volume, transformed_volume);
-  _clutter_paint_volume_transform (transformed_volume, &matrix);
+
-  _clutter_paint_volume_axis_align (transformed_volume);
+  _clutter_paint_volume_transform_relative (transformed_volume,
-  _clutter_paint_volume_set_reference_actor (transformed_volume,
+                                            relative_to_ancestor);
-                                             relative_to_ancestor);
 
   return transformed_volume;
 }
@@ -11776,18 +11797,14 @@ clutter_actor_get_transformed_paint_volume (ClutterActor *self,
  * Return value: %TRUE if a 2D paint box could be determined, else
  * %FALSE.
  *
- * Since: 1.4
+ * Since: 1.6
  */
 gboolean
 clutter_actor_get_paint_box (ClutterActor    *self,
                              ClutterActorBox *box)
 {
   ClutterActor *stage;
-  const ClutterPaintVolume *pv;
+  ClutterPaintVolume *pv;
-  CoglMatrix modelview;
-  CoglMatrix projection;
-  float viewport[4];
-  ClutterPaintVolume projected_pv;
 
   g_return_val_if_fail (CLUTTER_IS_ACTOR (self), FALSE);
   g_return_val_if_fail (box != NULL, FALSE);
@@ -11796,36 +11813,11 @@ clutter_actor_get_paint_box (ClutterActor    *self,
   if (G_UNLIKELY (!stage))
     return FALSE;
 
-  pv = clutter_actor_get_paint_volume (self);
+  pv = _clutter_actor_get_paint_volume_mutable (self);
   if (G_UNLIKELY (!pv))
     return FALSE;
 
-  /* NB: _clutter_actor_apply_modelview_transform_recursive will never
+  _clutter_paint_volume_get_stage_paint_box (pv, CLUTTER_STAGE (stage), box);
-   * include the transformation between stage coordinates and OpenGL
-   * window coordinates, we have to explicitly use the
-   * stage->apply_transform to get that... */
-  cogl_matrix_init_identity (&modelview);
-  _clutter_actor_apply_modelview_transform (stage, &modelview);
-  _clutter_actor_apply_modelview_transform_recursive (pv->actor,
-                                                      stage, &modelview);
-
-  _clutter_stage_get_projection_matrix (CLUTTER_STAGE (stage), &projection);
-  _clutter_stage_get_viewport (CLUTTER_STAGE (stage),
-                               &viewport[0],
-                               &viewport[1],
-                               &viewport[2],
-                               &viewport[3]);
-
-  _clutter_paint_volume_copy_static (pv, &projected_pv);
-  _clutter_paint_volume_project (&projected_pv,
-                                 &modelview,
-                                 &projection,
-                                 viewport);
-
-  _clutter_paint_volume_get_bounding_box (&projected_pv, box);
-  clutter_actor_box_clamp_to_pixel (box);
-
-  clutter_paint_volume_free (&projected_pv);
 
   return TRUE;
 }
@@ -11990,4 +11982,3 @@ _clutter_actor_traverse (ClutterActor              *actor,
                                    0, /* start depth */
                                    user_data);
 }
-

clutter/clutter-debug.h

@@ -25,7 +25,8 @@ typedef enum {
   CLUTTER_DEBUG_ANIMATION           = 1 << 14,
   CLUTTER_DEBUG_LAYOUT              = 1 << 15,
   CLUTTER_DEBUG_PICK                = 1 << 16,
-  CLUTTER_DEBUG_EVENTLOOP           = 1 << 17
+  CLUTTER_DEBUG_EVENTLOOP           = 1 << 17,
+  CLUTTER_DEBUG_CLIPPING            = 1 << 18
 } ClutterDebugFlag;
 
 typedef enum {

clutter/clutter-main.c

@@ -169,7 +169,8 @@ static const GDebugKey clutter_debug_keys[] = {
   { "shader", CLUTTER_DEBUG_SHADER },
   { "multistage", CLUTTER_DEBUG_MULTISTAGE },
   { "animation", CLUTTER_DEBUG_ANIMATION },
-  { "layout", CLUTTER_DEBUG_LAYOUT }
+  { "layout", CLUTTER_DEBUG_LAYOUT },
+  { "clipping", CLUTTER_DEBUG_CLIPPING }
 };
 #endif /* CLUTTER_ENABLE_DEBUG */
 

clutter/clutter-paint-volume-private.h

@@ -23,11 +23,14 @@
 #define __CLUTTER_PAINT_VOLUME_PRIVATE_H__
 
 #include <clutter/clutter-types.h>
+#include <clutter/clutter-private.h>
 
 G_BEGIN_DECLS
 
 struct _ClutterPaintVolume
 {
+  /* A paint volume represents a volume in a given actors private
+   * coordinate system. */
   ClutterActor *actor;
 
   /* cuboid for the volume:
@@ -99,8 +102,8 @@ struct _ClutterPaintVolume
    */
 };
 
-void                _clutter_paint_volume_init_static          (ClutterActor *actor,
+void                _clutter_paint_volume_init_static          (ClutterPaintVolume *pv,
-                                                                ClutterPaintVolume *pv);
+                                                                ClutterActor *actor);
 ClutterPaintVolume *_clutter_paint_volume_new                  (ClutterActor       *actor);
 void                _clutter_paint_volume_copy_static          (const ClutterPaintVolume *src_pv,
                                                                 ClutterPaintVolume *dst_pv);
@@ -120,6 +123,16 @@ void                _clutter_paint_volume_axis_align           (ClutterPaintVolu
 void                _clutter_paint_volume_set_reference_actor  (ClutterPaintVolume *pv,
                                                                 ClutterActor *actor);
 
+ClutterCullResult   _clutter_paint_volume_cull                 (ClutterPaintVolume *pv,
+                                                                const ClutterPlane       *planes);
+
+void                _clutter_paint_volume_get_stage_paint_box  (ClutterPaintVolume *pv,
+                                                                ClutterStage *stage,
+                                                                ClutterActorBox *box);
+
+void                _clutter_paint_volume_transform_relative   (ClutterPaintVolume *pv,
+                                                                ClutterActor *relative_to_ancestor);
+
 G_END_DECLS
 
 #endif /* __CLUTTER_PAINT_VOLUME_PRIVATE_H__ */

clutter/clutter-paint-volume.c

@@ -35,6 +35,7 @@
 #include "clutter-actor-private.h"
 #include "clutter-paint-volume-private.h"
 #include "clutter-private.h"
+#include "clutter-stage-private.h"
 
 G_DEFINE_BOXED_TYPE (ClutterPaintVolume, clutter_paint_volume,
                      clutter_paint_volume_copy,
@@ -90,11 +91,9 @@ _clutter_paint_volume_new (ClutterActor *actor)
  * free it during _paint_volume_free().
  */
 void
-_clutter_paint_volume_init_static (ClutterActor *actor,
+_clutter_paint_volume_init_static (ClutterPaintVolume *pv,
-                                   ClutterPaintVolume *pv)
+                                   ClutterActor *actor)
 {
-  g_return_if_fail (actor != NULL);
-
   pv->actor = actor;
 
   memset (pv->vertices, 0, 8 * sizeof (ClutterVertex));
@@ -433,12 +432,12 @@ void
 clutter_paint_volume_union (ClutterPaintVolume *pv,
                             const ClutterPaintVolume *another_pv)
 {
+  ClutterPaintVolume aligned_pv;
   static const int key_vertices[4] = { 0, 1, 3, 4 };
 
   g_return_if_fail (pv != NULL);
   g_return_if_fail (pv->is_axis_aligned);
   g_return_if_fail (another_pv != NULL);
-  g_return_if_fail (another_pv->is_axis_aligned);
 
   /* NB: we only have to update vertices 0, 1, 3 and 4
    * (See the ClutterPaintVolume typedef for more details) */
@@ -459,6 +458,13 @@ clutter_paint_volume_union (ClutterPaintVolume *pv,
       goto done;
     }
 
+  if (!another_pv->is_axis_aligned)
+    {
+      _clutter_paint_volume_copy_static (another_pv, &aligned_pv);
+      _clutter_paint_volume_axis_align (&aligned_pv);
+      another_pv = &aligned_pv;
+    }
+
   /* grow left*/
   /* left vertices 0, 3, 4, 7 */
   if (another_pv->vertices[0].x < pv->vertices[0].x)
@@ -905,3 +911,154 @@ _clutter_paint_volume_set_reference_actor (ClutterPaintVolume *pv,
 
   pv->actor = actor;
 }
+
+ClutterCullResult
+_clutter_paint_volume_cull (ClutterPaintVolume *pv,
+                            const ClutterPlane *planes)
+{
+  int vertex_count;
+  ClutterVertex *vertices = pv->vertices;
+  gboolean in = TRUE;
+  gboolean out = TRUE;
+  int i;
+  int j;
+
+  /* We expect the volume to already be transformed into eye coordinates
+   */
+  g_return_val_if_fail (pv->is_complete == TRUE, CLUTTER_CULL_RESULT_IN);
+  g_return_val_if_fail (pv->actor == NULL, CLUTTER_CULL_RESULT_IN);
+
+  if (pv->is_empty)
+    return CLUTTER_CULL_RESULT_OUT;
+
+  /* Most actors are 2D so we only have to transform the front 4
+   * vertices of the paint volume... */
+  if (G_LIKELY (pv->is_2d))
+    vertex_count = 4;
+  else
+    vertex_count = 8;
+
+  for (i = 0; i < vertex_count; i++)
+    {
+      gboolean point_in = TRUE;
+      for (j = 0; j < 4; j++)
+        {
+          ClutterVertex p;
+          float distance;
+
+          /* XXX: for perspective projections this can be optimized
+           * out because all the planes should pass through the origin
+           * so (0,0,0) is a valid v0. */
+          p.x = vertices[i].x - planes[j].v0.x;
+          p.y = vertices[i].y - planes[j].v0.y;
+          p.z = vertices[i].z - planes[j].v0.z;
+
+          distance =
+            planes[j].n.x * p.x + planes[j].n.y * p.y + planes[j].n.z * p.z;
+
+          if (distance < 0)
+            {
+              point_in = FALSE;
+              break;
+            }
+        }
+
+      if (!point_in)
+        in = FALSE;
+      else
+        out = FALSE;
+    }
+
+  if (in)
+    return CLUTTER_CULL_RESULT_IN;
+  else if (out)
+    return CLUTTER_CULL_RESULT_OUT;
+  else
+    return CLUTTER_CULL_RESULT_PARTIAL;
+}
+
+void
+_clutter_paint_volume_get_stage_paint_box (ClutterPaintVolume *pv,
+                                           ClutterStage *stage,
+                                           ClutterActorBox *box)
+{
+  ClutterPaintVolume projected_pv;
+  CoglMatrix modelview;
+  CoglMatrix projection;
+  float viewport[4];
+
+  _clutter_paint_volume_copy_static (pv, &projected_pv);
+
+  /* NB: _clutter_actor_apply_modelview_transform_recursive will never
+   * include the transformation between stage coordinates and OpenGL
+   * eye coordinates, we have to explicitly use the
+   * stage->apply_transform to get that... */
+  cogl_matrix_init_identity (&modelview);
+
+  /* If the paint volume isn't already in eye coordinates... */
+  if (pv->actor)
+    {
+      ClutterActor *stage_actor = CLUTTER_ACTOR (stage);
+      _clutter_actor_apply_modelview_transform (stage_actor, &modelview);
+      _clutter_actor_apply_modelview_transform_recursive (pv->actor,
+                                                          stage_actor,
+                                                          &modelview);
+    }
+
+  _clutter_stage_get_projection_matrix (stage, &projection);
+  _clutter_stage_get_viewport (stage,
+                               &viewport[0],
+                               &viewport[1],
+                               &viewport[2],
+                               &viewport[3]);
+
+  _clutter_paint_volume_project (&projected_pv,
+                                 &modelview,
+                                 &projection,
+                                 viewport);
+
+  _clutter_paint_volume_get_bounding_box (&projected_pv, box);
+  clutter_actor_box_clamp_to_pixel (box);
+
+  clutter_paint_volume_free (&projected_pv);
+}
+
+void
+_clutter_paint_volume_transform_relative (ClutterPaintVolume *pv,
+                                          ClutterActor *relative_to_ancestor)
+{
+  CoglMatrix matrix;
+  ClutterActor *actor;
+
+  actor = pv->actor;
+
+  g_return_if_fail (actor != NULL);
+
+  _clutter_paint_volume_set_reference_actor (pv, relative_to_ancestor);
+
+  cogl_matrix_init_identity (&matrix);
+
+  if (relative_to_ancestor == NULL)
+    {
+      /* NB: _clutter_actor_apply_modelview_transform_recursive will never
+       * include the transformation between stage coordinates and OpenGL
+       * eye coordinates, we have to explicitly use the
+       * stage->apply_transform to get that... */
+      ClutterActor *stage = _clutter_actor_get_stage_internal (actor);
+
+      /* We really can't do anything meaningful in this case so don't try
+       * to do any transform */
+      if (G_UNLIKELY (stage == NULL))
+        return;
+
+      _clutter_actor_apply_modelview_transform (stage, &matrix);
+
+      relative_to_ancestor = stage;
+    }
+
+  _clutter_actor_apply_modelview_transform_recursive (actor,
+                                                      relative_to_ancestor,
+                                                      &matrix);
+
+  _clutter_paint_volume_transform (pv, &matrix);
+}

clutter/clutter-private.h

@@ -232,6 +232,14 @@ typedef struct _ClutterPlane
   CoglVector3 n;
 } ClutterPlane;
 
+typedef enum _ClutterCullResult
+{
+  CLUTTER_CULL_RESULT_UNKNOWN,
+  CLUTTER_CULL_RESULT_IN,
+  CLUTTER_CULL_RESULT_OUT,
+  CLUTTER_CULL_RESULT_PARTIAL
+} ClutterCullResult;
+
 G_END_DECLS
 
 #endif /* __CLUTTER_PRIVATE_H__ */

clutter/clutter-stage-private.h

@@ -25,6 +25,7 @@
 #include <clutter/clutter-stage-window.h>
 #include <clutter/clutter-stage.h>
 #include <clutter/clutter-input-device.h>
+#include <clutter/clutter-private.h>
 
 G_BEGIN_DECLS
 
@@ -75,7 +76,7 @@ guint    _clutter_stage_get_picks_per_frame_counter       (ClutterStage *stage);
 ClutterPaintVolume *_clutter_stage_paint_volume_stack_allocate (ClutterStage *stage);
 void                _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage);
 
-const ClutterGeometry *_clutter_stage_get_clip (ClutterStage *stage);
+const ClutterPlane *_clutter_stage_get_clip (ClutterStage *stage);
 
 ClutterStageQueueRedrawEntry *_clutter_stage_queue_actor_redraw            (ClutterStage                 *stage,
                                                                             ClutterStageQueueRedrawEntry *entry,

clutter/clutter-stage.c

@@ -133,7 +133,7 @@ struct _ClutterStagePrivate
 
   GArray             *paint_volume_stack;
 
-  const ClutterGeometry *current_paint_clip;
+  ClutterPlane        current_clip_planes[4];
 
   GList              *pending_queue_redraws;
 
@@ -380,6 +380,110 @@ clutter_stage_allocate (ClutterActor           *self,
     }
 }
 
+typedef struct _Vector4
+{
+  float x, y, z, w;
+} Vector4;
+
+static void
+_cogl_util_get_eye_planes_for_screen_poly (float *polygon,
+                                           int n_vertices,
+                                           float *viewport,
+                                           const CoglMatrix *projection,
+                                           const CoglMatrix *inverse_project,
+                                           ClutterPlane *planes)
+{
+  float Wc;
+  Vector4 *tmp_poly;
+  ClutterPlane *plane;
+  int i;
+  CoglVector3 b;
+  CoglVector3 c;
+  int count;
+
+  tmp_poly = g_alloca (sizeof (Vector4) * n_vertices * 2);
+
+#define DEPTH -50
+
+  /* Determine W in clip-space (Wc) for a point (0, 0, DEPTH, 1)
+   *
+   * Note: the depth could be anything except 0.
+   *
+   * We will transform the polygon into clip coordinates using this
+   * depth and then into eye coordinates. Our clip planes will be
+   * defined by triangles that extend between points of the polygon at
+   * DEPTH and corresponding points of the same polygon at DEPTH * 2.
+   *
+   * NB: Wc defines the position of the clip planes in clip
+   * coordinates. Given a screen aligned cross section through the
+   * frustum; coordinates range from [-Wc,Wc] left to right on the
+   * x-axis and [Wc,-Wc] top to bottom on the y-axis.
+   */
+  Wc = DEPTH * projection->wz + projection->ww;
+
+#define CLIP_X(X) ((((float)X - viewport[0]) * (2.0 / viewport[2])) - 1) * Wc
+#define CLIP_Y(Y) ((((float)Y - viewport[1]) * (2.0 / viewport[3])) - 1) * -Wc
+
+  for (i = 0; i < n_vertices; i++)
+    {
+      tmp_poly[i].x = CLIP_X (polygon[i * 2]);
+      tmp_poly[i].y = CLIP_Y (polygon[i * 2 + 1]);
+      tmp_poly[i].z = DEPTH;
+      tmp_poly[i].w = Wc;
+    }
+
+  Wc = DEPTH * 2 * projection->wz + projection->ww;
+
+  /* FIXME: technically we don't need to project all of the points
+   * twice, it would be enough project every other point since
+   * we can share points in this set to define the plane vectors. */
+  for (i = 0; i < n_vertices; i++)
+    {
+      tmp_poly[n_vertices + i].x = CLIP_X (polygon[i * 2]);
+      tmp_poly[n_vertices + i].y = CLIP_Y (polygon[i * 2 + 1]);
+      tmp_poly[n_vertices + i].z = DEPTH * 2;
+      tmp_poly[n_vertices + i].w = Wc;
+    }
+
+#undef CLIP_X
+#undef CLIP_Y
+
+  cogl_matrix_project_points (inverse_project,
+                              4,
+                              sizeof (Vector4),
+                              tmp_poly,
+                              sizeof (Vector4),
+                              tmp_poly,
+                              n_vertices * 2);
+
+  /* XXX: It's quite ugly that we end up with these casts between
+   * Vector4 types and CoglVector3s, it might be better if the
+   * cogl_vector APIs just took pointers to floats.
+   */
+
+  count = n_vertices - 1;
+  for (i = 0; i < count; i++)
+    {
+      plane = &planes[i];
+      plane->v0 = *(CoglVector3 *)&tmp_poly[i];
+      b = *(CoglVector3 *)&tmp_poly[n_vertices + i];
+      c = *(CoglVector3 *)&tmp_poly[n_vertices + i + 1];
+      cogl_vector3_subtract (&b, &b, &plane->v0);
+      cogl_vector3_subtract (&c, &c, &plane->v0);
+      cogl_vector3_cross_product (&plane->n, &b, &c);
+      cogl_vector3_normalize (&plane->n);
+    }
+
+  plane = &planes[n_vertices - 1];
+  plane->v0 = *(CoglVector3 *)&tmp_poly[0];
+  b = *(CoglVector3 *)&tmp_poly[2 * n_vertices - 1];
+  c = *(CoglVector3 *)&tmp_poly[n_vertices];
+  cogl_vector3_subtract (&b, &b, &plane->v0);
+  cogl_vector3_subtract (&c, &c, &plane->v0);
+  cogl_vector3_cross_product (&plane->n, &b, &c);
+  cogl_vector3_normalize (&plane->n);
+}
+
 /* This provides a common point of entry for painting the scenegraph
  * for picking or painting...
  *
@@ -392,10 +496,44 @@ void
 _clutter_stage_do_paint (ClutterStage *stage, const ClutterGeometry *clip)
 {
   ClutterStagePrivate *priv = stage->priv;
-  priv->current_paint_clip = clip;
+  float clip_poly[8];
+
+  if (clip)
+    {
+      clip_poly[0] = clip->x;
+      clip_poly[1] = clip->y;
+      clip_poly[2] = clip->x + clip->width;
+      clip_poly[3] = clip->y;
+      clip_poly[4] = clip->x + clip->width;
+      clip_poly[5] = clip->y + clip->height;
+      clip_poly[6] = clip->x;
+      clip_poly[7] = clip->y + clip->height;
+    }
+  else
+    {
+      ClutterGeometry geom;
+
+      _clutter_stage_window_get_geometry (priv->impl, &geom);
+
+      clip_poly[0] = 0;
+      clip_poly[1] = 0;
+      clip_poly[2] = geom.width;
+      clip_poly[3] = 0;
+      clip_poly[4] = geom.width;
+      clip_poly[5] = geom.height;
+      clip_poly[6] = 0;
+      clip_poly[7] = geom.height;
+    }
+
+  _cogl_util_get_eye_planes_for_screen_poly (clip_poly,
+                                             4,
+                                             priv->viewport,
+                                             &priv->projection,
+                                             &priv->inverse_projection,
+                                             priv->current_clip_planes);
+
   _clutter_stage_paint_volume_stack_free_all (stage);
   clutter_actor_paint (CLUTTER_ACTOR (stage));
-  priv->current_paint_clip = NULL;
 }
 
 static void
@@ -924,12 +1062,9 @@ clutter_stage_real_queue_redraw (ClutterActor *actor,
                                  ClutterActor *leaf)
 {
   ClutterStage *stage = CLUTTER_STAGE (actor);
-  ClutterStagePrivate *priv = stage->priv;
   ClutterStageWindow *stage_window;
   ClutterGeometry stage_clip;
-  const ClutterPaintVolume *redraw_clip;
+  ClutterPaintVolume *redraw_clip;
-  ClutterPaintVolume projected_clip;
-  CoglMatrix modelview;
   ClutterActorBox bounding_box;
 
   if (CLUTTER_ACTOR_IN_DESTRUCTION (actor))
@@ -948,9 +1083,8 @@ clutter_stage_real_queue_redraw (ClutterActor *actor,
       return;
     }
 
-  /* Convert the clip volume (which is in leaf actor coordinates) into stage
+  /* Convert the clip volume into stage coordinates and then into an
-   * coordinates and then into an axis aligned stage coordinates bounding
+   * axis aligned stage coordinates bounding box...
-   * box...
    */
 
   if (!_clutter_actor_get_queue_redraw_clip (leaf))
@@ -961,25 +1095,9 @@ clutter_stage_real_queue_redraw (ClutterActor *actor,
 
   redraw_clip = _clutter_actor_get_queue_redraw_clip (leaf);
 
-  _clutter_paint_volume_copy_static (redraw_clip, &projected_clip);
+  _clutter_paint_volume_get_stage_paint_box (redraw_clip,
-
+                                             stage,
-  /* NB: _clutter_actor_apply_modelview_transform_recursive will never
+                                             &bounding_box);
-   * include the transformation between stage coordinates and OpenGL
-   * window coordinates, we have to explicitly use the
-   * stage->apply_transform to get that... */
-  cogl_matrix_init_identity (&modelview);
-  _clutter_actor_apply_modelview_transform (CLUTTER_ACTOR (stage), &modelview);
-  _clutter_actor_apply_modelview_transform_recursive (leaf, NULL, &modelview);
-
-  _clutter_paint_volume_project (&projected_clip,
-                                 &modelview,
-                                 &priv->projection,
-                                 priv->viewport);
-
-  _clutter_paint_volume_get_bounding_box (&projected_clip, &bounding_box);
-  clutter_paint_volume_free (&projected_clip);
-
-  clutter_actor_box_clamp_to_pixel (&bounding_box);
 
   /* when converting to integer coordinates make sure we round the edges of the
    * clip rectangle outwards... */
@@ -3195,10 +3313,10 @@ _clutter_stage_paint_volume_stack_free_all (ClutterStage *stage)
 
 /* The is an out-of-band paramater available while painting that
  * can be used to cull actors. */
-const ClutterGeometry *
+const ClutterPlane *
 _clutter_stage_get_clip (ClutterStage *stage)
 {
-  return stage->priv->current_paint_clip;
+  return stage->priv->current_clip_planes;
 }
 
 /* When an actor queues a redraw we add it to a list on the stage that
@@ -3220,6 +3338,9 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
 {
   ClutterStagePrivate *priv = stage->priv;
 
+  CLUTTER_NOTE (CLIPPING, "stage_queue_actor_redraw (actor=%s, clip=%p): ",
+                G_OBJECT_TYPE_NAME (actor), clip);
+
   if (!priv->redraw_pending)
     {
       ClutterMasterClock *master_clock;
@@ -3256,7 +3377,12 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
       /* Ignore all requests to queue a redraw for an actor if a full
        * (non-clipped) redraw of the actor has already been queued. */
       if (!entry->has_clip)
-        return entry;
+        {
+          CLUTTER_NOTE (CLIPPING, "Bail from stage_queue_actor_redraw (%s): "
+                        "Unclipped redraw of actor already queued",
+                        G_OBJECT_CLASS_NAME (actor));
+          return entry;
+        }
 
       /* If queuing a clipped redraw and a clipped redraw has
        * previously been queued for this actor then combine the latest
@@ -3278,7 +3404,7 @@ _clutter_stage_queue_actor_redraw (ClutterStage *stage,
       if (clip)
         {
           entry->has_clip = TRUE;
-          _clutter_paint_volume_init_static (actor, &entry->clip);
+          _clutter_paint_volume_init_static (&entry->clip, actor);
           _clutter_paint_volume_set_from_volume (&entry->clip, clip);
         }
       else

clutter/glx/clutter-stage-glx.c

@@ -425,6 +425,12 @@ clutter_stage_glx_redraw (ClutterStageWindow *stage_window)
 
   if (use_clipped_redraw)
     {
+      CLUTTER_NOTE (CLIPPING,
+                    "Stage clip pushed: x=%d, y=%d, width=%d, height=%d\n",
+                    stage_glx->bounding_redraw_clip.x,
+                    stage_glx->bounding_redraw_clip.y,
+                    stage_glx->bounding_redraw_clip.width,
+                    stage_glx->bounding_redraw_clip.height);
       cogl_clip_push_window_rectangle (stage_glx->bounding_redraw_clip.x,
                                        stage_glx->bounding_redraw_clip.y,
                                        stage_glx->bounding_redraw_clip.width,
@@ -434,7 +440,10 @@ clutter_stage_glx_redraw (ClutterStageWindow *stage_window)
       cogl_clip_pop ();
     }
   else
-    _clutter_stage_do_paint (stage_x11->wrapper, NULL);
+    {
+      CLUTTER_NOTE (CLIPPING, "Unclipped stage paint\n");
+      _clutter_stage_do_paint (stage_x11->wrapper, NULL);
+    }
 
   if (may_use_clipped_redraw &&
       G_UNLIKELY ((clutter_paint_debug_flags & CLUTTER_DEBUG_REDRAWS)))

clutter/x11/clutter-stage-x11.c

@@ -1046,7 +1046,7 @@ clutter_stage_x11_translate_event (ClutterEventTranslator *translator,
         origin.y = expose->y;
         origin.z = 0;
 
-        _clutter_paint_volume_init_static (CLUTTER_ACTOR (stage), &clip);
+        _clutter_paint_volume_init_static (&clip, CLUTTER_ACTOR (stage));
 
         clutter_paint_volume_set_origin (&clip, &origin);
         clutter_paint_volume_set_width (&clip, expose->width);

clutter/x11/clutter-x11-texture-pixmap.c

@@ -372,7 +372,7 @@ clutter_x11_texture_pixmap_real_queue_damage_redraw (
   scale_x = (allocation.x2 - allocation.x1) / priv->pixmap_width;
   scale_y = (allocation.y2 - allocation.y1) / priv->pixmap_height;
 
-  _clutter_paint_volume_init_static (self, &clip);
+  _clutter_paint_volume_init_static (&clip, self);
 
   origin.x = x * scale_x;
   origin.y = y * scale_y;