Skip to content

Commit

Permalink
!the algorithm fixup
Browse files Browse the repository at this point in the history
  • Loading branch information
@Hywan
Hywan committed Jun 14, 2024
1 parent 09bcc94 commit d07a56f
Show file tree
Hide file tree
Showing 3 changed files with 242 additions and 44 deletions.
23 changes: 21 additions & 2 deletions crates/matrix-sdk-ui/src/timeline/day_dividers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,15 @@ impl DayDividerAdjuster {

let item = meta.new_timeline_item(VirtualTimelineItem::DayDivider(ts));

// Shift all timeline item indices to the right after `at`!
for event_meta in &mut meta.all_events {
if let Some(index) = event_meta.timeline_item_index.as_mut() {
if *index >= at {
*index += 1;
}
}
}

// Keep push semantics, if we're inserting at the front or the back.
if at == items.len() {
items.push_back(item);
Expand Down Expand Up @@ -343,12 +352,22 @@ impl DayDividerAdjuster {
}

DayDividerOperation::Remove(i) => {
assert!(i >= max_i, "trying to replace at {i} < max_i={max_i}");
assert!(i >= max_i, "trying to remove at {i} < max_i={max_i}");

let at = i64::try_from(i).unwrap() + offset;
assert!(at >= 0);
let at = at as usize;

// Shift all timeline item indices to the left after `at`!
for event_meta in &mut meta.all_events {
if let Some(index) = event_meta.timeline_item_index.as_mut() {
if *index >= at {
*index -= 1;
}
}
}

let removed = items.remove(at as usize);
let removed = items.remove(at);
if !removed.is_day_divider() {
error!("we removed a non day-divider @ {i}: {:?}", removed.kind());
}
Expand Down
120 changes: 84 additions & 36 deletions crates/matrix-sdk-ui/src/timeline/event_handler.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ use super::{
EventTimelineItem, InReplyToDetails, Message, OtherState, ReactionGroup, ReactionSenderData,
Sticker, TimelineDetails, TimelineItem, TimelineItemContent,
};
use crate::{events::SyncTimelineEventWithoutContent, DEFAULT_SANITIZER_MODE};
use crate::{
events::SyncTimelineEventWithoutContent, timeline::inner::EventMeta, DEFAULT_SANITIZER_MODE,
};

/// When adding an event, useful information related to the source of the event.
#[derive(Clone)]
Expand Down Expand Up @@ -475,14 +477,24 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {

#[cfg(feature = "e2e-encryption")]
if let Flow::Remote {
position: TimelineItemPosition::Update { timeline_item_index: index },
position: TimelineItemPosition::Update { timeline_item_index },
..
} = self.ctx.flow
{
// If add was not called, that means the UTD event is one that
// wouldn't normally be visible. Remove it.
trace!("Removing UTD that was successfully retried");
self.items.remove(index);

// Shift all timeline item indices to the left after `timeline_item_index`!
for event_meta in &mut self.meta.all_events {
if let Some(index) = event_meta.timeline_item_index.as_mut() {
if *index >= timeline_item_index {
*index -= 1;
}
}
}

self.items.remove(timeline_item_index);

self.result.item_removed = true;
}
Expand Down Expand Up @@ -1020,6 +1032,47 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
// In more complex cases, remove the item before re-adding the item.
trace!("Removing local echo or duplicate timeline item");

dbg!(idx);
dbg!(&self.meta.all_events);

/*
// OK so we have a duplicated timeline item for sure. We must also remove
// the duplicated event in `self.meta.all_events`!
// However, we must be careful here. The timeline item can be duplicated
// without the event being duplicated: it happens when the timeline item is
// virtual for example. So we need to check there is an actual duplicated
// event, and record the position of the first event
let mut first_duplicated_event_index = None;
let mut number_of_duplications: usize = 0;
for (index, EventMeta { event_id: evt_id, .. }) in
self.meta.all_events.iter().enumerate()
{
if evt_id == event_id {
if first_duplicated_event_index.is_none() {
first_duplicated_event_index = Some(index);
}
number_of_duplications += 1;
}
}
if number_of_duplications > 1 {
if let Some(event_index) = first_duplicated_event_index {
self.meta.all_events.remove(event_index);
}
}
*/

// Shift all timeline item indices to the left after `idx`!
for event_meta in &mut self.meta.all_events {
if let Some(index) = event_meta.timeline_item_index.as_mut() {
if *index >= idx {
*index -= 1;
}
}
}

Some(self.items.remove(idx).internal_id.clone())

// no return here, below code for adding a new event
Expand All @@ -1037,32 +1090,23 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
};

let (event_index, timeline_item_index) = match position {
TimelineItemPosition::Start { .. } => (0, 0),
TimelineItemPosition::Start { .. }
| TimelineItemPosition::At { event_index: 0, .. } => (0, 0),
TimelineItemPosition::End { .. } => {
// Local echoes that are pending should stick to the bottom,
// find the latest event that isn't that.
let latest_event_idx =
self.items.iter().enumerate().rev().find_map(|(idx, item)| {
(!item.as_event()?.is_local_echo()).then_some(idx)
});

// Insert the next item after the latest event item that's not a
// pending local echo, or at the start if there is no such item.
(
self.meta
.all_events
.len()
.checked_sub(1)
// SAFETY: This is supposed to be unreachable because the event is
// first added into `all_events`
// before the timeline item is created.
// Consequently, `all_events` is never empty, and thus it's smallest
// size is 1, making the subtraction
// always valid.
.expect(
"The event is missing from `all_events`, it must not be empty",
),
latest_event_idx.map_or(0, |idx| idx + 1),
self.meta.all_events.len().checked_sub(1).unwrap_or(0),
// Insert the next item after the latest timeline item that's not a
// pending local echo, or at the start if there is no such item.
// Local echoes that are pending should stick to the bottom,
// find the latest event that isn't that.
self.items
.iter()
.enumerate()
.rev()
.find_map(|(index, item)| {
(!item.as_event()?.is_local_echo()).then_some(index)
})
.map_or(0, |index| index + 1),
)
}
TimelineItemPosition::At { event_index, .. } => {
Expand All @@ -1074,7 +1118,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
.meta
.all_events
// The events at the left.
.range(0..event_index)
//
// Note: the case where `event_index = 0` is matched with
// `TimelineItemPosition::Start`.
.range(0..=event_index)
// In reverse order, because we want to find the closest.
.rev()
.find_map(|event_meta| event_meta.timeline_item_index)
Expand Down Expand Up @@ -1113,13 +1160,6 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
event_id
);

let Some(event_meta) = self.meta.all_events.get_mut(event_index) else {
panic!("Event `{event_index}` is missing from `all_events`, it is inconsistent");
};

// The event index maps to a timeline item index!
event_meta.timeline_item_index = Some(timeline_item_index);

// Shift all timeline item indices to the right after `timeline_item_index`!
for event_meta in &mut self.meta.all_events {
if let Some(index) = event_meta.timeline_item_index.as_mut() {
Expand All @@ -1128,6 +1168,13 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
}
}
}

let Some(event_meta) = self.meta.all_events.get_mut(event_index) else {
panic!("Event `{event_index}` is missing from `all_events`, it is inconsistent");
};

// The event index maps to a timeline item index!
event_meta.timeline_item_index = Some(timeline_item_index);
}

trace!("Adding new remote timeline item after all non-pending events");
Expand All @@ -1139,7 +1186,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
None => self.meta.new_timeline_item(item),
};

// Keep push semantics, if we're inserting at the front or the back.
// Keep push semantics, if we're inserting at the front, the back, or in the
// middle.
if timeline_item_index == self.items.len() {
trace!("Adding new remote timeline item at the back");
self.items.push_back(new_item);
Expand Down
Loading

0 comments on commit d07a56f

Please sign in to comment.