Issue #11389: crash when using layer auto-expansion.
The fix is basically to make gimp_projection_flush() thread-safe, so that it can be called from any thread. The actual rendering was actually already run in the main thread since it was in an idle function, but update_region could be touched both from the main thread and another thread (e.g. the "paint" thread). An alternative could have been to put some mutex to protect usage, but then I realized that iter variable was also to be protected, and there was some code where I am unsure if we had to protect larger parts of code (in particular with gimp_projection_projectable_bounds_changed() which was touching update region through gimp_projection_chunk_render_stop() then directly). The nice headaches of multi-threading! Moving the whole actual logic of gimp_projection_flush() in the main thread seems much more robust and maintainable.
This commit is contained in:
Jehan
@ -124,11 +124,8 @@ static void gimp_projection_add_update_area (GimpProjection *proj,
|
||||
gint y,
|
||||
gint w,
|
||||
gint h);
|
||||
static void gimp_projection_flush_whenever (GimpProjection *proj,
|
||||
gboolean now,
|
||||
gboolean direct);
|
||||
static void gimp_projection_update_priority_rect (GimpProjection *proj);
|
||||
static void gimp_projection_chunk_render_start (GimpProjection *proj);
|
||||
static gboolean gimp_projection_chunk_render_start (GimpProjection *proj);
|
||||
static void gimp_projection_chunk_render_stop (GimpProjection *proj,
|
||||
gboolean merge);
|
||||
static gboolean gimp_projection_chunk_render_callback (GimpProjection *proj);
|
||||
@ -493,23 +490,67 @@ gimp_projection_stop_rendering (GimpProjection *proj)
|
||||
gimp_projection_chunk_render_stop (proj, TRUE);
|
||||
}
|
||||
|
||||
/**
|
||||
* gimp_projection_flush:
|
||||
* @proj:
|
||||
*
|
||||
* This requests to render the projection. This function is thread-safe
|
||||
* and can be called in any thread.
|
||||
*
|
||||
* The actual projection painting will happen in the main thread.
|
||||
*/
|
||||
void
|
||||
gimp_projection_flush (GimpProjection *proj)
|
||||
{
|
||||
g_return_if_fail (GIMP_IS_PROJECTION (proj));
|
||||
|
||||
/* Construct in chunks */
|
||||
gimp_projection_flush_whenever (proj, FALSE, FALSE);
|
||||
/* Construct in chunks - asynchronously in the main thread */
|
||||
g_idle_add_full (G_PRIORITY_HIGH_IDLE,
|
||||
(GSourceFunc) gimp_projection_chunk_render_start,
|
||||
proj, NULL);
|
||||
}
|
||||
|
||||
/**
|
||||
* gimp_projection_flush_now:
|
||||
* @proj:
|
||||
* @direct:
|
||||
*
|
||||
* This renders the projection immediately. You can only call this from
|
||||
* the main thread.
|
||||
*/
|
||||
void
|
||||
gimp_projection_flush_now (GimpProjection *proj,
|
||||
gboolean direct)
|
||||
{
|
||||
g_return_if_fail (GIMP_IS_PROJECTION (proj));
|
||||
|
||||
/* Construct NOW */
|
||||
gimp_projection_flush_whenever (proj, TRUE, direct);
|
||||
/* Construct NOW - synchronously */
|
||||
if (proj->priv->update_region)
|
||||
{
|
||||
gint n_rects = cairo_region_num_rectangles (proj->priv->update_region);
|
||||
gint i;
|
||||
|
||||
/* Make sure we have a buffer */
|
||||
gimp_projection_allocate_buffer (proj);
|
||||
|
||||
for (i = 0; i < n_rects; i++)
|
||||
{
|
||||
cairo_rectangle_int_t rect;
|
||||
|
||||
cairo_region_get_rectangle (proj->priv->update_region,
|
||||
i, &rect);
|
||||
|
||||
gimp_projection_paint_area (proj,
|
||||
direct,
|
||||
rect.x,
|
||||
rect.y,
|
||||
rect.width,
|
||||
rect.height);
|
||||
}
|
||||
|
||||
/* Free the update region */
|
||||
g_clear_pointer (&proj->priv->update_region, cairo_region_destroy);
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
@ -611,56 +652,6 @@ gimp_projection_add_update_area (GimpProjection *proj,
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
gimp_projection_flush_whenever (GimpProjection *proj,
|
||||
gboolean now,
|
||||
gboolean direct)
|
||||
{
|
||||
if (proj->priv->update_region)
|
||||
{
|
||||
/* Make sure we have a buffer */
|
||||
gimp_projection_allocate_buffer (proj);
|
||||
|
||||
if (now) /* Synchronous */
|
||||
{
|
||||
gint n_rects = cairo_region_num_rectangles (proj->priv->update_region);
|
||||
gint i;
|
||||
|
||||
for (i = 0; i < n_rects; i++)
|
||||
{
|
||||
cairo_rectangle_int_t rect;
|
||||
|
||||
cairo_region_get_rectangle (proj->priv->update_region,
|
||||
i, &rect);
|
||||
|
||||
gimp_projection_paint_area (proj,
|
||||
direct,
|
||||
rect.x,
|
||||
rect.y,
|
||||
rect.width,
|
||||
rect.height);
|
||||
}
|
||||
|
||||
/* Free the update region */
|
||||
g_clear_pointer (&proj->priv->update_region, cairo_region_destroy);
|
||||
}
|
||||
else /* Asynchronous */
|
||||
{
|
||||
/* Consumes the update region */
|
||||
gimp_projection_chunk_render_start (proj);
|
||||
}
|
||||
}
|
||||
else if (! now && ! proj->priv->iter && proj->priv->invalidate_preview)
|
||||
{
|
||||
/* invalidate the preview here since it is constructed from
|
||||
* the projection
|
||||
*/
|
||||
proj->priv->invalidate_preview = FALSE;
|
||||
|
||||
gimp_projectable_invalidate_preview (proj->priv->projectable);
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
gimp_projection_update_priority_rect (GimpProjection *proj)
|
||||
{
|
||||
@ -688,66 +679,82 @@ gimp_projection_update_priority_rect (GimpProjection *proj)
|
||||
}
|
||||
}
|
||||
|
||||
static void
|
||||
static gboolean
|
||||
gimp_projection_chunk_render_start (GimpProjection *proj)
|
||||
{
|
||||
cairo_region_t *region = proj->priv->update_region;
|
||||
gboolean invalidate_preview = FALSE;
|
||||
|
||||
if (proj->priv->iter)
|
||||
if (proj->priv->update_region)
|
||||
{
|
||||
region = gimp_chunk_iterator_stop (proj->priv->iter, FALSE);
|
||||
cairo_region_t *region = proj->priv->update_region;
|
||||
gboolean invalidate_preview = FALSE;
|
||||
|
||||
proj->priv->iter = NULL;
|
||||
/* Make sure we have a buffer */
|
||||
gimp_projection_allocate_buffer (proj);
|
||||
|
||||
if (cairo_region_is_empty (region))
|
||||
invalidate_preview = proj->priv->invalidate_preview;
|
||||
|
||||
if (proj->priv->update_region)
|
||||
if (proj->priv->iter)
|
||||
{
|
||||
cairo_region_union (region, proj->priv->update_region);
|
||||
region = gimp_chunk_iterator_stop (proj->priv->iter, FALSE);
|
||||
|
||||
cairo_region_destroy (proj->priv->update_region);
|
||||
proj->priv->iter = NULL;
|
||||
|
||||
if (cairo_region_is_empty (region))
|
||||
invalidate_preview = proj->priv->invalidate_preview;
|
||||
|
||||
if (proj->priv->update_region)
|
||||
{
|
||||
cairo_region_union (region, proj->priv->update_region);
|
||||
|
||||
cairo_region_destroy (proj->priv->update_region);
|
||||
}
|
||||
}
|
||||
|
||||
proj->priv->update_region = NULL;
|
||||
|
||||
if (region && ! cairo_region_is_empty (region))
|
||||
{
|
||||
proj->priv->iter = gimp_chunk_iterator_new (region);
|
||||
|
||||
gimp_projection_update_priority_rect (proj);
|
||||
|
||||
if (! proj->priv->idle_id)
|
||||
{
|
||||
proj->priv->idle_id = g_idle_add_full (GIMP_PRIORITY_PROJECTION_IDLE + proj->priv->priority,
|
||||
(GSourceFunc) gimp_projection_chunk_render_callback,
|
||||
proj, NULL);
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
if (region)
|
||||
cairo_region_destroy (region);
|
||||
|
||||
if (proj->priv->idle_id)
|
||||
{
|
||||
g_source_remove (proj->priv->idle_id);
|
||||
proj->priv->idle_id = 0;
|
||||
}
|
||||
|
||||
if (invalidate_preview)
|
||||
{
|
||||
/* invalidate the preview here since it is constructed from
|
||||
* the projection
|
||||
*/
|
||||
proj->priv->invalidate_preview = FALSE;
|
||||
|
||||
gimp_projectable_invalidate_preview (proj->priv->projectable);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
proj->priv->update_region = NULL;
|
||||
|
||||
if (region && ! cairo_region_is_empty (region))
|
||||
else if (! proj->priv->iter && proj->priv->invalidate_preview)
|
||||
{
|
||||
proj->priv->iter = gimp_chunk_iterator_new (region);
|
||||
/* invalidate the preview here since it is constructed from
|
||||
* the projection
|
||||
*/
|
||||
proj->priv->invalidate_preview = FALSE;
|
||||
|
||||
gimp_projection_update_priority_rect (proj);
|
||||
|
||||
if (! proj->priv->idle_id)
|
||||
{
|
||||
proj->priv->idle_id = g_idle_add_full (
|
||||
GIMP_PRIORITY_PROJECTION_IDLE + proj->priv->priority,
|
||||
(GSourceFunc) gimp_projection_chunk_render_callback,
|
||||
proj, NULL);
|
||||
}
|
||||
gimp_projectable_invalidate_preview (proj->priv->projectable);
|
||||
}
|
||||
else
|
||||
{
|
||||
if (region)
|
||||
cairo_region_destroy (region);
|
||||
|
||||
if (proj->priv->idle_id)
|
||||
{
|
||||
g_source_remove (proj->priv->idle_id);
|
||||
proj->priv->idle_id = 0;
|
||||
}
|
||||
|
||||
if (invalidate_preview)
|
||||
{
|
||||
/* invalidate the preview here since it is constructed from
|
||||
* the projection
|
||||
*/
|
||||
proj->priv->invalidate_preview = FALSE;
|
||||
|
||||
gimp_projectable_invalidate_preview (proj->priv->projectable);
|
||||
}
|
||||
}
|
||||
return G_SOURCE_REMOVE;
|
||||
}
|
||||
|
||||
static void
|
||||
|
||||
Reference in New Issue
Block a user