[gedit] tab: a bug fix for the text-cut-off problem



commit 21ae0bab6bc0eb552b05a3cead0da2ac405aad2a
Author: Sébastien Wilmet <swilmet informatique-libre be>
Date:   Mon Jul 25 20:47:30 2022 +0200

    tab: a bug fix for the text-cut-off problem
    
    It's a hack, but it works in all situations, as far as I've tested.
    
    Hopefully a proper fix in GTK will be submitted (but it requires more
    investigation work in gtktextview.c).
    
    I will copy/paste my detailed debugging of this, written here:
    https://wiki.gnome.org/Apps/Gedit/FixingTextCutOffBug
    (a commit message or something in git has a better chance to still exist
    in the distant future).
    
    Fixes https://gitlab.gnome.org/GNOME/gedit/issues/42
    
    -----
    
    = Apps/Gedit/FixingTextCutOffBug =
    
    See:
     * https://gitlab.gnome.org/GNOME/gedit/-/blob/master/docs/common-bugs.md
     * https://gitlab.gnome.org/GNOME/gedit/issues/42
     * https://gitlab.gnome.org/GNOME/gedit/-/issues/42#note_774722 and further comments
    
    100% reproducible with certain files.
    
    Generating a file to reproduce the bug (here 100 lines, might need to be adapted for your screen size):
    {{{
    $ seq 1 100 > text-cut-off
    }}}
    
    Open the file from the terminal, with gedit window already maximized.
    
    Some printf-debugging is possible (gdb might not be appropriate because of switching windows between 
gedit and the terminal, which triggers different events in the gedit app).
    
    See the wip/text-cut-off git branches in gedit for some tests.
    
    == Workarounds ==
    
    === Workaround 1 ===
    
    {{{
    g_object_set (gtk_settings_get_default (),
                  "gtk-enable-animations", FALSE,
                  NULL);
    }}}
    
    === Workaround 2 ===
    
    Unrelated to workaround 1 (so it's an OR, not an AND).
    
    After loading a file, do not scroll to the cursor.
    
    == Hints for finding the location of the bug ==
    
    Most probably related to the '''combination''' of: animations, and scrolling the !GtkTextView (see the 
two workarounds above).
    
    In gtkadjustment.c? If gtk_adjustment_enable_animation() (internal function) does nothing, the bug 
doesn't occur.
    
    A small digression with a new mini Adjustment class (ideas such as: respect class invariants at all 
times, and store the original value before being clamped):
     * https://gitlab.gnome.org/swilmet/gte-adjustment
     * https://gitlab.gnome.org/swilmet/gte-adjustment/-/blob/main/NOTES.md
    
    == First conclusion ==
    
    It's related to !GtkAdjustment. Either its implementation, or its use by !GtkTextView.
    
    In gtktextview.c, after `s/gtk_adjustment_animate_to_value/gtk_adjustment_set_value/`, the bug doesn't 
occur anymore.
    
    == Locate the bug more precisely ==
    
    I think it's that ('''Update:''' yes it's that):
     * https://gitlab.gnome.org/GNOME/gtk/-/commit/9d7f1caca7073cdf4af3bd85b24dbe0209abdde2
    
    On the ''last'' call to gtk_text_view_size_allocate() (while the text is still cut off), the adjustments 
are animating, but ''some'' adjustment values need to change (at least the ''page_size'' needs to be updated 
to its final value).
    
    Because gtk_text_view_size_allocate() calls text_window_size_allocate(), which can change the 
SCREEN_HEIGHT(). And SCREEN_HEIGHT() is used as the vadjustment page-size.
    
    Suggestion (?): take into account gtk_adjustment_is_animating() ''inside'' 
gtk_text_view_set_vadjustment_values() (and for the horizontal one too).
    
    See comments in https://bugzilla.gnome.org/show_bug.cgi?id=733406
    
    == What happens in gedit ==
    
    Example:
    {{{
    GtkTextView vadjustment: value=0.0 lower=0.0 upper=2000.0 page_size=969.0
    
    Then gtk_text_view_size_allocate() is called one more time,
    but the vadjustment is animating, so the page_size is not updated correctly.
    The vadjustment value can go to (upper - page_size = 1031.0) max, so the text is cut off.
    
    After unmaximize/maximize the gedit window, we get the right page_size:
    
    GtkTextView vadjustment: value=0.0 lower=0.0 upper=2000.0 page_size=917.0
    
    upper - page_size = 1083.0, so value can go further and show the bottom of the text.
    }}}
    
    Even without animation, there is a problem to restore the cursor position to the last line(s) (even 
though the text is not cut off and can be scrolled down manually):
    {{{
    We request to scroll to the last line.
    
    GtkTextView vadjustment: value=1031.0 lower=0.0 upper=2000.0 page_size=969.0
    
    value has been clamped.
    
    Then we get the additional call to gtk_text_view_size_allocate(),
    and because animations are disabled here, the vadjustment is updated.
    
    But the value of 1031.0 is not updated, while page_size is:
    
    GtkTextView vadjustment: value=1031.0 lower=0.0 upper=2000.0 page_size=917.0
    
    To show the last line, it should instead be:
    
    [scrolling manually here]
    
    GtkTextView vadjustment: value=1083.0 lower=0.0 upper=2000.0 page_size=917.0
    }}}
    
    == Simple fix in gedit ==
    
    We ask to scroll to the cursor too early. By changing the idle_scroll to a timeout (250ms seems fine), 
everything works. But it's a hack.
    
    '''Edit:''' actually no, doesn't work with larger files, even though the timeout is dispatched (so 
gtk_text_view_scroll_to_mark() is called after 250ms but it doesn't scroll to the position, weird). Need more 
debugging...
    
    '''Edit 2''': wait idle -> wait timeout of 250ms -> finally scroll: that works in all cases that I've 
tested :-) (but it's even more hacky). It's because the idle takes longer than 250ms with larger files: if we 
launch both the timeout and the idle at the same time, we first get the timeout and then the idle (while with 
smaller files it's the reverse).
    
    == Why there is an extra call to gtk_text_view_size_allocate() ==
    
    We get the extra call to gtk_text_view_size_allocate() via gtk_container_idle_sizer() on the 
!GeditWindow. gtk_container_idle_sizer() is called during !GdkFrameClock::layout, which is thus called after 
the idle_scroll (idle_scroll from gedit-tab.c).
    
    '''Better solution:''' we can expect that gtk_text_view_scroll_to_mark() works in all situations. Maybe 
with an implementation that connects to the !GdkFrameClock (?).
    
    == gtk_text_view_flush_scroll() is maybe called too early ==
    
    With this code inserted at the beginning of gtk_text_view_flush_scroll():
    
    {{{
      if (text_view->priv->layout == NULL ||
          !gtk_text_layout_is_valid (text_view->priv->layout))
        g_warning ("%s() layout not valid", G_STRFUNC);
    }}}
    
    we get several warnings.
    
    gtk_text_view_scroll_to_mark() directly calls gtk_text_view_flush_scroll() if the layout is valid. (If 
not, gtk_text_view_flush_scroll() is called later on).

 gedit/gedit-tab.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)
---
diff --git a/gedit/gedit-tab.c b/gedit/gedit-tab.c
index bf70ce0c3..65f3953a2 100644
--- a/gedit/gedit-tab.c
+++ b/gedit/gedit-tab.c
@@ -60,7 +60,8 @@ struct _GeditTab
 
        GtkSourceFileSaverFlags save_flags;
 
-       guint idle_scroll;
+       guint scroll_timeout;
+       guint scroll_idle;
 
        gint auto_save_interval;
        guint auto_save_timeout;
@@ -330,10 +331,16 @@ gedit_tab_dispose (GObject *object)
 
        remove_auto_save_timeout (tab);
 
-       if (tab->idle_scroll != 0)
+       if (tab->scroll_timeout != 0)
        {
-               g_source_remove (tab->idle_scroll);
-               tab->idle_scroll = 0;
+               g_source_remove (tab->scroll_timeout);
+               tab->scroll_timeout = 0;
+       }
+
+       if (tab->scroll_idle != 0)
+       {
+               g_source_remove (tab->scroll_idle);
+               tab->scroll_idle = 0;
        }
 
        if (tab->cancellable != NULL)
@@ -1029,14 +1036,32 @@ should_show_progress_info (GTimer  **timer,
 }
 
 static gboolean
-scroll_to_cursor (GeditTab *tab)
+scroll_timeout_cb (GeditTab *tab)
 {
        GeditView *view;
 
        view = gedit_tab_get_view (tab);
        tepl_view_scroll_to_cursor (TEPL_VIEW (view));
 
-       tab->idle_scroll = 0;
+       tab->scroll_timeout = 0;
+       return G_SOURCE_REMOVE;
+}
+
+static gboolean
+scroll_idle_cb (GeditTab *tab)
+{
+       /* The idle is not enough, for a detailed analysis of this, see:
+        * https://wiki.gnome.org/Apps/Gedit/FixingTextCutOffBug
+        * or the commit message that changed this.
+        * (here it's a hack, a proper solution in GTK/GtkTextView should be
+        * found).
+        */
+       if (tab->scroll_timeout == 0)
+       {
+               tab->scroll_timeout = g_timeout_add (250, (GSourceFunc)scroll_timeout_cb, tab);
+       }
+
+       tab->scroll_idle = 0;
        return G_SOURCE_REMOVE;
 }
 
@@ -1685,9 +1710,9 @@ goto_line (GTask *loading_task)
         * an idle as after the document is loaded the textview is still
         * redrawing and relocating its internals.
         */
-       if (data->tab->idle_scroll == 0)
+       if (data->tab->scroll_idle == 0)
        {
-               data->tab->idle_scroll = g_idle_add ((GSourceFunc)scroll_to_cursor, data->tab);
+               data->tab->scroll_idle = g_idle_add ((GSourceFunc)scroll_idle_cb, data->tab);
        }
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]