Give a chance to effects for running when picking

Some effects can change the actor's shape and position, so they need
to run when picking.

https://bugzilla.gnome.org/show_bug.cgi?id=651700

clutter/clutter-actor.c

@@ -2865,79 +2865,67 @@ clutter_actor_paint (ClutterActor *self)
          applications to notify when the value of the
          has_overlaps virtual changes. */
       add_or_remove_flatten_effect (self);
-
-      /* We save the current paint volume 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 volume to perform culling so
-       * we can avoid painting actors outside the current clip region.
-       *
-       * If we are painting inside a clone, we should neither update
-       * 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.
-       */
-      if (!in_clone_paint ())
-        {
-          gboolean success;
-          /* annoyingly gcc warns if uninitialized even though
-           * the initialization is redundant :-( */
-          ClutterCullResult result = CLUTTER_CULL_RESULT_IN;
-
-          if (G_LIKELY ((clutter_paint_debug_flags &
-                         (CLUTTER_DEBUG_DISABLE_CULLING |
-                          CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)) !=
-                        (CLUTTER_DEBUG_DISABLE_CULLING |
-                         CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)))
-            _clutter_actor_update_last_paint_volume (self);
-
-          success = cull_actor (self, &result);
-
-          if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_REDRAWS))
-            _clutter_actor_paint_cull_result (self, success, result);
-          else if (result == CLUTTER_CULL_RESULT_OUT && success)
-            goto done;
-        }
-
-      if (priv->effects == NULL)
-        {
-          if (actor_has_shader_data (self))
-            clutter_actor_shader_pre_paint (self, FALSE);
-          priv->next_effect_to_paint = NULL;
-        }
-      else
-        priv->next_effect_to_paint =
-          _clutter_meta_group_peek_metas (priv->effects);
-
-      clutter_actor_continue_paint (self);
-
-      if (priv->effects == NULL &&
-          actor_has_shader_data (self))
-        clutter_actor_shader_post_paint (self);
-
-      if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_PAINT_VOLUMES))
-        _clutter_actor_draw_paint_volume (self);
     }
   else
+    CLUTTER_COUNTER_INC (_clutter_uprof_context, actor_pick_counter);
+
+  /* We save the current paint volume 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 volume to perform culling so
+   * we can avoid painting actors outside the current clip region.
+   *
+   * If we are painting inside a clone, we should neither update
+   * 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.
+   */
+  if (!in_clone_paint ())
     {
-      ClutterColor col = { 0, };
+      gboolean success;
+      /* annoyingly gcc warns if uninitialized even though
+       * the initialization is redundant :-( */
+      ClutterCullResult result = CLUTTER_CULL_RESULT_IN;
 
-      CLUTTER_COUNTER_INC (_clutter_uprof_context, actor_pick_counter);
+      if (G_LIKELY ((clutter_paint_debug_flags &
+                     (CLUTTER_DEBUG_DISABLE_CULLING |
+                      CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)) !=
+                    (CLUTTER_DEBUG_DISABLE_CULLING |
+                     CLUTTER_DEBUG_DISABLE_CLIPPED_REDRAWS)))
+        _clutter_actor_update_last_paint_volume (self);
 
-      _clutter_id_to_color (_clutter_actor_get_pick_id (self), &col);
+      success = cull_actor (self, &result);
 
-      /* Actor will then paint silhouette of itself in supplied
+      if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_REDRAWS))
-       * color.  See clutter_stage_get_actor_at_pos() for where
+        _clutter_actor_paint_cull_result (self, success, result);
-       * picking is enabled.
+      else if (result == CLUTTER_CULL_RESULT_OUT && success)
-       */
+        goto done;
-      g_signal_emit (self, actor_signals[PICK], 0, &col);
     }
 
+  if (priv->effects == NULL)
+    {
+      if (actor_has_shader_data (self))
+        clutter_actor_shader_pre_paint (self, FALSE);
+      priv->next_effect_to_paint = NULL;
+    }
+  else
+    priv->next_effect_to_paint =
+      _clutter_meta_group_peek_metas (priv->effects);
+
+  clutter_actor_continue_paint (self);
+
+  if (priv->effects == NULL &&
+      actor_has_shader_data (self))
+    clutter_actor_shader_post_paint (self);
+
+  if (G_UNLIKELY (clutter_paint_debug_flags & CLUTTER_DEBUG_PAINT_VOLUMES))
+    _clutter_actor_draw_paint_volume (self);
+
 done:
   if (clip_set)
     cogl_clip_pop();
@@ -2981,9 +2969,24 @@ clutter_actor_continue_paint (ClutterActor *self)
      actual actor */
   if (priv->next_effect_to_paint == NULL)
     {
-      priv->propagated_one_redraw = FALSE;
+      if (_clutter_context_get_pick_mode () == CLUTTER_PICK_NONE)
+        {
+          priv->propagated_one_redraw = FALSE;
 
-      g_signal_emit (self, actor_signals[PAINT], 0);
+          g_signal_emit (self, actor_signals[PAINT], 0);
+        }
+      else
+        {
+          ClutterColor col = { 0, };
+
+          _clutter_id_to_color (_clutter_actor_get_pick_id (self), &col);
+
+          /* Actor will then paint silhouette of itself in supplied
+           * color.  See clutter_stage_get_actor_at_pos() for where
+           * picking is enabled.
+           */
+          g_signal_emit (self, actor_signals[PICK], 0, &col);
+        }
     }
   else
     {
@@ -2997,19 +3000,31 @@ clutter_actor_continue_paint (ClutterActor *self)
       priv->current_effect = priv->next_effect_to_paint->data;
       priv->next_effect_to_paint = priv->next_effect_to_paint->next;
 
-      if (priv->propagated_one_redraw)
+      if (_clutter_context_get_pick_mode () == CLUTTER_PICK_NONE)
         {
-          /* If there's an effect queued with this redraw then all
+          if (priv->propagated_one_redraw)
-             effects up to that one will be considered dirty. It is
+            {
-             expected the queued effect will paint the cached image
+              /* If there's an effect queued with this redraw then all
-             and not call clutter_actor_continue_paint again (although
+                 effects up to that one will be considered dirty. It
-             it should work ok if it does) */
+                 is expected the queued effect will paint the cached
-          if (priv->effect_to_redraw == NULL ||
+                 image and not call clutter_actor_continue_paint again
-              priv->current_effect != priv->effect_to_redraw)
+                 (although it should work ok if it does) */
-            run_flags |= CLUTTER_EFFECT_RUN_ACTOR_DIRTY;
+              if (priv->effect_to_redraw == NULL ||
-        }
+                  priv->current_effect != priv->effect_to_redraw)
+                run_flags |= CLUTTER_EFFECT_RUN_ACTOR_DIRTY;
+            }
 
-      _clutter_effect_paint (priv->current_effect, run_flags);
+          _clutter_effect_paint (priv->current_effect, run_flags);
+        }
+      else
+        {
+          /* We can't determine when an actor has been modified since
+             its last pick so lets just assume it has always been
+             modified */
+          run_flags |= CLUTTER_EFFECT_RUN_ACTOR_DIRTY;
+
+          _clutter_effect_pick (priv->current_effect, run_flags);
+        }
 
       priv->current_effect = old_current_effect;
     }

clutter/clutter-effect-private.h

@@ -11,6 +11,8 @@ gboolean        _clutter_effect_get_paint_volume        (ClutterEffect      *eff
                                                          ClutterPaintVolume *volume);
 void            _clutter_effect_paint                   (ClutterEffect      *effect,
                                                          ClutterEffectRunFlags flags);
+void            _clutter_effect_pick                    (ClutterEffect      *effect,
+                                                         ClutterEffectRunFlags flags);
 
 G_END_DECLS
 

clutter/clutter-effect.c

@@ -248,6 +248,17 @@ clutter_effect_real_paint (ClutterEffect         *effect,
     _clutter_effect_post_paint (effect);
 }
 
+static void
+clutter_effect_real_pick (ClutterEffect         *effect,
+                          ClutterEffectRunFlags  flags)
+{
+  ClutterActorMeta *actor_meta = CLUTTER_ACTOR_META (effect);
+  ClutterActor *actor;
+
+  actor = clutter_actor_meta_get_actor (actor_meta);
+  clutter_actor_continue_paint (actor);
+}
+
 static void
 clutter_effect_notify (GObject    *gobject,
                        GParamSpec *pspec)
@@ -276,6 +287,7 @@ clutter_effect_class_init (ClutterEffectClass *klass)
   klass->post_paint = clutter_effect_real_post_paint;
   klass->get_paint_volume = clutter_effect_real_get_paint_volume;
   klass->paint = clutter_effect_real_paint;
+  klass->pick = clutter_effect_real_pick;
 }
 
 static void
@@ -308,6 +320,15 @@ _clutter_effect_paint (ClutterEffect         *effect,
   CLUTTER_EFFECT_GET_CLASS (effect)->paint (effect, flags);
 }
 
+void
+_clutter_effect_pick (ClutterEffect         *effect,
+                      ClutterEffectRunFlags  flags)
+{
+  g_return_if_fail (CLUTTER_IS_EFFECT (effect));
+
+  CLUTTER_EFFECT_GET_CLASS (effect)->pick (effect, flags);
+}
+
 gboolean
 _clutter_effect_get_paint_volume (ClutterEffect      *effect,
                                   ClutterPaintVolume *volume)

clutter/clutter-effect.h

@@ -49,7 +49,7 @@ typedef struct _ClutterEffectClass      ClutterEffectClass;
  * should call clutter_actor_continue_paint() to chain to the next
  * effect and can not cache any results from a previous paint.
  *
- * Flags passed to the ‘run’ method of #ClutterEffect.
+ * Flags passed to the ‘paint’ or ‘pick’ method of #ClutterEffect.
  */
 typedef enum
 {
@@ -96,8 +96,10 @@ struct _ClutterEffectClass
   void     (* paint)            (ClutterEffect        *effect,
                                  ClutterEffectRunFlags flags);
 
+  void     (* pick)             (ClutterEffect        *effect,
+                                 ClutterEffectRunFlags flags);
+
   /*< private >*/
-  void (* _clutter_effect3) (void);
   void (* _clutter_effect4) (void);
   void (* _clutter_effect5) (void);
   void (* _clutter_effect6) (void);

tests/conform/test-pick.c

@@ -6,6 +6,7 @@
 #define STAGE_HEIGHT 480
 #define ACTORS_X 12
 #define ACTORS_Y 16
+#define SHIFT_STEP STAGE_WIDTH / ACTORS_X
 
 typedef struct _State State;
 
@@ -18,6 +19,75 @@ struct _State
   gboolean pass;
 };
 
+struct _ShiftEffect
+{
+  ClutterShaderEffect parent_instance;
+};
+
+struct _ShiftEffectClass
+{
+  ClutterShaderEffectClass parent_class;
+};
+
+typedef struct _ShiftEffect       ShiftEffect;
+typedef struct _ShiftEffectClass  ShiftEffectClass;
+
+#define TYPE_SHIFT_EFFECT        (shift_effect_get_type ())
+
+G_DEFINE_TYPE (ShiftEffect,
+               shift_effect,
+               CLUTTER_TYPE_SHADER_EFFECT);
+
+static void
+shader_paint (ClutterEffect         *effect,
+              ClutterEffectRunFlags  flags)
+{
+  ClutterShaderEffect *shader = CLUTTER_SHADER_EFFECT (effect);
+  float tex_width;
+  ClutterActor *actor =
+    clutter_actor_meta_get_actor (CLUTTER_ACTOR_META (effect));
+
+  g_debug ("shader_paint");
+
+  clutter_shader_effect_set_shader_source (shader,
+    "uniform sampler2D tex;\n"
+    "uniform float step;\n"
+    "void main (void)\n"
+    "{\n"
+    "  gl_FragColor = texture2D(tex, vec2 (gl_TexCoord[0].s + step,\n"
+    "                                      gl_TexCoord[0].t));\n"
+    "}\n");
+
+  tex_width = clutter_actor_get_width (actor);
+
+  clutter_shader_effect_set_uniform (shader, "tex", G_TYPE_INT, 1, 0);
+  clutter_shader_effect_set_uniform (shader, "step", G_TYPE_FLOAT, 1,
+                                     SHIFT_STEP / tex_width);
+
+  CLUTTER_EFFECT_CLASS (shift_effect_parent_class)->paint (effect, flags);
+}
+
+static void
+shader_pick (ClutterEffect         *effect,
+             ClutterEffectRunFlags  flags)
+{
+  shader_paint (effect, flags);
+}
+
+static void
+shift_effect_class_init (ShiftEffectClass *klass)
+{
+  ClutterEffectClass *shader_class = CLUTTER_EFFECT_CLASS (klass);
+
+  shader_class->paint = shader_paint;
+  shader_class->pick = shader_pick;
+}
+
+static void
+shift_effect_init (ShiftEffect *self)
+{
+}
+
 static gboolean
 on_timeout (State *state)
 {
@@ -34,7 +104,7 @@ on_timeout (State *state)
   clutter_stage_get_actor_at_pos (CLUTTER_STAGE (state->stage),
                                   CLUTTER_PICK_REACTIVE, 10, 10);
 
-  for (test_num = 0; test_num < 3; test_num++)
+  for (test_num = 0; test_num < 5; test_num++)
     {
       if (test_num == 0)
         {
@@ -69,53 +139,91 @@ on_timeout (State *state)
           if (g_test_verbose ())
             g_print ("Clipped covering actor:\n");
         }
+      else if (test_num == 3)
+        {
+          clutter_actor_hide (over_actor);
+
+          clutter_actor_add_effect_with_name (CLUTTER_ACTOR (state->stage),
+                                              "blur",
+                                              clutter_blur_effect_new ());
+
+          if (g_test_verbose ())
+            g_print ("With blur effect:\n");
+        }
+      else if (test_num == 4)
+        {
+          clutter_actor_hide (over_actor);
+          clutter_actor_remove_effect_by_name (CLUTTER_ACTOR (state->stage),
+                                               "blur");
+
+          clutter_actor_add_effect_with_name (CLUTTER_ACTOR (state->stage),
+            "shift",
+            g_object_new (TYPE_SHIFT_EFFECT, NULL));
+
+          if (g_test_verbose ())
+            g_print ("With shift effect:\n");
+        }
 
       for (y = 0; y < ACTORS_Y; y++)
-        for (x = 0; x < ACTORS_X; x++)
+        {
-          {
+          if (test_num == 4)
-            gboolean pass = FALSE;
+            x = 1;
-            ClutterActor *actor
+          else
-              = clutter_stage_get_actor_at_pos (CLUTTER_STAGE (state->stage),
+            x = 0;
-                                                CLUTTER_PICK_ALL,
-                                                x * state->actor_width
-                                                + state->actor_width / 2,
-                                                y * state->actor_height
-                                                + state->actor_height / 2);
 
-            if (g_test_verbose ())
+          for (; x < ACTORS_X; x++)
-              g_print ("% 3i,% 3i / %p -> ",
+            {
-                       x, y, state->actors[y * ACTORS_X + x]);
+              gboolean pass = FALSE;
+              gfloat pick_x;
+              ClutterActor *actor;
 
-            if (actor == NULL)
+              pick_x = x * state->actor_width + state->actor_width / 2;
-              {
-                if (g_test_verbose ())
-                  g_print ("NULL:       FAIL\n");
-              }
-            else if (actor == over_actor)
-              {
-                if (test_num == 2
-                    && x >= 2 && x < ACTORS_X - 2
-                    && y >= 2 && y < ACTORS_Y - 2)
-                  pass = TRUE;
 
-                if (g_test_verbose ())
+              if (test_num == 4)
-                  g_print ("over_actor: %s\n", pass ? "pass" : "FAIL");
+                pick_x -= SHIFT_STEP;
-              }
-            else
-              {
-                if (actor == state->actors[y * ACTORS_X + x]
-                    && (test_num != 2
-                        || x < 2 || x >= ACTORS_X - 2
-                        || y < 2 || y >= ACTORS_Y - 2))
-                  pass = TRUE;
 
-                if (g_test_verbose ())
+              actor
-                  g_print ("%p: %s\n", actor, pass ? "pass" : "FAIL");
+                = clutter_stage_get_actor_at_pos (CLUTTER_STAGE (state->stage),
-              }
+                                                  CLUTTER_PICK_ALL,
+                                                  pick_x,
+                                                  y * state->actor_height
+                                                  + state->actor_height / 2);
 
-            if (!pass)
+              if (g_test_verbose ())
-              state->pass = FALSE;
+                g_print ("% 3i,% 3i / %p -> ",
-          }
+                         x, y, state->actors[y * ACTORS_X + x]);
+
+              if (actor == NULL)
+                {
+                  if (g_test_verbose ())
+                    g_print ("NULL:       FAIL\n");
+                }
+              else if (actor == over_actor)
+                {
+                  if (test_num == 2
+                      && x >= 2 && x < ACTORS_X - 2
+                      && y >= 2 && y < ACTORS_Y - 2)
+                    pass = TRUE;
+
+                  if (g_test_verbose ())
+                    g_print ("over_actor: %s\n", pass ? "pass" : "FAIL");
+                }
+              else
+                {
+                  if (actor == state->actors[y * ACTORS_X + x]
+                      && (test_num != 2
+                          || x < 2 || x >= ACTORS_X - 2
+                          || y < 2 || y >= ACTORS_Y - 2))
+                    pass = TRUE;
+
+                  if (g_test_verbose ())
+                    g_print ("%p: %s\n", actor, pass ? "pass" : "FAIL");
+                }
+
+              if (!pass)
+                state->pass = FALSE;
+            }
+        }
     }
 
   clutter_main_quit ();