Skip to content

Commit

Permalink
Make sure events get loaded if the timeline is empty
Browse files Browse the repository at this point in the history
After fixing the race in the room switching procedure (see 4ce59f7)
another race showed up, this time between the model reset and updating
root.room property in QML. Because this property is distinct from
messageModel.room, it gets updated later, specifically after
onModelReset() is called. onModelReset() itself already uses
messageModel.room instead of root.room exactly for that reason but
ensurePreviousContent() called from there still operates on root.room.
That alone wouldn't be much of a hurdle (ensurePreviousContent() could
be rewired to messageModel.room just as well) but at the moment
onModelReset() is called chatView has not created any delegates for
the model yet - meaning that contentY and originY used in
ensurePreviousContent() are 0 even when the model has plenty of events
loaded, leading to a false-positive condition and an unneeded request
to the homeserver for more messages.

This commit moves requesting the initial batch of historical events
to the model. It does not request a lot, therefore the turnaround is
short; and that solves the timeline bootstrapping after a room switch.
Eventually this might even move to libQuotient, because events have
to be loaded to the room when it is displayed, regardless of
the client - but that's something to ponder separately.

And while we're at it, the property tracking the number of requested
historical events has been moved to QuaternionRoom, anticipating its
further move to libQuotient.
  • Loading branch information
Kitsune Ral committed Jan 12, 2024
1 parent 25ce1c6 commit aef84cf
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 19 deletions.
3 changes: 3 additions & 0 deletions client/models/messageeventmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ void MessageEventModel::changeRoom(QuaternionRoom* room)
qCDebug(EVENTMODEL)
<< "Event model connected to room" << room->objectName() //
<< "as" << room->localUser()->id();
// If the timeline isn't loaded, ask for at least something right away
if (room->timelineSize() == 0)
room->getHistory(30);
}
endResetModel();
emit readMarkerUpdated();
Expand Down
30 changes: 11 additions & 19 deletions client/qml/Timeline.qml
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,6 @@ Page {
contentHeight > 0 && count > 0 ? count / contentHeight : 0.03
// 0.03 is just an arbitrary reasonable number

property int lastRequestedEvents: 0
readonly property int currentRequestedEvents:
room && room.eventsHistoryJob ? lastRequestedEvents : 0

property var textEditWithSelection
property real readMarkerContentPos: originY
readonly property real readMarkerViewportPos:
Expand Down Expand Up @@ -287,10 +283,8 @@ Page {
// Check if we're about to bump into the ceiling in
// 2 seconds and if yes, request the amount of messages
// enough to scroll at this rate for 3 more seconds
if (velocity > 0 && contentY - velocity*2 < originY) {
lastRequestedEvents = velocity * eventDensity * 3
room.getPreviousContent(lastRequestedEvents)
}
if (velocity > 0 && contentY - velocity*2 < originY)
room.getHistory(velocity * eventDensity * 3)
}
onContentYChanged: ensurePreviousContent()
onContentHeightChanged: ensurePreviousContent()
Expand Down Expand Up @@ -346,11 +340,9 @@ Page {
chatView.saveViewport(true)
}
function onModelReset() {
if (messageModel.room) {
// Load events if there are not enough of them
chatView.ensurePreviousContent()
chatView.positionViewAtBeginning()
}
// NB: at this point, the actual delegates are not instantiated
// yet, so defer all actions to when at least some are
scrollFinisher.scrollViewTo(0, ListView.Contain)
}
}

Expand Down Expand Up @@ -479,7 +471,7 @@ Page {

// A proxy property for animation
property int requestedHistoryEventsCount:
chatView.currentRequestedEvents
room ? room.requestedEventsCount : 0
AnimationBehavior on requestedHistoryEventsCount {
NormalNumberAnimation { }
}
Expand Down Expand Up @@ -619,8 +611,8 @@ Page {
color: palette.alternateBase
property bool shown:
(chatView.bottommostVisibleIndex >= 0
&& (scrollerArea.containsMouse || scrollAnimation.running))
|| chatView.currentRequestedEvents > 0
&& (scrollerArea.containsMouse || scrollAnimation.running))
|| (room && room.requestedEventsCount > 0)

onShownChanged: {
if (shown) {
Expand Down Expand Up @@ -648,10 +640,10 @@ Page {
chatView.bottommostVisibleIndex))
+ "\n" + qsTr("%Ln events cached", "", chatView.count)
: "")
+ (chatView.currentRequestedEvents > 0
+ (room && room.requestedEventsCount > 0
? (chatView.count > 0 ? "\n" : "")
+ qsTr("%Ln events requested from the server",
"", chatView.currentRequestedEvents)
"", room.requestedEventsCount)
: "")
horizontalAlignment: Label.AlignRight
}
Expand Down Expand Up @@ -720,7 +712,7 @@ Page {
scrollFinisher.scrollViewTo(messageModel.readMarkerVisualIndex,
ListView.Center)
else
room.getPreviousContent(chatView.count / 2) // FIXME, #799
room.getHistory(chatView.count / 2) // FIXME, #799
}
}
}
13 changes: 13 additions & 0 deletions client/quaternionroom.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ QuaternionRoom::QuaternionRoom(Connection* connection, QString roomId,
{
connect(this, &Room::namesChanged,
this, &QuaternionRoom::htmlSafeDisplayNameChanged);
connect(this, &Room::eventsHistoryJobChanged,
this, &QuaternionRoom::requestedEventsCountChanged);
}

const QString& QuaternionRoom::cachedUserFilter() const
Expand Down Expand Up @@ -80,6 +82,17 @@ QString QuaternionRoom::htmlSafeDisplayName() const
return displayName().toHtmlEscaped();
}

int QuaternionRoom::requestedEventsCount() const
{
return eventsHistoryJob() != nullptr ? m_requestedEventsCount : 0;
}

void QuaternionRoom::getHistory(int limit)
{
m_requestedEventsCount = limit;
getPreviousContent(m_requestedEventsCount);
}

void QuaternionRoom::onAddNewTimelineEvents(timeline_iter_t from)
{
std::for_each(from, messageEvents().cend(),
Expand Down
9 changes: 9 additions & 0 deletions client/quaternionroom.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class QuaternionRoom: public Quotient::Room
{
Q_OBJECT
Q_PROPERTY(QString htmlSafeDisplayName READ htmlSafeDisplayName NOTIFY htmlSafeDisplayNameChanged)
Q_PROPERTY(int requestedEventsCount READ requestedEventsCount NOTIFY requestedEventsCountChanged)
public:
QuaternionRoom(Quotient::Connection* connection,
QString roomId, Quotient::JoinState joinState);
Expand All @@ -29,16 +30,24 @@ class QuaternionRoom: public Quotient::Room
bool force = false);

QString htmlSafeDisplayName() const;
int requestedEventsCount() const;

public slots:
// TODO, 0.0.96: move logic to libQuotient 0.9 and get rid of it here
void getHistory(int limit);

signals:
// Gotta wrap the Room::namesChanged signal because it has parameters
// and moc cannot use signals with parameters defined in the parent
// class as NOTIFY targets
void htmlSafeDisplayNameChanged();
// TODO, 0.0.96: same as for getHistory()
void requestedEventsCountChanged();

private:
QSet<const Quotient::RoomEvent*> highlights;
QString m_cachedUserFilter;
int m_requestedEventsCount = 0;

void onAddNewTimelineEvents(timeline_iter_t from) override;
void onAddHistoricalTimelineEvents(rev_iter_t from) override;
Expand Down

0 comments on commit aef84cf

Please sign in to comment.