From 8506064d310a3e0831b9f5ad3e53e6a7dc6571a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Wed, 5 Apr 2023 15:36:02 +0200 Subject: [PATCH] sidebar: Replace order-changed signal by watching expressions Triggers fewer updates --- src/session/room/mod.rs | 19 +- src/session/room_list.rs | 16 +- .../sidebar/category/category_filter.rs | 165 +++++++++++++ src/session/sidebar/category/mod.rs | 41 +++- src/utils/expression_list_model.rs | 216 ++++++++++++++++++ src/utils/mod.rs | 69 +++++- 6 files changed, 475 insertions(+), 51 deletions(-) create mode 100644 src/session/sidebar/category/category_filter.rs create mode 100644 src/utils/expression_list_model.rs diff --git a/src/session/room/mod.rs b/src/session/room/mod.rs index 313ef6d8..60b031b3 100644 --- a/src/session/room/mod.rs +++ b/src/session/room/mod.rs @@ -216,12 +216,8 @@ mod imp { } fn signals() -> &'static [Signal] { - static SIGNALS: Lazy> = Lazy::new(|| { - vec![ - Signal::builder("order-changed").build(), - Signal::builder("room-forgotten").build(), - ] - }); + static SIGNALS: Lazy> = + 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(&self, f: F) -> glib::SignalHandlerId { - self.connect_local("order-changed", true, move |values| { - let obj = values[0].get::().unwrap(); - f(&obj); - None - }) - } - /// Connect to the signal sent when a room was forgotten. pub fn connect_room_forgotten(&self, f: F) -> glib::SignalHandlerId { self.connect_local("room-forgotten", true, move |values| { diff --git a/src/session/room_list.rs b/src/session/room_list.rs index 43881d4a..2b38ffed 100644 --- a/src/session/room_list.rs +++ b/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()); })); diff --git a/src/session/sidebar/category/category_filter.rs b/src/session/sidebar/category/category_filter.rs new file mode 100644 index 00000000..12300283 --- /dev/null +++ b/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>, + /// The category type to filter. + pub category_type: Cell, + } + + #[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> = Lazy::new(|| { + vec![ + gtk::ParamSpecExpression::builder("expression") + .explicit_notify() + .build(), + glib::ParamSpecEnum::builder::("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::().unwrap()) else { + return false; + }; + + value == category_type + } + } +} + +glib::wrapper! { + /// A filter by `CategoryType`. + pub struct CategoryFilter(ObjectSubclass) + @extends gtk::Filter; +} + +impl CategoryFilter { + pub fn new(expression: impl AsRef, 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 { + self.imp().expression.borrow().clone() + } + + /// Set the expression to watch. + /// + /// This expression must return a [`CategoryType`]. + pub fn set_expression(&self, expression: Option) { + 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"); + } +} diff --git a/src/session/sidebar/category/mod.rs b/src/session/sidebar/category/mod.rs index 166a3caf..44e73ac4 100644 --- a/src/session/sidebar/category/mod.rs +++ b/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::() { - let filter = gtk::CustomFilter::new(move |o| { - o.downcast_ref::() - .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::(closure!( + |_: Option, 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 diff --git a/src/utils/expression_list_model.rs b/src/utils/expression_list_model.rs new file mode 100644 index 00000000..25a67224 --- /dev/null +++ b/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, + pub expression: RefCell>, + pub watches: RefCell>, + } + + #[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> = Lazy::new(|| { + vec![ + glib::ParamSpecObject::builder::("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 { + 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) + @implements gio::ListModel; +} + +impl ExpressionListModel { + pub fn new(model: impl IsA, expression: impl AsRef) -> Self { + glib::Object::builder() + .property("model", model.upcast()) + .property("expression", expression.as_ref()) + .build() + } + + /// The underlying model. + pub fn model(&self) -> Option { + self.imp().model.obj() + } + + /// Set the underlying model. + pub fn set_model(&self, model: Option) { + 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 { + self.imp().expression.borrow().clone() + } + + /// Set the watched expression. + pub fn set_expression(&self, expression: Option) { + 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; + } + } + } +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index be0babeb..585215c7 100644 --- a/src/utils/mod.rs +++ b/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 = Lazy::new(|| { .unwrap() }); +/// Inner to manage a bound object. +#[derive(Debug)] +pub struct BoundObjectInner { + obj: T, + signal_handler_ids: Vec, +} + +/// Wrapper to manage a bound object. +/// +/// This keeps a strong reference to the object. +#[derive(Debug)] +pub struct BoundObject { + inner: RefCell>>, +} + +impl BoundObject { + /// 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) { + 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 { + 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 Default for BoundObject { + 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 BoundObjectWeakRef { self.weak_obj.upgrade() } - /// Add `SignalHandlerId`s to this `BoundObjectWeakRef`. - pub fn add_signal_handler_ids(&mut self, signal_handler_ids: Vec) { - 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();