Fix smooth scrolling when ChannelView is full (#4501)

Co-authored-by: Rasmus Karlsson <rasmus.karlsson@pajlada.com>

CHANGELOG.md

@@ -63,6 +63,7 @@
 - Bugfix: Fixed blocked user list sticking around when switching from a logged in user to being logged out. (#4437)
 - Bugfix: Fixed search popup ignoring setting for message scrollback limit. (#4496)
 - Bugfix: Fixed a memory leak that occurred when loading message history. This was mostly noticeable with unstable internet connections where reconnections were frequent or long-running instances of Chatterino. (#4499)
+- Bugfix: Fix visual glitches with smooth scrolling. (#4501)
 - Bugfix: Fixed Twitch channel-specific filters not being applied correctly. (#4529)
 - Bugfix: Fixed `/mods` displaying incorrectly when the channel has no mods. (#4546)
 - Bugfix: Fixed emote & badge tooltips not showing up when thumbnails were hidden. (#4509)

src/messages/Selection.hpp

@@ -78,6 +78,7 @@ struct Selection {
         if (offset > this->selectionMin.messageIndex)
         {
             this->selectionMin.messageIndex = 0;
+            this->selectionMin.charIndex = 0;
         }
         else
         {
@@ -87,6 +88,7 @@ struct Selection {
         if (offset > this->selectionMax.messageIndex)
         {
             this->selectionMax.messageIndex = 0;
+            this->selectionMax.charIndex = 0;
         }
         else
         {
@@ -96,6 +98,7 @@ struct Selection {
         if (offset > this->start.messageIndex)
         {
             this->start.messageIndex = 0;
+            this->start.charIndex = 0;
         }
         else
         {
@@ -105,6 +108,7 @@ struct Selection {
         if (offset > this->end.messageIndex)
         {
             this->end.messageIndex = 0;
+            this->end.charIndex = 0;
         }
         else
         {

src/widgets/Scrollbar.cpp

@@ -4,6 +4,7 @@
 #include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
 #include "singletons/WindowManager.hpp"
+#include "util/Clamp.hpp"
 #include "widgets/helper/ChannelView.hpp"
 
 #include <QMouseEvent>
@@ -21,12 +22,14 @@ Scrollbar::Scrollbar(size_t messagesLimit, ChannelView *parent)
     , currentValueAnimation_(this, "currentValue_")
     , highlights_(messagesLimit)
 {
-    resize(int(16 * this->scale()), 100);
+    this->resize(int(16 * this->scale()), 100);
     this->currentValueAnimation_.setDuration(150);
     this->currentValueAnimation_.setEasingCurve(
         QEasingCurve(QEasingCurve::OutCubic));
+    connect(&this->currentValueAnimation_, &QAbstractAnimation::finished, this,
+            &Scrollbar::resetMaximum);
 
-    setMouseTracking(true);
+    this->setMouseTracking(true);
 }
 
 void Scrollbar::addHighlight(ScrollbarHighlight highlight)
@@ -72,12 +75,12 @@ LimitedQueueSnapshot<ScrollbarHighlight> &Scrollbar::getHighlightSnapshot()
 
 void Scrollbar::scrollToBottom(bool animate)
 {
-    this->setDesiredValue(this->maximum_ - this->getLargeChange(), animate);
+    this->setDesiredValue(this->getBottom(), animate);
 }
 
 void Scrollbar::scrollToTop(bool animate)
 {
-    this->setDesiredValue(this->minimum_ - this->getLargeChange(), animate);
+    this->setDesiredValue(this->minimum_, animate);
 }
 
 bool Scrollbar::isAtBottom() const
@@ -89,80 +92,95 @@ void Scrollbar::setMaximum(qreal value)
 {
     this->maximum_ = value;
 
-    updateScroll();
+    this->updateScroll();
+}
+
+void Scrollbar::offsetMaximum(qreal value)
+{
+    this->maximum_ += value;
+
+    this->updateScroll();
+}
+
+void Scrollbar::resetMaximum()
+{
+    if (this->minimum_ > 0)
+    {
+        this->maximum_ -= this->minimum_;
+        this->desiredValue_ -= this->minimum_;
+        this->currentValue_ -= this->minimum_;
+        this->minimum_ = 0;
+    }
 }
 
 void Scrollbar::setMinimum(qreal value)
 {
     this->minimum_ = value;
 
-    updateScroll();
+    this->updateScroll();
+}
+
+void Scrollbar::offsetMinimum(qreal value)
+{
+    this->minimum_ += value;
+
+    if (this->minimum_ > this->desiredValue_)
+    {
+        this->scrollToTop();
+    }
+
+    this->updateScroll();
 }
 
 void Scrollbar::setLargeChange(qreal value)
 {
     this->largeChange_ = value;
 
-    updateScroll();
+    this->updateScroll();
 }
 
 void Scrollbar::setSmallChange(qreal value)
 {
     this->smallChange_ = value;
 
-    updateScroll();
+    this->updateScroll();
 }
 
 void Scrollbar::setDesiredValue(qreal value, bool animated)
 {
-    animated &= getSettings()->enableSmoothScrolling;
+    value = std::max(this->minimum_, std::min(this->getBottom(), value));
-    value = std::max(this->minimum_,
-                     std::min(this->maximum_ - this->largeChange_, value));
 
-    if (std::abs(this->currentValue_ + this->smoothScrollingOffset_ - value) >
+    if (std::abs(this->currentValue_ - value) <= 0.0001)
-        0.0001)
     {
-        if (animated)
+        return;
-        {
-            this->currentValueAnimation_.stop();
-            this->currentValueAnimation_.setStartValue(
-                this->currentValue_ + this->smoothScrollingOffset_);
-
-            //            if (((this->getMaximum() - this->getLargeChange()) -
-            //            value) <= 0.01) {
-            //                value += 1;
-            //            }
-
-            this->currentValueAnimation_.setEndValue(value);
-            this->smoothScrollingOffset_ = 0;
-            this->atBottom_ = ((this->getMaximum() - this->getLargeChange()) -
-                               value) <= 0.0001;
-            this->currentValueAnimation_.start();
-        }
-        else
-        {
-            if (this->currentValueAnimation_.state() ==
-                QPropertyAnimation::Running)
-            {
-                this->currentValueAnimation_.setEndValue(value);
-            }
-            else
-            {
-                this->smoothScrollingOffset_ = 0;
-                this->desiredValue_ = value;
-                this->currentValueAnimation_.stop();
-                this->atBottom_ =
-                    ((this->getMaximum() - this->getLargeChange()) - value) <=
-                    0.0001;
-                setCurrentValue(value);
-            }
-        }
     }
 
-    this->smoothScrollingOffset_ = 0;
     this->desiredValue_ = value;
 
     this->desiredValueChanged_.invoke();
+
+    this->atBottom_ = (this->getBottom() - value) <= 0.0001;
+
+    if (animated && getSettings()->enableSmoothScrolling)
+    {
+        // stop() does not emit QAbstractAnimation::finished().
+        this->currentValueAnimation_.stop();
+        this->currentValueAnimation_.setStartValue(this->currentValue_);
+        this->currentValueAnimation_.setEndValue(value);
+        this->currentValueAnimation_.start();
+    }
+    else
+    {
+        if (this->currentValueAnimation_.state() != QPropertyAnimation::Stopped)
+        {
+            this->currentValueAnimation_.setEndValue(value);
+        }
+        else
+        {
+            this->setCurrentValue(value);
+            this->resetMaximum();
+        }
+    }
 }
 
 qreal Scrollbar::getMaximum() const
@@ -180,6 +198,11 @@ qreal Scrollbar::getLargeChange() const
     return this->largeChange_;
 }
 
+qreal Scrollbar::getBottom() const
+{
+    return this->maximum_ - this->largeChange_;
+}
+
 qreal Scrollbar::getSmallChange() const
 {
     return this->smallChange_;
@@ -187,7 +210,7 @@ qreal Scrollbar::getSmallChange() const
 
 qreal Scrollbar::getDesiredValue() const
 {
-    return this->desiredValue_ + this->smoothScrollingOffset_;
+    return this->desiredValue_;
 }
 
 qreal Scrollbar::getCurrentValue() const
@@ -195,21 +218,17 @@ qreal Scrollbar::getCurrentValue() const
     return this->currentValue_;
 }
 
-const QPropertyAnimation &Scrollbar::getCurrentValueAnimation() const
+qreal Scrollbar::getRelativeCurrentValue() const
 {
-    return this->currentValueAnimation_;
+    // currentValue - minimum can be negative if minimum is incremented while
+    // scrolling up to or down from the top when smooth scrolling is enabled.
+    return clamp(this->currentValue_ - this->minimum_, qreal(0.0),
+                 this->currentValue_);
 }
 
 void Scrollbar::offset(qreal value)
 {
-    if (this->currentValueAnimation_.state() == QPropertyAnimation::Running)
+    this->setDesiredValue(this->desiredValue_ + value);
-    {
-        this->smoothScrollingOffset_ += value;
-    }
-    else
-    {
-        this->setDesiredValue(this->getDesiredValue() + value);
-    }
 }
 
 pajlada::Signals::NoArgSignal &Scrollbar::getCurrentValueChanged()
@@ -224,19 +243,17 @@ pajlada::Signals::NoArgSignal &Scrollbar::getDesiredValueChanged()
 
 void Scrollbar::setCurrentValue(qreal value)
 {
-    value = std::max(this->minimum_,
+    value = std::max(this->minimum_, std::min(this->getBottom(), value));
-                     std::min(this->maximum_ - this->largeChange_,
-                              value + this->smoothScrollingOffset_));
 
-    if (std::abs(this->currentValue_ - value) > 0.0001)
+    if (std::abs(this->currentValue_ - value) <= 0.0001)
     {
-        this->currentValue_ = value;
+        return;
-
-        this->updateScroll();
-        this->currentValueChanged_.invoke();
-
-        this->update();
     }
+
+    this->currentValue_ = value;
+
+    this->updateScroll();
+    this->currentValueChanged_.invoke();
 }
 
 void Scrollbar::printCurrentState(const QString &prefix) const
@@ -381,14 +398,14 @@ void Scrollbar::mouseMoveEvent(QMouseEvent *event)
 
         if (oldIndex != this->mouseOverIndex_)
         {
-            update();
+            this->update();
         }
     }
     else if (this->mouseDownIndex_ == 2)
     {
         int delta = event->pos().y() - this->lastMousePosition_.y();
 
-        setDesiredValue(
+        this->setDesiredValue(
             this->desiredValue_ +
             (qreal(delta) / std::max<qreal>(0.00000002, this->trackHeight_)) *
                 this->maximum_);
@@ -431,14 +448,16 @@ void Scrollbar::mouseReleaseEvent(QMouseEvent *event)
     {
         if (this->mouseDownIndex_ == 0)
         {
-            setDesiredValue(this->desiredValue_ - this->smallChange_, true);
+            this->setDesiredValue(this->desiredValue_ - this->smallChange_,
+                                  true);
         }
     }
     else if (y < this->thumbRect_.y())
     {
         if (this->mouseDownIndex_ == 1)
         {
-            setDesiredValue(this->desiredValue_ - this->smallChange_, true);
+            this->setDesiredValue(this->desiredValue_ - this->smallChange_,
+                                  true);
         }
     }
     else if (this->thumbRect_.contains(2, y))
@@ -449,26 +468,29 @@ void Scrollbar::mouseReleaseEvent(QMouseEvent *event)
     {
         if (this->mouseDownIndex_ == 3)
         {
-            setDesiredValue(this->desiredValue_ + this->smallChange_, true);
+            this->setDesiredValue(this->desiredValue_ + this->smallChange_,
+                                  true);
         }
     }
     else
     {
         if (this->mouseDownIndex_ == 4)
         {
-            setDesiredValue(this->desiredValue_ + this->smallChange_, true);
+            this->setDesiredValue(this->desiredValue_ + this->smallChange_,
+                                  true);
         }
     }
 
     this->mouseDownIndex_ = -1;
-    update();
+
+    this->update();
 }
 
 void Scrollbar::leaveEvent(QEvent *)
 {
     this->mouseOverIndex_ = -1;
 
-    update();
+    this->update();
 }
 
 void Scrollbar::updateScroll()
@@ -476,11 +498,11 @@ void Scrollbar::updateScroll()
     this->trackHeight_ = this->height() - this->buttonHeight_ -
                          this->buttonHeight_ - MIN_THUMB_HEIGHT - 1;
 
-    auto div = std::max<qreal>(0.0000001, this->maximum_);
+    auto div = std::max<qreal>(0.0000001, this->maximum_ - this->minimum_);
 
     this->thumbRect_ = QRect(
         0,
-        int(this->currentValue_ / div * this->trackHeight_) + 1 +
+        int((this->getRelativeCurrentValue()) / div * this->trackHeight_) + 1 +
             this->buttonHeight_,
         this->width(),
         int(this->largeChange_ / div * this->trackHeight_) + MIN_THUMB_HEIGHT);

src/widgets/Scrollbar.hpp

@@ -34,18 +34,21 @@ public:
     bool isAtBottom() const;
 
     void setMaximum(qreal value);
+    void offsetMaximum(qreal value);
+    void resetMaximum();
     void setMinimum(qreal value);
+    void offsetMinimum(qreal value);
     void setLargeChange(qreal value);
     void setSmallChange(qreal value);
     void setDesiredValue(qreal value, bool animated = false);
     qreal getMaximum() const;
     qreal getMinimum() const;
     qreal getLargeChange() const;
+    qreal getBottom() const;
     qreal getSmallChange() const;
     qreal getDesiredValue() const;
     qreal getCurrentValue() const;
-
+    qreal getRelativeCurrentValue() const;
-    const QPropertyAnimation &getCurrentValueAnimation() const;
 
     // offset the desired value without breaking smooth scolling
     void offset(qreal value);
@@ -96,7 +99,6 @@ private:
     qreal smallChange_ = 5;
     qreal desiredValue_ = 0;
     qreal currentValue_ = 0;
-    qreal smoothScrollingOffset_ = 0;
 
     pajlada::Signals::NoArgSignal currentValueChanged_;
     pajlada::Signals::NoArgSignal desiredValueChanged_;

src/widgets/helper/ChannelView.cpp

@@ -328,8 +328,10 @@ void ChannelView::updatePauses()
         this->pauseEnd_ = boost::none;
         this->pauseTimer_.stop();
 
-        this->scrollBar_->offset(this->pauseScrollOffset_);
+        this->scrollBar_->offsetMaximum(this->pauseScrollMaximumOffset_);
-        this->pauseScrollOffset_ = 0;
+        this->scrollBar_->offsetMinimum(this->pauseScrollMinimumOffset_);
+        this->pauseScrollMinimumOffset_ = 0;
+        this->pauseScrollMaximumOffset_ = 0;
 
         this->queueLayout();
     }
@@ -453,7 +455,7 @@ void ChannelView::performLayout(bool causedByScrollbar)
 void ChannelView::layoutVisibleMessages(
     const LimitedQueueSnapshot<MessageLayoutPtr> &messages)
 {
-    const auto start = size_t(this->scrollBar_->getCurrentValue());
+    const auto start = size_t(this->scrollBar_->getRelativeCurrentValue());
     const auto layoutWidth = this->getLayoutWidth();
     const auto flags = this->getFlags();
     auto redrawRequired = false;
@@ -461,7 +463,7 @@ void ChannelView::layoutVisibleMessages(
     if (messages.size() > start)
     {
         auto y = int(-(messages[start]->getHeight() *
-                       (fmod(this->scrollBar_->getCurrentValue(), 1))));
+                       (fmod(this->scrollBar_->getRelativeCurrentValue(), 1))));
 
         for (auto i = start; i < messages.size() && y <= this->height(); i++)
         {
@@ -521,21 +523,17 @@ void ChannelView::updateScrollbar(
 
     if (!showScrollbar && !causedByScrollbar)
     {
-        this->scrollBar_->setDesiredValue(0);
+        this->scrollBar_->scrollToTop();
     }
     this->showScrollBar_ = showScrollbar;
 
-    this->scrollBar_->setMaximum(messages.size());
-
     // If we were showing the latest messages and the scrollbar now wants to be
     // rendered, scroll to bottom
     if (this->enableScrollingToBottom_ && this->showingLatestMessages_ &&
-        showScrollbar)
+        showScrollbar && !causedByScrollbar)
     {
         this->scrollBar_->scrollToBottom(
-            // this->messageWasAdded &&
             getSettings()->enableSmoothScrollingNewMessages.getValue());
-        this->messageWasAdded_ = false;
     }
 }
 
@@ -544,6 +542,9 @@ void ChannelView::clearMessages()
     // Clear all stored messages in this chat widget
     this->messages_.clear();
     this->scrollBar_->clearHighlights();
+    this->scrollBar_->resetMaximum();
+    this->scrollBar_->setMaximum(0);
+    this->scrollBar_->setMinimum(0);
     this->queueLayout();
 
     this->lastMessageHasAlternateBackground_ = false;
@@ -750,12 +751,6 @@ void ChannelView::setChannel(ChannelPtr underlyingChannel)
             this->messageAddedAtStart(messages);
         });
 
-    // on message removed
-    this->channelConnections_.managedConnect(
-        this->channel_->messageRemovedFromStart, [this](MessagePtr &message) {
-            this->messageRemoveFromStart(message);
-        });
-
     // on message replaced
     this->channelConnections_.managedConnect(
         this->channel_->messageReplaced,
@@ -771,6 +766,8 @@ void ChannelView::setChannel(ChannelPtr underlyingChannel)
 
     auto snapshot = underlyingChannel->getMessageSnapshot();
 
+    this->scrollBar_->setMaximum(qreal(snapshot.size()));
+
     for (const auto &msg : snapshot)
     {
         auto messageLayout = std::make_shared<MessageLayout>(msg);
@@ -881,31 +878,26 @@ void ChannelView::messageAppended(MessagePtr &message,
     this->lastMessageHasAlternateBackground_ =
         !this->lastMessageHasAlternateBackground_;
 
-    if (!this->scrollBar_->isAtBottom() &&
+    if (this->paused())
-        this->scrollBar_->getCurrentValueAnimation().state() ==
-            QPropertyAnimation::Running)
     {
-        QEventLoop loop;
+        this->pauseScrollMaximumOffset_++;
-
+    }
-        connect(&this->scrollBar_->getCurrentValueAnimation(),
+    else
-                &QAbstractAnimation::stateChanged, &loop, &QEventLoop::quit);
+    {
-
+        this->scrollBar_->offsetMaximum(1);
-        loop.exec();
     }
 
     if (this->messages_.pushBack(messageRef))
     {
         if (this->paused())
         {
-            if (!this->scrollBar_->isAtBottom())
+            this->pauseScrollMinimumOffset_++;
-                this->pauseScrollOffset_--;
+            this->pauseSelectionOffset_++;
         }
         else
         {
-            if (this->scrollBar_->isAtBottom())
+            this->scrollBar_->offsetMinimum(1);
-                this->scrollBar_->scrollToBottom();
+            this->selection_.shiftMessageIndex(1);
-            else
-                this->scrollBar_->offset(-1);
         }
     }
 
@@ -931,7 +923,6 @@ void ChannelView::messageAppended(MessagePtr &message,
         this->scrollBar_->addHighlight(message->getScrollBarHighlight());
     }
 
-    this->messageWasAdded_ = true;
     this->queueLayout();
 }
 
@@ -956,12 +947,14 @@ void ChannelView::messageAddedAtStart(std::vector<MessagePtr> &messages)
     }
 
     /// Add the messages at the start
-    if (this->messages_.pushFront(messageRefs).size() > 0)
+    auto addedMessages = this->messages_.pushFront(messageRefs);
+    if (!addedMessages.empty())
     {
         if (this->scrollBar_->isAtBottom())
             this->scrollBar_->scrollToBottom();
         else
-            this->scrollBar_->offset(qreal(messages.size()));
+            this->scrollBar_->offset(qreal(addedMessages.size()));
+        this->scrollBar_->offsetMaximum(qreal(addedMessages.size()));
     }
 
     if (this->showScrollbarHighlights())
@@ -976,21 +969,6 @@ void ChannelView::messageAddedAtStart(std::vector<MessagePtr> &messages)
         this->scrollBar_->addHighlightsAtStart(highlights);
     }
 
-    this->messageWasAdded_ = true;
-    this->queueLayout();
-}
-
-void ChannelView::messageRemoveFromStart(MessagePtr &message)
-{
-    if (this->paused())
-    {
-        this->pauseSelectionOffset_ += 1;
-    }
-    else
-    {
-        this->selection_.shiftMessageIndex(1);
-    }
-
     this->queueLayout();
 }
 
@@ -1024,6 +1002,9 @@ void ChannelView::messagesUpdated()
 
     this->messages_.clear();
     this->scrollBar_->clearHighlights();
+    this->scrollBar_->resetMaximum();
+    this->scrollBar_->setMaximum(qreal(snapshot.size()));
+    this->scrollBar_->setMinimum(0);
     this->lastMessageHasAlternateBackground_ = false;
     this->lastMessageHasAlternateBackgroundReverse_ = true;
 
@@ -1227,7 +1208,8 @@ void ChannelView::scrollToMessageLayout(MessageLayout *layout,
 
     if (this->showScrollBar_)
     {
-        this->getScrollBar().setDesiredValue(messageIdx);
+        this->getScrollBar().setDesiredValue(this->scrollBar_->getMinimum() +
+                                             qreal(messageIdx));
     }
 }
 
@@ -1258,7 +1240,7 @@ void ChannelView::drawMessages(QPainter &painter)
 {
     auto &messagesSnapshot = this->getMessagesSnapshot();
 
-    size_t start = size_t(this->scrollBar_->getCurrentValue());
+    const auto start = size_t(this->scrollBar_->getRelativeCurrentValue());
 
     if (start >= messagesSnapshot.size())
     {
@@ -1266,7 +1248,7 @@ void ChannelView::drawMessages(QPainter &painter)
     }
 
     int y = int(-(messagesSnapshot[start].get()->getHeight() *
-                  (fmod(this->scrollBar_->getCurrentValue(), 1))));
+                  (fmod(this->scrollBar_->getRelativeCurrentValue(), 1))));
 
     MessageLayout *end = nullptr;
     bool windowFocused = this->window() == QApplication::activeWindow();
@@ -1368,7 +1350,8 @@ void ChannelView::wheelEvent(QWheelEvent *event)
 
         auto &snapshot = this->getMessagesSnapshot();
         int snapshotLength = int(snapshot.size());
-        int i = std::min<int>(int(desired), snapshotLength);
+        int i = std::min<int>(int(desired - this->scrollBar_->getMinimum()),
+                              snapshotLength - 1);
 
         if (delta > 0)
         {
@@ -2779,7 +2762,7 @@ bool ChannelView::tryGetMessageAt(QPoint p,
 {
     auto &messagesSnapshot = this->getMessagesSnapshot();
 
-    size_t start = this->scrollBar_->getCurrentValue();
+    const auto start = size_t(this->scrollBar_->getRelativeCurrentValue());
 
     if (start >= messagesSnapshot.size())
     {
@@ -2787,7 +2770,7 @@ bool ChannelView::tryGetMessageAt(QPoint p,
     }
 
     int y = -(messagesSnapshot[start]->getHeight() *
-              (fmod(this->scrollBar_->getCurrentValue(), 1)));
+              (fmod(this->scrollBar_->getRelativeCurrentValue(), 1)));
 
     for (size_t i = start; i < messagesSnapshot.size(); ++i)
     {

src/widgets/helper/ChannelView.hpp

@@ -260,7 +260,6 @@ private:
 
     QTimer updateTimer_;
     bool updateQueued_ = false;
-    bool messageWasAdded_ = false;
     bool lastMessageHasAlternateBackground_ = false;
     bool lastMessageHasAlternateBackgroundReverse_ = true;
 
@@ -269,7 +268,8 @@ private:
     std::unordered_map<PauseReason, boost::optional<SteadyClock::time_point>>
         pauses_;
     boost::optional<SteadyClock::time_point> pauseEnd_;
-    int pauseScrollOffset_ = 0;
+    int pauseScrollMinimumOffset_ = 0;
+    int pauseScrollMaximumOffset_ = 0;
     // Keeps track how many message indices we need to offset the selection when we resume scrolling
     uint32_t pauseSelectionOffset_ = 0;