[vte/wip/issue-335: 3/6] widget: Move GtkAdjustment handling to Widget

[Christian Persch <chpe src gnome org>]

commit ca3528138b9a7f5eaf15338d49f0fbafed386b5c
Author: Christian Persch <chpe src gnome org>
Date:   Thu Mar 4 23:17:41 2021 +0100

    widget: Move GtkAdjustment handling to Widget

 src/vte.cc         | 197 ++++++++++-------------------------------------------
 src/vteinternal.hh |  12 +---
 src/vteseq.cc      |   3 +-
 src/widget.cc      | 136 ++++++++++++++++++++++++++++++++++++
 src/widget.hh      |  12 +++-
 5 files changed, 187 insertions(+), 173 deletions(-)
---
diff --git a/src/vte.cc b/src/vte.cc
index 534b722c..53ccf0cc 100644
--- a/src/vte.cc
+++ b/src/vte.cc
@@ -1941,92 +1941,22 @@ Terminal::regex_match_check_extra(vte::grid::column_t col,
 void
 Terminal::emit_adjustment_changed()
 {
-       if (m_adjustment_changed_pending) {
-               bool changed = false;
-               gdouble current, v;
-
-                auto vadjustment = m_vadjustment.get();
-
-                auto const freezer = vte::glib::FreezeObjectNotify{vadjustment};
-
-               v = _vte_ring_delta (m_screen->row_data);
-                current = gtk_adjustment_get_lower(vadjustment);
-               if (!_vte_double_equal(current, v)) {
-                       _vte_debug_print(VTE_DEBUG_ADJ,
-                                       "Changing lower bound from %.0f to %f\n",
-                                        current, v);
-                        gtk_adjustment_set_lower(vadjustment, v);
-                       changed = true;
-               }
-
-               v = m_screen->insert_delta + m_row_count;
-                current = gtk_adjustment_get_upper(vadjustment);
-               if (!_vte_double_equal(current, v)) {
-                       _vte_debug_print(VTE_DEBUG_ADJ,
-                                       "Changing upper bound from %.0f to %f\n",
-                                        current, v);
-                        gtk_adjustment_set_upper(vadjustment, v);
-                       changed = true;
-               }
+        if (!widget())
+                return;
 
-               /* The step increment should always be one. */
-                v = gtk_adjustment_get_step_increment(vadjustment);
-               if (!_vte_double_equal(v, 1)) {
-                       _vte_debug_print(VTE_DEBUG_ADJ,
-                                       "Changing step increment from %.0lf to 1\n", v);
-                        gtk_adjustment_set_step_increment(vadjustment, 1);
-                       changed = true;
-               }
+        if (m_adjustment_changed_pending) {
+                m_adjustment_changed_pending = false;
 
-               /* Set the number of rows the user sees to the number of rows the
-                * user sees. */
-                v = gtk_adjustment_get_page_size(vadjustment);
-               if (!_vte_double_equal(v, m_row_count)) {
-                       _vte_debug_print(VTE_DEBUG_ADJ,
-                                       "Changing page size from %.0f to %ld\n",
-                                        v, m_row_count);
-                        gtk_adjustment_set_page_size(vadjustment,
-                                                    m_row_count);
-                       changed = true;
-               }
+                auto const lower = _vte_ring_delta (m_screen->row_data);
+                auto const upper = m_screen->insert_delta + m_row_count;
+                widget()->notify_scroll_bounds_changed({lower}, {upper}, {m_row_count});
+        }
 
-               /* Clicking in the empty area should scroll one screen, so set the
-                * page size to the number of visible rows. */
-                v = gtk_adjustment_get_page_increment(vadjustment);
-               if (!_vte_double_equal(v, m_row_count)) {
-                       _vte_debug_print(VTE_DEBUG_ADJ,
-                                       "Changing page increment from "
-                                       "%.0f to %ld\n",
-                                       v, m_row_count);
-                        gtk_adjustment_set_page_increment(vadjustment,
-                                                         m_row_count);
-                       changed = true;
-               }
+        if (m_adjustment_value_changed_pending) {
+                m_adjustment_value_changed_pending = false;
 
-               if (changed)
-                       _vte_debug_print(VTE_DEBUG_SIGNALS,
-                                       "Emitting adjustment_changed.\n");
-               m_adjustment_changed_pending = FALSE;
-       }
-       if (m_adjustment_value_changed_pending) {
-               double v, delta;
-               _vte_debug_print(VTE_DEBUG_SIGNALS,
-                               "Emitting adjustment_value_changed.\n");
-               m_adjustment_value_changed_pending = FALSE;
-
-                auto vadjustment = m_vadjustment.get();
-                v = gtk_adjustment_get_value(vadjustment);
-               if (!_vte_double_equal(v, m_screen->scroll_delta)) {
-                       /* this little dance is so that the scroll_delta is
-                        * updated immediately, but we still handled scrolling
-                        * via the adjustment - e.g. user interaction with the
-                        * scrollbar
-                        */
-                       delta = m_screen->scroll_delta;
-                       m_screen->scroll_delta = v;
-                        gtk_adjustment_set_value(vadjustment, delta);
-               }
-       }
+                widget()->notify_scroll_value_changed(m_screen->scroll_delta);
+        }
 }
 
 /* Queue an adjustment-changed signal to be delivered when convenient. */
@@ -2054,12 +1984,11 @@ Terminal::queue_adjustment_value_changed(double v)
 void
 Terminal::queue_adjustment_value_changed_clamped(double v)
 {
-        auto vadjustment = m_vadjustment.get();
-        auto const lower = gtk_adjustment_get_lower(vadjustment);
-        auto const upper = gtk_adjustment_get_upper(vadjustment);
-
-       v = CLAMP(v, lower, MAX (lower, upper - m_row_count));
+        auto const lower = _vte_ring_delta (m_screen->row_data);
 
+        v = std::clamp(v,
+                       double(lower),
+                       double(std::max(lower, m_screen->insert_delta)));
        queue_adjustment_value_changed(v);
 }
 
@@ -6726,21 +6655,19 @@ Terminal::mouse_autoscroll_timer_callback()
 
        /* Provide an immediate effect for mouse wigglers. */
        if (m_mouse_last_position.y < 0) {
-               if (m_vadjustment) {
-                       /* Try to scroll up by one line. */
-                       adj = m_screen->scroll_delta - 1;
-                       queue_adjustment_value_changed_clamped(adj);
-                       extend = true;
-               }
+                /* Try to scroll up by one line. */
+                adj = m_screen->scroll_delta - 1;
+                queue_adjustment_value_changed_clamped(adj);
+                extend = true;
+
                _vte_debug_print(VTE_DEBUG_EVENTS, "Autoscrolling down.\n");
        }
        if (m_mouse_last_position.y >= m_view_usable_extents.height()) {
-               if (m_vadjustment) {
-                       /* Try to scroll up by one line. */
-                       adj = m_screen->scroll_delta + 1;
-                       queue_adjustment_value_changed_clamped(adj);
-                       extend = true;
-               }
+                /* Try to scroll up by one line. */
+                adj = m_screen->scroll_delta + 1;
+                queue_adjustment_value_changed_clamped(adj);
+                extend = true;
+
                _vte_debug_print(VTE_DEBUG_EVENTS, "Autoscrolling up.\n");
        }
        if (extend) {
@@ -7694,25 +7621,13 @@ Terminal::set_size(long columns,
        }
 }
 
-/* Redraw the widget. */
-static void
-vte_terminal_vadjustment_value_changed_cb(vte::terminal::Terminal* that) noexcept
-try
-{
-        that->vadjustment_value_changed();
-}
-catch (...)
-{
-        vte::log_exception();
-}
-
 void
-Terminal::vadjustment_value_changed()
+Terminal::set_scroll_value(double value)
 {
-       /* Read the new adjustment value and save the difference. */
-        auto const adj = gtk_adjustment_get_value(m_vadjustment.get());
-       double dy = adj - m_screen->scroll_delta;
-       m_screen->scroll_delta = adj;
+       /* Save the difference. */
+       auto const dy = value - m_screen->scroll_delta;
+
+       m_screen->scroll_delta = value;
 
        /* Sanity checks. */
         if (G_UNLIKELY(!widget_realized()))
@@ -7721,7 +7636,7 @@ Terminal::vadjustment_value_changed()
         /* FIXME: do this check in pixel space */
        if (!_vte_double_equal(dy, 0)) {
                _vte_debug_print(VTE_DEBUG_ADJ,
-                           "Scrolling by %f\n", dy);
+                                 "Scrolling by %f\n", dy);
 
                 invalidate_all();
                 match_contents_clear();
@@ -7732,33 +7647,6 @@ Terminal::vadjustment_value_changed()
        }
 }
 
-void
-Terminal::widget_set_vadjustment(vte::glib::RefPtr<GtkAdjustment>&& adjustment)
-{
-        if (adjustment && adjustment == m_vadjustment)
-                return;
-        if (!adjustment && m_vadjustment)
-                return;
-
-        if (m_vadjustment) {
-               /* Disconnect our signal handlers from this object. */
-                g_signal_handlers_disconnect_by_func(m_vadjustment.get(),
-                                                    (void*)vte_terminal_vadjustment_value_changed_cb,
-                                                    this);
-       }
-
-        if (adjustment)
-                m_vadjustment = std::move(adjustment);
-        else
-                m_vadjustment = vte::glib::make_ref_sink(GTK_ADJUSTMENT(gtk_adjustment_new(0, 0, 0, 0, 0, 
0)));
-
-       /* We care about the offset, not the top or bottom. */
-        g_signal_connect_swapped(m_vadjustment.get(),
-                                "value-changed",
-                                G_CALLBACK(vte_terminal_vadjustment_value_changed_cb),
-                                this);
-}
-
 Terminal::Terminal(vte::platform::Widget* w,
                    VteTerminal *t) :
         m_real_widget(w),
@@ -7768,8 +7656,6 @@ Terminal::Terminal(vte::platform::Widget* w,
         m_alternate_screen(VTE_ROWS, false),
         m_screen(&m_normal_screen)
 {
-       widget_set_vadjustment({});
-
         /* Inits allocation to 1x1 @ -1,-1 */
         cairo_rectangle_int_t allocation;
         gtk_widget_get_allocation(m_widget, &allocation);
@@ -8054,7 +7940,7 @@ Terminal::~Terminal()
        stop_autoscroll();
 
        /* Cancel pending adjustment change notifications. */
-       m_adjustment_changed_pending = FALSE;
+       m_adjustment_changed_pending = false;
 
        /* Free any selected text, but if we currently own the selection,
         * throw the text onto the clipboard without an owner so that it
@@ -8087,14 +7973,6 @@ Terminal::~Terminal()
        _vte_byte_array_free(m_outgoing);
         m_outgoing = nullptr;
 
-       /* Free public-facing data. */
-        if (m_vadjustment) {
-               /* Disconnect our signal handlers from this object. */
-                g_signal_handlers_disconnect_by_func(m_vadjustment.get(),
-                                                    (void*)vte_terminal_vadjustment_value_changed_cb,
-                                                    this);
-       }
-
         /* Update rects */
         g_array_free(m_update_rects, TRUE /* free segment */);
 }
@@ -9531,7 +9409,7 @@ Terminal::widget_mouse_scroll(vte::platform::ScrollEvent const& event)
                return true;
        }
 
-        v = MAX (1., ceil (gtk_adjustment_get_page_increment (m_vadjustment.get()) / 10.));
+        v = MAX (1., ceil (m_row_count /* page increment */ / 10.));
        _vte_debug_print(VTE_DEBUG_EVENTS,
                        "Scroll speed is %d lines per non-smooth scroll unit\n",
                        (int) v);
@@ -10768,7 +10646,6 @@ Terminal::search_rows(pcre2_match_context_8 *match_context,
        long start_col, end_col;
        VteCharAttributes *ca;
        GArray *attrs;
-       gdouble value, page_size;
 
        auto row_text = get_text(start_row, 0,
                                  end_row, 0,
@@ -10837,9 +10714,9 @@ Terminal::search_rows(pcre2_match_context_8 *match_context,
        g_string_free (row_text, TRUE);
 
        select_text(start_col, start_row, end_col, end_row);
-       /* Quite possibly the math here should not access adjustment directly... */
-        value = gtk_adjustment_get_value(m_vadjustment.get());
-        page_size = gtk_adjustment_get_page_size(m_vadjustment.get());
+       /* Quite possibly the math here should not access the scroll values directly... */
+        auto const value = m_screen->scroll_delta;
+        auto const page_size = m_row_count;
        if (backward) {
                if (end_row < value || end_row > value + page_size - 1)
                        queue_adjustment_value_changed_clamped(end_row - page_size + 1);
diff --git a/src/vteinternal.hh b/src/vteinternal.hh
index e5dd7fe3..878c4196 100644
--- a/src/vteinternal.hh
+++ b/src/vteinternal.hh
@@ -657,8 +657,8 @@ public:
         int m_im_preedit_cursor;
 
         /* Adjustment updates pending. */
-        gboolean m_adjustment_changed_pending;
-        gboolean m_adjustment_value_changed_pending;
+        bool m_adjustment_changed_pending;
+        bool m_adjustment_value_changed_pending;
         gboolean m_cursor_moved_pending;
         gboolean m_contents_changed_pending;
 
@@ -713,9 +713,6 @@ public:
 #endif
         auto padding() const noexcept { return &m_padding; }
 
-        vte::glib::RefPtr<GtkAdjustment> m_vadjustment{};
-        auto vadjustment() noexcept { return m_vadjustment.get(); }
-
         /* Hyperlinks */
         bool m_allow_hyperlink{false};
         vte::base::Ring::hyperlink_idx_t m_hyperlink_hover_idx;
@@ -878,8 +875,6 @@ public:
                                                                   vte::platform::ClipboardFormat format);
         void widget_clipboard_data_clear(vte::platform::Clipboard const& clipboard);
 
-        void widget_set_vadjustment(vte::glib::RefPtr<GtkAdjustment>&& adjustment);
-
         void widget_realize();
         void widget_unrealize();
         void widget_unmap();
@@ -1097,8 +1092,6 @@ public:
                              long old_rows,
                              bool do_rewrap);
 
-        void vadjustment_value_changed();
-
         unsigned translate_ctrlkey(vte::platform::KeyEvent const& event) const noexcept;
 
         void apply_mouse_cursor();
@@ -1106,6 +1099,7 @@ public:
 
         void beep();
 
+        void set_scroll_value(double value);
         void emit_adjustment_changed();
         void emit_commit(std::string_view const& str);
         void emit_eof();
diff --git a/src/vteseq.cc b/src/vteseq.cc
index 2a8d777d..833167c6 100644
--- a/src/vteseq.cc
+++ b/src/vteseq.cc
@@ -545,8 +545,7 @@ Terminal::set_mode_private(int mode,
                 }
 
                 /* Reset scrollbars and repaint everything. */
-                gtk_adjustment_set_value(m_vadjustment.get(),
-                                         m_screen->scroll_delta);
+                queue_adjustment_value_changed(m_screen->scroll_delta);
                 set_scrollback_lines(m_scrollback_lines);
                 queue_contents_changed();
                 invalidate_all();
diff --git a/src/widget.cc b/src/widget.cc
index 6c5ca11c..b9265750 100644
--- a/src/widget.cc
+++ b/src/widget.cc
@@ -30,6 +30,7 @@
 #include "vtegtk.hh"
 #include "vteptyinternal.hh"
 #include "debug.h"
+#include "gobject-glue.hh"
 
 #if VTE_GTK == 4
 #include "graphene-glue.hh"
@@ -47,6 +48,8 @@ namespace vte {
 
 namespace platform {
 
+static void vadjustment_value_changed_cb(vte::platform::Widget* that) noexcept;
+
 static void
 im_commit_cb(GtkIMContext* im_context,
              char const* text,
@@ -151,6 +154,9 @@ Widget::Widget(VteTerminal* t)
           m_hscroll_policy{GTK_SCROLL_NATURAL},
           m_vscroll_policy{GTK_SCROLL_NATURAL}
 {
+        // Create a default adjustment
+        set_vadjustment({});
+
 #if VTE_GTK == 3
         gtk_widget_set_can_focus(gtk(), true);
 #endif
@@ -178,6 +184,12 @@ try
                                              0, 0, NULL, NULL,
                                              this);
 
+        if (m_vadjustment) {
+                g_signal_handlers_disconnect_by_func(m_vadjustment.get(),
+                                                     (void*)vadjustment_value_changed_cb,
+                                                     this);
+        }
+
         m_widget = nullptr;
 
         m_terminal->~Terminal();
@@ -724,6 +736,82 @@ Widget::mouse_event_from_gdk(GdkEvent* event) const /* throws */
                 y};
 }
 
+/* Emit an adjustment changed signal on our adjustment object. */
+void
+Widget::notify_scroll_bounds_changed(long lower,
+                                     long upper,
+                                     long row_count)
+{
+        auto const freezer = vte::glib::FreezeObjectNotify{m_vadjustment.get()};
+        auto changed = false;
+
+        auto const dlower = double(lower);
+        auto const dupper = double(upper);
+
+        auto current = gtk_adjustment_get_lower(m_vadjustment.get());
+        if (!_vte_double_equal(current, dlower)) {
+                _vte_debug_print(VTE_DEBUG_ADJ,
+                                 "Changing lower bound from %.0f to %f\n",
+                                 current, dlower);
+                gtk_adjustment_set_lower(m_vadjustment.get(), dlower);
+                changed = true;
+        }
+
+        current = gtk_adjustment_get_upper(m_vadjustment.get());
+        if (!_vte_double_equal(current, dupper)) {
+                _vte_debug_print(VTE_DEBUG_ADJ,
+                                 "Changing upper bound from %.0f to %f\n",
+                                 current, dupper);
+                gtk_adjustment_set_upper(m_vadjustment.get(), dupper);
+                changed = true;
+        }
+
+        /* The step increment should always be one. */
+        auto v = gtk_adjustment_get_step_increment(m_vadjustment.get());
+        if (!_vte_double_equal(v, 1.)) {
+                _vte_debug_print(VTE_DEBUG_ADJ,
+                                 "Changing step increment from %.0lf to 1.0\n", v);
+                gtk_adjustment_set_step_increment(m_vadjustment.get(), 1.);
+                changed = true;
+        }
+
+        v = gtk_adjustment_get_page_size(m_vadjustment.get());
+        if (!_vte_double_equal(v, row_count)) {
+                _vte_debug_print(VTE_DEBUG_ADJ,
+                                 "Changing page size from %.0f to %ld\n",
+                                 v, row_count);
+                gtk_adjustment_set_page_size(m_vadjustment.get(), row_count);
+                changed = true;
+        }
+
+        /* Clicking in the empty area should scroll exactly one screen,
+         * so set the page size to the number of visible rows.
+         */
+        v = gtk_adjustment_get_page_increment(m_vadjustment.get());
+        if (!_vte_double_equal(v, row_count)) {
+                _vte_debug_print(VTE_DEBUG_ADJ,
+                                 "Changing page increment from "
+                                 "%.0f to %ld\n",
+                                 v, row_count);
+                gtk_adjustment_set_page_increment(m_vadjustment.get(), row_count);
+                changed = true;
+        }
+
+        if (changed)
+                _vte_debug_print(VTE_DEBUG_SIGNALS,
+                                 "Adjustment changed.\n");
+}
+
+void
+Widget::notify_scroll_value_changed(double value)
+{
+        auto const v = gtk_adjustment_get_value(m_vadjustment.get());
+        if (!_vte_double_equal(v, value)) {
+                /* Note that this will generate a 'value-changed' signal */
+                gtk_adjustment_set_value(m_vadjustment.get(), value);
+        }
+}
+
 ScrollEvent
 Widget::scroll_event_from_gdk(GdkEvent* event) const /* throws */
 {
@@ -1041,6 +1129,44 @@ Widget::set_pty(VtePty* pty_obj) noexcept
         return true;
 }
 
+
+static void
+vadjustment_value_changed_cb(vte::platform::Widget* that) noexcept
+try
+{
+        that->vadjustment_value_changed();
+}
+catch (...)
+{
+        vte::log_exception();
+}
+
+void
+Widget::set_vadjustment(vte::glib::RefPtr<GtkAdjustment> adjustment)
+{
+        if (adjustment && adjustment == m_vadjustment)
+                return;
+        if (!adjustment && m_vadjustment)
+                return;
+
+        if (m_vadjustment) {
+                g_signal_handlers_disconnect_by_func(m_vadjustment.get(),
+                                                     (void*)vadjustment_value_changed_cb,
+                                                     this);
+        }
+
+        if (adjustment)
+                m_vadjustment = std::move(adjustment);
+        else
+                m_vadjustment = vte::glib::make_ref_sink(GTK_ADJUSTMENT(gtk_adjustment_new(0, 0, 0, 0, 0, 
0)));
+
+        /* We care about the offset only, not the top or bottom. */
+        g_signal_connect_swapped(m_vadjustment.get(),
+                                 "value-changed",
+                                 G_CALLBACK(vadjustment_value_changed_cb),
+                                 this);
+}
+
 bool
 Widget::set_word_char_exceptions(std::optional<std::string_view> stropt)
 {
@@ -1252,6 +1378,16 @@ Widget::unroot()
 
 #endif /* VTE_GTK == 4 */
 
+void
+Widget::vadjustment_value_changed()
+{
+        if (!m_terminal)
+                return;
+
+        auto const adj = gtk_adjustment_get_value(m_vadjustment.get());
+        m_terminal->set_scroll_value(adj);
+}
+
 } // namespace platform
 
 } // namespace vte
diff --git a/src/widget.hh b/src/widget.hh
index 813b3625..5e9b5a59 100644
--- a/src/widget.hh
+++ b/src/widget.hh
@@ -343,9 +343,9 @@ public:
         void beep() noexcept;
 
         void set_hadjustment(vte::glib::RefPtr<GtkAdjustment> adjustment) noexcept { m_hadjustment = 
std::move(adjustment); }
-        void set_vadjustment(vte::glib::RefPtr<GtkAdjustment> adjustment) { 
terminal()->widget_set_vadjustment(std::move(adjustment)); }
+        void set_vadjustment(vte::glib::RefPtr<GtkAdjustment> adjustment);
         auto hadjustment() noexcept { return m_hadjustment.get(); }
-        auto vadjustment() noexcept { return terminal()->vadjustment(); }
+        auto vadjustment() noexcept { return m_vadjustment.get(); }
         void set_hscroll_policy(GtkScrollablePolicy policy);
         void set_vscroll_policy(GtkScrollablePolicy policy);
         auto hscroll_policy() const noexcept { return m_hscroll_policy; }
@@ -464,8 +464,14 @@ protected:
 
         unsigned key_event_translate_ctrlkey(KeyEvent const& event) const noexcept;
 
+        void notify_scroll_bounds_changed(long lower,
+                                          long upper,
+                                          long row_count);
+        void notify_scroll_value_changed(double value);
+
 public: // FIXMEchpe
         void im_preedit_changed() noexcept;
+        void vadjustment_value_changed();
 
 private:
         KeyEvent key_event_from_gdk(GdkEvent* event) const;
@@ -513,7 +519,9 @@ private:
         /* Misc */
         std::optional<std::string> m_word_char_exceptions{};
 
+        vte::glib::RefPtr<GtkAdjustment> m_vadjustment{};
         vte::glib::RefPtr<GtkAdjustment> m_hadjustment{};
+
         uint32_t m_hscroll_policy : 1;
         uint32_t m_vscroll_policy : 1;
 };