From b66f890546431ed42c7281a486d6aff4fc7a252a Mon Sep 17 00:00:00 2001 From: Karim Jerbi Date: Sat, 10 Oct 2026 08:18:56 +0100 Subject: [PATCH] fix(statusbar): scale pill number, label and badge follow one viewport The pill number used the first-content fallback while the label and sync badge used explicit-only resolution, showing e.g. a 1:50 number beside a 1:1 label with no badge. Label and badge now resolve through target_viewport_handle like the number; render-path handle fn, set-paths and Model branches untouched. Tests: scale_pill_sources_agree_without_targeted_viewport (RED: live probe Some(0.02)/1:1/None) plus Model-unchanged pin. Full suite: 2027 passed / 0 failed / 25 ignored. --- src/scene/mod.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/src/scene/mod.rs b/src/scene/mod.rs index 1272673c4..9af6c1224 100644 --- a/src/scene/mod.rs +++ b/src/scene/mod.rs @@ -6386,7 +6386,13 @@ impl Scene { if self.current_layout == "Model" { return self.document.header.current_annotation_scale.clone(); } - self.explicit_viewport_handle() + // Same viewport the pill number comes from (active → selected → + // first-content fallback), so label, number and badge always describe + // one viewport (#48). Degenerate case: with zero Scale objects the + // name falls back to paper/"1:1" while the number stays factual. + // The render paths use displayed_annotation_scale_HANDLE, which stays + // explicit-only — untouched by this fix. + self.target_viewport_handle() .and_then(|viewport| self.viewport_scale_handle(viewport)) .and_then(|handle| match self.document.objects.get(&handle) { Some(ObjectType::Scale(scale)) => Some(scale.name.clone()), @@ -6480,7 +6486,8 @@ impl Scene { } pub fn viewport_annotation_scale_synced(&self) -> Option { - let viewport = self.explicit_viewport_handle()?; + // Same viewport as the pill number and label (#48). + let viewport = self.target_viewport_handle()?; let EntityType::Viewport(vp) = self.document.get_entity(viewport)? else { return None; }; @@ -14447,6 +14454,48 @@ mod layout_cache_tests { ); assert!(after.iter().any(|n| n == "TEST_CACHED_UCS")); } + + // #48: with no active/selected viewport, the pill number, label and sync + // badge must all describe the SAME viewport (the first-content fallback + // the number already used) — not a mix of viewport number, paper label + // and hidden badge. + #[test] + fn scale_pill_sources_agree_without_targeted_viewport() { + let mut scene = Scene::new(); + scene.add_layout("P1").unwrap(); + scene.current_layout = "P1".to_string(); + assert!(scene.add_scale("1:50", 1.0, 50.0)); + let mut vp = codec::entities::Viewport::new(); + vp.width = 100.0; + vp.height = 100.0; + let handle = scene + .document + .add_entity_to_layout(codec::EntityType::Viewport(vp), "P1") + .unwrap(); + assert!(scene.set_viewport_scale_named_for(handle, "1:50").is_some()); + assert!(scene.active_viewport.is_none()); + let number = scene.first_viewport_scale(); + let name = scene.displayed_annotation_scale_name(); + let synced = scene.viewport_annotation_scale_synced(); + eprintln!("number={number:?} name={name:?} synced={synced:?}"); + assert!( + number.is_some() && name == "1:50" && synced == Some(true), + "pill trio must agree" + ); + } + + #[test] + fn scale_pill_model_layout_unchanged() { + // Model layout has no viewports: number None, header label, no badge. + let scene = Scene::new(); + assert!(scene.current_layout == "Model"); + assert!(scene.first_viewport_scale().is_none()); + assert_eq!( + scene.displayed_annotation_scale_name(), + scene.document.header.current_annotation_scale + ); + assert!(scene.viewport_annotation_scale_synced().is_none()); + } } #[cfg(test)]