mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-06 10:47:27 +00:00
fix(ui): a press where two pages touch is one page's; the guard sees PageHits::hits
Found in review: - **A press where two pages touch.** With the page gaps hidden, pages can share a row of pixels, and a press there was claimed by both: the first page acted on it, then the second took it over, and the text layer could count it twice. Only a press just begun is claimed now, by the first page that does. This was so on `main` too. - **The guard** listed who asks `PageHits::at` and `context_at`, but not `PageHits::hits`, which they wrap. A new call of it in `page_input.rs` now fails, and the guard's doc says what it still can't see. - **Docs.** `Hit::declines_drag` (a link's drag with the Link tool), `over_mark`, and when the Select tool still unselects under another tool's frame. Tests: two pages claiming one press, seen failing without the fix. A planted call of `PageHits::hits` fails the guard. The router's unit tests now begin a press before claiming it, as `draw_pages` does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c1d34fc9ea
commit
2a4032b2fc
4 files changed
+34
-10
No files matched your search
@@ -236,8 +236,9 @@ fn link_of(owner: Option<Owner>) -> Option<usize> {
|
||||
/// stamps, Fill & Sign, Sign area, zoom and snapshot, the camera's quadrilateral and crop act only
|
||||
/// on a press that began on this page and that no hit won (`press` is Free), and keep their cursor
|
||||
/// and gestures. The camera also drags a corner whose press it holds, and crop's double-click
|
||||
/// opens Set Page Boxes wherever it is. `over_mark`: a Fill & Sign mark takes the press, or would take one under the
|
||||
/// pointer.
|
||||
/// opens Set Page Boxes wherever it is. `over_mark`: with Fill & Sign, a hit holds the press (a
|
||||
/// mark's, or a Prepare field's), or with no button down a mark lies under the pointer, where a
|
||||
/// press would pick it up.
|
||||
fn tool_input(
|
||||
ui: &egui::Ui,
|
||||
view: &mut DocView,
|
||||
|
||||
@@ -453,13 +453,17 @@ const HIT_TESTERS: [(&str, &str, &str); 1] = [(
|
||||
"`PageHits::hits`: every module's hits at a point, for the pointer's cursor and tip, a right-click and, once, the press's owner",
|
||||
)];
|
||||
|
||||
/// The page view's asks for every module's hits at a point (`PageHits::at`, `PageHits::context_at`).
|
||||
const PAGE_HITS_ASKS: [&str; 2] = [".at(", ".context_at("];
|
||||
/// The page view's asks for every module's hits at a point (`PageHits::hits`, and `at` and
|
||||
/// `context_at`, which wrap it).
|
||||
const PAGE_HITS_ASKS: [&str; 3] = [".hits(", ".at(", ".context_at("];
|
||||
|
||||
/// The functions that ask the page view's hits at a point: (file, function, why). `PageHits` is
|
||||
/// private to `page_input.rs`, so this keeps a new place there from testing a point again; a
|
||||
/// second ask inside one of these is for review to catch.
|
||||
const PAGE_HITS_ASKERS: [(&str, &str, &str); 3] = [
|
||||
/// private to `page_input.rs`, so this keeps a new function there from asking by these names; a
|
||||
/// call by path (`PageHits::at(&hits, …)`) and a second ask inside a listed function are for
|
||||
/// review to catch.
|
||||
const PAGE_HITS_ASKERS: [(&str, &str, &str); 5] = [
|
||||
("canvas/page_input.rs", "at", "`PageHits::at`: the hits at a point, as a press there would take them"),
|
||||
("canvas/page_input.rs", "context_at", "`PageHits::context_at`: what a right-click at a point is on"),
|
||||
(
|
||||
"canvas/page_input.rs",
|
||||
"page_tool_input",
|
||||
|
||||
@@ -90,7 +90,7 @@ pub(crate) struct Hit {
|
||||
/// Beside the pointer where this hit wins (a link's target).
|
||||
pub tip: Option<String>,
|
||||
/// A drag from here is not this owner's: a click on a highlight selects it, a drag across it
|
||||
/// selects the text underneath.
|
||||
/// selects the text underneath; with the Link tool, a drag from a link draws a new one.
|
||||
pub declines_drag: bool,
|
||||
}
|
||||
|
||||
@@ -159,7 +159,12 @@ impl PageRouter {
|
||||
|
||||
/// A press on `page`: `hits` at the press point decide who it belongs to. A press nobody's
|
||||
/// hit claims is held by nobody (the page's text, or a tool that acts anywhere on the page).
|
||||
/// Only a press just begun ([`Self::begin`]) is claimed, by the first page that does: pages
|
||||
/// that touch (the gaps hidden) can share a row of pixels, and a press there is one page's.
|
||||
pub(crate) fn press(&mut self, page: usize, hits: impl IntoIterator<Item = Hit>, tool: QuickTool) {
|
||||
if self.began != Some(None) {
|
||||
return;
|
||||
}
|
||||
self.capture = top(hits, tool).map(|h| Capture { owner: h.owner, declines_drag: h.declines_drag });
|
||||
self.began = Some(Some(page));
|
||||
}
|
||||
@@ -222,6 +227,7 @@ mod tests {
|
||||
#[test]
|
||||
fn a_press_is_held_until_the_button_is_up_and_its_release_seen() {
|
||||
let mut r = PageRouter::default();
|
||||
r.begin();
|
||||
r.press(2, [hit(Layer::Comment, 7)], QuickTool::Select);
|
||||
assert_eq!(r.owner(2), Some(hit(Layer::Comment, 7).owner));
|
||||
assert_eq!(r.owner(1), None, "only on its page");
|
||||
@@ -230,6 +236,7 @@ mod tests {
|
||||
assert!(r.owner(2).is_some(), "the release frame still belongs to the press");
|
||||
r.settle(false, false);
|
||||
assert_eq!(r.owner(2), None);
|
||||
r.begin();
|
||||
r.press(0, [], QuickTool::Select);
|
||||
assert_eq!(r.owner(0), None, "a press nobody claims is held by nobody");
|
||||
assert!(!r.keeps_drag());
|
||||
@@ -257,9 +264,21 @@ mod tests {
|
||||
assert_eq!(r.press_for(0), PagePress::Free, "a cancelled press leaves every page to the tool");
|
||||
}
|
||||
|
||||
/// Pages that touch (the gaps hidden) can share a row of pixels: a press there is the first
|
||||
/// page's to claim it, and the next page's claim is ignored.
|
||||
#[test]
|
||||
fn a_press_where_two_pages_touch_is_the_first_pages() {
|
||||
let mut r = PageRouter::default();
|
||||
r.begin();
|
||||
r.press(0, [hit(Layer::Comment, 1)], QuickTool::Select);
|
||||
r.press(1, [hit(Layer::Comment, 2)], QuickTool::Select);
|
||||
assert_eq!((r.press_for(0), r.press_for(1)), (PagePress::Mine(hit(Layer::Comment, 1).owner), PagePress::Elsewhere));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_highlight_lets_its_drag_go_to_the_text() {
|
||||
let mut r = PageRouter::default();
|
||||
r.begin();
|
||||
r.press(0, [Hit { declines_drag: true, ..hit(Layer::Comment, 0) }], QuickTool::Select);
|
||||
assert!(r.owner(0).is_some() && !r.keeps_drag());
|
||||
r.cancel();
|
||||
|
||||
@@ -13,8 +13,8 @@ use crate::page_router::{CommentPart, Hit, Owner, PagePress};
|
||||
/// tool, Fill & Sign over its marks, a shape tool over the selected shape's handles) is that
|
||||
/// comment's, and one nobody holds is a commenting tool's. When another tool took this frame
|
||||
/// (`taken`: Prepare, added content, which still decide for themselves), the commenting tools leave
|
||||
/// it to that tool, as before, though a click there still unselects. Returns `true` when the
|
||||
/// pointer gesture belongs to commenting (text selection must ignore it).
|
||||
/// it to that tool, as before; the Select tool's click there still unselects. Returns `true` when
|
||||
/// the pointer gesture belongs to commenting (text selection must ignore it).
|
||||
pub(crate) fn page_input(ui: &egui::Ui, resp: &egui::Response, cx: &PageCx<'_>, press: PagePress, taken: bool, view: &mut DocView) -> bool {
|
||||
let (pointer, origin) = (cx.ptr.pos, cx.ptr.origin);
|
||||
let page_rect = cx.xf.rect;
|
||||
|
||||
Reference in new issue
Block a user