sidebar: Replace order-changed signal by watching expressions

Triggers fewer updates

src/session/room/mod.rs

@@ -216,12 +216,8 @@ mod imp {
         }
 
         fn signals() -> &'static [Signal] {
-            static SIGNALS: Lazy<Vec<Signal>> = Lazy::new(|| {
-                vec![
-                    Signal::builder("order-changed").build(),
-                    Signal::builder("room-forgotten").build(),
-                ]
-            });
+            static SIGNALS: Lazy<Vec<Signal>> =
+                Lazy::new(|| vec![Signal::builder("room-forgotten").build()]);
             SIGNALS.as_ref()
         }
 
@@ -395,7 +391,6 @@ impl Room {
 
         self.imp().category.set(category);
         self.notify("category");
-        self.emit_by_name::<()>("order-changed", &[]);
     }
 
     /// Set the category of this room.
@@ -1108,8 +1103,6 @@ impl Room {
         self.session()
             .verification_list()
             .handle_response_room(self, events.iter());
-
-        self.emit_by_name::<()>("order-changed", &[]);
     }
 
     /// The timestamp of the room's latest possibly unread event.
@@ -1366,14 +1359,6 @@ impl Room {
         )
     }
 
-    pub fn connect_order_changed<F: Fn(&Self) + 'static>(&self, f: F) -> glib::SignalHandlerId {
-        self.connect_local("order-changed", true, move |values| {
-            let obj = values[0].get::<Self>().unwrap();
-            f(&obj);
-            None
-        })
-    }
-
     /// Connect to the signal sent when a room was forgotten.
     pub fn connect_room_forgotten<F: Fn(&Self) + 'static>(&self, f: F) -> glib::SignalHandlerId {
         self.connect_local("room-forgotten", true, move |values| {

src/session/room_list.rs

@@ -89,8 +89,7 @@ glib::wrapper! {
     /// List of all joined rooms of the user.
     ///
     /// This is the parent ListModel of the sidebar from which all other models
-    /// are derived. If a room is updated in an order-relevant manner, use
-    /// `room.emit_by_name::<()>("order-changed", &[])` to fix the sorting.
+    /// are derived.
     ///
     /// The `RoomList` also takes care of all so called *pending rooms*, i.e.
     /// rooms the user requested to join, but received no response from the
@@ -162,14 +161,6 @@ impl RoomList {
         }
     }
 
-    fn get_full(&self, room_id: &RoomId) -> Option<(usize, OwnedRoomId, Room)> {
-        self.imp()
-            .list
-            .borrow()
-            .get_full(room_id)
-            .map(|(pos, room_id, room)| (pos, room_id.clone(), room.clone()))
-    }
-
     pub fn contains_key(&self, room_id: &RoomId) -> bool {
         self.imp().list.borrow().contains_key(room_id)
     }
@@ -192,11 +183,6 @@ impl RoomList {
         let position = list.len() - added;
 
         for (_room_id, room) in list.iter().skip(position) {
-            room.connect_order_changed(clone!(@weak self as obj => move |room| {
-                if let Some((position, _, _)) = obj.get_full(room.room_id()) {
-                    obj.items_changed(position as u32, 1, 1);
-                }
-            }));
             room.connect_room_forgotten(clone!(@weak self as obj => move |room| {
                 obj.remove(room.room_id());
             }));

src/session/sidebar/category/category_filter.rs

@@ -0,0 +1,165 @@
+use gtk::{glib, prelude::*, subclass::prelude::*};
+
+use super::CategoryType;
+
+mod imp {
+    use std::cell::{Cell, RefCell};
+
+    use super::*;
+
+    #[derive(Debug, Default)]
+    pub struct CategoryFilter {
+        /// The expression to watch.
+        pub expression: RefCell<Option<gtk::Expression>>,
+        /// The category type to filter.
+        pub category_type: Cell<CategoryType>,
+    }
+
+    #[glib::object_subclass]
+    impl ObjectSubclass for CategoryFilter {
+        const NAME: &'static str = "CategoryFilter";
+        type Type = super::CategoryFilter;
+        type ParentType = gtk::Filter;
+    }
+
+    impl ObjectImpl for CategoryFilter {
+        fn properties() -> &'static [glib::ParamSpec] {
+            use once_cell::sync::Lazy;
+            static PROPERTIES: Lazy<Vec<glib::ParamSpec>> = Lazy::new(|| {
+                vec![
+                    gtk::ParamSpecExpression::builder("expression")
+                        .explicit_notify()
+                        .build(),
+                    glib::ParamSpecEnum::builder::<CategoryType>("category-type")
+                        .explicit_notify()
+                        .build(),
+                ]
+            });
+
+            PROPERTIES.as_ref()
+        }
+
+        fn set_property(&self, _id: usize, value: &glib::Value, pspec: &glib::ParamSpec) {
+            let obj = self.obj();
+            match pspec.name() {
+                "expression" => obj.set_expression(value.get().unwrap()),
+                "category-type" => obj.set_category_type(value.get().unwrap()),
+                _ => unimplemented!(),
+            }
+        }
+
+        fn property(&self, _id: usize, pspec: &glib::ParamSpec) -> glib::Value {
+            let obj = self.obj();
+
+            match pspec.name() {
+                "expression" => obj.expression().to_value(),
+                "category-type" => obj.category_type().to_value(),
+                _ => unimplemented!(),
+            }
+        }
+    }
+
+    impl FilterImpl for CategoryFilter {
+        fn strictness(&self) -> gtk::FilterMatch {
+            if self.category_type.get() == CategoryType::None {
+                return gtk::FilterMatch::All;
+            }
+
+            if self.expression.borrow().is_none() {
+                return gtk::FilterMatch::None;
+            }
+
+            gtk::FilterMatch::Some
+        }
+
+        fn match_(&self, item: &glib::Object) -> bool {
+            let category_type = self.category_type.get();
+            if category_type == CategoryType::None {
+                return true;
+            }
+
+            let Some(value) = self.expression.borrow().as_ref().and_then(|e| e.evaluate(Some(item))).map(|v| v.get::<CategoryType>().unwrap()) else {
+                return false;
+            };
+
+            value == category_type
+        }
+    }
+}
+
+glib::wrapper! {
+    /// A filter by `CategoryType`.
+    pub struct CategoryFilter(ObjectSubclass<imp::CategoryFilter>)
+        @extends gtk::Filter;
+}
+
+impl CategoryFilter {
+    pub fn new(expression: impl AsRef<gtk::Expression>, category_type: CategoryType) -> Self {
+        glib::Object::builder()
+            .property("expression", expression.as_ref())
+            .property("category-type", category_type)
+            .build()
+    }
+
+    /// The expression to watch.
+    pub fn expression(&self) -> Option<gtk::Expression> {
+        self.imp().expression.borrow().clone()
+    }
+
+    /// Set the expression to watch.
+    ///
+    /// This expression must return a [`CategoryType`].
+    pub fn set_expression(&self, expression: Option<gtk::Expression>) {
+        let prev_expression = self.expression();
+
+        if prev_expression.is_none() && expression.is_none() {
+            return;
+        }
+
+        let change = if self.category_type() == CategoryType::None {
+            None
+        } else if prev_expression.is_none() {
+            Some(gtk::FilterChange::LessStrict)
+        } else if expression.is_none() {
+            Some(gtk::FilterChange::MoreStrict)
+        } else {
+            Some(gtk::FilterChange::Different)
+        };
+
+        self.imp().expression.replace(expression);
+        if let Some(change) = change {
+            self.changed(change)
+        }
+        self.notify("expression");
+    }
+
+    /// The category type to filter.
+    pub fn category_type(&self) -> CategoryType {
+        self.imp().category_type.get()
+    }
+
+    /// Set the category type to filter.
+    pub fn set_category_type(&self, category_type: CategoryType) {
+        let prev_category_type = self.category_type();
+
+        if prev_category_type == category_type {
+            return;
+        }
+
+        let change = if self.expression().is_none() {
+            None
+        } else if prev_category_type == CategoryType::None {
+            Some(gtk::FilterChange::MoreStrict)
+        } else if category_type == CategoryType::None {
+            Some(gtk::FilterChange::LessStrict)
+        } else {
+            Some(gtk::FilterChange::Different)
+        };
+
+        self.imp().category_type.set(category_type);
+        if let Some(change) = change {
+            self.changed(change)
+        }
+        self.notify("category-type");
+    }
+}

src/session/sidebar/category/mod.rs

@@ -1,13 +1,23 @@
-use gtk::{gio, glib, glib::clone, prelude::*, subclass::prelude::*};
+use gtk::{
+    gio, glib,
+    glib::{clone, closure},
+    prelude::*,
+    subclass::prelude::*,
+};
 
+mod category_filter;
 mod category_row;
 mod category_type;
 
+use self::category_filter::CategoryFilter;
 pub use self::{category_row::CategoryRow, category_type::CategoryType};
 use super::{SidebarItem, SidebarItemExt, SidebarItemImpl};
-use crate::session::{
-    room::{Room, RoomType},
-    room_list::RoomList,
+use crate::{
+    session::{
+        room::{Room, RoomType},
+        room_list::RoomList,
+    },
+    utils::ExpressionListModel,
 };
 
 mod imp {
@@ -145,18 +155,25 @@ impl Category {
 
         // Special case room lists so that they are sorted and in the right category
         let model = if model.is::<RoomList>() {
-            let filter = gtk::CustomFilter::new(move |o| {
-                o.downcast_ref::<Room>()
-                    .filter(|r| CategoryType::from(r.category()) == type_)
-                    .is_some()
-            });
-            let filter_model = gtk::FilterListModel::new(Some(model), Some(filter));
+            let room_category_type = Room::this_expression("category")
+                .chain_closure::<CategoryType>(closure!(
+                    |_: Option<glib::Object>, room_type: RoomType| {
+                        CategoryType::from(room_type)
+                    }
+                ));
+            let filter = CategoryFilter::new(&room_category_type, type_);
+            let category_type_expr_model = ExpressionListModel::new(model, room_category_type);
+            let filter_model =
+                gtk::FilterListModel::new(Some(category_type_expr_model), Some(filter));
 
+            let room_latest_unread = Room::this_expression("latest-unread");
             let sorter = gtk::NumericSorter::builder()
-                .expression(Room::this_expression("latest-unread"))
+                .expression(&room_latest_unread)
                 .sort_order(gtk::SortType::Descending)
                 .build();
-            let sort_model = gtk::SortListModel::new(Some(filter_model), Some(sorter));
+            let latest_unread_expr_model =
+                ExpressionListModel::new(filter_model, room_latest_unread);
+            let sort_model = gtk::SortListModel::new(Some(latest_unread_expr_model), Some(sorter));
             sort_model.upcast()
         } else {
             model

src/utils/expression_list_model.rs

@@ -0,0 +1,216 @@
+use gtk::{gio, glib, glib::clone, prelude::*, subclass::prelude::*};
+use log::error;
+
+use crate::utils::BoundObject;
+
+mod imp {
+    use std::cell::RefCell;
+
+    use once_cell::sync::Lazy;
+
+    use super::*;
+
+    #[derive(Debug, Default)]
+    pub struct ExpressionListModel {
+        pub model: BoundObject<gio::ListModel>,
+        pub expression: RefCell<Option<gtk::Expression>>,
+        pub watches: RefCell<Vec<gtk::ExpressionWatch>>,
+    }
+
+    #[glib::object_subclass]
+    impl ObjectSubclass for ExpressionListModel {
+        const NAME: &'static str = "ExpressionListModel";
+        type Type = super::ExpressionListModel;
+        type Interfaces = (gio::ListModel,);
+    }
+
+    impl ObjectImpl for ExpressionListModel {
+        fn properties() -> &'static [glib::ParamSpec] {
+            static PROPERTIES: Lazy<Vec<glib::ParamSpec>> = Lazy::new(|| {
+                vec![
+                    glib::ParamSpecObject::builder::<gio::ListModel>("model")
+                        .explicit_notify()
+                        .build(),
+                    gtk::ParamSpecExpression::builder("expression")
+                        .explicit_notify()
+                        .build(),
+                ]
+            });
+
+            PROPERTIES.as_ref()
+        }
+
+        fn set_property(&self, _id: usize, value: &glib::Value, pspec: &glib::ParamSpec) {
+            let obj = self.obj();
+
+            match pspec.name() {
+                "model" => obj.set_model(value.get().unwrap()),
+                "expression" => obj.set_expression(value.get().unwrap()),
+                _ => unimplemented!(),
+            }
+        }
+
+        fn property(&self, _id: usize, pspec: &glib::ParamSpec) -> glib::Value {
+            let obj = self.obj();
+
+            match pspec.name() {
+                "model" => obj.model().to_value(),
+                "expression" => obj.expression().to_value(),
+                _ => unimplemented!(),
+            }
+        }
+
+        fn dispose(&self) {
+            self.model.disconnect_signals();
+
+            for watch in self.watches.take() {
+                watch.unwatch()
+            }
+        }
+    }
+
+    impl ListModelImpl for ExpressionListModel {
+        fn item_type(&self) -> glib::Type {
+            self.model
+                .obj()
+                .map(|m| m.item_type())
+                .unwrap_or_else(glib::Object::static_type)
+        }
+
+        fn n_items(&self) -> u32 {
+            self.model.obj().map(|m| m.n_items()).unwrap_or_default()
+        }
+
+        fn item(&self, position: u32) -> Option<glib::Object> {
+            self.model.obj().and_then(|m| m.item(position))
+        }
+    }
+}
+
+glib::wrapper! {
+    /// A list model that signals an item as changed when the expression's value changes.
+    pub struct ExpressionListModel(ObjectSubclass<imp::ExpressionListModel>)
+        @implements gio::ListModel;
+}
+
+impl ExpressionListModel {
+    pub fn new(model: impl IsA<gio::ListModel>, expression: impl AsRef<gtk::Expression>) -> Self {
+        glib::Object::builder()
+            .property("model", model.upcast())
+            .property("expression", expression.as_ref())
+            .build()
+    }
+
+    /// The underlying model.
+    pub fn model(&self) -> Option<gio::ListModel> {
+        self.imp().model.obj()
+    }
+
+    /// Set the underlying model.
+    pub fn set_model(&self, model: Option<gio::ListModel>) {
+        let imp = self.imp();
+
+        if imp.model.obj() == model {
+            return;
+        }
+
+        let removed = self.n_items();
+
+        imp.model.disconnect_signals();
+        for watch in imp.watches.take() {
+            watch.unwatch();
+        }
+
+        let added = if let Some(model) = model {
+            let items_changed_handler = model.connect_items_changed(
+                clone!(@weak self as obj => move |_, pos, removed, added| {
+                    obj.watch_items(pos, removed, added);
+                    obj.items_changed(pos, removed, added);
+                }),
+            );
+
+            let added = model.n_items();
+            imp.model.set(model, vec![items_changed_handler]);
+
+            self.watch_items(0, removed, added);
+            added
+        } else {
+            0
+        };
+
+        self.items_changed(0, removed, added);
+        self.notify("model");
+    }
+
+    /// The watched expression.
+    pub fn expression(&self) -> Option<gtk::Expression> {
+        self.imp().expression.borrow().clone()
+    }
+
+    /// Set the watched expression.
+    pub fn set_expression(&self, expression: Option<gtk::Expression>) {
+        if self.expression().is_none() && expression.is_none() {
+            return;
+        }
+
+        let imp = self.imp();
+
+        // Reset expression watches.
+        for watch in imp.watches.take() {
+            watch.unwatch();
+        }
+
+        imp.expression.replace(expression);
+
+        // Watch items again.
+        let added = self.n_items();
+        self.watch_items(0, 0, added);
+
+        self.notify("expression");
+    }
+
+    /// Watch and unwatch items according to changes in the underlying model.
+    fn watch_items(&self, pos: u32, removed: u32, added: u32) {
+        let Some(expression) = self.expression() else {
+            return;
+        };
+        let Some(model) = self.model() else {
+            return;
+        };
+        let imp = self.imp();
+
+        let mut new_watches = Vec::with_capacity(added as usize);
+        for item_pos in pos..pos + added {
+            let Some(item) = model.item(item_pos) else {
+                error!("Out of bounds item");
+                break;
+            };
+
+            new_watches.push(expression.watch(
+                Some(&item),
+                clone!(@weak self as obj, @weak item => move || {
+                    obj.item_expr_changed(&item);
+                }),
+            ));
+        }
+
+        let mut watches = imp.watches.borrow_mut();
+        let removed_range = (pos as usize)..((pos + removed) as usize);
+        for watch in watches.splice(removed_range, new_watches) {
+            watch.unwatch()
+        }
+    }
+
+    fn item_expr_changed(&self, item: &glib::Object) {
+        let Some(model) = self.model() else {
+            return;
+        };
+
+        for (pos, obj) in model.snapshot().iter().enumerate() {
+            if obj == item {
+                self.items_changed(pos as u32, 1, 1);
+                break;
+            }
+        }
+    }
+}

src/utils/mod.rs

@@ -1,5 +1,6 @@
 //! Collection of common methods and types.
 
+mod expression_list_model;
 pub mod macros;
 pub mod matrix;
 pub mod media;
@@ -25,6 +26,8 @@ use matrix_sdk::ruma::UInt;
 use once_cell::sync::Lazy;
 use regex::Regex;
 
+pub use self::expression_list_model::ExpressionListModel;
+
 /// Returns an expression that is the and’ed result of the given boolean
 /// expressions.
 #[allow(dead_code)]
@@ -152,6 +155,65 @@ pub static EMOJI_REGEX: Lazy<Regex> = Lazy::new(|| {
     .unwrap()
 });
 
+/// Inner to manage a bound object.
+#[derive(Debug)]
+pub struct BoundObjectInner<T: glib::ObjectType> {
+    obj: T,
+    signal_handler_ids: Vec<glib::SignalHandlerId>,
+}
+
+/// Wrapper to manage a bound object.
+///
+/// This keeps a strong reference to the object.
+#[derive(Debug)]
+pub struct BoundObject<T: glib::ObjectType> {
+    inner: RefCell<Option<BoundObjectInner<T>>>,
+}
+
+impl<T: glib::ObjectType> BoundObject<T> {
+    /// Creates a new empty `BoundObjectWeakRef`.
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    /// Set the given object and signal handlers IDs.
+    ///
+    /// Calls `disconnect_signals` first to drop the previous strong reference
+    /// and disconnect the previous signal handlers.
+    pub fn set(&self, obj: T, signal_handler_ids: Vec<glib::SignalHandlerId>) {
+        self.disconnect_signals();
+
+        let inner = BoundObjectInner {
+            obj,
+            signal_handler_ids,
+        };
+
+        self.inner.replace(Some(inner));
+    }
+
+    /// Get the object, if any.
+    pub fn obj(&self) -> Option<T> {
+        self.inner.borrow().as_ref().map(|inner| inner.obj.clone())
+    }
+
+    /// Disconnect the signal handlers and drop the strong reference.
+    pub fn disconnect_signals(&self) {
+        if let Some(inner) = self.inner.take() {
+            for signal_handler_id in inner.signal_handler_ids {
+                inner.obj.disconnect(signal_handler_id)
+            }
+        }
+    }
+}
+
+impl<T: glib::ObjectType> Default for BoundObject<T> {
+    fn default() -> Self {
+        Self {
+            inner: Default::default(),
+        }
+    }
+}
+
 /// Wrapper to manage a bound object.
 ///
 /// This keeps a weak reference to the object.
@@ -184,13 +246,6 @@ impl<T: glib::ObjectType> BoundObjectWeakRef<T> {
         self.weak_obj.upgrade()
     }
 
-    /// Add `SignalHandlerId`s to this `BoundObjectWeakRef`.
-    pub fn add_signal_handler_ids(&mut self, signal_handler_ids: Vec<glib::SignalHandlerId>) {
-        self.signal_handler_ids
-            .borrow_mut()
-            .extend(signal_handler_ids);
-    }
-
     /// Disconnect the signal handlers and drop the weak reference.
     pub fn disconnect_signals(&self) {
         let signal_handler_ids = self.signal_handler_ids.take();