[gimp] Bug 795257 - Segmentation fault crash using the clone tool
- From: N/A <ell src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp] Bug 795257 - Segmentation fault crash using the clone tool
- Date: Sat, 14 Apr 2018 14:15:16 +0000 (UTC)
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]