[gimp] app: fix leak of a GdkPoint and improve stable zoom centering logics.
- From: Jehan <jehanp src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gimp] app: fix leak of a GdkPoint and improve stable zoom centering logics.
- Date: Fri, 20 Aug 2021 18:56:30 +0000 (UTC)
commit f5ea8e9b2aaa9c82692127685d8aafc285d0ff31
Author: Jehan <jehan girinstud io>
Date: Fri Aug 20 20:09:50 2021 +0200
app: fix leak of a GdkPoint and improve stable zoom centering logics.
This started as yet another report of leak by Massimo. But really the
leak of the GdkPoint created by the function
gimp_display_shell_push_zoom_focus_pointer_pos() is not only when
delta_y is 0. There are a few code paths in gimp_display_shell_scale()
when we would not pop this point. One of them is for instance when
window resizing in multi-window mode is allowed. There might be more
(but the code is convoluted enough not to be 100% sure if these are
possible with our specific case).
This specific function was initially created only to be used for unit
testing code (commit 7e3898da093), but it ended up being also used
internally (commit 792cd581a21). Since I see that the test for which
this code was initially created even seem broken right now (the assert
part for position check is commented out!), I even wonder if we should
keep it. We could indeed instead just add optional start_x|y arguments
to gimp_display_shell_scale(), which would be much cleaner. But I leave
it for now.
Instead I just make sure we clean the created GdkPoint after calling
gimp_display_shell_scale(). Also I get rid of the GQueue. It is clear in
the code that we are not expecting queuing interaction of several
positions. Worse right now, we could end up in weird cases where the
pushed points are not used when they should, then could end up being
used later in totally unrelated interactions (this would make the shell
position jump here and there). So let's just make it a single point.
Finally adding some appropriate comments in parts which are still a bit
wrong.
app/display/gimpdisplayshell-scale.c | 69 ++++++++++++++++++++++++------------
app/display/gimpdisplayshell.c | 8 +++--
app/display/gimpdisplayshell.h | 2 +-
3 files changed, 53 insertions(+), 26 deletions(-)
---
diff --git a/app/display/gimpdisplayshell-scale.c b/app/display/gimpdisplayshell-scale.c
index 26afbbbf22..24c7d234f9 100644
--- a/app/display/gimpdisplayshell-scale.c
+++ b/app/display/gimpdisplayshell-scale.c
@@ -539,6 +539,10 @@ gimp_display_shell_scale (GimpDisplayShell *shell,
gimp_zoom_model_zoom (shell->zoom, GIMP_ZOOM_TO, new_scale);
gimp_display_shell_scale_resize (shell, TRUE, FALSE);
+
+ /* XXX The @zoom_focus policy is clearly not working in this code
+ * path. This should be fixed.
+ */
}
else
{
@@ -805,21 +809,33 @@ gimp_display_shell_scale_drag (GimpDisplayShell *shell,
scale = gimp_zoom_model_get_factor (shell->zoom);
- gimp_display_shell_push_zoom_focus_pointer_pos (shell, start_x, start_y);
-
- if (delta_y > 0)
- {
- gimp_display_shell_scale (shell,
- GIMP_ZOOM_TO,
- scale * 1.1,
- GIMP_ZOOM_FOCUS_POINTER);
- }
- else if (delta_y < 0)
+ if (delta_y != 0.0)
{
- gimp_display_shell_scale (shell,
- GIMP_ZOOM_TO,
- scale * 0.9,
- GIMP_ZOOM_FOCUS_POINTER);
+ gimp_display_shell_push_zoom_focus_pointer_pos (shell, start_x, start_y);
+
+ if (delta_y > 0.0)
+ {
+ gimp_display_shell_scale (shell,
+ GIMP_ZOOM_TO,
+ scale * 1.1,
+ GIMP_ZOOM_FOCUS_POINTER);
+ }
+ else /* delta_y < 0.0 */
+ {
+ gimp_display_shell_scale (shell,
+ GIMP_ZOOM_TO,
+ scale * 0.9,
+ GIMP_ZOOM_FOCUS_POINTER);
+ }
+
+ if (shell->zoom_focus_point)
+ {
+ /* In case we hit one of the cases when the focus pointer
+ * position was unused.
+ */
+ g_slice_free (GdkPoint, shell->zoom_focus_point);
+ shell->zoom_focus_point = NULL;
+ }
}
}
@@ -997,6 +1013,13 @@ gimp_display_shell_get_rotated_scale (GimpDisplayShell *shell,
*
* When the zoom focus mechanism asks for the pointer the next time,
* use @x and @y.
+ *
+ * It was primarily created for unit testing (see commit 7e3898da093).
+ * Therefore it should not be used by our code. When it is, we should
+ * make sure that @shell->zoom_focus_point has been properly used and
+ * freed, if we don't want it to leak.
+ * Just calling gimp_display_shell_scale() is not enough as there are
+ * currently some code paths where the values is not used.
**/
void
gimp_display_shell_push_zoom_focus_pointer_pos (GimpDisplayShell *shell,
@@ -1007,8 +1030,8 @@ gimp_display_shell_push_zoom_focus_pointer_pos (GimpDisplayShell *shell,
point->x = x;
point->y = y;
- g_queue_push_head (shell->zoom_focus_pointer_queue,
- point);
+ g_slice_free (GdkPoint, shell->zoom_focus_point);
+ shell->zoom_focus_point = point;
}
@@ -1372,16 +1395,16 @@ gimp_display_shell_scale_get_zoom_focus (GimpDisplayShell *shell,
gtk_get_event_widget (event) == shell->canvas ||
gtk_get_event_widget (event) == window)
{
- GdkPoint *point = g_queue_pop_head (shell->zoom_focus_pointer_queue);
- gint canvas_pointer_x;
- gint canvas_pointer_y;
+ gint canvas_pointer_x;
+ gint canvas_pointer_y;
- if (point)
+ if (shell->zoom_focus_point)
{
- canvas_pointer_x = point->x;
- canvas_pointer_y = point->y;
+ canvas_pointer_x = shell->zoom_focus_point->x;
+ canvas_pointer_y = shell->zoom_focus_point->y;
- g_slice_free (GdkPoint, point);
+ g_slice_free (GdkPoint, shell->zoom_focus_point);
+ shell->zoom_focus_point = NULL;
}
else
{
diff --git a/app/display/gimpdisplayshell.c b/app/display/gimpdisplayshell.c
index e0a553c21c..998500e874 100644
--- a/app/display/gimpdisplayshell.c
+++ b/app/display/gimpdisplayshell.c
@@ -374,7 +374,7 @@ gimp_display_shell_init (GimpDisplayShell *shell)
G_CALLBACK (gimp_display_shell_buffer_hover),
shell);
- shell->zoom_focus_pointer_queue = g_queue_new ();
+ shell->zoom_focus_point = NULL;
gtk_widget_set_events (GTK_WIDGET (shell), (GDK_POINTER_MOTION_MASK |
GDK_BUTTON_PRESS_MASK |
@@ -782,7 +782,11 @@ gimp_display_shell_dispose (GObject *object)
g_clear_object (&shell->motion_buffer);
- g_clear_pointer (&shell->zoom_focus_pointer_queue, g_queue_free);
+ if (shell->zoom_focus_point)
+ {
+ g_slice_free (GdkPoint, shell->zoom_focus_point);
+ shell->zoom_focus_point = NULL;
+ }
if (shell->title_idle_id)
{
diff --git a/app/display/gimpdisplayshell.h b/app/display/gimpdisplayshell.h
index 2f63a88c53..f9bf6360f0 100644
--- a/app/display/gimpdisplayshell.h
+++ b/app/display/gimpdisplayshell.h
@@ -232,7 +232,7 @@ struct _GimpDisplayShell
GimpMotionBuffer *motion_buffer;
- GQueue *zoom_focus_pointer_queue;
+ GdkPoint *zoom_focus_point;
gboolean blink;
guint blink_timeout_id;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]