GNOME.org
 

[gimp] app: fix text tool drawing for good this time (hopefully)

commit 5284c9c836198f74197e2f365d5b5955d6a16b10
Author: Michael Natterer <mitch gimp org>
Date:   Thu Mar 4 23:47:23 2010 +0100

    app: fix text tool drawing for good this time (hopefully)
    
    Earlier I claimed that drawing would work now because we make sure
    that buffer and layout are always in sync. This was nonsense. In fact,
    we constantly ran into the sutiation where the buffer was modified,
    and we were drawing with the previous layout. It's unclear why this
    didn't cause drawing artifacts, but it did cause e.g. the cursor
    temporarily jumping to the next position while editing in the middle
    of text (especially visible at line ends).
    
    Several underlying problems existed: first, we now modify the buffer
    from outside the text tool (from GimpTextStyleEditor) where we can't
    pause the tool; second, proxy changes are handled asymetrically
    (property changes are queued and processed all together in an idle
    function) so we can't pause/resume drawing across the entire operation
    because it has many beginnings and only one end.
    
    Therefore:
    
    - add gimp_text_tool_block_drawing()/unblock_drawing(), where block()
      can be called as many times as needed, and a single unblock()
      enables drawing again. block() also clears the layout, because it
      served its purpose (it was just used to pause drawing, and we know
      the buffer will change, so kill it).
    - connect to GtkTextBuffer::begin-user-action and call block() from
      the callback, so we undraw stuff and kill the cached layout before
      any buffer change happens.
    - call unblock() at the end of gimp_text_tool_apply() because then
      the text and the buffer are in sync again, the tool is undrawn and
      we can safely create the layout again to draw our stuff.
    - also call block()/unblock() from some other places, like when a
      new text layer is created.
    - get rid of *all* calls to draw_tool_pause()/resume() around buffer
      modifications, they are not needed any longer.
    - add calls to begin/end_user_action() where they were missing.

 app/tools/gimptexttool-editor.c |   36 +------------
 app/tools/gimptexttool.c        |  112 +++++++++++++++++++++++++++------------
 app/tools/gimptexttool.h        |    1 +
 3 files changed, 80 insertions(+), 69 deletions(-)
---
diff --git a/app/tools/gimptexttool-editor.c b/app/tools/gimptexttool-editor.c
index d55fdd1..59a8cd9 100644
--- a/app/tools/gimptexttool-editor.c
+++ b/app/tools/gimptexttool-editor.c
@@ -416,15 +416,11 @@ gimp_text_tool_editor_key_press (GimpTextTool *text_tool,
   gint           x_pos  = -1;
   gboolean       retval = TRUE;
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   if (gtk_im_context_filter_keypress (text_tool->im_context, kevent))
     {
       text_tool->needs_im_reset = TRUE;
       text_tool->x_pos          = -1;
 
-      gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
-
      return TRUE;
     }
 
@@ -435,8 +431,6 @@ gimp_text_tool_editor_key_press (GimpTextTool *text_tool,
     {
       GIMP_LOG (TEXT_EDITING, "binding handled event");
 
-      gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
-
       return TRUE;
     }
 
@@ -473,8 +467,6 @@ gimp_text_tool_editor_key_press (GimpTextTool *text_tool,
 
   text_tool->x_pos = x_pos;
 
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
-
   return retval;
 }
 
@@ -833,11 +825,7 @@ static void
 gimp_text_tool_insert_at_cursor (GimpTextTool *text_tool,
                                  const gchar  *str)
 {
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   gimp_text_buffer_insert (text_tool->buffer, str);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 static gboolean
@@ -896,11 +884,7 @@ gimp_text_tool_delete_from_cursor (GimpTextTool  *text_tool,
     case GTK_DELETE_CHARS:
       if (gtk_text_buffer_get_has_selection (buffer))
         {
-          gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
           gtk_text_buffer_delete_selection (buffer, TRUE, TRUE);
-
-          gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
           return;
         }
       else
@@ -962,11 +946,7 @@ gimp_text_tool_delete_from_cursor (GimpTextTool  *text_tool,
 
   if (! gtk_text_iter_equal (&cursor, &end))
     {
-      gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
       gtk_text_buffer_delete_interactive (buffer, &cursor, &end, TRUE);
-
-      gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
     }
 }
 
@@ -977,8 +957,6 @@ gimp_text_tool_backspace (GimpTextTool *text_tool)
 
   gimp_text_tool_reset_im_context (text_tool);
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   if (gtk_text_buffer_get_has_selection (buffer))
     {
       gtk_text_buffer_delete_selection (buffer, TRUE, TRUE);
@@ -992,8 +970,6 @@ gimp_text_tool_backspace (GimpTextTool *text_tool)
 
       gtk_text_buffer_backspace (buffer, &cursor, TRUE, TRUE);
     }
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 static void
@@ -1048,13 +1024,9 @@ gimp_text_tool_change_baseline (GimpTextTool *text_tool,
       gtk_text_buffer_get_end_iter (buffer, &end);
     }
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   gtk_text_iter_order (&start, &end);
   gimp_text_buffer_change_baseline (text_tool->buffer, &start, &end,
                                     count * PANGO_SCALE);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 static void
@@ -1073,13 +1045,9 @@ gimp_text_tool_change_kerning (GimpTextTool *text_tool,
       gtk_text_iter_forward_char (&end);
     }
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   gtk_text_iter_order (&start, &end);
   gimp_text_buffer_change_kerning (text_tool->buffer, &start, &end,
                                    count * PANGO_SCALE);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 static void
@@ -1167,7 +1135,7 @@ gimp_text_tool_enter_text (GimpTextTool *text_tool,
 
   had_selection = gtk_text_buffer_get_has_selection (buffer);
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
+  gtk_text_buffer_begin_user_action (buffer);
 
   gimp_text_tool_delete_selection (text_tool);
 
@@ -1184,7 +1152,7 @@ gimp_text_tool_enter_text (GimpTextTool *text_tool,
 
   gimp_text_buffer_insert (text_tool->buffer, str);
 
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
+  gtk_text_buffer_end_user_action (buffer);
 }
 
 static void
diff --git a/app/tools/gimptexttool.c b/app/tools/gimptexttool.c
index c815d54..e68cc93 100644
--- a/app/tools/gimptexttool.c
+++ b/app/tools/gimptexttool.c
@@ -163,7 +163,12 @@ static gboolean  gimp_text_tool_set_drawable    (GimpTextTool      *text_tool,
                                                  GimpDrawable      *drawable,
                                                  gboolean           confirm);
 
-static void      gimp_text_tool_buffer_edited   (GimpTextBuffer    *buffer,
+static void      gimp_text_tool_block_drawing   (GimpTextTool      *text_tool);
+static void      gimp_text_tool_unblock_drawing (GimpTextTool      *text_tool);
+
+static void    gimp_text_tool_buffer_begin_edit (GimpTextBuffer    *buffer,
+                                                 GimpTextTool      *text_tool);
+static void    gimp_text_tool_buffer_end_edit   (GimpTextBuffer    *buffer,
                                                  GimpTextTool      *text_tool);
 
 
@@ -246,8 +251,11 @@ gimp_text_tool_init (GimpTextTool *text_tool)
 
   text_tool->buffer = gimp_text_buffer_new ();
 
+  g_signal_connect (text_tool->buffer, "begin-user-action",
+                    G_CALLBACK (gimp_text_tool_buffer_begin_edit),
+                    text_tool);
   g_signal_connect (text_tool->buffer, "end-user-action",
-                    G_CALLBACK (gimp_text_tool_buffer_edited),
+                    G_CALLBACK (gimp_text_tool_buffer_end_edit),
                     text_tool);
 
   text_tool->handle_rectangle_change_complete = TRUE;
@@ -496,13 +504,19 @@ gimp_text_tool_button_release (GimpTool              *tool,
        *  can we do...
        */
       g_signal_handlers_block_by_func (text_tool->buffer,
-                                       gimp_text_tool_buffer_edited,
+                                       gimp_text_tool_buffer_begin_edit,
+                                       text_tool);
+      g_signal_handlers_block_by_func (text_tool->buffer,
+                                       gimp_text_tool_buffer_end_edit,
                                        text_tool);
 
       gimp_text_tool_editor_button_release (text_tool);
 
       g_signal_handlers_unblock_by_func (text_tool->buffer,
-                                         gimp_text_tool_buffer_edited,
+                                         gimp_text_tool_buffer_end_edit,
+                                         text_tool);
+      g_signal_handlers_unblock_by_func (text_tool->buffer,
+                                         gimp_text_tool_buffer_begin_edit,
                                          text_tool);
 
       text_tool->selecting = FALSE;
@@ -872,6 +886,8 @@ gimp_text_tool_rectangle_change_complete (GimpRectangleTool *rect_tool)
 
           gimp_image_get_resolution (text_tool->image, &xres, &yres);
 
+          gimp_text_tool_block_drawing (text_tool);
+
           g_object_set (text_tool->proxy,
                         "box-mode",   GIMP_TEXT_BOX_FIXED,
                         "box-width",  gimp_pixels_to_units (x2 - x1,
@@ -924,7 +940,10 @@ gimp_text_tool_connect (GimpTextTool  *text_tool,
       GimpTextOptions *options = GIMP_TEXT_TOOL_GET_OPTIONS (tool);
 
       g_signal_handlers_block_by_func (text_tool->buffer,
-                                       gimp_text_tool_buffer_edited,
+                                       gimp_text_tool_buffer_begin_edit,
+                                       text_tool);
+      g_signal_handlers_block_by_func (text_tool->buffer,
+                                       gimp_text_tool_buffer_end_edit,
                                        text_tool);
 
       if (text_tool->text)
@@ -977,7 +996,10 @@ gimp_text_tool_connect (GimpTextTool  *text_tool,
         }
 
       g_signal_handlers_unblock_by_func (text_tool->buffer,
-                                         gimp_text_tool_buffer_edited,
+                                         gimp_text_tool_buffer_end_edit,
+                                         text_tool);
+      g_signal_handlers_unblock_by_func (text_tool->buffer,
+                                         gimp_text_tool_buffer_begin_edit,
                                          text_tool);
     }
 
@@ -1036,7 +1058,7 @@ gimp_text_tool_text_notify (GimpText     *text,
 {
   g_return_if_fail (text == text_tool->text);
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
+  gimp_text_tool_block_drawing (text_tool);
 
   if ((pspec->flags & G_PARAM_READWRITE) == G_PARAM_READWRITE)
     {
@@ -1066,7 +1088,10 @@ gimp_text_tool_text_notify (GimpText     *text,
       strcmp (pspec->name, "markup") == 0)
     {
       g_signal_handlers_block_by_func (text_tool->buffer,
-                                       gimp_text_tool_buffer_edited,
+                                       gimp_text_tool_buffer_begin_edit,
+                                       text_tool);
+      g_signal_handlers_block_by_func (text_tool->buffer,
+                                       gimp_text_tool_buffer_end_edit,
                                        text_tool);
 
       if (pspec->name[0] == 't')
@@ -1075,27 +1100,24 @@ gimp_text_tool_text_notify (GimpText     *text,
         gimp_text_buffer_set_markup (text_tool->buffer, text->markup);
 
       g_signal_handlers_unblock_by_func (text_tool->buffer,
-                                         gimp_text_tool_buffer_edited,
+                                         gimp_text_tool_buffer_end_edit,
+                                         text_tool);
+      g_signal_handlers_unblock_by_func (text_tool->buffer,
+                                         gimp_text_tool_buffer_begin_edit,
                                          text_tool);
     }
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 static void
 gimp_text_tool_text_changed (GimpText     *text,
                              GimpTextTool *text_tool)
 {
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   /* we need to redraw the rectangle in any case because whatever
    * changes to the text can change its size
    */
   gimp_text_tool_frame_item (text_tool);
 
-  gimp_text_tool_clear_layout (text_tool);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
+  gimp_text_tool_unblock_drawing (text_tool);
 }
 
 static gboolean
@@ -1168,8 +1190,6 @@ gimp_text_tool_apply (GimpTextTool *text_tool)
         }
     }
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   if (push_undo)
     {
       if (layer->modified)
@@ -1242,9 +1262,7 @@ gimp_text_tool_apply (GimpTextTool *text_tool)
 
   gimp_image_flush (image);
 
-  gimp_text_tool_clear_layout (text_tool);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
+  gimp_text_tool_unblock_drawing (text_tool);
 
   return FALSE;
 }
@@ -1260,6 +1278,8 @@ gimp_text_tool_create_layer (GimpTextTool *text_tool,
   gint               x1, y1;
   gint               x2, y2;
 
+  gimp_text_tool_block_drawing (text_tool);
+
   if (text)
     {
       text = gimp_config_duplicate (GIMP_CONFIG (text));
@@ -1344,10 +1364,14 @@ gimp_text_tool_create_layer (GimpTextTool *text_tool,
                                                         text_tool->proxy->box_unit,
                                                         yres),
                     NULL);
+
+      gimp_text_tool_apply (text_tool); /* unblocks drawing */
     }
   else
     {
       gimp_text_tool_frame_item (text_tool);
+
+      gimp_text_tool_unblock_drawing (text_tool);
     }
 
   gimp_image_undo_group_end (image);
@@ -1582,8 +1606,38 @@ gimp_text_tool_set_drawable (GimpTextTool *text_tool,
 }
 
 static void
-gimp_text_tool_buffer_edited (GimpTextBuffer *buffer,
-                              GimpTextTool   *text_tool)
+gimp_text_tool_block_drawing (GimpTextTool *text_tool)
+{
+  if (! text_tool->drawing_blocked)
+    {
+      gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
+
+      gimp_text_tool_clear_layout (text_tool);
+
+      text_tool->drawing_blocked = TRUE;
+    }
+}
+
+static void
+gimp_text_tool_unblock_drawing (GimpTextTool *text_tool)
+{
+  g_return_if_fail (text_tool->drawing_blocked == TRUE);
+
+  text_tool->drawing_blocked = FALSE;
+
+  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
+}
+
+static void
+gimp_text_tool_buffer_begin_edit (GimpTextBuffer *buffer,
+                                  GimpTextTool   *text_tool)
+{
+  gimp_text_tool_block_drawing (text_tool);
+}
+
+static void
+gimp_text_tool_buffer_end_edit (GimpTextBuffer *buffer,
+                                GimpTextTool   *text_tool)
 {
   if (text_tool->text)
     {
@@ -1713,11 +1767,7 @@ gimp_text_tool_delete_selection (GimpTextTool *text_tool)
 
   if (gtk_text_buffer_get_has_selection (buffer))
     {
-      gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
       gtk_text_buffer_delete_selection (buffer, TRUE, TRUE);
-
-      gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
     }
 }
 
@@ -1734,12 +1784,8 @@ gimp_text_tool_cut_clipboard (GimpTextTool *text_tool)
   clipboard = gtk_widget_get_clipboard (GTK_WIDGET (shell),
                                         GDK_SELECTION_CLIPBOARD);
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   gtk_text_buffer_cut_clipboard (GTK_TEXT_BUFFER (text_tool->buffer),
                                  clipboard, TRUE);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 void
@@ -1772,12 +1818,8 @@ gimp_text_tool_paste_clipboard (GimpTextTool *text_tool)
   clipboard = gtk_widget_get_clipboard (GTK_WIDGET (shell),
                                         GDK_SELECTION_CLIPBOARD);
 
-  gimp_draw_tool_pause (GIMP_DRAW_TOOL (text_tool));
-
   gtk_text_buffer_paste_clipboard (GTK_TEXT_BUFFER (text_tool->buffer),
                                    clipboard, NULL, TRUE);
-
-  gimp_draw_tool_resume (GIMP_DRAW_TOOL (text_tool));
 }
 
 void
diff --git a/app/tools/gimptexttool.h b/app/tools/gimptexttool.h
index 4e9da23..194a426 100644
--- a/app/tools/gimptexttool.h
+++ b/app/tools/gimptexttool.h
@@ -62,6 +62,7 @@ struct _GimpTextTool
   gboolean        text_box_fixed;
 
   GimpTextLayout *layout;
+  gboolean        drawing_blocked;
 
   /* text editor state: */
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]