dnrops
/
gimp
mirror of https://github.com/GNOME/gimp.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 7e3898da09), but it ended up being also used
internally (commit 792cd581a2). 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.
This commit is contained in:
Jehan 2021-08-20 20:09:50 +02:00
parent 23da87aab4
commit f5ea8e9b2a
3 changed files with 53 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
app/display/gimpdisplayshell-scale.c
View File

@ -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.0)
{
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)
{
gimp_display_shell_scale (shell,
GIMP_ZOOM_TO,
scale * 0.9,
GIMP_ZOOM_FOCUS_POINTER);
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
{

8
app/display/gimpdisplayshell.c
View File

@ -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)
{

2
app/display/gimpdisplayshell.h
View File

@ -232,7 +232,7 @@ struct _GimpDisplayShell
GimpMotionBuffer *motion_buffer;
GQueue *zoom_focus_pointer_queue;
GdkPoint *zoom_focus_point;
gboolean blink;
guint blink_timeout_id;