GNOME.org
 

[gimp] macos: Fix 7690 (slow drawing)

commit 1baeffc913884e3421a9ff458839cd55692efc48
Author: Lukas Oberhuber <lukaso gmail com>
Date:   Sat Feb 19 01:25:51 2022 +0000

    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 6de9ea70223 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.

 app/display/gimpstatusbar.c | 103 +++++++++++++++++++++++++++++++++-----------
 app/display/gimpstatusbar.h |   3 ++
 libgimpwidgets/gimpruler.c  |  55 ++++++++++++++++++++++-
 3 files changed, 133 insertions(+), 28 deletions(-)
---
diff --git a/app/display/gimpstatusbar.c b/app/display/gimpstatusbar.c
index a6b16b158e..df1ba12140 100644
--- a/app/display/gimpstatusbar.c
+++ b/app/display/gimpstatusbar.c
@@ -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);
+}
diff --git a/app/display/gimpstatusbar.h b/app/display/gimpstatusbar.h
index c1d9f78630..69646dd320 100644
--- a/app/display/gimpstatusbar.h
+++ b/app/display/gimpstatusbar.h
@@ -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;
diff --git a/libgimpwidgets/gimpruler.c b/libgimpwidgets/gimpruler.c
index 2c519132c2..670ced1e7d 100644
--- a/libgimpwidgets/gimpruler.c
+++ b/libgimpwidgets/gimpruler.c
@@ -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;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]