From 50cb21e4d4458ea5b32f77a5ca9233b537d77637 Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Wed, 2 Sep 2026 18:20:29 +0100 Subject: [PATCH] refactor(ribbon): make StyleContext borrowed to avoid 8 clones per view Replace the 8 owned clones in StyleContext construction (4 Vec + 4 String) at ribbon/mod.rs:568 and :1059 with borrowed refs. StyleContext is now StyleContext<'a> with &'a [String]/&'a str (widgets.rs:313), RenderCtx holds &'a StyleContext<'a> (:633), and call sites pass &self.x instead of .clone(). The view-local borrow is valid for the duration of Ribbon::view() and the style dropdown overlay. TDD: RED style_context_tests failed E0308 (owned vs borrowed), GREEN added lifetime and updated 4 type sites + 2 construction sites. --- src/ui/ribbon/mod.rs | 36 ++++++++-------- src/ui/ribbon/widgets.rs | 91 +++++++++++++++++++++++++++++++--------- 2 files changed, 90 insertions(+), 37 deletions(-) diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index 5b3308611..39d7444d3 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -566,14 +566,14 @@ impl Ribbon { if let Some(module) = self.modules.get(effective_active) { let groups = module.ribbon_groups(); let style_ctx = StyleContext { - text_style_names: self.text_style_names.clone(), - active_text_style: self.active_text_style.clone(), - dim_style_names: self.dim_style_names.clone(), - active_dim_style: self.active_dim_style.clone(), - mleader_style_names: self.mleader_style_names.clone(), - active_mleader_style: self.active_mleader_style.clone(), - table_style_names: self.table_style_names.clone(), - active_table_style: self.active_table_style.clone(), + text_style_names: &self.text_style_names, + active_text_style: &self.active_text_style, + dim_style_names: &self.dim_style_names, + active_dim_style: &self.active_dim_style, + mleader_style_names: &self.mleader_style_names, + active_mleader_style: &self.active_mleader_style, + table_style_names: &self.table_style_names, + active_table_style: &self.active_table_style, }; // Adaptive tool area: panels sit on one row; when they don't all @@ -1057,14 +1057,14 @@ impl Ribbon { let (style_key, manager_cmd) = found?; let ctx = StyleContext { - text_style_names: self.text_style_names.clone(), - active_text_style: self.active_text_style.clone(), - dim_style_names: self.dim_style_names.clone(), - active_dim_style: self.active_dim_style.clone(), - mleader_style_names: self.mleader_style_names.clone(), - active_mleader_style: self.active_mleader_style.clone(), - table_style_names: self.table_style_names.clone(), - active_table_style: self.active_table_style.clone(), + text_style_names: &self.text_style_names, + active_text_style: &self.active_text_style, + dim_style_names: &self.dim_style_names, + active_dim_style: &self.active_dim_style, + mleader_style_names: &self.mleader_style_names, + active_mleader_style: &self.active_mleader_style, + table_style_names: &self.table_style_names, + active_table_style: &self.active_table_style, }; let active = ctx.active_for(style_key).to_string(); @@ -1298,7 +1298,7 @@ fn render_group<'a>( active_color: AcadColor, active_linetype: &'a str, active_lineweight: LineWeight, - style_ctx: &StyleContext, + style_ctx: &StyleContext<'_>, ) -> Element<'a, Message> { let mut items_row: Vec> = Vec::new(); let mut small_buf: Vec> = Vec::new(); @@ -1419,7 +1419,7 @@ fn collapse_button<'a>( active_color: AcadColor, active_linetype: &'a str, active_lineweight: LineWeight, - style_ctx: &StyleContext, + style_ctx: &StyleContext<'_>, // When set, this is the TIGHT step (reached once even the all-collapsed row // overflows): the whole panel becomes a single non-running button — the first // tool's icon + a ▾ that opens the tools flyout. Nothing runs directly here. diff --git a/src/ui/ribbon/widgets.rs b/src/ui/ribbon/widgets.rs index edbc651d0..333147351 100644 --- a/src/ui/ribbon/widgets.rs +++ b/src/ui/ribbon/widgets.rs @@ -310,32 +310,32 @@ pub(super) const PROP_LW_ID: &str = "PROP_LW"; // ── Style context (passed from Ribbon to render_large) ──────────────────── -pub(super) struct StyleContext { - pub text_style_names: Vec, - pub active_text_style: String, - pub dim_style_names: Vec, - pub active_dim_style: String, - pub mleader_style_names: Vec, - pub active_mleader_style: String, - pub table_style_names: Vec, - pub active_table_style: String, +pub(super) struct StyleContext<'a> { + pub text_style_names: &'a [String], + pub active_text_style: &'a str, + pub dim_style_names: &'a [String], + pub active_dim_style: &'a str, + pub mleader_style_names: &'a [String], + pub active_mleader_style: &'a str, + pub table_style_names: &'a [String], + pub active_table_style: &'a str, } -impl StyleContext { +impl<'a> StyleContext<'a> { pub(super) fn names_for(&self, key: StyleKey) -> &[String] { match key { - StyleKey::TextStyle => &self.text_style_names, - StyleKey::DimStyle => &self.dim_style_names, - StyleKey::MLeaderStyle => &self.mleader_style_names, - StyleKey::TableStyle => &self.table_style_names, + StyleKey::TextStyle => self.text_style_names, + StyleKey::DimStyle => self.dim_style_names, + StyleKey::MLeaderStyle => self.mleader_style_names, + StyleKey::TableStyle => self.table_style_names, } } pub(super) fn active_for(&self, key: StyleKey) -> &str { match key { - StyleKey::TextStyle => &self.active_text_style, - StyleKey::DimStyle => &self.active_dim_style, - StyleKey::MLeaderStyle => &self.active_mleader_style, - StyleKey::TableStyle => &self.active_table_style, + StyleKey::TextStyle => self.active_text_style, + StyleKey::DimStyle => self.active_dim_style, + StyleKey::MLeaderStyle => self.active_mleader_style, + StyleKey::TableStyle => self.active_table_style, } } } @@ -630,7 +630,7 @@ pub(super) struct RenderCtx<'a> { pub active_color: AcadColor, pub active_linetype: &'a str, pub active_lineweight: LineWeight, - pub style_ctx: &'a StyleContext, + pub style_ctx: &'a StyleContext<'a>, /// When compact, the Properties panel's Match button shrinks to a small icon. pub compact: bool, } @@ -1274,3 +1274,56 @@ pub(super) fn top_hist_btn_style( snap: false, } } + +#[cfg(test)] +mod style_context_tests { + use super::StyleContext; + use crate::modules::StyleKey; + + #[test] + fn style_context_borrowed_without_cloning() { + // RED: should construct from borrowed slices/strs without cloning. + let text_names = vec!["Standard".to_string(), "MyStyle".to_string()]; + let active_text = "Standard".to_string(); + let dim_names = vec!["DimStd".to_string()]; + let active_dim = "DimStd".to_string(); + let mleader_names: Vec = vec![]; + let active_mleader = String::new(); + let table_names: Vec = vec![]; + let active_table = String::new(); + + let ctx = StyleContext { + text_style_names: &text_names, + active_text_style: &active_text, + dim_style_names: &dim_names, + active_dim_style: &active_dim, + mleader_style_names: &mleader_names, + active_mleader_style: &active_mleader, + table_style_names: &table_names, + active_table_style: &active_table, + }; + assert_eq!(ctx.names_for(StyleKey::TextStyle), &text_names); + assert_eq!(ctx.active_for(StyleKey::TextStyle), "Standard"); + assert_eq!(ctx.names_for(StyleKey::DimStyle), &dim_names); + } + + #[test] + fn style_context_no_clone_needed() { + let names = vec!["A".to_string(), "B".to_string()]; + let active = "A".to_string(); + let empty: Vec = vec![]; + let empty_s = String::new(); + let ctx = StyleContext { + text_style_names: &names, + active_text_style: &active, + dim_style_names: &empty, + active_dim_style: &empty_s, + mleader_style_names: &empty, + active_mleader_style: &empty_s, + table_style_names: &empty, + active_table_style: &empty_s, + }; + assert_eq!(ctx.active_for(StyleKey::TableStyle), ""); + assert!(ctx.names_for(StyleKey::MLeaderStyle).is_empty()); + } +}