From 0e60ca10d0dd89fe8c6087b2a8c5212f7cec0170 Mon Sep 17 00:00:00 2001 From: Andrew Opalach Date: Sun, 18 Jun 2023 12:09:11 +0000 Subject: [PATCH] Fix smooth scrolling when ChannelView is full (#4501) Co-authored-by: Rasmus Karlsson --- CHANGELOG.md | 1 + src/messages/Selection.hpp | 4 + src/widgets/Scrollbar.cpp | 182 ++++++++++++++++------------- src/widgets/Scrollbar.hpp | 8 +- src/widgets/helper/ChannelView.cpp | 93 ++++++--------- src/widgets/helper/ChannelView.hpp | 4 +- 6 files changed, 152 insertions(+), 140 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 000f7a050..1fb9fcd6f 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/messages/Selection.hpp b/src/messages/Selection.hpp index f16e6ce29..8c879c0ae 100644 --- a/src/messages/Selection.hpp +++ b/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 { diff --git a/src/widgets/Scrollbar.cpp b/src/widgets/Scrollbar.cpp index b63e57227..6ca0e248e 100644 --- a/src/widgets/Scrollbar.cpp +++ b/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 @@ -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 &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->maximum_ - this->largeChange_, value)); + value = std::max(this->minimum_, std::min(this->getBottom(), value)); - if (std::abs(this->currentValue_ + this->smoothScrollingOffset_ - value) > - 0.0001) + if (std::abs(this->currentValue_ - value) <= 0.0001) { - if (animated) - { - 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); - } - } + return; } - 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->smoothScrollingOffset_ += value; - } - else - { - this->setDesiredValue(this->getDesiredValue() + value); - } + this->setDesiredValue(this->desiredValue_ + 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_, - std::min(this->maximum_ - this->largeChange_, - value + this->smoothScrollingOffset_)); + value = std::max(this->minimum_, std::min(this->getBottom(), value)); - if (std::abs(this->currentValue_ - value) > 0.0001) + if (std::abs(this->currentValue_ - value) <= 0.0001) { - this->currentValue_ = value; - - this->updateScroll(); - this->currentValueChanged_.invoke(); - - this->update(); + return; } + + 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(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(0.0000001, this->maximum_); + auto div = std::max(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); diff --git a/src/widgets/Scrollbar.hpp b/src/widgets/Scrollbar.hpp index 41ea34b0c..d025539c1 100644 --- a/src/widgets/Scrollbar.hpp +++ b/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; - - const QPropertyAnimation &getCurrentValueAnimation() const; + qreal getRelativeCurrentValue() 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_; diff --git a/src/widgets/helper/ChannelView.cpp b/src/widgets/helper/ChannelView.cpp index 3d031dfa1..6676c2734 100644 --- a/src/widgets/helper/ChannelView.cpp +++ b/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->pauseScrollOffset_ = 0; + this->scrollBar_->offsetMaximum(this->pauseScrollMaximumOffset_); + 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 &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(msg); @@ -881,31 +878,26 @@ void ChannelView::messageAppended(MessagePtr &message, this->lastMessageHasAlternateBackground_ = !this->lastMessageHasAlternateBackground_; - if (!this->scrollBar_->isAtBottom() && - this->scrollBar_->getCurrentValueAnimation().state() == - QPropertyAnimation::Running) + if (this->paused()) { - QEventLoop loop; - - connect(&this->scrollBar_->getCurrentValueAnimation(), - &QAbstractAnimation::stateChanged, &loop, &QEventLoop::quit); - - loop.exec(); + this->pauseScrollMaximumOffset_++; + } + else + { + this->scrollBar_->offsetMaximum(1); } if (this->messages_.pushBack(messageRef)) { if (this->paused()) { - if (!this->scrollBar_->isAtBottom()) - this->pauseScrollOffset_--; + this->pauseScrollMinimumOffset_++; + this->pauseSelectionOffset_++; } else { - if (this->scrollBar_->isAtBottom()) - this->scrollBar_->scrollToBottom(); - else - this->scrollBar_->offset(-1); + this->scrollBar_->offsetMinimum(1); + this->selection_.shiftMessageIndex(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 &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 &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(desired), snapshotLength); + int i = std::min(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) { diff --git a/src/widgets/helper/ChannelView.hpp b/src/widgets/helper/ChannelView.hpp index 3fb47c04d..a31121768 100644 --- a/src/widgets/helper/ChannelView.hpp +++ b/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> pauses_; boost::optional 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;