[gimp] Bug 795257 - Segmentation fault crash using the clone tool



commit 45c172a885dfa1684e3b0fad1a6e5cde5a1ccb0f
Author: Ell <ell_se yahoo com>
Date:   Sat Apr 14 09:48:10 2018 -0400

    Bug 795257 - Segmentation fault crash using the clone tool
    
    Commit f5cb1fed85341a9d0a46fb1391b19fa9ea3ccb42, which performed
    brush outline generation in GimpPaintTool in synchrony with the
    paint thread, wasn't enough, since GimpSourceTool could still call
    gimp_brush_tool_create_outline() directly during its
    GimpDrawTool::draw() method, leading to the same race condition
    when executed concurrently with the paint thread.
    
    Partially revert the above commit, so that outline generation is
    handled as before, as far as GimpPaintTool is concenered.  Instead,
    add GimpPaintTool::{start,end,flush}_paint() virtual functions; the
    first two are called when starting/ending painting using the paint
    thread, while the third is called during the display-update
    timeout, while the main thread and the paint thread are
    synchronized.  This allows subclasses to perform non-thread-safe
    actions while the threads are synchronized.
    
    Override these functions in GimpBrushTool, and cache the brush
    boundary in the flush() function.  Use the cached boundary in
    gimp_brush_tool_create_outline() while painting, to avoid the above
    race condition, both when this function is called through
    GimpPaintTool, and through GimpSourceTool.

 app/tools/gimpbrushtool.c       |  109 ++++++++++++++++++++++++++++++++-------
 app/tools/gimpbrushtool.h       |    6 ++-
 app/tools/gimppainttool-paint.c |   60 ++++++---------------
 app/tools/gimppainttool.c       |   34 +++++-------
 app/tools/gimppainttool.h       |   58 +++++++++++---------
 5 files changed, 160 insertions(+), 107 deletions(-)
---
diff --git a/app/tools/gimpbrushtool.c b/app/tools/gimpbrushtool.c
index 7cd9fe6..5b35573 100644
--- a/app/tools/gimpbrushtool.c
+++ b/app/tools/gimpbrushtool.c
@@ -61,6 +61,9 @@ static void   gimp_brush_tool_options_notify  (GimpTool          *tool,
                                                GimpToolOptions   *options,
                                                const GParamSpec  *pspec);
 
+static void   gimp_brush_tool_start_paint     (GimpPaintTool     *paint_tool);
+static void   gimp_brush_tool_end_paint       (GimpPaintTool     *paint_tool);
+static void   gimp_brush_tool_flush_paint     (GimpPaintTool     *paint_tool);
 static GimpCanvasItem *
               gimp_brush_tool_get_outline     (GimpPaintTool     *paint_tool,
                                                GimpDisplay       *display,
@@ -74,6 +77,11 @@ static void   gimp_brush_tool_set_brush       (GimpBrushCore     *brush_core,
                                                GimpBrush         *brush,
                                                GimpBrushTool     *brush_tool);
 
+static const GimpBezierDesc *
+                 gimp_brush_tool_get_boundary (GimpBrushTool     *brush_tool,
+                                               gint              *width,
+                                               gint              *height);
+
 
 G_DEFINE_TYPE (GimpBrushTool, gimp_brush_tool, GIMP_TYPE_PAINT_TOOL)
 
@@ -93,6 +101,9 @@ gimp_brush_tool_class_init (GimpBrushToolClass *klass)
   tool_class->cursor_update     = gimp_brush_tool_cursor_update;
   tool_class->options_notify    = gimp_brush_tool_options_notify;
 
+  paint_tool_class->start_paint = gimp_brush_tool_start_paint;
+  paint_tool_class->end_paint   = gimp_brush_tool_end_paint;
+  paint_tool_class->flush_paint = gimp_brush_tool_flush_paint;
   paint_tool_class->get_outline = gimp_brush_tool_get_outline;
 }
 
@@ -221,6 +232,45 @@ gimp_brush_tool_options_notify (GimpTool         *tool,
     }
 }
 
+static void
+gimp_brush_tool_start_paint (GimpPaintTool *paint_tool)
+{
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->start_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->start_paint (paint_tool);
+
+  gimp_brush_tool_flush_paint (paint_tool);
+}
+
+static void
+gimp_brush_tool_end_paint (GimpPaintTool *paint_tool)
+{
+  GimpBrushTool *brush_tool = GIMP_BRUSH_TOOL (paint_tool);
+
+  g_clear_pointer (&brush_tool->boundary, gimp_bezier_desc_free);
+
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->end_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->end_paint (paint_tool);
+}
+
+static void
+gimp_brush_tool_flush_paint (GimpPaintTool *paint_tool)
+{
+  GimpBrushTool        *brush_tool = GIMP_BRUSH_TOOL (paint_tool);
+  const GimpBezierDesc *boundary;
+
+  if (GIMP_PAINT_TOOL_CLASS (parent_class)->flush_paint)
+    GIMP_PAINT_TOOL_CLASS (parent_class)->flush_paint (paint_tool);
+
+  g_clear_pointer (&brush_tool->boundary, gimp_bezier_desc_free);
+
+  boundary = gimp_brush_tool_get_boundary (brush_tool,
+                                           &brush_tool->boundary_width,
+                                           &brush_tool->boundary_height);
+
+  if (boundary)
+    brush_tool->boundary = gimp_bezier_desc_copy (boundary);
+}
+
 static GimpCanvasItem *
 gimp_brush_tool_get_outline (GimpPaintTool *paint_tool,
                              GimpDisplay   *display,
@@ -259,7 +309,6 @@ gimp_brush_tool_create_outline (GimpBrushTool *brush_tool,
                                 gdouble        x,
                                 gdouble        y)
 {
-  GimpBrushCore        *brush_core;
   GimpPaintOptions     *options;
   GimpDisplayShell     *shell;
   const GimpBezierDesc *boundary = NULL;
@@ -269,29 +318,25 @@ gimp_brush_tool_create_outline (GimpBrushTool *brush_tool,
   g_return_val_if_fail (GIMP_IS_BRUSH_TOOL (brush_tool), NULL);
   g_return_val_if_fail (GIMP_IS_DISPLAY (display), NULL);
 
-  if (! GIMP_PAINT_TOOL (brush_tool)->draw_brush)
-    return NULL;
-
-  brush_core = GIMP_BRUSH_CORE (GIMP_PAINT_TOOL (brush_tool)->core);
-  options    = GIMP_PAINT_TOOL_GET_OPTIONS (brush_tool);
-  shell      = gimp_display_get_shell (display);
+  if (gimp_paint_tool_is_painting (GIMP_PAINT_TOOL (brush_tool)))
+    {
+      boundary = brush_tool->boundary;
+      width    = brush_tool->boundary_width;
+      height   = brush_tool->boundary_height;
+    }
+  else
+    {
+      boundary = gimp_brush_tool_get_boundary (brush_tool, &width, &height);
+    }
 
-  if (! brush_core->main_brush || ! brush_core->dynamics)
+  if (! boundary)
     return NULL;
 
-  if (brush_core->scale > 0.0)
-    boundary = gimp_brush_transform_boundary (brush_core->main_brush,
-                                              brush_core->scale,
-                                              brush_core->aspect_ratio,
-                                              brush_core->angle,
-                                              brush_core->reflect,
-                                              brush_core->hardness,
-                                              &width,
-                                              &height);
+  options = GIMP_PAINT_TOOL_GET_OPTIONS (brush_tool);
+  shell   = gimp_display_get_shell (display);
 
   /*  don't draw the boundary if it becomes too small  */
-  if (boundary                   &&
-      SCALEX (shell, width)  > 4 &&
+  if (SCALEX (shell, width)  > 4 &&
       SCALEY (shell, height) > 4)
     {
       x -= width  / 2.0;
@@ -347,3 +392,29 @@ gimp_brush_tool_set_brush (GimpBrushCore *brush_core,
 
   gimp_draw_tool_resume (GIMP_DRAW_TOOL (brush_tool));
 }
+
+static const GimpBezierDesc *
+gimp_brush_tool_get_boundary (GimpBrushTool *brush_tool,
+                              gint          *width,
+                              gint          *height)
+{
+  GimpPaintTool *paint_tool = GIMP_PAINT_TOOL (brush_tool);
+  GimpBrushCore *brush_core = GIMP_BRUSH_CORE (paint_tool->core);
+
+  if (paint_tool->draw_brush &&
+      brush_core->main_brush &&
+      brush_core->dynamics   &&
+      brush_core->scale > 0.0)
+    {
+      return gimp_brush_transform_boundary (brush_core->main_brush,
+                                            brush_core->scale,
+                                            brush_core->aspect_ratio,
+                                            brush_core->angle,
+                                            brush_core->reflect,
+                                            brush_core->hardness,
+                                            width,
+                                            height);
+    }
+
+  return NULL;
+}
diff --git a/app/tools/gimpbrushtool.h b/app/tools/gimpbrushtool.h
index 79225f3..3145557 100644
--- a/app/tools/gimpbrushtool.h
+++ b/app/tools/gimpbrushtool.h
@@ -34,7 +34,11 @@ typedef struct _GimpBrushToolClass GimpBrushToolClass;
 
 struct _GimpBrushTool
 {
-  GimpPaintTool  parent_instance;
+  GimpPaintTool   parent_instance;
+
+  GimpBezierDesc *boundary;
+  gint            boundary_width;
+  gint            boundary_height;
 };
 
 struct _GimpBrushToolClass
diff --git a/app/tools/gimppainttool-paint.c b/app/tools/gimppainttool-paint.c
index 72614e6..1a4e513 100644
--- a/app/tools/gimppainttool-paint.c
+++ b/app/tools/gimppainttool-paint.c
@@ -68,12 +68,10 @@ typedef struct
 
 /*  local function prototypes  */
 
-static gboolean   gimp_paint_tool_paint_use_thread     (GimpPaintTool *paint_tool);
-static gpointer   gimp_paint_tool_paint_thread         (gpointer       data);
+static gboolean   gimp_paint_tool_paint_use_thread (GimpPaintTool *paint_tool);
+static gpointer   gimp_paint_tool_paint_thread     (gpointer       data);
 
-static gboolean   gimp_paint_tool_paint_timeout        (GimpPaintTool *paint_tool);
-
-static void       gimp_paint_tool_paint_update_outline (GimpPaintTool *paint_tool);
+static gboolean   gimp_paint_tool_paint_timeout    (GimpPaintTool *paint_tool);
 
 
 /*  static variables  */
@@ -182,8 +180,8 @@ gimp_paint_tool_paint_timeout (GimpPaintTool *paint_tool)
 
   update = gimp_drawable_flush_paint (drawable);
 
-  if (update)
-    gimp_paint_tool_paint_update_outline (paint_tool);
+  if (update && GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->flush_paint)
+    GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->flush_paint (paint_tool);
 
   paint_timeout_pending = FALSE;
   g_cond_signal (&paint_cond);
@@ -207,36 +205,6 @@ gimp_paint_tool_paint_timeout (GimpPaintTool *paint_tool)
   return G_SOURCE_CONTINUE;
 }
 
-static void
-gimp_paint_tool_paint_update_outline (GimpPaintTool *paint_tool)
-{
-  if (gimp_paint_tool_paint_use_thread (paint_tool))
-    {
-      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
-
-      if (paint_tool->draw_brush)
-        {
-          GimpPaintCore *core     = paint_tool->core;
-          GimpDisplay   *display  = paint_tool->display;
-          GimpDrawable  *drawable = paint_tool->drawable;
-          gint           off_x, off_y;
-          gdouble        x, y;
-
-          gimp_item_get_offset (GIMP_ITEM (drawable), &off_x, &off_y);
-
-          x = core->cur_coords.x + off_x;
-          y = core->cur_coords.y + off_y;
-
-          if (paint_tool->outline)
-            g_object_unref (paint_tool->outline);
-
-          paint_tool->outline =
-            GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (paint_tool,
-                                                                 display, x, y);
-        }
-    }
-}
-
 
 /*  public functions  */
 
@@ -322,6 +290,13 @@ gimp_paint_tool_paint_start (GimpPaintTool     *paint_tool,
         gimp_display_shell_get_constrained_line_offset_angle (shell));
     }
 
+  /*  Notify subclasses  */
+  if (gimp_paint_tool_paint_use_thread (paint_tool) &&
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->start_paint)
+    {
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->start_paint (paint_tool);
+    }
+
   /*  Let the specific painting function initialize itself  */
   gimp_paint_core_paint (core, drawable, paint_options,
                          GIMP_PAINT_STATE_INIT, time);
@@ -338,9 +313,6 @@ gimp_paint_tool_paint_start (GimpPaintTool     *paint_tool,
                              GIMP_PAINT_STATE_MOTION, time);
     }
 
-  /*  Update the brush outline  */
-  gimp_paint_tool_paint_update_outline (paint_tool);
-
   gimp_projection_flush_now (gimp_image_get_projection (image));
   gimp_display_flush_now (display);
 
@@ -422,8 +394,12 @@ gimp_paint_tool_paint_end (GimpPaintTool *paint_tool,
   else
     gimp_paint_core_finish (core, drawable, TRUE);
 
-  /*  Clear the brush outline  */
-  g_clear_object (&paint_tool->outline);
+  /*  Notify subclasses  */
+  if (gimp_paint_tool_paint_use_thread (paint_tool) &&
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->end_paint)
+    {
+      GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->end_paint (paint_tool);
+    }
 
   /*  Exit paint mode  */
   if (gimp_paint_tool_paint_use_thread (paint_tool))
diff --git a/app/tools/gimppainttool.c b/app/tools/gimppainttool.c
index 626a0d8..ebc25f6 100644
--- a/app/tools/gimppainttool.c
+++ b/app/tools/gimppainttool.c
@@ -672,9 +672,12 @@ gimp_paint_tool_draw (GimpDrawTool *draw_tool)
           line_drawn = TRUE;
         }
 
-      outline = gimp_paint_tool_get_outline (paint_tool,
-                                             draw_tool->display,
-                                             cur_x, cur_y);
+      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
+
+      if (paint_tool->draw_brush)
+        outline = gimp_paint_tool_get_outline (paint_tool,
+                                               draw_tool->display,
+                                               cur_x, cur_y);
 
       if (outline)
         {
@@ -767,22 +770,9 @@ gimp_paint_tool_get_outline (GimpPaintTool *paint_tool,
                              gdouble        x,
                              gdouble        y)
 {
-  if (paint_tool->drawable && gimp_drawable_is_painting (paint_tool->drawable))
-    {
-      if (paint_tool->outline)
-        return g_object_ref (paint_tool->outline);
-    }
-  else
-    {
-      gimp_paint_tool_set_draw_fallback (paint_tool, FALSE, 0.0);
-
-      if (paint_tool->draw_brush &&
-          GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline)
-        {
-          return GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (
-            paint_tool, display, x, y);
-        }
-    }
+  if (GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline)
+    return GIMP_PAINT_TOOL_GET_CLASS (paint_tool)->get_outline (paint_tool,
+                                                                display, x, y);
 
   return NULL;
 }
@@ -853,3 +843,9 @@ gimp_paint_tool_set_draw_circle (GimpPaintTool *tool,
   tool->draw_circle = draw_circle;
   tool->circle_size = circle_size;
 }
+
+gboolean
+gimp_paint_tool_is_painting (GimpPaintTool *tool)
+{
+  return tool->drawable != NULL && gimp_drawable_is_painting (tool->drawable);
+}
diff --git a/app/tools/gimppainttool.h b/app/tools/gimppainttool.h
index 1164009..6153335 100644
--- a/app/tools/gimppainttool.h
+++ b/app/tools/gimppainttool.h
@@ -39,34 +39,36 @@ typedef struct _GimpPaintToolClass GimpPaintToolClass;
 
 struct _GimpPaintTool
 {
-  GimpColorTool   parent_instance;
+  GimpColorTool  parent_instance;
 
-  gboolean        pick_colors;  /*  pick color if ctrl is pressed   */
-  gboolean        draw_line;
+  gboolean       pick_colors;  /*  pick color if ctrl is pressed   */
+  gboolean       draw_line;
 
-  gboolean        show_cursor;
-  gboolean        draw_brush;
-  gboolean        draw_fallback;
-  gint            fallback_size;
-  gboolean        draw_circle;
-  gint            circle_size;
+  gboolean       show_cursor;
+  gboolean       draw_brush;
+  gboolean       draw_fallback;
+  gint           fallback_size;
+  gboolean       draw_circle;
+  gint           circle_size;
 
-  const gchar    *status;       /* status message */
-  const gchar    *status_line;  /* status message when drawing a line */
-  const gchar    *status_ctrl;  /* additional message for the ctrl modifier */
+  const gchar   *status;       /* status message */
+  const gchar   *status_line;  /* status message when drawing a line */
+  const gchar   *status_ctrl;  /* additional message for the ctrl modifier */
 
-  GimpPaintCore  *core;
+  GimpPaintCore *core;
 
-  GimpDisplay    *display;
-  GimpDrawable   *drawable;
-
-  GimpCanvasItem *outline;
+  GimpDisplay   *display;
+  GimpDrawable  *drawable;
 };
 
 struct _GimpPaintToolClass
 {
   GimpColorToolClass  parent_class;
 
+  void             (* start_paint) (GimpPaintTool *paint_tool);
+  void             (* end_paint)   (GimpPaintTool *paint_tool);
+  void             (* flush_paint) (GimpPaintTool *paint_tool);
+
   GimpCanvasItem * (* get_outline) (GimpPaintTool *paint_tool,
                                     GimpDisplay   *display,
                                     gdouble        x,
@@ -74,16 +76,20 @@ struct _GimpPaintToolClass
 };
 
 
-GType   gimp_paint_tool_get_type            (void) G_GNUC_CONST;
+GType      gimp_paint_tool_get_type            (void) G_GNUC_CONST;
+
+void       gimp_paint_tool_enable_color_picker (GimpPaintTool     *tool,
+                                                GimpColorPickMode  mode);
+
+void       gimp_paint_tool_set_draw_fallback   (GimpPaintTool     *tool,
+                                                gboolean           draw_fallback,
+                                                gint               fallback_size);
+
+void       gimp_paint_tool_set_draw_circle     (GimpPaintTool     *tool,
+                                                gboolean           draw_circle,
+                                                gint               circle_size);
 
-void    gimp_paint_tool_enable_color_picker (GimpPaintTool     *tool,
-                                             GimpColorPickMode  mode);
+gboolean   gimp_paint_tool_is_painting         (GimpPaintTool     *tool);
 
-void    gimp_paint_tool_set_draw_fallback   (GimpPaintTool     *tool,
-                                             gboolean           draw_fallback,
-                                             gint               fallback_size);
 
-void    gimp_paint_tool_set_draw_circle     (GimpPaintTool     *tool,
-                                             gboolean           draw_circle,
-                                             gint               circle_size);
 #endif  /*  __GIMP_PAINT_TOOL_H__  */


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]