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

macos: Fix 7690 (slow drawing) 

Gets drawing in the canvas speed on retina displays up to the speed
of FullHD displays on macOS, making 2.99 usable on macOS.

Generic change:

Changes the cursor_label to delay the drawing phase to the idle
queue, from immediate draw on all platforms.

Before the fix in 32049afd (using a deprecated function in Gtk3)
any draws on this label forced a full canvas redraw. This is due
to a quirk in how GtkLabel functions.

The redraw occurred because GtkLabels resize themselves and everything
around them by sending a resize  message any time they receive new
text. These resizes then trigger the full canvas resize which triggers
a full canvas redraw that cannot be optimized by either Gtk or Big Sur.

MacOS changes:

Only redraws the cursor position label and each of the horizontal and
vertical rules (cursor tracking widgets) 3 times a second max for a
total of 9 redraws a second (ideally out of 60, though I don't believe
under any circumstances that GIMP achieves a 60fps).

Each of the cursor tracking widgets gets its own timeslice, and so
will not redraw when the other cursor tracking widgets are drawing.

This is required because Big Sur is merging all draw rects into
one large rect, dramatically slowing down draws.

This timeslicing ensures that draw rects are maintained at the smallest
possible size. So the typical redraw is a small rect around the
brush. However, 9 times a second, the rect will include one of the
3 cursor tracking widgets (rulers and cursor label).

Additionally, the code tries to minimize resizing the width of the
cursor label by checking if the widget is too small for the text,
then setting the char_width to a greater size so that resizes won't
be that common.

This improves the appearance of the widget as it no longer
constantly jumps about in size on each cursor move.

Here is a discussion of the issue:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445

Reviewer's (Jehan) notes:

* The whole issue about GtkLabel resizing is no more after 32049afd. It 
  is normal for a widget to request a resize when needed. We just don't
  want the statusbar to resize and triggering canvas redraws.
* Changing cursor position text into an idle function generally makes
  sense.
  Also it reverts commit 6de9ea7022 which had a bug I hadn't realized
  when I accepted it: when we test for time, we don't know yet if it
  will be the last position change, hence we could "refuse" the last
  update. Therefore displayed cursor position would end up outdated
  on macOS. This new implementation doesn't have the problem (the last
  idle update always happens after the last move).
* The change about giving 1/3 timeslices to side canvas components 
  (rulers and statusbar) is a complete hack to work around the fact that
  macOs doesn't report properly each damaged rectangle. Instead it
  returns a huge bounding box. The workaround here is to expose these
  area separately.
  We have not been able to find a proper solution yet. This is the only
  reason why I accept this code, for macOS only, to at least have
  something usable there.
  See discussions in MRs gimp!572 and gimp-macos-build!86. With these 2 
  MRs, Lukas reported GIMP 2.99 to perform even better than GIMP 2.10 on
  Monterey, though it could not be tested on Big Sur unfortunately.
* Lastly the set_width_chars() thing is also an ugly hack which I will
  try later to revisit (see !581). I only accepted it (with mandatory 
  macOS-only macro) to have an acceptable state for release after seeing
  a screencast where the label size indeed "jumps around" on macOS.
This commit is contained in:
Lukas Oberhuber 2022-02-19 01:25:51 +00:00 committed by Jehan
parent 7056f1b960
commit 1baeffc913
3 changed files with 133 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
app/display/gimpstatusbar.c
View File

@ -152,6 +152,11 @@ static gchar * gimp_statusbar_vprintf (const gchar *format,
static GdkPixbuf * gimp_statusbar_load_icon (GimpStatusbar *statusbar,
const gchar *icon_name);
static void gimp_statusbar_cursor_label_set_text (GimpStatusbar *statusbar,
gchar *buffer);
static gboolean gimp_statusbar_queue_pos_redraw (gpointer data);
G_DEFINE_TYPE_WITH_CODE (GimpStatusbar, gimp_statusbar, GTK_TYPE_FRAME,
G_IMPLEMENT_INTERFACE (GIMP_TYPE_PROGRESS,
@ -395,6 +400,12 @@ gimp_statusbar_dispose (GObject *object)
statusbar->temp_timeout_id = 0;
}
if (statusbar->statusbar_pos_redraw_idle_id)
{
g_source_remove (statusbar->statusbar_pos_redraw_idle_id);
statusbar->statusbar_pos_redraw_idle_id = 0;
}
g_clear_pointer (&statusbar->size_widgets, g_slist_free);
G_OBJECT_CLASS (parent_class)->dispose (object);
@ -1232,32 +1243,8 @@ gimp_statusbar_update_cursor (GimpStatusbar *statusbar,
GimpImage *image;
gchar buffer[CURSOR_LEN];
#ifdef GDK_WINDOWING_QUARTZ
/*
* This optimization dramatically improves drawing refresh speed on Macs with retina
* displays, which is all macbook pros since 2016 and macbook airs since 2018 and
* running Big Sur (released Nov 2020) or higher.
* https://gitlab.gnome.org/GNOME/gimp/-/issues/7690
*/
gint64 curr_time = g_get_monotonic_time ();
#endif
g_return_if_fail (GIMP_IS_STATUSBAR (statusbar));
#ifdef GDK_WINDOWING_QUARTZ
/*
* This optimization dramatically improves drawing refresh speed on Macs with retina
* displays, which is all macbook pros since 2016 and macbook airs since 2018 and
* running Big Sur (released Nov 2020) or higher.
* https://gitlab.gnome.org/GNOME/gimp/-/issues/7690
*/
/* only redraw max every 100ms */
if (curr_time - statusbar->last_frame_time < 1000 * 300)
return;
statusbar->last_frame_time = curr_time;
#endif
shell = statusbar->shell;
image = gimp_display_get_image (shell->display);
@ -1321,13 +1308,26 @@ gimp_statusbar_update_cursor (GimpStatusbar *statusbar,
"", x, ", ", y, "");
}
gtk_label_set_text (GTK_LABEL (statusbar->cursor_label), buffer);
if (g_strcmp0 (buffer, statusbar->cursor_string_last) == 0)
return;
g_free (statusbar->cursor_string_todraw);
statusbar->cursor_string_todraw = g_strdup (buffer);
if (statusbar->statusbar_pos_redraw_idle_id == 0)
{
statusbar->statusbar_pos_redraw_idle_id =
g_idle_add_full (G_PRIORITY_LOW,
gimp_statusbar_queue_pos_redraw,
statusbar,
NULL);
}
}
void
gimp_statusbar_clear_cursor (GimpStatusbar *statusbar)
{
gtk_label_set_text (GTK_LABEL (statusbar->cursor_label), "");
gimp_statusbar_cursor_label_set_text (statusbar, "");
gtk_widget_set_sensitive (statusbar->cursor_label, TRUE);
}
@ -1797,3 +1797,54 @@ gimp_statusbar_load_icon (GimpStatusbar *statusbar,
return icon;
}
static gboolean gimp_statusbar_queue_pos_redraw (gpointer data)
{
GimpStatusbar *statusbar = GIMP_STATUSBAR (data);
#ifdef GDK_WINDOWING_QUARTZ
/*
* This optimization dramatically improves drawing refresh speed on Macs with retina
* displays, which is all macbook pros since 2016 and macbook airs since 2018 and
* running Big Sur (released Nov 2020) or higher.
* https://gitlab.gnome.org/GNOME/gimp/-/issues/7690
*
* only redraw max every 333ms and only redraw when the other two decorations aren't
* redrawing (cursor_label, horizontal and vertical rulers). This will keep the draw
* rects of limited size.
*/
gint64 curr_time = g_get_monotonic_time ();
gint64 mod_time = curr_time % G_TIME_SPAN_SECOND;
gint timeslice_num = mod_time / (G_TIME_SPAN_SECOND / 9) % 3;
if (curr_time - statusbar->last_frame_time < G_TIME_SPAN_SECOND / 3 || timeslice_num != 0)
return G_SOURCE_CONTINUE;
statusbar->last_frame_time = curr_time;
#endif
g_free (statusbar->cursor_string_last);
gimp_statusbar_cursor_label_set_text (statusbar, statusbar->cursor_string_todraw);
statusbar->cursor_string_last = statusbar->cursor_string_todraw;
statusbar->cursor_string_todraw = NULL;
statusbar->statusbar_pos_redraw_idle_id = 0;
return G_SOURCE_REMOVE;
}
static void gimp_statusbar_cursor_label_set_text (GimpStatusbar *statusbar, gchar *buffer)
{
#ifdef GDK_WINDOWING_QUARTZ
/*
* Without this code, on MacOS the cursor label leaps about in width as the label width is
* closely wrapped to the content which changes with each cursor move.
* Here is a discussion of the issue:
* https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/572#note_1389445
*/
if (gtk_label_get_width_chars (GTK_LABEL (statusbar->cursor_label)) <= (gint)strlen (buffer) + 4)
gtk_label_set_width_chars (GTK_LABEL (statusbar->cursor_label), strlen (buffer) + 6);
#endif
gtk_label_set_text (GTK_LABEL (statusbar->cursor_label), buffer);
}

3
app/display/gimpstatusbar.h
View File

@ -53,6 +53,9 @@ struct _GimpStatusbar
*/
gint64 last_frame_time;
#endif
guint statusbar_pos_redraw_idle_id;
gchar *cursor_string_todraw;
gchar *cursor_string_last;
GdkPixbuf *icon;
GHashTable *icon_hash;

55
libgimpwidgets/gimpruler.c
View File

@ -77,6 +77,16 @@ struct _GimpRulerPrivate
PangoLayout *layout;
GList *track_widgets;
#ifdef GDK_WINDOWING_QUARTZ
/*
* This optimization dramatically improves drawing refresh speed on Macs with retina
* displays, which is all macbook pros since 2016 and macbook airs since 2018 and
* running Big Sur (released Nov 2020) or higher.
* https://gitlab.gnome.org/GNOME/gimp/-/issues/7690
*/
gint64 last_frame_time;
#endif
};
#define GET_PRIVATE(obj) (((GimpRuler *) (obj))->priv)
@ -652,6 +662,7 @@ gimp_ruler_set_position (GimpRuler *ruler,
xdiff = rect.x - priv->last_pos_rect.x;
ydiff = rect.y - priv->last_pos_rect.y;
#ifndef GDK_WINDOWING_QUARTZ
/*
* If the position has changed far enough, queue a redraw immediately.
* Otherwise, we only queue a redraw in a low priority idle handler, to
@ -674,6 +685,15 @@ gimp_ruler_set_position (GimpRuler *ruler,
gimp_ruler_queue_pos_redraw (ruler);
}
else if (! priv->pos_redraw_idle_id)
#else
/*
* pos_redraw_idle_id being set can be counted on to mean
* a redraw is needed (on mac only) since we will not do
* a high priority draws due to the dramatic performance
* they will have.
*/
if (! priv->pos_redraw_idle_id)
#endif
{
priv->pos_redraw_idle_id =
g_idle_add_full (G_PRIORITY_LOW,
@ -1251,11 +1271,42 @@ gimp_ruler_get_pos_rect (GimpRuler *ruler,
static gboolean
gimp_ruler_idle_queue_pos_redraw (gpointer data)
{
GimpRuler *ruler = data;
GimpRulerPrivate *priv = GET_PRIVATE (ruler);
GimpRuler *ruler = data;
GimpRulerPrivate *priv = GET_PRIVATE (ruler);
#ifdef GDK_WINDOWING_QUARTZ
/*
* This optimization dramatically improves drawing refresh speed on Macs with retina
* displays, which is all macbook pros since 2016 and macbook airs since 2018 and
* running Big Sur (released Nov 2020) or higher.
* https://gitlab.gnome.org/GNOME/gimp/-/issues/7690
*
* only redraw max every 333ms and only redraw when the other two decorations aren't
* redrawing (cursor_label, horizontal and vertical rulers). This will keep the draw
* rects of limited size.
*/
gint64 curr_time = g_get_monotonic_time ();
gint64 mod_time = curr_time % G_TIME_SPAN_SECOND;
gint timeslice_num = mod_time / (G_TIME_SPAN_SECOND / 9) % 3;
if (curr_time - priv->last_frame_time < G_TIME_SPAN_SECOND / 3)
return G_SOURCE_CONTINUE;
if (priv->orientation == GTK_ORIENTATION_HORIZONTAL && timeslice_num != 1)
return G_SOURCE_CONTINUE;
if (priv->orientation == GTK_ORIENTATION_VERTICAL && timeslice_num != 2)
return G_SOURCE_CONTINUE;
priv->last_frame_time = curr_time;
#endif
gimp_ruler_queue_pos_redraw (ruler);
/*
* pos_redraw_idle_id being set can be counted on to mean
* a redraw is needed (on mac only) since we will not do
* a high priority draws due to the dramatic performance
* they will have.
*/
priv->pos_redraw_idle_id = 0;
return G_SOURCE_REMOVE;