From 8bafe305be064627aa2527be1b4c4b45efb185b0 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 15 Oct 2026 23:28:56 -0700 Subject: [PATCH] feat(mobile): leave drawings out of the Reading View A page with drawings on it is now offered the Reading View too, unless drawings cover most of it. The drawings stay out of the column, and a small grey line at its end says the page has them and how to see them. They cannot be picked, moved or deleted from the column, and any picked in the page view let go when it opens. Handwriting inside a text box still flows with its text where it fits. Refs #128 Assisted-by: claude-opus-5.5 --- crates/canvas/src/gpu/page.rs | 7 + .../canvas/src/interaction/accessibility.rs | 5 +- crates/canvas/src/interaction/ink.rs | 6 + crates/canvas/src/interaction/mod.rs | 13 +- crates/canvas/src/interaction/tests.rs | 124 ++++++++++++++++++ crates/canvas/src/reading.rs | 80 ++++++++--- crates/mobile/src/tests.rs | 83 ++++++++++++ 7 files changed, 295 insertions(+), 23 deletions(-) diff --git a/crates/canvas/src/gpu/page.rs b/crates/canvas/src/gpu/page.rs index 9f702348e84820e29a6637f25411a7c7f417db5b..f726cc1d7b8e68aba5d4d34c82f3543d15c29092 100644 --- a/crates/canvas/src/gpu/page.rs +++ b/crates/canvas/src/gpu/page.rs @@ -741,10 +741,16 @@ impl PageScene { } }, Content::File { .. } => Some((object.file()?.1, true)), + Content::Ink(_) if column.is_some() => None, Content::Ink(ink) => Some((crate::editor::page::ink_bounds(ink)?, true)), Content::ReadOnly(object) => Some((object.rect(), true)), Content::Editable(_) => None, }) + .chain( + column + .and_then(crate::reading::Column::drawings_rect) + .map(|rect| (rect, true)), + ) } pub(crate) fn header_text<'a>( @@ -1043,6 +1049,7 @@ impl PageScene { outline(*id, offset, primitives)?; continue; } + Content::Ink(_) if column.is_some() => continue, Content::Ink(ink) => { let [x, y] = crate::origin(&ink.layout); let [dx, dy] = editor.map_or([0.0; 2], |editor| editor.ink_drag_offset(ink.id)); diff --git a/crates/canvas/src/interaction/accessibility.rs b/crates/canvas/src/interaction/accessibility.rs index 793ad7859612e1dc51b885e39d3cf3ea9af19828..dd904270bb1cfe46ee9f3766494b5abfe3437bc7 100644 --- a/crates/canvas/src/interaction/accessibility.rs +++ b/crates/canvas/src/interaction/accessibility.rs @@ -358,7 +358,10 @@ impl Accessibility { self.read_only.push((id, node)); } if let Some(column) = column { - for (text, bounds) in scene.header_text(editor, column) { + let note = column + .drawings_rect() + .map(|bounds| (crate::reading::DRAWINGS_NOTE.to_owned(), bounds)); + for (text, bounds) in scene.header_text(editor, column).chain(note) { let index = self.read_only.len(); let mut node = Node::new(Role::Label); node.set_value(text); diff --git a/crates/canvas/src/interaction/ink.rs b/crates/canvas/src/interaction/ink.rs index e26e83624f47321575d47c6a9c073a6e43adbd0c..2df49174303620456260765a6aeb718f371cfc17 100644 --- a/crates/canvas/src/interaction/ink.rs +++ b/crates/canvas/src/interaction/ink.rs @@ -149,6 +149,9 @@ impl PageView { /// A press at page point `point`, when a drawing tool takes it or it picks ink. pub(super) fn ink_pressed(&mut self, point: [f32; 2]) -> Result> { + if self.column.is_some() { + return Ok(None); + } let selected = !self.ink.selection.is_empty() && self .editor @@ -282,6 +285,9 @@ impl PageView { /// Delete and Backspace delete picked drawings; Escape lets them go, then leaves a /// drawing tool for Select & Type, as OneNote's Escape does. pub(super) fn ink_key(&mut self, key: &Key) -> Result> { + if self.column.is_some() { + return Ok(None); + } match key { Key::Named(NamedKey::Backspace | NamedKey::Delete) if !self.ink.selection.is_empty() => diff --git a/crates/canvas/src/interaction/mod.rs b/crates/canvas/src/interaction/mod.rs index 0e7bb59c3992458794c2c6dacc07a44db1efdffc..7c22dccca651dc5747ba221e2b051b40b187b33c 100644 --- a/crates/canvas/src/interaction/mod.rs +++ b/crates/canvas/src/interaction/mod.rs @@ -1022,9 +1022,11 @@ impl PageView { .ok() .flatten() }); - // The column shows pictures only; one picked in the page view lets go. + // The column only shows pictures and leaves drawings out; what the page view + // picked lets go. if self.column.is_some() { self.set_object_focus(None); + self.leave_ink(); } self.column.is_some() } @@ -2985,6 +2987,15 @@ fn page_primitives<'a>( &mut primitives, )?; } + if let Some(([x, y], note)) = column.and_then(|column| column.drawings.as_ref()) { + let [dx, dy] = scene.map_or([0.0; 2], |(_, offset)| *offset); + primitives.push(Primitive::Text { + clip: None, + text: note, + origin: [x + dx, y + dy], + ink: paint.colors.paper.shade(crate::gpu::colorref(0x00767676)), + }); + } if let Some(ObjectFocus::Image(id)) = object_focus { let (origin, size) = match preview { Some(PointerFeedback::Image(moving, origin, size)) if moving == id => (origin, size), diff --git a/crates/canvas/src/interaction/tests.rs b/crates/canvas/src/interaction/tests.rs index 1d24e624df7c02a642b90f38149102428c387c96..15025153a835c1a69a161f508409aef3f45d7a39 100644 --- a/crates/canvas/src/interaction/tests.rs +++ b/crates/canvas/src/interaction/tests.rs @@ -4055,6 +4055,130 @@ fn reader_shrinks_pictures_to_the_column_in_reading_order_and_leaves_them_alone( assert!(!view.set_reading(None) && view.column.is_none()); } +#[test] +fn reader_leaves_drawings_out_and_ends_with_a_note_that_the_page_has_them() { + use onestore::page::{Ink, InkStroke, PageObject, ink::ShapeKind}; + // A mostly drawn page is a canvas, not offered. + let drawn = scene_view( + include_bytes!("../../../../corpus/ink-edit/drawing/candidate/ink.one"), + "Beside the drawing", + ); + assert!(!drawn.reading_status(200.0).0); + let (mut page, _) = reader_columns_page(); + let pen = InkStroke { + id: onestore::page::text::new_id().unwrap(), + points: Vec::new(), + width: 2.0, + height: 2.0, + color: None, + transparency: None, + pen_tip: None, + raster_operation: None, + pressure: Vec::new(), + }; + let ink = Ink::drawn(ShapeKind::Ellipse, [320.0, 380.0], [400.0, 420.0], &pen).unwrap(); + let ink_id = ink.id; + page.objects.push(PageObject::Ink(ink)); + let mut view = reader_view(page); + view.viewport.size = [800, 900]; + let original = view.editor.page().unwrap(); + let strokes = |view: &PageView| { + view.primitives(COLORS) + .unwrap() + .iter() + .filter(|primitive| { + matches!( + primitive, + Primitive::Segment { .. } | Primitive::Taper { .. } + ) + }) + .count() + }; + assert!(strokes(&view) > 0); + // A drawing picked in the page view lets go in the column. + let offset = view.scene.as_ref().unwrap().1; + click(&mut view, [400.0, 400.0], Instant::now()); + assert_eq!(view.ink_selection(), [ink_id]); + assert!(view.set_reading(Some(200.0))); + assert!(view.ink_selection().is_empty()); + assert_eq!(strokes(&view), 0); + // Where the drawing lies on the page, the column can neither pick, move nor delete it. + for point in [ + [400.0, 400.0], + [360.0, 380.0], + [400.0 + offset[0], 400.0 + offset[1]], + ] { + let _ = view.pointer_moved(point).unwrap(); + let _ = view.pointer_pressed(Instant::now()).unwrap(); + assert!(view.ink_selection().is_empty()); + let _ = view + .pointer_moved([point[0] + 30.0, point[1] + 30.0]) + .unwrap(); + let _ = view.pointer_released().unwrap(); + assert!(view.ink_selection().is_empty()); + } + let _ = view.key(&Key::Named(NamedKey::Delete), None).unwrap(); + let drawing = |page: &Page| { + page.objects + .iter() + .find(|object| object.id() == ink_id) + .cloned() + }; + assert!(drawing(&view.editor.page().unwrap()).is_some()); + assert_eq!(drawing(&view.editor.page().unwrap()), drawing(&original)); + let column = view.column.as_ref().unwrap(); + let note = column.drawings_rect().unwrap(); + let last = column + .outlines + .iter() + .map(|(_, origin, shaped)| origin[1] + shaped.size[1]) + .fold(f32::NEG_INFINITY, f32::max); + assert!(note[1] >= last + 18.0); + assert!(note[2] <= 36.0 + 200.0); + let (_, text) = column.drawings.as_ref().unwrap(); + assert!( + view.primitives(COLORS) + .unwrap() + .iter() + .any(|primitive| matches!( + primitive, + Primitive::Text { text: drawn, origin, .. } + if painted_layout(*drawn).id() == text.id() + && *origin == [note[0] + offset[0], note[1] + offset[1]] + )) + ); + let bounds: Vec<_> = view + .scene + .as_ref() + .unwrap() + .0 + .content_bounds(&view.editor, view.column.as_ref()) + .map(|(rect, _)| rect) + .collect(); + assert_eq!( + bounds + .iter() + .map(|rect| rect[3]) + .fold(f32::NEG_INFINITY, f32::max), + note[3] + ); + let mut access = accessibility::Accessibility::default(); + let mut tree = None; + let nodes = accessibility::tests::nodes(accessibility::tests::apply( + &mut tree, + access.update(&view, view.viewport, "Reader").unwrap(), + )); + assert!( + nodes + .iter() + .any(|node| node.role() == accesskit::Role::Label + && node.data().value() == Some(crate::reading::DRAWINGS_NOTE)) + ); + assert!(!view.set_reading(None)); + assert!(strokes(&view) > 0); + assert_eq!(drawing(&view.editor.page().unwrap()), drawing(&original)); +} + #[test] fn reader_consumers_share_title_header_date_and_body_geometry() { let mut view = reader_view(reader_page()); diff --git a/crates/canvas/src/reading.rs b/crates/canvas/src/reading.rs index d1bb7471755a7cd208b44f076760d2ef3dc7beb7..601fdce1bb7c1fe15d0831612cd4d5e0d62095b1 100644 --- a/crates/canvas/src/reading.rs +++ b/crates/canvas/src/reading.rs @@ -30,10 +30,18 @@ pub(crate) struct Column { pub(crate) outlines: Vec<(ExGuid, [f32; 2], crate::outline::OutlineLayout)>, /// Page pictures as the column shows them, `[x0, y0, x1, y1]` in page points. pub(crate) pictures: Vec<(ExGuid, [f32; 4])>, + /// Where the note that the page has drawings, which the column leaves out, starts. + pub(crate) drawings: Option<([f32; 2], crate::layout::TextLayout)>, } #[cfg(feature = "gpu")] impl Column { + /// The note's `[x0, y0, x1, y1]` in page points. + pub(crate) fn drawings_rect(&self) -> Option<[f32; 4]> { + let ([x, y], note) = self.drawings.as_ref()?; + Some([*x, *y, x + note.shaped.width(), y + note.height()]) + } + /// Where the column shows page picture `id`; None for one it leaves out. pub(crate) fn picture(&self, id: ExGuid) -> Option<[f32; 4]> { self.pictures @@ -109,6 +117,7 @@ impl Column { .. } | Content::Image(_) + | Content::Ink(_) ) }) { return Ok(None); @@ -127,14 +136,14 @@ impl Column { .map(|outline| (outline.id, root_tops(outline))) .collect(); // Emptied outlines, and one being started, keep their place while typing. - let ordered = order( - blocks( - editor, - editor.visible_outlines().chain(editor.caret_outline()), - ), - &tops, - &mut Vec::new(), + let blocks = blocks( + editor, + editor.visible_outlines().chain(editor.caret_outline()), ); + if drawn_or_arranged(&blocks).is_some() { + return Ok(None); + } + let ordered = order(blocks, &tops, &mut Vec::new()); let rank = |id: ExGuid| { ordered .iter() @@ -175,6 +184,7 @@ impl Column { width, outlines: Vec::new(), pictures: Vec::new(), + drawings: None, }; let mut top = 14.4; if let Some(title) = title { @@ -212,6 +222,23 @@ impl Column { }; top += height + GAP; } + if editor + .objects + .iter() + .any(|object| matches!(object, Content::Ink(_))) + { + let note = engine.layout( + &onestore::page::text::Paragraph::new( + DRAWINGS_NOTE.into(), + onestore::document::Format { + font_size: Some(10.0), + ..Default::default() + }, + ), + width, + )?; + column.drawings = Some(([36.0, top], note)); + } Ok(Some(column)) } } @@ -236,7 +263,7 @@ fn column_layout( let mut nodes = source.document().nodes().to_vec(); for node in &mut nodes { match &mut node.content { - ParagraphContent::Text(_) => {} + ParagraphContent::Text(_) | ParagraphContent::Ink(_) => {} ParagraphContent::Image(image) => { let indent = crate::outline::indentation(node.level, &source.indents, width)?; shrink(image, width - indent); @@ -271,6 +298,9 @@ fn column_layout( Ok(fits.then_some(layout)) } +/// Ends a column that leaves out the page's drawings. +pub(crate) const DRAWINGS_NOTE: &str = "This page has drawings. Turn off Reading View to see them."; + /// Points between blocks in the column: OneNote's grid row. const GAP: f32 = 18.0; /// Strokes this near one another are one drawing, as handwriting is many strokes. @@ -390,19 +420,8 @@ pub fn read(page: &Page, column: f32, engine: &mut TextEngine) -> Result| -> f64 { - blocks - .iter() - .filter(|b| kind.is_none_or(|kind| b.kind == kind)) - .map(|b| f64::from(b.bounds[2] - b.bounds[0]) * f64::from(b.bounds[3] - b.bounds[1])) - .sum() - }; - let total = area(None).max(1.0); - if area(Some(Kind::Drawing)) / total >= CANVAS_SHARE { - return Ok(refused(Refusal::Drawn, blocks)); - } - if area(Some(Kind::Group)) / total >= CANVAS_SHARE { - return Ok(refused(Refusal::Arranged, blocks)); + if let Some(refusal) = drawn_or_arranged(&blocks) { + return Ok(refused(refusal, blocks)); } let mut kept = Vec::new(); if blocks.iter().any(|b| b.kind == Kind::Group) { @@ -438,6 +457,25 @@ pub fn read(page: &Page, column: f32, engine: &mut TextEngine) -> Result Option { + let area = |kind: Option| -> f64 { + blocks + .iter() + .filter(|b| kind.is_none_or(|kind| b.kind == kind)) + .map(|b| f64::from(b.bounds[2] - b.bounds[0]) * f64::from(b.bounds[3] - b.bounds[1])) + .sum() + }; + let total = area(None).max(1.0); + if area(Some(Kind::Drawing)) / total >= CANVAS_SHARE { + Some(Refusal::Drawn) + } else if area(Some(Kind::Group)) / total >= CANVAS_SHARE { + Some(Refusal::Arranged) + } else { + None + } +} + /// Where `outline`'s lines, table cells and pictures lie, in page points. fn marks(outline: &TextOutline) -> Vec<[f32; 4]> { let [x, y] = outline.origin(); diff --git a/crates/mobile/src/tests.rs b/crates/mobile/src/tests.rs index 4ca3ed62effea7908c21de63f9ff32e38b8b3f04..053e0ecec558edb98be14e6147347ffc22ce816a 100644 --- a/crates/mobile/src/tests.rs +++ b/crates/mobile/src/tests.rs @@ -3482,6 +3482,89 @@ fn reader_shows_a_corpus_pages_picture_no_wider_than_the_column() { assert!(pictures[0][2] - pictures[0][0] <= 240.0 / POINT + 0.01); } +#[test] +fn reader_leaves_a_small_drawing_out_and_refuses_a_mostly_drawn_page() { + let reading = |view: &View| { + let query = sb_view_reading(view, 240.0); + let status = serde_json::from_str::(&string(query)).unwrap(); + unsafe { sb_string_free(query) }; + status + }; + let (_directory, _section, drawn) = + reader_corpus_view("ink-edit/drawing/candidate/ink.one", "Beside the drawing"); + assert_eq!(reading(&drawn)["offered"], false); + let (directory, section) = features(); + let space = section.new_page(None, "Mon", "6:14 AM").unwrap(); + let (page, read_only) = section.shared.page(space).unwrap(); + let mut view = View { + canvas: ManuallyDrop::new(Canvas::new(space, page, [720, 1280], 2.0).unwrap()), + presentation: None, + frame: Arc::default(), + section: Arc::clone(§ion.shared), + read_only, + reload_pending: false, + local_edits: Arc::new(Mutex::new(LocalEdits::default())), + }; + view.canvas.set_transform(1.0, [0.0; 2]); + assert!(sb_view_focus_title(&mut view)); + assert!(unsafe { sb_insert(&mut view, c"Notes with a doodle".as_ptr()) }); + assert!(unsafe { sb_insert(&mut view, c"\n".as_ptr()) }); + assert!(unsafe { + sb_insert( + &mut view, + c"A paragraph long enough to fill several lines of the column on a phone, with a small drawing beside it." + .as_ptr(), + ) + }); + sb_view_set_tool(&mut view, 1, 0, 0); + let stroke = + [[300.0, 400.0], [320.0, 410.0], [340.0, 400.0]].map(|[x, y]| [x * POINT, y * POINT]); + assert!(sb_view_press(&mut view, stroke[0][0], stroke[0][1])); + for [x, y] in &stroke[1..] { + sb_view_drag(&mut view, *x, *y); + } + sb_view_release(&mut view); + sb_view_set_tool(&mut view, 0, 0, 0); + let strokes = |view: &View| { + view.canvas + .page + .primitives(view.canvas.colors()) + .unwrap() + .iter() + .filter(|primitive| { + matches!( + primitive, + draw::Primitive::Segment { .. } | draw::Primitive::Taper { .. } + ) + }) + .count() + }; + assert!(strokes(&view) > 0); + assert_eq!(reading(&view)["offered"], true); + let mut anchor = [0.0; 2]; + assert!(sb_view_set_reading(&mut view, 240.0, &mut anchor)); + assert_eq!(strokes(&view), 0); + // A tap where the drawing lies on the page picks nothing to delete. + assert!(sb_view_press(&mut view, stroke[1][0], stroke[1][1])); + sb_view_release(&mut view); + let mut rect = [0.0; 4]; + assert!(!sb_view_ink_selection(&view, &mut rect)); + assert!(!sb_view_set_reading(&mut view, 0.0, &mut anchor)); + assert!(strokes(&view) > 0); + assert!(section.flush(Duration::from_secs(20))); + let arena = onestore::Arena::default(); + let file = directory.path().join("Features.one"); + let stored = onestore::Section::open(&arena, onestore::read_file(file).unwrap()).unwrap(); + assert!( + stored + .page(space) + .unwrap() + .objects + .iter() + .any(|object| matches!(object, onestore::page::PageObject::Ink(_))) + ); +} + #[test] fn reader_local_apply_fence_tracks_overlapping_replies_and_view_lifetime() { use std::sync::mpsc; -- 2.54.0