From 4d89082ab54633979193e0f8132a2cbadd089369 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Thu, 5 Nov 2026 04:22:50 +0800 Subject: [PATCH] M5: group selection goes through the page router's one look at the point (#4358) main now checks that only listed functions ask which comment lies under a point. The group lookup is `click_target` (a subordinate stands for its primary, ISO 32000-2 12.5.6.2), listed with `picked`; a right-click acts on the comment the router already found instead of looking again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/ui-egui/src/canvas/page_pointer/tests.rs | 9 +++++++-- crates/ui-egui/src/tools/comments/input.rs | 7 ++++--- crates/ui-egui/src/tools/comments/state.rs | 4 ++-- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index 3f5d1e105f..634faf0cbc 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -559,9 +559,14 @@ const PAGE_HITS_ASKERS: [(&str, &str, &str); 5] = [ const COMMENT_POINT_TESTS: [&str; 2] = [".comment_at(", "PageCx::comment_at("]; /// The functions that ask which comment lies under a point: (file, function, why). -const COMMENT_POINT_TESTERS: [(&str, &str, &str); 3] = [ +const COMMENT_POINT_TESTERS: [(&str, &str, &str); 4] = [ ("tools/comments/input.rs", "hit", "the comments' hit test"), - ("tools/comments/input.rs", "context_hit", "the comments' hit test for a right-click"), + ( + "tools/comments/state.rs", + "click_target", + "the comments' hit tests' lookup: a group subordinate (`/RT /Group`, ISO 32000-2 ยง12.5.6.2) stands for its primary", + ), + ("tools/comments/state.rs", "picked", "which group subordinate a click selected its primary through, when the click is acted on"), ("tools/comments/paint.rs", "paint_page", "a redaction mark shows its fill under the pointer with every tool, as comments' tips show"), ]; diff --git a/crates/ui-egui/src/tools/comments/input.rs b/crates/ui-egui/src/tools/comments/input.rs index d0a1b3fdc1..5012c821a7 100644 --- a/crates/ui-egui/src/tools/comments/input.rs +++ b/crates/ui-egui/src/tools/comments/input.rs @@ -103,7 +103,7 @@ pub(crate) fn hit(cx: &PageCx<'_>, view: &DocView, p: Pos2, on_link: bool) -> Op // there selects the group. Only a click: a drag still moves what is under the pointer. let (a, member) = match cx.comment_at(p) { Some(a) => (a, false), - None => (cx.click_hit(p)?, true), + None => (cx.click_target(p)?, true), }; let a = Some(a).filter(|a| bodies && pick(a))?; // A click on a highlight over a link follows the link, so the link's hand and tip show there. @@ -131,7 +131,7 @@ pub(crate) fn context_hit(cx: &PageCx<'_>, p: Pos2) -> Option { if cx.hidden || !cx.xf.rect.contains(p) { return None; } - let a = cx.click_hit(p)?; + let a = cx.click_target(p)?; Some(Hit::new(Owner::Comment { index: a.index, part: CommentPart::Body }, layer(a), egui::CursorIcon::Default)) } @@ -146,7 +146,8 @@ pub(crate) fn anchor(cx: &PageCx<'_>, p: Pos2, view: &mut DocView) { /// anything else selects none. pub(crate) fn context(cx: &PageCx<'_>, p: Pos2, index: Option, view: &mut DocView) { match index { - Some(i) => match cx.click_hit(p).filter(|a| a.index == i) { + // `index` is what the right-click is on ([`context_hit`]): no second look at the point. + Some(i) => match cx.get(i) { Some(a) => view.comments.pick(cx, p, a), None => view.comments.select(Some((cx.page, i))), }, diff --git a/crates/ui-egui/src/tools/comments/state.rs b/crates/ui-egui/src/tools/comments/state.rs index 4d5f63e792..33177590ba 100644 --- a/crates/ui-egui/src/tools/comments/state.rs +++ b/crates/ui-egui/src/tools/comments/state.rs @@ -208,7 +208,7 @@ impl CommentView { self.member = None; } - /// Select `a`, the comment a press at `p` on this page selects ([`PageCx::click_hit`]), + /// Select `a`, the comment a press at `p` on this page selects ([`PageCx::click_target`]), /// remembering the group subordinate under `p` when `a` is its group's primary /// ([`PageCx::picked`]). The only place that sets [`Self::member`]. pub(crate) fn pick(&mut self, cx: &PageCx<'_>, p: Pos2, a: &Annotation) { @@ -439,7 +439,7 @@ impl PageCx<'_> { /// The comment a click at `p` selects: the topmost one under it or, on a group subordinate, /// the group's primary, which stands for the group. - pub(super) fn click_hit(&self, p: Pos2) -> Option<&Annotation> { + pub(super) fn click_target(&self, p: Pos2) -> Option<&Annotation> { self.comment_at(p).or_else(|| self.member_at(p).map(|(_, p)| p)) }