[gimp] app: add a "visible" property and API to GimpCanvasItem

[Michael Natterer <mitch src gnome org>]

commit 10111126aed16c27ddf0ab24d363327054f70991
Author: Michael Natterer <mitch gimp org>
Date:   Fri Oct 1 20:41:27 2010 +0200

    app: add a "visible" property and API to GimpCanvasItem
    
    and use it to set visibility of guides and sample points, so this
    reression is fixed.
    
    - return NULL extents for invisible items and never draw them
    - do nothing for invisible items in more places, like the group
    
    While hacking this, it turned out that it was a braino to collect an
    item's region before and after a change in
    dispatch_properties_changed(), so a new update had to be devised:
    
    - add a "change count" and new API begin_change()/end_change()
    - in begin_change(), remember the item's extents before the change
    - in end_change(), combine old and current extents and emit "update"
    - add some protected function to emit "update", and to figure if
      it makes sense at all to emit "update" on an item.

 app/display/gimpcanvasgroup.c             |   31 +++--
 app/display/gimpcanvasitem.c              |  194 ++++++++++++++++++++++-------
 app/display/gimpcanvasitem.h              |   10 ++
 app/display/gimpdisplayshell-appearance.c |   17 +--
 app/display/gimpdisplayshell-handlers.c   |    4 +
 5 files changed, 186 insertions(+), 70 deletions(-)
---
diff --git a/app/display/gimpcanvasgroup.c b/app/display/gimpcanvasgroup.c
index ad123e6..75da839 100644
--- a/app/display/gimpcanvasgroup.c
+++ b/app/display/gimpcanvasgroup.c
@@ -230,7 +230,8 @@ gimp_canvas_group_child_update (GimpCanvasItem  *item,
                                 GdkRegion       *region,
                                 GimpCanvasGroup *group)
 {
-  g_signal_emit_by_name (group, "update", region);
+  if (_gimp_canvas_item_needs_update (GIMP_CANVAS_ITEM (group)))
+    _gimp_canvas_item_update (GIMP_CANVAS_ITEM (group), region);
 }
 
 
@@ -251,7 +252,6 @@ gimp_canvas_group_add_item (GimpCanvasGroup *group,
                             GimpCanvasItem  *item)
 {
   GimpCanvasGroupPrivate *private;
-  GdkRegion              *region;
 
   g_return_if_fail (GIMP_IS_CANVAS_GROUP (group));
   g_return_if_fail (GIMP_IS_CANVAS_ITEM (item));
@@ -267,12 +267,15 @@ gimp_canvas_group_add_item (GimpCanvasGroup *group,
 
   private->items = g_list_append (private->items, g_object_ref (item));
 
-  region = gimp_canvas_item_get_extents (item);
-
-  if (region)
+  if (_gimp_canvas_item_needs_update (GIMP_CANVAS_ITEM (group)))
     {
-      g_signal_emit_by_name (group, "update", region);
-      gdk_region_destroy (region);
+      GdkRegion *region = gimp_canvas_item_get_extents (item);
+
+      if (region)
+        {
+          _gimp_canvas_item_update (GIMP_CANVAS_ITEM (group), region);
+          gdk_region_destroy (region);
+        }
     }
 
   g_signal_connect (item, "update",
@@ -285,7 +288,6 @@ gimp_canvas_group_remove_item (GimpCanvasGroup *group,
                                GimpCanvasItem  *item)
 {
   GimpCanvasGroupPrivate *private;
-  GdkRegion              *region;
 
   g_return_if_fail (GIMP_IS_CANVAS_GROUP (group));
   g_return_if_fail (GIMP_IS_CANVAS_ITEM (item));
@@ -296,12 +298,15 @@ gimp_canvas_group_remove_item (GimpCanvasGroup *group,
 
   private->items = g_list_remove (private->items, item);
 
-  region = gimp_canvas_item_get_extents (item);
-
-  if (region)
+  if (_gimp_canvas_item_needs_update (GIMP_CANVAS_ITEM (group)))
     {
-      g_signal_emit_by_name (group, "update", region);
-      gdk_region_destroy (region);
+      GdkRegion *region = gimp_canvas_item_get_extents (item);
+
+      if (region)
+        {
+          _gimp_canvas_item_update (GIMP_CANVAS_ITEM (group), region);
+          gdk_region_destroy (region);
+        }
     }
 
   g_signal_handlers_disconnect_by_func (item,
diff --git a/app/display/gimpcanvasitem.c b/app/display/gimpcanvasitem.c
index 6304927..ff467e6 100644
--- a/app/display/gimpcanvasitem.c
+++ b/app/display/gimpcanvasitem.c
@@ -38,6 +38,7 @@ enum
 {
   PROP_0,
   PROP_SHELL,
+  PROP_VISIBLE,
   PROP_LINE_CAP,
   PROP_HIGHLIGHT
 };
@@ -54,10 +55,13 @@ typedef struct _GimpCanvasItemPrivate GimpCanvasItemPrivate;
 struct _GimpCanvasItemPrivate
 {
   GimpDisplayShell *shell;
+  gboolean          visible;
   cairo_line_cap_t  line_cap;
   gboolean          highlight;
   gint              suspend_stroking;
   gint              suspend_filling;
+  gint              change_count;
+  GdkRegion        *change_region;
 };
 
 #define GET_PRIVATE(item) \
@@ -136,6 +140,12 @@ gimp_canvas_item_class_init (GimpCanvasItemClass *klass)
                                                         GIMP_PARAM_READWRITE |
                                                         G_PARAM_CONSTRUCT_ONLY));
 
+  g_object_class_install_property (object_class, PROP_VISIBLE,
+                                   g_param_spec_boolean ("visible",
+                                                         NULL, NULL,
+                                                         TRUE,
+                                                         GIMP_PARAM_READWRITE));
+
   g_object_class_install_property (object_class, PROP_LINE_CAP,
                                    g_param_spec_int ("line-cap",
                                                      NULL, NULL,
@@ -159,10 +169,13 @@ gimp_canvas_item_init (GimpCanvasItem *item)
   GimpCanvasItemPrivate *private = GET_PRIVATE (item);
 
   private->shell            = NULL;
+  private->visible          = TRUE;
   private->line_cap         = CAIRO_LINE_CAP_ROUND;
   private->highlight        = FALSE;
   private->suspend_stroking = 0;
   private->suspend_filling  = 0;
+  private->change_count     = 1; /* avoid emissions during construction */
+  private->change_region    = NULL;
 }
 
 static void
@@ -172,6 +185,8 @@ gimp_canvas_item_constructed (GObject *object)
 
   g_assert (GIMP_IS_DISPLAY_SHELL (private->shell));
 
+  private->change_count = 0; /* undo hack from init() */
+
   if (G_OBJECT_CLASS (parent_class)->constructed)
     G_OBJECT_CLASS (parent_class)->constructed (object);
 }
@@ -189,6 +204,9 @@ gimp_canvas_item_set_property (GObject      *object,
     case PROP_SHELL:
       private->shell = g_value_get_object (value); /* don't ref */
       break;
+    case PROP_VISIBLE:
+      private->visible = g_value_get_boolean (value);
+      break;
     case PROP_LINE_CAP:
       private->line_cap = g_value_get_int (value);
       break;
@@ -215,6 +233,9 @@ gimp_canvas_item_get_property (GObject    *object,
     case PROP_SHELL:
       g_value_set_object (value, private->shell);
       break;
+    case PROP_VISIBLE:
+      g_value_set_boolean (value, private->visible);
+      break;
     case PROP_LINE_CAP:
       g_value_set_int (value, private->line_cap);
       break;
@@ -235,28 +256,13 @@ gimp_canvas_item_dispatch_properties_changed (GObject     *object,
 {
   GimpCanvasItem *item = GIMP_CANVAS_ITEM (object);
 
-  if (g_signal_has_handler_pending (object, item_signals[UPDATE], 0, FALSE))
-    {
-      GdkRegion *before;
-      GdkRegion *region;
+  G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object,
+                                                              n_pspecs,
+                                                              pspecs);
 
-      before = gimp_canvas_item_get_extents (item);
-
-      G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object,
-                                                                  n_pspecs,
-                                                                  pspecs);
-
-      region = gimp_canvas_item_get_extents (item);
-
-      if (! region)
-        {
-          region = before;
-        }
-      else if (before)
-        {
-          gdk_region_union (region, before);
-          gdk_region_destroy (before);
-        }
+  if (_gimp_canvas_item_needs_update (item))
+    {
+      GdkRegion *region = gimp_canvas_item_get_extents (item);
 
       if (region)
         {
@@ -265,12 +271,6 @@ gimp_canvas_item_dispatch_properties_changed (GObject     *object,
           gdk_region_destroy (region);
         }
     }
-  else
-    {
-      G_OBJECT_CLASS (parent_class)->dispatch_properties_changed (object,
-                                                                  n_pspecs,
-                                                                  pspecs);
-    }
 }
 
 static void
@@ -333,11 +333,12 @@ gimp_canvas_item_draw (GimpCanvasItem *item,
 
   private = GET_PRIVATE (item);
 
-  cairo_save (cr);
-
-  GIMP_CANVAS_ITEM_GET_CLASS (item)->draw (item, private->shell, cr);
-
-  cairo_restore (cr);
+  if (private->visible)
+    {
+      cairo_save (cr);
+      GIMP_CANVAS_ITEM_GET_CLASS (item)->draw (item, private->shell, cr);
+      cairo_restore (cr);
+    }
 }
 
 GdkRegion *
@@ -349,7 +350,42 @@ gimp_canvas_item_get_extents (GimpCanvasItem *item)
 
   private = GET_PRIVATE (item);
 
-  return GIMP_CANVAS_ITEM_GET_CLASS (item)->get_extents (item, private->shell);
+  if (private->visible)
+    return GIMP_CANVAS_ITEM_GET_CLASS (item)->get_extents (item, private->shell);
+
+  return NULL;
+}
+
+void
+gimp_canvas_item_set_visible (GimpCanvasItem *item,
+                              gboolean        visible)
+{
+  GimpCanvasItemPrivate *private;
+
+  g_return_if_fail (GIMP_IS_CANVAS_ITEM (item));
+
+  private = GET_PRIVATE (item);
+
+  if (private->visible != visible)
+    {
+      gimp_canvas_item_begin_change (item);
+      g_object_set (G_OBJECT (item),
+                    "visible", visible,
+                    NULL);
+      gimp_canvas_item_end_change (item);
+    }
+}
+
+gboolean
+gimp_canvas_item_get_visible (GimpCanvasItem *item)
+{
+  GimpCanvasItemPrivate *private;
+
+  g_return_val_if_fail (GIMP_IS_CANVAS_ITEM (item), FALSE);
+
+  private = GET_PRIVATE (item);
+
+  return private->visible;
 }
 
 void
@@ -362,14 +398,13 @@ gimp_canvas_item_set_line_cap (GimpCanvasItem   *item,
 
   private = GET_PRIVATE (item);
 
-  line_cap = CLAMP (line_cap,
-                    CAIRO_LINE_CAP_BUTT,
-                    CAIRO_LINE_CAP_SQUARE);
-
   if (private->line_cap != line_cap)
     {
-      private->line_cap = line_cap;
-      g_object_notify (G_OBJECT (item), "line-cap");
+      gimp_canvas_item_begin_change (item);
+      g_object_set (G_OBJECT (item),
+                    "line-cap", line_cap,
+                    NULL);
+      gimp_canvas_item_end_change (item);
     }
 }
 
@@ -383,12 +418,11 @@ gimp_canvas_item_set_highlight (GimpCanvasItem *item,
 
   private = GET_PRIVATE (item);
 
-  highlight = highlight ? TRUE : FALSE;
-
   if (private->highlight != highlight)
     {
-      private->highlight = highlight;
-      g_object_notify (G_OBJECT (item), "highlight");
+      g_object_set (G_OBJECT (item),
+                    "highlight", highlight,
+                    NULL);
     }
 }
 
@@ -405,6 +439,63 @@ gimp_canvas_item_get_highlight (GimpCanvasItem *item)
 }
 
 void
+gimp_canvas_item_begin_change (GimpCanvasItem *item)
+{
+  GimpCanvasItemPrivate *private;
+
+  g_return_if_fail (GIMP_IS_CANVAS_ITEM (item));
+
+  private = GET_PRIVATE (item);
+
+  private->change_count++;
+
+  if (private->change_count == 1 &&
+      g_signal_has_handler_pending (item, item_signals[UPDATE], 0, FALSE))
+    {
+      private->change_region = gimp_canvas_item_get_extents (item);
+    }
+}
+
+void
+gimp_canvas_item_end_change (GimpCanvasItem *item)
+{
+  GimpCanvasItemPrivate *private;
+
+  g_return_if_fail (GIMP_IS_CANVAS_ITEM (item));
+
+  private = GET_PRIVATE (item);
+
+  g_return_if_fail (private->change_count > 0);
+
+  private->change_count--;
+
+  if (private->change_count == 0 &&
+      g_signal_has_handler_pending (item, item_signals[UPDATE], 0, FALSE))
+    {
+      GdkRegion *region = gimp_canvas_item_get_extents (item);
+
+      if (! region)
+        {
+          region = private->change_region;
+        }
+      else if (private->change_region)
+        {
+          gdk_region_union (region, private->change_region);
+          gdk_region_destroy (private->change_region);
+        }
+
+      private->change_region = NULL;
+
+      if (region)
+        {
+          g_signal_emit (item, item_signals[UPDATE], 0,
+                         region);
+          gdk_region_destroy (region);
+        }
+    }
+}
+
+void
 gimp_canvas_item_suspend_stroking (GimpCanvasItem *item)
 {
   GimpCanvasItemPrivate *private;
@@ -460,6 +551,23 @@ gimp_canvas_item_resume_filling (GimpCanvasItem *item)
 /*  protected functions  */
 
 void
+_gimp_canvas_item_update (GimpCanvasItem *item,
+                          GdkRegion      *region)
+{
+  g_signal_emit (item, item_signals[UPDATE], 0,
+                 region);
+}
+
+gboolean
+_gimp_canvas_item_needs_update (GimpCanvasItem *item)
+{
+  GimpCanvasItemPrivate *private = GET_PRIVATE (item);
+
+  return (private->change_count == 0 &&
+          g_signal_has_handler_pending (item, item_signals[UPDATE], 0, FALSE));
+}
+
+void
 _gimp_canvas_item_stroke (GimpCanvasItem *item,
                           cairo_t        *cr)
 {
diff --git a/app/display/gimpcanvasitem.h b/app/display/gimpcanvasitem.h
index 61867d4..95ea3f2 100644
--- a/app/display/gimpcanvasitem.h
+++ b/app/display/gimpcanvasitem.h
@@ -70,6 +70,10 @@ void        gimp_canvas_item_draw             (GimpCanvasItem   *item,
                                                cairo_t          *cr);
 GdkRegion * gimp_canvas_item_get_extents      (GimpCanvasItem   *item);
 
+void        gimp_canvas_item_set_visible      (GimpCanvasItem   *item,
+                                               gboolean          visible);
+gboolean    gimp_canvas_item_get_visible      (GimpCanvasItem   *item);
+
 void        gimp_canvas_item_set_line_cap     (GimpCanvasItem   *item,
                                                cairo_line_cap_t  line_cap);
 
@@ -77,6 +81,9 @@ void        gimp_canvas_item_set_highlight    (GimpCanvasItem   *item,
                                                gboolean          highlight);
 gboolean    gimp_canvas_item_get_highlight    (GimpCanvasItem   *item);
 
+void        gimp_canvas_item_begin_change     (GimpCanvasItem   *item);
+void        gimp_canvas_item_end_change       (GimpCanvasItem   *item);
+
 void        gimp_canvas_item_suspend_stroking (GimpCanvasItem   *item);
 void        gimp_canvas_item_resume_stroking  (GimpCanvasItem   *item);
 
@@ -86,6 +93,9 @@ void        gimp_canvas_item_resume_filling   (GimpCanvasItem   *item);
 
 /*  protected  */
 
+void        _gimp_canvas_item_update          (GimpCanvasItem   *item,
+                                               GdkRegion        *region);
+gboolean    _gimp_canvas_item_needs_update    (GimpCanvasItem   *item);
 void        _gimp_canvas_item_stroke          (GimpCanvasItem   *item,
                                                cairo_t          *cr);
 void        _gimp_canvas_item_fill            (GimpCanvasItem   *item,
diff --git a/app/display/gimpdisplayshell-appearance.c b/app/display/gimpdisplayshell-appearance.c
index e92b152..86a91ef 100644
--- a/app/display/gimpdisplayshell-appearance.c
+++ b/app/display/gimpdisplayshell-appearance.c
@@ -40,6 +40,7 @@
 #include "widgets/gimpwidgets-utils.h"
 
 #include "gimpcanvas.h"
+#include "gimpcanvasitem.h"
 #include "gimpdisplay.h"
 #include "gimpdisplayshell.h"
 #include "gimpdisplayshell-appearance.h"
@@ -295,7 +296,6 @@ gimp_display_shell_set_show_guides (GimpDisplayShell *shell,
                                     gboolean          show)
 {
   GimpDisplayOptions *options;
-  GimpImage          *image;
 
   g_return_if_fail (GIMP_IS_DISPLAY_SHELL (shell));
 
@@ -303,12 +303,7 @@ gimp_display_shell_set_show_guides (GimpDisplayShell *shell,
 
   g_object_set (options, "show-guides", show, NULL);
 
-  image = gimp_display_get_image (shell->display);
-
-  if (image && gimp_image_get_guides (image))
-    {
-      gimp_display_shell_expose_full (shell);
-    }
+  gimp_canvas_item_set_visible (shell->guides, show);
 
   appearance_set_action_active (shell, "view-show-guides", show);
 }
@@ -357,7 +352,6 @@ gimp_display_shell_set_show_sample_points (GimpDisplayShell *shell,
                                            gboolean          show)
 {
   GimpDisplayOptions *options;
-  GimpImage          *image;
 
   g_return_if_fail (GIMP_IS_DISPLAY_SHELL (shell));
 
@@ -365,12 +359,7 @@ gimp_display_shell_set_show_sample_points (GimpDisplayShell *shell,
 
   g_object_set (options, "show-sample-points", show, NULL);
 
-  image = gimp_display_get_image (shell->display);
-
-  if (image && gimp_image_get_sample_points (image))
-    {
-      gimp_display_shell_expose_full (shell);
-    }
+  gimp_canvas_item_set_visible (shell->sample_points, show);
 
   appearance_set_action_active (shell, "view-show-sample-points", show);
 }
diff --git a/app/display/gimpdisplayshell-handlers.c b/app/display/gimpdisplayshell-handlers.c
index 6838caa..406e95b 100644
--- a/app/display/gimpdisplayshell-handlers.c
+++ b/app/display/gimpdisplayshell-handlers.c
@@ -572,10 +572,12 @@ gimp_display_shell_guide_move_handler (GimpImage        *image,
 
   item = gimp_canvas_proxy_group_get_item (group, guide);
 
+  gimp_canvas_item_begin_change (item);
   g_object_set (item,
                 "orientation", gimp_guide_get_orientation (guide),
                 "position",    gimp_guide_get_position (guide),
                 NULL);
+  gimp_canvas_item_end_change (item);
 }
 
 static void
@@ -647,10 +649,12 @@ gimp_display_shell_sample_point_move_handler (GimpImage        *image,
 
   item = gimp_canvas_proxy_group_get_item (group, sample_point);
 
+  gimp_canvas_item_begin_change (item);
   g_object_set (item,
                 "x", sample_point->x,
                 "y", sample_point->y,
                 NULL);
+  gimp_canvas_item_end_change (item);
 }
 
 static void