Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Propagate RoomEventCache's VectorDiff to Timeline #3512

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

Hywan
Copy link
Member

@Hywan Hywan commented Jun 5, 2024

@Hywan Hywan force-pushed the feat-sdk-evenet-cache-room-update-vectordiff branch 4 times, most recently from c0a9cd5 to 66847ad Compare June 12, 2024 12:09
Hywan added a commit to Hywan/matrix-rust-sdk that referenced this pull request Jun 13, 2024
…`Timeline`.

This patch changes the behaviour of the `Timeline` when a duplicated
event is received. Prior to this patch, the old event was removed, and
the one was added. With this patch, the old event is kept, and the new
one is skipped.

This is important for matrix-org#3512
where events are mapped to the timeline item indices. When an event is
removed, it becomes difficult to keep the mapping correct.

This patch also adds duplication detection for pagination (with
`TimelineItemPosition::Start`).
Hywan added 12 commits June 14, 2024 14:58
This patch adds the `EventsOrigin::Pagination` variant. It does nothing
more than that. This variant is going to be used in the following
patches.
This patch adds the `RoomEvents::chunks_updates_as_vector` field,
and the `updates_as_vector_diffs` method. Let's use
`linked_chunk::AsVector` inside `EventCache`.
This patch implements `Add` for `HandleManyEventsResult` so that it's
easier to accumulate many of them.
…<VectorDiff<_>>`.

The type of `RoomEventCacheUpdate::AddTimelineEvents::events` is updated
from `Vec<SyncTimelineEvent>` to `Vec<VectorDiff<SyncTimelineEvent>>`.
So far, only `VectorDiff::Append` is supported, but more will come in
the next patches.

A new `TimelineInner::add_events_with_diffs` method is added.
It's pretty similar to `add_events_at`, except that it receives
`VectorDiff`s.
…ept an iterator of events.

This patch updates `TimelineInnerStateTransaction::add_remote_events_at`
to accept a generic parameter for `events`, it must implement
`IntoIterator::Item: Into<SyncTimelineEvent>`. At an upper level,
`TimelineStateInner::add_events_at` at the same constraint except
`IntoIterator::IntoIter: ExactSizeIterator>`.

This modification propagates up to `TimelineInner::add_remote_events_at`
and `TimelineInner::add_events_with_diffs`. It removes the need to clone
the `VectorDiff` values which are inside a `Vector` to get a `Vec`.
Basically, it saves memory and computations!
This patch renames `TimelineEnd` into `TimelineNewItemPosition` for
2 reasons:

1. In the following patch, we will introduce a new variant to insert at
   a specific index, so the suffix `End` would no longer make sense.

2. It's exactly like `TimelineItemPosition` except that it's used
   only and strictly only to add **new** items, which is why we can't use
   `TimelineItemPosition` because it contains the `Update` variant. This
   renaming reflects it's only about **new** items.
This patch updates `TimelineItemPosition::Update` from a tuple to a
struct with an `index`, i.e. from `Update(usize)` to `Update { index:
usize }`, for the sake of consistency with the other variants.
This patch adds the `TimelineItemPosition::At` variant that allows to
insert an event at any position.
…em index.

This patch tries to clear confusion between event index and timeline
item index in `TimelineItemPosition`.
…date_event`.

This patch applies event duplication detection in
`TimelineInnerState::add_or_update_event` when the the position is
`TimelineItemPosition::At`.
This patch tries to unify the code to insert timeline item at position
`Start` and `End` (plus `At`, but with a `todo` for the moment). We can
clearly have the same code for these `TimelineItemPosition`s. It doesn't
make sense to have separate implementations, especially knowing that
`End` had duplicated events check while `Start` had not.

This code only and strictly code moving, modulo the `position` handling,
but nothing has changed apart from that.
@Hywan Hywan force-pushed the feat-sdk-evenet-cache-room-update-vectordiff branch from 66847ad to d07a56f Compare June 14, 2024 13:04
…rDiff`.

This patch updates `TimelineInner::add_events_with_diffs` to
support more `VectorDiff` variant, notably `Insert` which uses
`TimelineNewItemPosition::At`.
…CacheUpdate`!

Prior to this patch, when the `Timeline` was doing a pagination, the
new events were inside the `EventCache` and also added directly onto
the timeline. However, we already have a mechanism to receive new events
in the `Timeline`, and it's a task run by the `Timeline` builder. New
events are received via `RoomEventCacheUpdate`. The problem is: we have
two entry points to add events: one with `RoomEventCacheUpdate`, and one
with paginations.

This patch removes the second entry point! Now the `Timeline` receives
**all** its events via `RoomEventCacheUpdate`.

This patch updates all the appropriate tests accordingly.
This patch inlines the `Add` implementation for `HandleManyEventsResult`
since it's used only once.
…nction.

This patch is a refactoring. It moves the code block responsible to
handle the `RoomEventCacheUpdate`s into its own function for the sake
of clarity.
@Hywan Hywan force-pushed the feat-sdk-evenet-cache-room-update-vectordiff branch from 59251f0 to 049de27 Compare June 17, 2024 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants