Scrolling jumps

[lukebarnard1]

The logic handling [un]pagination and scrolling needs a bit of thinking about. What we really want is smooth scrolling through the timeline, with pagination and unpagination, with the assumption that events could change size or be re-rendered at any time.

Occasionally the scroll position "jumps" as images are loaded (see #2640, #2624) and possibly when new messages arrive (#2619) or just when reading backlog (#2138, although this issue was submitted before the most recent unpagination logic was merged, and so may not be useful at this point).

The ScrollPanel concept of stickiness appears to be lacking in these cases. Given that the bugs are seemingly to do with scrolling to the top of the screen, and given that ScrollPanel only has the concept of stickyBottom (and not stickyTop) https://github.com/matrix-org/matrix-react-sdk/blob/v0.8.0/src/components/structures/ScrollPanel.js#L66, maybe we should have more control of "sticking" and instead of just sticking to the bottom, perhaps we could stick to an arbitrary point in the timeline (e.g. keep the event under the mouse/at the top/at the bottom in the same place). UPDATE: this is already done, and it's most likely that _onWidgetLoad is not being called when something changes height in the MessagePanel.

The solution to this might be being more sensible with unpagination. At the moment, events are unpaginated when they are really (arbitrarily) far away from the view-port (see matrix-org/matrix-react-sdk#567 for the latest fix).

Update: #2624 (encrypted images mess up scroll position) should have been fixed by matrix-org/matrix-react-sdk#577

Update: #2744 says that encrypted videos mess up scroll position still

To prevent future regressions and hopefully get reliable visibility on these bugs, it'd probably be a good idea to add tests that can determine that these "jumps" do not happen during [un]pagination, event tiles changing size, new messages arriving, etc. A "jump" probably can't simply be defined as a big change in scrollTop of the ScrollPanel, because this happens when events are [un]paginated. A better definition could be "when an event in view changes position without a scroll event having been fired".

[Qwertie-]

One way to solve this would be getting the image sizes from the server and load a blank rectangle the same size as the image will be as a place holder until the image loads. Telegram also does this along with loading a blurry version of the image until the full thing has loaded

[richvdh]

what's the tl;dr on this? I think the relevant bugs are fixed?

[richvdh]

assuming it's fixed

[ara4n]

it's very much a problem still. i'm not sure what triggers it, but certainly rooms with images or URL previews can jump around enormously when you are backpaginating, landing you on an entirely different pages. it used to be fine (about 6 months ago)

[ara4n]

this is reproduceable right now in #riot-dev

[ara4n]

gah - i've seen this about 10 times in the last 24 hours, but on trying to get a video i couldn't reproduce it. i think it may be particularly obvious when the server is slow (so you have more time to stare at the spinner, and perhaps for a race to happen). Or it may be specifically related to rooms with URL previews.

[lukebarnard1]

Having turned URL previews on on my homeserver, I have noticed more jumping. Thinking about it, the previews have a preset height - we should make them that height whilst they're loading, surely?

[ara4n]

Ironically i've only spotted this when scrolling backwards, but in general i tend to only ever read scrollback in reverse chronological order anyway (given read markers are still flakey).

I just saw this (using #dendrite-dev) on a room with no images at all, only URL previews... however, I wonder if this is a separate problem; that simply we're lazy-loading the URL previews on content which has just been paginated into the top of the screen, which cosmetically looks identical to the scroll offset jumping....

[lukebarnard1]

whoops..

[ollieh]

Part of the scrolling jumpiness seems to not be to do with things that change in size (images loading etc), but the way messages are added and removed. What it looks like is before any filling or unfilling takes place, the scroll position is saved. After the filling or unfilling has completed, the scroll position is restored by calculating the new scrolltop from the old one. I tried measuring the time between most recent save and restores, and I got values ranging from 10ms to 400ms, and a couple that were more than a second! If that's the case, it's not surprising if it's jumping around a bit.

Maybe what happens between the save and the restore could be sufficiently optimized. I'm not sure if things like image downloads happen in this time (which would explain some of the large timings), or if it would be possible to do them outside the save-restore window.

EDIT: It looks like all the network stuff for filling is happening inside the save-restore window. Also, I tried scrolling in a room with url previews, and I was regularly getting timings of over half a second, and it was very jumpy!

[lukebarnard1]

I'm getting some quite serious scroll jumps on /develop at the moment
riot-web version: b54dabd matrix-org/matrix-react-sdk@a0fe3d1cb00d matrix-org/matrix-js-sdk@f7fee29c76d8

[lukebarnard1]

I've done a bit of digging and have found that the save-restore window is indeed the problem here. This was a reproduceable scroll jump when scrolling back (earlier) through the timeline from https://riot.im/develop/#/room/#riot:matrix.org/$15034002194798QLmXG:cervoi.se. It wasn't always in the same place and could be explained by the inline md-style mxc images that were being used in nearby messages.

...
Saved scroll state $1503356208162tOIjL:riot.ovh
Saved scroll state $1503356196161GznwZ:riot.ovh

1.
Saved scroll state $15033561705555307YtYuy:matrix.org
2.
Scrolling to token '$15033561705555307YtYuy:matrix.org'+364 (delta: 0)
Scrolling to token '$15033561705555307YtYuy:matrix.org'+364 (delta: 0)

3.
**** SCROLL JUMP ***

4.
Saved scroll state $15033551415543372ziHbq:matrix.org
5.
Scrolling to token '$15033551415543372ziHbq:matrix.org'+369 (delta: 0)
Scrolling to token '$15033551415543372ziHbq:matrix.org'+369 (delta: 0)

Saved scroll state $1503354949137sGgss:riot.ovh
Saved scroll state $1503354949137sGgss:riot.ovh
...

The order of events is:

1. Save scroll state correctly
2. Restore scroll state correctly
3. Something causes the timeline to change scroll position the app does not otherwise counteract(?)
4. Save scroll state at the incorrect position
5. Restore scroll state incorrectly.

On step 3, if the scroll state had been restored following the timeline changing, the next scroll event would have caused the following save to be done in the correct, non-jumped position.

Other cases

I've seen cases in other rooms where a jump has happened seemingly without inline images being present and will investigate.

[lukebarnard1]

Having checked the maths on our impl, it looks sound. The only issue is that we save the scroll state at times where it might not be correct (i.e. following a change in scrollHeight or scrollTop that was not accounted for). Perhaps our method for getting the previous scroll state should be adjusted such that we know that the calculations we're doing at this exact point in time are based on correct scroll state. This might mean ignoring scroll state saves that are a literal jump in the timeline.

[ara4n]

This is getting worse and worse; especially in E2E rooms I regularly see jumps of more than a page whilst scrolling through scrollback :(

[lukebarnard1]

This is quite bad on Quantum, even with smooth scrolling disabled.

[lukebarnard1]

So somehow we weren't calling onWidgetLoad from MImageBody when an <img> is finished loading. I think I assumed previously that we were, but this is just not true. See matrix-org/matrix-react-sdk@641add49

[lukebarnard1]

I seemed to have neglected to doc this but I had an attempt at using Chrome's new support for scroll anchors and plonked it a branch. Here's the PR to merge said branch, but it's just a POC; not for merging.

It answers the question "what happens when we disable our own scroll management and let Chrome do it's thing".

The answer is that it works remarkably well (from memory) apart from when the top of the page appears in the viewport. (See matrix-org/matrix-react-sdk#1787)

---

The issue for me is that we have an implementation that saves & restores the scroll position. In theory we could have one the relies entirely on a new technology - web anchoring. Interestingly, Chrome does this remarkably well and I even tested this in the past with Riot.

The conclusion here was that until all major browsers support this, we continue to maintain a single code path as we don't have capacity to maintain two different code paths for the same component.

It's possible that we're missing out though.

[lukebarnard1]

Empirically, back paginating in an e2e room seems very scroll-jumpy, especially when the probability of a jump is increased by scroll events being fired more frequently (i.e. when using a Mac trackpad).

See #6265

[Ablu]

For me this happens very severely in the Android version. I usually click "Jump to first unread messages" there and start to scroll down. As soon messages are loaded as part of the scroll process the view jumps for me, which always disturbes the reading flow. I can reproduce this on basically any channel that I am in (normal IRC bridged rooms, so text messages only).

[lukebarnard1]

@Ablu that is an issue with https://github.com/vector-im/riot-android, please report it in that repo.

[Ablu]

Ah. I thought the code was shared since the Android client felt like the wrapped web version. But thanks, I will see whether this is already reported there! :)

…

[ara4n]

I think the E2E issue is probably due to fixupHeight() not considering the fact that E2E images are sent with a clientside thumbnail which is typically smaller than the 800x600 bounding box (perhaps)?

[aaronraimist]

Firefox console says:

This site appears to use a scroll-linked positioning effect. This may not work well with asynchronous panning; see https://developer.mozilla.org/docs/Mozilla/Performance/ScrollLinkedEffects for further details and to join the discussion on related tools and features!

From https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Scroll-linked_effects:

In the asynchronous scrolling model, the visual scroll position is updated in the compositor thread and is visible to the user before the scroll event is updated in the DOM and fired on the main thread. This means that the effects implemented will lag a little bit behind what the user sees the scroll position to be. This can cause the effect to be laggy, janky, or jittery — in short, something we want to avoid.

[ara4n]

this is obsoleted by #8565