diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index 0f76817d..fed79d4d 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -212,8 +212,10 @@ fn model_pos(body: &TextBody, pos: (usize, usize)) -> Option<(usize, usize)> { /// characters), stopping at the end of the body. Returns the position reached and the characters /// and bytes consumed. /// -/// The walk is what bounds the surrounding query: it never looks further than `limit`, so a -/// megabyte body costs no more than a short one. +/// The returned window is bounded by `limit`, but locating `from` inside a paragraph still walks +/// that paragraph's runs from their start: runs do not cache character lengths, and a position is +/// a character index. A megabyte paragraph therefore costs time proportional to the caret's +/// character offset, even though the copied surrounding text stays within `limit`. fn walk_forward( body: &TextBody, comp: Option<(usize, usize, usize)>, @@ -330,7 +332,9 @@ fn logical_between(body: &TextBody, comp: Option<(usize, usize, usize)>, start: /// from an unavailable context. `cursor > anchor` is preserved for a reversed selection. /// /// Only a bounded window is measured and copied: the query walks at most the budget out from the -/// selection on each side and never visits the rest of the body. +/// selection on each side and does not copy the rest of the body. Finding the selection's +/// character offsets inside one paragraph still scans that paragraph's prefix, so the time is not +/// bounded independently of the paragraph length. pub fn ime_surrounding(st: &DocState, t: &TextSel) -> Option { let body = body_of(st, t)?; let comp = composition_range(body, st.composition.as_ref()); @@ -1449,21 +1453,6 @@ fn delete_surrounding(s: &mut Session, p: &Value) -> Result { let before_sel = st.selection.clone(); let composing = st.composition.is_some(); let vertex_kind = s.tool.vertex_kind.clone(); - // Where the deletion before `start` begins decides how the selection moves: within the start - // paragraph its characters shift; from an earlier paragraph those paragraphs merge, so later - // paragraph numbers shift too. - let (from_para, from_char) = from; - let remap = move |pos: (usize, usize)| -> (usize, usize) { - if before_chars == 0 || pos < start { - return pos; - } - if from_para == start.0 { - // Only endpoints in the start paragraph move; later paragraphs keep their text. - if pos.0 == start.0 { (pos.0, pos.1 - before_chars) } else { pos } - } else { - (from_para + (pos.0 - start.0), if pos.0 == start.0 { from_char + (pos.1 - start.1) } else { pos.1 }) - } - }; edit_text(s, |body, t| { // Delete the after-range first, so the positions of the earlier range stay valid; then // the text before the selection. @@ -1473,15 +1462,15 @@ fn delete_surrounding(s: &mut Session, p: &Value) -> Result { if before_chars > 0 { ed::delete(body, from, start); } - t.anchor = remap(t.anchor); - t.caret = remap(t.caret); + t.anchor = remap_after_delete(t.anchor, start, from, before_chars); + t.caret = remap_after_delete(t.caret, start, from, before_chars); t.line_end = None; })?; if composing { if let Some(c) = s.doc_mut()?.composition.as_mut() { c.start = if before_chars == 0 { start - } else if from_para == start.0 { + } else if from.0 == start.0 { (start.0, start.1 - before_chars) } else { from @@ -1493,6 +1482,21 @@ fn delete_surrounding(s: &mut Session, p: &Value) -> Result { Ok(json!({"deleted": deleted})) } +/// Map a text position across the deletion of `before_chars` characters that begins at `start` +/// and reaches back to `from`. Within the start paragraph its characters shift; from an earlier +/// paragraph those paragraphs merge, so later paragraph numbers shift too. +fn remap_after_delete(pos: (usize, usize), start: (usize, usize), from: (usize, usize), before_chars: usize) -> (usize, usize) { + if before_chars == 0 || pos < start { + return pos; + } + if from.0 == start.0 { + // Only endpoints in the start paragraph move; later paragraphs keep their text. + if pos.0 == start.0 { (pos.0, pos.1 - before_chars) } else { pos } + } else { + (from.0 + (pos.0 - start.0), if pos.0 == start.0 { from.1 + (pos.1 - start.1) } else { pos.1 }) + } +} + fn delete(s: &mut Session, p: &Value) -> Result { delete_dir(s, str_param(p, "dir").unwrap_or("backward")) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3fdd19d2..0d5eff41 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4031,7 +4031,12 @@ fn tables_with_a_file_defined_style_edit_and_resize() { && regular.style == "Regular" && bold.style == "Bold" && regular.weight < bold.weight; - assert!(metric_pair || option_env!("CRAFT_FONTS_REQUIRED").is_none(), "required BIZ Regular/Bold pair is absent"); + // Required native builds validate and embed both BIZ faces (crates/fonts/build.rs); + // optional local builds may only have Regular, so the weight assertion is skipped there. + assert!( + metric_pair || option_env!("CRAFT_FONTS_REQUIRED").is_none(), + "required native build is missing the BIZ UDPGothic Regular/Bold metric pair" + ); if metric_pair { let doc = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); let slide = std::sync::Arc::make_mut(&mut doc.slides[0]); diff --git a/crates/fonts/build.rs b/crates/fonts/build.rs index 432ccbc1..a33c340e 100644 --- a/crates/fonts/build.rs +++ b/crates/fonts/build.rs @@ -5,9 +5,10 @@ //! `fonts/simplified-chinese-manifest.txt`; it is optional for ordinary local builds but a required //! native input (`CRAFT_FONTS_REQUIRED`) fails without it. Unset, `CRAFT_FONTS` is empty. Web //! (wasm32) builds embed only Inter Regular/Bold (or a supported Variable face) and BIZ UDPGothic -//! Regular, to stay within the web size budget. Required builds also verify those faces survive -//! exclusion (and, on wasm, the web filter), so a pin without the Latin manifest fails instead of -//! silently dropping Inter. It only reads the local checkout: no network. +//! Regular, to stay within the web size budget; native builds also carry BIZ UDPGothic Bold. +//! Required builds verify the faces they promise survive exclusion (and, on wasm, the web filter), +//! so a pin without the Latin manifest or the native bold face fails instead of silently dropping +//! them. It only reads the local checkout: no network. use std::fmt::Write as _; use std::path::PathBuf; @@ -156,21 +157,27 @@ fn manifest(dir: &std::path::Path, manifest: &std::path::Path, wasm: bool) -> Re } /// Required builds must actually carry the faces the platform promises: Inter Regular and Bold -/// (or one Variable face) and BIZ UDPGothic Regular everywhere, plus LXGW WenKai Lite Regular for -/// native builds. This runs after exclusion and, on wasm, the web filter, so an absent manifest, -/// empty/all-excluded rows, a wrong-family row or a partial catalog cannot satisfy the gate. +/// (or one Variable face) and BIZ UDPGothic Regular everywhere, BIZ UDPGothic Bold natively (the +/// weight-pair metric test relies on it), plus LXGW WenKai Lite Regular for native builds. This +/// runs after exclusion and, on wasm, the web filter, so an absent manifest, empty/all-excluded +/// rows, a wrong-family row or a partial catalog cannot satisfy the gate. Wasm deliberately +/// embeds only BIZ Regular to stay within the web size budget. fn validate_required_faces(faces: &[(String, String)], wasm: bool) -> Result<(), String> { let has = |family: &str, style: &str| faces.iter().any(|(f, s)| f == family && s == style); let inter = has("Inter", "Variable") || (has("Inter", "Regular") && has("Inter", "Bold")); - let biz = has("BIZ UDPGothic", "Regular"); + let biz_regular = has("BIZ UDPGothic", "Regular"); + let biz_bold = wasm || has("BIZ UDPGothic", "Bold"); let chinese = wasm || has("LXGW WenKai Lite", "Regular"); let mut missing = Vec::new(); if !inter { missing.push("Inter Regular/Bold (or Inter Variable)"); } - if !biz { + if !biz_regular { missing.push("BIZ UDPGothic Regular"); } + if !biz_bold { + missing.push("BIZ UDPGothic Bold"); + } if !chinese { missing.push("LXGW WenKai Lite Regular"); } @@ -199,7 +206,8 @@ mod tests { std::fs::write(path.join("fonts/fixture.ttf"), b"build-input path fixture").unwrap(); std::fs::write( path.join("fonts/manifest.txt"), - "BIZ UDPGothic | Regular | fonts/fixture.ttf | Jpan,Latn | OFL-1.1 | OFL.txt | fixture | https://example.invalid\n", + "BIZ UDPGothic | Regular | fonts/fixture.ttf | Jpan,Latn | OFL-1.1 | OFL.txt | fixture | https://example.invalid\n\ + BIZ UDPGothic | Bold | fonts/fixture.ttf | Jpan,Latn | OFL-1.1 | OFL.txt | fixture | https://example.invalid\n", ) .unwrap(); Self(path) @@ -355,7 +363,7 @@ mod tests { fixture.latin(include_str!("tests/fixtures/latin-families.txt")); fixture.optional(CHINESE_ROW); let native = input_fonts(Some(&fixture.0), false, true).unwrap(); - assert_eq!(native.matches("CraftFont {").count(), 45, "43 permitted Latin faces plus BIZ and LXGW"); + assert_eq!(native.matches("CraftFont {").count(), 46, "43 permitted Latin faces plus BIZ Regular/Bold and LXGW"); for family in [ "Carlito", "Caladea", @@ -424,10 +432,28 @@ mod tests { for wasm in [true, false] { let e = input_fonts(Some(&fixture.0), wasm, true).unwrap_err(); assert!(e.contains("BIZ UDPGothic Regular"), "wasm={wasm}: {e}"); + if !wasm { + assert!(e.contains("BIZ UDPGothic Bold"), "native builds need the weight pair: {e}"); + } assert!(input_fonts(Some(&fixture.0), wasm, false).is_ok(), "optional builds still work"); } } + /// Native required builds need the real BIZ Bold face for weight metrics; wasm deliberately + /// embeds only Regular to stay within its size budget. This pins the contract the engine's + /// required-build metric pair assertion relies on. + #[test] + fn required_native_builds_need_biz_bold_but_wasm_does_not() { + let fixture = InputFixture::new(); + fixture.valid_latin(); + fixture.optional(CHINESE_ROW); + fixture.main("BIZ UDPGothic | Regular | fonts/fixture.ttf | Jpan,Latn | OFL-1.1 | OFL.txt | fixture | https://example.invalid\n"); + let e = input_fonts(Some(&fixture.0), false, true).unwrap_err(); + assert!(e.contains("BIZ UDPGothic Bold"), "required native build accepted a Regular-only catalog: {e}"); + let web = input_fonts(Some(&fixture.0), true, true).unwrap(); + assert_eq!(web.matches("CraftFont {").count(), 3, "wasm keeps Inter Regular/Bold and BIZ Regular only"); + } + #[test] fn required_builds_accept_static_or_variable_inter_with_biz() { let fixture = InputFixture::new(); diff --git a/crates/ui-egui/src/datetime_tests.rs b/crates/ui-egui/src/datetime_tests.rs index c955aa2e..0a7dede1 100644 --- a/crates/ui-egui/src/datetime_tests.rs +++ b/crates/ui-egui/src/datetime_tests.rs @@ -46,7 +46,7 @@ fn insert_opens_the_dialog_and_ok_inserts_one_field() { h.run_steps(2); assert_eq!(h.state().dialog.as_ref().map(|d| d.id.as_str()), Some("dateTime")); let _ = h.get_by_label("Update automatically"); - let _ = h.get_by_label("English (United States)"); + assert_eq!(h.get_by_role_and_label(egui::accesskit::Role::ComboBox, "Language").value().as_deref(), Some("English (United States)")); let now = deckcraft_text::datetime::DateTime::now_local().unwrap(); let sample = deckcraft_text::datetime::field_text("datetime2", Some("en-US"), now).unwrap(); h.get_by_label(&sample).click(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 99858c68..71aece47 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -278,7 +278,7 @@ fn pick_album_picture(app: &mut SlideApp) -> Option { Some(json!({"name": name, "data": deckcraft_engine::cmd::base64_encode(&loaded)})) } -fn album_choice(ui: &mut Ui, d: &mut Dialog, id: &str, key: &str, default: &str, choices: &[(&str, &str)]) { +fn album_choice(ui: &mut Ui, d: &mut Dialog, id: &str, name: &str, key: &str, default: &str, choices: &[(&str, &str)]) { let cur = d.get(key, default); let label = choices.iter().find(|(k, _)| *k == cur).map(|(_, l)| *l).unwrap_or(default); let opened = egui::ComboBox::from_id_salt(id).selected_text(label).width(220.0).show_ui(ui, |ui| { @@ -288,7 +288,13 @@ fn album_choice(ui: &mut Ui, d: &mut Dialog, id: &str, key: &str, default: &str, } } }); - opened.response.widget_info(|| egui::WidgetInfo::labeled(egui::Role::ComboBox, true, label)); + let access = name.to_string(); + let shown = label.to_string(); + opened.response.widget_info(move || { + let mut info = egui::WidgetInfo::labeled(egui::Role::ComboBox, true, access.clone()); + info.current_text_value = Some(shown.clone()); + info + }); } const ALBUM_THEMES: &[&str] = &["DeckCraft", "Harbor", "Ember", "Meadow", "Nocturne", "Paper", "Coral Reef", "Slate"]; @@ -458,6 +464,7 @@ fn photo_album_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { ui, d, "album_layout", + "Picture layout", "layout", "1", &[ @@ -475,6 +482,7 @@ fn photo_album_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { ui, d, "album_frame", + "Frame shape", "frame", "rect", &[("none", "None"), ("rect", "Rectangle"), ("rounded", "Rounded"), ("simple", "Simple, white")], @@ -482,7 +490,7 @@ fn photo_album_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { check(ui, d, "Captions under pictures", "captions", false); check(ui, d, "ALL pictures black and white", "grayscale", false); ui.label("Album theme"); - album_choice(ui, d, "album_theme", "theme", "DeckCraft", &ALBUM_THEMES.iter().map(|n| (*n, *n)).collect::>()); + album_choice(ui, d, "album_theme", "Album theme", "theme", "DeckCraft", &ALBUM_THEMES.iter().map(|n| (*n, *n)).collect::>()); }); let captions = d.fields.get("captions").is_some_and(|v| v == "1"); let grayscale = d.fields.get("grayscale").is_some_and(|v| v == "1"); @@ -2279,7 +2287,7 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { let r = ui.add(egui::Slider::new(&mut scale, 0.75..=1.75)); // No visible label here (the row is laid out to fit); the accessible name only. let enabled = r.enabled(); - r.widget_info(|| egui::WidgetInfo::slider(enabled, scale as f64, "Interface size")); + r.widget_info(|| egui::WidgetInfo::slider(enabled, f64::from(scale), "Interface size")); if r.is_pointer_button_down_on() { if r.changed() || pending.is_some() { d.fields.insert("uiScale".into(), scale.to_string()); @@ -2622,7 +2630,12 @@ fn date_time_dialog(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool { } } }); - opened.response.widget_info(|| egui::WidgetInfo::labeled(egui::Role::ComboBox, true, lang_label)); + let shown = lang_label.to_string(); + opened.response.widget_info(move || { + let mut info = egui::WidgetInfo::labeled(egui::Role::ComboBox, true, "Language"); + info.current_text_value = Some(shown.clone()); + info + }); }); ui.label( egui::RichText::new("Month names and AM/PM are listed for English. Other languages list the numeric date and 24-hour time.") diff --git a/crates/ui-egui/src/fillui.rs b/crates/ui-egui/src/fillui.rs index 04307d2d..2410dc0a 100644 --- a/crates/ui-egui/src/fillui.rs +++ b/crates/ui-egui/src/fillui.rs @@ -212,7 +212,7 @@ pub fn gradient_editor(ui: &mut Ui, id: &str, g: &Gradient, sc: &ColorScheme) -> if let GradientShape::Linear { angle, .. } = &g.shape { ui.horizontal(|ui| { ui.add_sized(vec2(70.0, 20.0), egui::Label::new("Angle")); - if let Some(a) = widgets::spinner(ui, &format!("{id}-angle"), *angle, 15.0, "°", 70.0) { + if let Some(a) = widgets::spinner(ui, &format!("{id}-angle"), "Angle", *angle, 15.0, "°", 70.0) { out = Some(Gradient { shape: GradientShape::Linear { angle: a.rem_euclid(360.0), scaled: false }, ..g.clone() }); } }); @@ -299,7 +299,7 @@ pub fn gradient_editor(ui: &mut Ui, id: &str, g: &Gradient, sc: &ColorScheme) -> }); ui.horizontal(|ui| { ui.add_sized(vec2(70.0, 20.0), egui::Label::new("Position")); - if let Some(v) = widgets::spinner(ui, &format!("{id}-pos-{sel}"), (s.pos * 100.0).round(), 5.0, "%", 70.0) { + if let Some(v) = widgets::spinner(ui, &format!("{id}-pos-{sel}"), "Position", (s.pos * 100.0).round(), 5.0, "%", 70.0) { let mut ng = g.clone(); ng.stops[sel].pos = (v / 100.0).clamp(0.0, 1.0); ng.stops.sort_by(|a, b| a.pos.total_cmp(&b.pos)); diff --git a/crates/ui-egui/src/ime_tests.rs b/crates/ui-egui/src/ime_tests.rs index f57da69e..2c4563a9 100644 --- a/crates/ui-egui/src/ime_tests.rs +++ b/crates/ui-egui/src/ime_tests.rs @@ -377,6 +377,15 @@ fn system_has(s: &str) -> bool { s.chars().filter(|c| !c.is_whitespace()).all(|c| deckcraft_fonts::FontDb::global().fallback_for(c, u32::MAX).is_some_and(|f| !f.is_color())) } +/// Assert that both interface faces draw every non-space character of `text`; `painted` describes +/// the sample in a failure message. +fn assert_faces_draw(h: &egui_kittest::Harness<'_, SlideApp>, text: &str, painted: &str) { + for family in [FontId::proportional(13.0), crate::theme::bold(13.0)] { + let missing: String = text.chars().filter(|c| !c.is_whitespace() && !h.ctx.fonts_mut(|f| f.has_glyph(&family, *c))).collect(); + assert!(missing.is_empty(), "{family:?} draws {missing:?} of {painted} as boxes"); + } +} + /// #101: a Korean file name, title, notes and comments, a Japanese slide and a Chinese section are /// drawn with glyphs in the interface (title bar, outline, notes pane, comments), not boxes: faces /// that draw them are borrowed from the font database. A Latin presentation borrows nothing. @@ -419,10 +428,7 @@ fn cjk_text_in_the_interface_is_drawn_with_real_glyphs() { .collect(); assert_eq!(initial, [chinese.to_owned(), japanese.to_owned()]); for text in [chinese, japanese] { - for family in [FontId::proportional(13.0), crate::theme::bold(13.0)] { - let missing: String = text.chars().filter(|c| !c.is_whitespace() && !h.ctx.fonts_mut(|f| f.has_glyph(&family, *c))).collect(); - assert!(missing.is_empty(), "{family:?} draws {missing:?} of initially painted {text:?} as boxes"); - } + assert_faces_draw(&h, text, &format!("initially painted {text:?}")); } } else { eprintln!("optional font input: initial CJK paint skipped before layout (no complete local coverage)"); @@ -455,10 +461,7 @@ fn cjk_text_in_the_interface_is_drawn_with_real_glyphs() { eprintln!("skipped {text:?}: no font on this machine draws it"); continue; } - for family in [FontId::proportional(13.0), crate::theme::bold(13.0)] { - let missing: String = text.chars().filter(|c| !c.is_whitespace() && !h.ctx.fonts_mut(|f| f.has_glyph(&family, *c))).collect(); - assert!(missing.is_empty(), "{family:?} draws {missing:?} of {text:?} as boxes"); - } + assert_faces_draw(&h, text, &format!("{text:?}")); checked += 1; } if require_all || cfg!(windows) || cfg!(target_os = "macos") { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4152ef87..f306f4e6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2467,52 +2467,7 @@ impl SlideApp { let owner = target.map(|t| t.owner); let accepted = matches!(e, egui::ImeEvent::Commit(_) | egui::ImeEvent::DeleteSurrounding { .. } | egui::ImeEvent::DeleteSurroundingBytes { .. }); - // A commit or deletion belongs to the composition that began the pre-edit: another owner - // (the user clicked elsewhere) or a composition the app already settled makes it stale. - // While the reset is still pending, the tombstone rejects pre-edits too (the platform has - // not yet been told to cancel its copy). `observe_ime` retires the tombstone in the pass - // that publishes the interrupt (or in an accepted no-descriptor pass, which resets the - // backend just the same), so a later commit-only is not lost forever: it is a - // legitimate direct commit or a new platform composition. egui's events carry no - // generation, so an event queued by the OS across that reset is indistinguishable from - // one the user just typed; only events in the same input batch as the settle are rejected - // deterministically. A cleared-preedit terminal is a distinct case: its pending owner - // remains guarded across that output boundary until its actual commit/enable/disable - // arrives. Publishing a reset is not evidence that the separately queued terminal was - // delivered. With no incoming event epoch, the first ambiguous new composition after an - // unobserved reset may be rejected; no timer guesses that it became safe. - // The tombstone is not filtered by the *current* target: a click can change the owner in - // the settling pass, and every remaining event of that batch still belongs to the - // pre-reset composition. Filtering would let the event after the first dropped pre-edit - // start composing in the new target before the platform is reset. - let cancelled = self.ime_cancelled; - let pending_cancelled = self.ime_terminal_pending.as_ref().is_some_and(|p| p.cancelled); - #[allow(deprecated)] - let stale = !matches!(e, egui::ImeEvent::Disabled | egui::ImeEvent::Enabled) - && (self.ime_composition.is_some_and(|c| c.settled || Some(c.owner) != owner) || cancelled.is_some() || pending_cancelled); - if stale { - if pending_cancelled && matches!(e, egui::ImeEvent::Commit(_)) { - // The old terminal finally arrived: reject it and release its pending owner now, - // so a new preedit later in this same raw batch can belong to the current target. - self.ime_terminal_pending = None; - self.ime_composition = None; - self.ime_cancelled = None; - self.ime_interrupt_pending = false; - return; - } - if pending_cancelled { - // note_settled_composition already queued the reset. Do not also latch the - // frame's memory interrupt: a terminal later in this batch can consume it and - // permit a fresh preedit without resetting that new composition at pass end. - self.ime_composition = None; - return; - } - if let Some(c) = cancelled { - log::debug!("IGNORING IME event for settled composition owner {:?} generation {}", c.owner, c.generation); - } - self.ime_composition = None; - // Ask the backend to drop its copy, so this is one stale event and not a stream. - ctx.memory_mut(|m| m.interrupt_ime()); + if self.reject_stale_ime_event(ctx, e, owner) { return; } // Until the exact newest reset request completes at its ordered stream position, custom @@ -2523,7 +2478,82 @@ impl SlideApp { return; } let before = if accepted { self.ime_receipt_stamp() } else { None }; - let r = match e { + let r = self.apply_ime_event(e); + self.settle_ime_event_state(e, owner, r.is_ok()); + if r.is_ok() && accepted { + self.refresh_ime_terminal_selection(); + } + if let Err(e) = &r { + self.set_status(e.to_string()); + } else if accepted + && let (Some(target), Some(before), Some(after)) = (target, before, self.ime_receipt_stamp()) + && before != after + && r.as_ref().is_ok_and(|v| !v.is_null() && v.get("handled").and_then(Value::as_bool) != Some(false)) + { + // A no-op receipt (an empty commit, a zero-length delete, same text reinserted) is + // not a mutation and must not poison the frame for a real commit later in the batch. + self.ime_tracker.note_input_method_apply(viewport, frame, target, before, after); + } + } + + /// Drop an event that belongs to a composition the application already settled, or to another + /// owner; returns `true` when the event must not be applied. + /// + /// A commit or deletion belongs to the composition that began the pre-edit: another owner + /// (the user clicked elsewhere) or a composition the app already settled makes it stale. + /// While the reset is still pending, the tombstone rejects pre-edits too (the platform has + /// not yet been told to cancel its copy). `observe_ime` retires the tombstone in the pass + /// that publishes the interrupt (or in an accepted no-descriptor pass, which resets the + /// backend just the same), so a later commit-only is not lost forever: it is a + /// legitimate direct commit or a new platform composition. egui's events carry no + /// generation, so an event queued by the OS across that reset is indistinguishable from + /// one the user just typed; only events in the same input batch as the settle are rejected + /// deterministically. A cleared-preedit terminal is a distinct case: its pending owner + /// remains guarded across that output boundary until its actual commit/enable/disable + /// arrives. Publishing a reset is not evidence that the separately queued terminal was + /// delivered. With no incoming event epoch, the first ambiguous new composition after an + /// unobserved reset may be rejected; no timer guesses that it became safe. + /// The tombstone is not filtered by the *current* target: a click can change the owner in + /// the settling pass, and every remaining event of that batch still belongs to the + /// pre-reset composition. Filtering would let the event after the first dropped pre-edit + /// start composing in the new target before the platform is reset. + fn reject_stale_ime_event(&mut self, ctx: &egui::Context, e: &egui::ImeEvent, owner: Option) -> bool { + let cancelled = self.ime_cancelled; + let pending_cancelled = self.ime_terminal_pending.as_ref().is_some_and(|p| p.cancelled); + #[allow(deprecated)] + let stale = !matches!(e, egui::ImeEvent::Disabled | egui::ImeEvent::Enabled) + && (self.ime_composition.is_some_and(|c| c.settled || Some(c.owner) != owner) || cancelled.is_some() || pending_cancelled); + if !stale { + return false; + } + if pending_cancelled && matches!(e, egui::ImeEvent::Commit(_)) { + // The old terminal finally arrived: reject it and release its pending owner now, + // so a new preedit later in this same raw batch can belong to the current target. + self.ime_terminal_pending = None; + self.ime_composition = None; + self.ime_cancelled = None; + self.ime_interrupt_pending = false; + return true; + } + if pending_cancelled { + // note_settled_composition already queued the reset. Do not also latch the + // frame's memory interrupt: a terminal later in this batch can consume it and + // permit a fresh preedit without resetting that new composition at pass end. + self.ime_composition = None; + return true; + } + if let Some(c) = cancelled { + log::debug!("IGNORING IME event for settled composition owner {:?} generation {}", c.owner, c.generation); + } + self.ime_composition = None; + // Ask the backend to drop its copy, so this is one stale event and not a stream. + ctx.memory_mut(|m| m.interrupt_ime()); + true + } + + /// Apply the document mutation an IME event asks for, before any receipt or tracker state. + fn apply_ime_event(&mut self, e: &egui::ImeEvent) -> deckcraft_engine::Result { + match e { egui::ImeEvent::Preedit { text, active_range_chars } => { self.session.compose_text(text, active_range_chars.as_ref().map(|r| (r.start, r.end))) } @@ -2557,7 +2587,12 @@ impl SlideApp { self.session.end_composition(); Ok(Value::Null) } - }; + } + } + + /// Update the tracked composition after `e` has been applied; `applied` says whether the + /// document mutation succeeded. + fn settle_ime_event_state(&mut self, e: &egui::ImeEvent, owner: Option, applied: bool) { match e { // The platform ends its composition on a commit (and on Disabled/Enabled): nothing is // left to interrupt. A pre-edit keeps (or starts) the tracked composition. @@ -2577,7 +2612,7 @@ impl SlideApp { self.ime_interrupt_pending = false; } egui::ImeEvent::Preedit { text, .. } => { - match (owner, r.is_ok(), self.session.active().is_some_and(|d| d.composition.is_some())) { + match (owner, applied, self.session.active().is_some_and(|d| d.composition.is_some())) { (Some(owner), true, true) => { // Updating the same composition keeps its generation. After a settle (or // a first pre-edit) this is a new generation and clears the tombstone. @@ -2618,20 +2653,6 @@ impl SlideApp { } _ => {} } - if r.is_ok() && accepted { - self.refresh_ime_terminal_selection(); - } - if let Err(e) = &r { - self.set_status(e.to_string()); - } else if accepted - && let (Some(target), Some(before), Some(after)) = (target, before, self.ime_receipt_stamp()) - && before != after - && r.as_ref().is_ok_and(|v| !v.is_null() && v.get("handled").and_then(Value::as_bool) != Some(false)) - { - // A no-op receipt (an empty commit, a zero-length delete, same text reinserted) is - // not a mutation and must not poison the frame for a real commit later in the batch. - self.ime_tracker.note_input_method_apply(viewport, frame, target, before, after); - } } /// Observe the canvas IME owner once per pass and publish its descriptor. diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index c94af8b0..0708713d 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -333,7 +333,7 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { let mut fields = |ui: &mut Ui, label: &str, id: &str, v: f64, key: &str, cmd: &str, unit: f64| { ui.horizontal(|ui| { ui.add_sized(vec2(90.0, 20.0), egui::Label::new(label)); - if let Some(nv) = widgets::spinner(ui, id, v / unit, 0.1, if unit == 72.0 { "\"" } else { "°" }, 70.0) { + if let Some(nv) = widgets::spinner(ui, id, label, v / unit, 0.1, if unit == 72.0 { "\"" } else { "°" }, 70.0) { let mut p = json!({key: nv * unit}); if cmd == "shape.resize" && locked { p["lockAspect"] = json!(true); @@ -403,7 +403,7 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { ] { ui.horizontal(|ui| { ui.add_sized(vec2(60.0, 20.0), egui::Label::new(l)); - if let Some(nv) = widgets::spinner(ui, &format!("m{k}"), v, 1.0, " pt", 60.0) { + if let Some(nv) = widgets::spinner(ui, &format!("m{k}"), l, v, 1.0, " pt", 60.0) { run(app, "format.margins", json!({k: nv.max(0.0)})); } }); @@ -551,7 +551,7 @@ fn format_shape(app: &mut SlideApp, ui: &mut Ui) { ui.horizontal(|ui| { ui.label("Width"); let w = sh.line.as_ref().and_then(|l| l.width).unwrap_or(0.75); - if let Some(v) = widgets::spinner(ui, "lw", w, 0.25, " pt", 60.0) { + if let Some(v) = widgets::spinner(ui, "lw", "Width", w, 0.25, " pt", 60.0) { run(app, "shape.line", json!({"width": v.max(0.0)})); } }); @@ -956,7 +956,7 @@ fn picture_background_options(app: &mut SlideApp, ui: &mut Ui, pf: &deckcraft_mo ] { ui.horizontal(|ui| { ui.add_sized(vec2(72.0, 20.0), egui::Label::new(label)); - if let Some(v) = widgets::spinner(ui, id, value, if scale < 1.0 { 5.0 } else { 1.0 }, suffix, 64.0) { + if let Some(v) = widgets::spinner(ui, id, label, value, if scale < 1.0 { 5.0 } else { 1.0 }, suffix, 64.0) { let key = match id { "bg-ox" => "offsetX", "bg-oy" => "offsetY", diff --git a/crates/ui-egui/src/photo_album_tests.rs b/crates/ui-egui/src/photo_album_tests.rs index b97abe4e..26c608da 100644 --- a/crates/ui-egui/src/photo_album_tests.rs +++ b/crates/ui-egui/src/photo_album_tests.rs @@ -89,7 +89,9 @@ fn update_reflows_the_open_album() { assert!(h.query_by_label("Update").is_some()); h.get_by_label("ALL pictures black and white").click(); h.run_steps(2); - h.get_by_label("1 picture").click(); + let layout = h.get_by_role_and_label(egui::accesskit::Role::ComboBox, "Picture layout"); + assert_eq!(layout.value().as_deref(), Some("1 picture")); + layout.click(); h.run_steps(2); h.get_by_label("2 pictures").click(); h.run_steps(2); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 4c53824f..fe9b129a 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -488,24 +488,7 @@ fn slides_group(app: &mut SlideApp, ui: &mut Ui) { run(app, "slide.new", json!({})); } if let Some(a) = arrow { - egui::Popup::menu(&a).width(420.0).show(|ui| { - layout_menu(app, ui, "slide.new"); - ui.separator(); - if ui.button("Duplicate Selected Slides").clicked() { - run(app, "slide.duplicate", json!({})); - } - if ui.button("Slides from Outline…").clicked() { - app.dialog = Some(crate::dialogs::Dialog::new("outline")); - } - let desktop = app.services.pick_open.is_some(); - for (label, keep) in [("Reuse Slides…", false), ("Reuse Slides (Keep Source Formatting)…", true)] { - let r = ui.add_enabled(desktop, egui::Button::new(label)); - if r.clicked() { - run(app, "app.reuseSlidesDialog", json!({"keepFormatting": keep})); - } - r.on_disabled_hover_text("Choosing a presentation to reuse needs the desktop app"); - } - }); + egui::Popup::menu(&a).width(420.0).show(|ui| new_slide_menu(app, ui)); } rows(ui, |ui| { let (main, arrow) = small_button_ex(ui, Icon::Layout, "Layout", enabled(app, "slide.layout"), false, true); @@ -515,19 +498,42 @@ fn slides_group(app: &mut SlideApp, ui: &mut Ui) { small(app, ui, Icon::ResetSlide, "Reset", "slide.reset", json!({})); let (main, arrow) = small_button_ex(ui, Icon::Section, "Section", enabled(app, "section.add"), false, true); if let Some(arrow) = arrow { - widgets::split_menu(&main, &arrow, |ui| { - if ui.button("Add Section").clicked() { - run(app, "section.add", json!({})); - } - if ui.button("Remove All Sections").clicked() { - run(app, "section.removeAll", json!({})); - } - }); + widgets::split_menu(&main, &arrow, |ui| section_menu(app, ui)); } }); }); } +/// The New Slide button's menu: layouts, duplication, outline import and slide reuse. +fn new_slide_menu(app: &mut SlideApp, ui: &mut Ui) { + layout_menu(app, ui, "slide.new"); + ui.separator(); + if ui.button("Duplicate Selected Slides").clicked() { + run(app, "slide.duplicate", json!({})); + } + if ui.button("Slides from Outline…").clicked() { + app.dialog = Some(crate::dialogs::Dialog::new("outline")); + } + let desktop = app.services.pick_open.is_some(); + for (label, keep) in [("Reuse Slides…", false), ("Reuse Slides (Keep Source Formatting)…", true)] { + let r = ui.add_enabled(desktop, egui::Button::new(label)); + if r.clicked() { + run(app, "app.reuseSlidesDialog", json!({"keepFormatting": keep})); + } + r.on_disabled_hover_text("Choosing a presentation to reuse needs the desktop app"); + } +} + +/// The Section split button's menu. +fn section_menu(app: &mut SlideApp, ui: &mut Ui) { + if ui.button("Add Section").clicked() { + run(app, "section.add", json!({})); + } + if ui.button("Remove All Sections").clicked() { + run(app, "section.removeAll", json!({})); + } +} + pub fn font_families(app: &SlideApp) -> Vec { let mut v = vec![]; if let Some(m) = app.session.active().and_then(|d| d.doc.masters.first()) { @@ -1857,7 +1863,7 @@ fn transitions(app: &mut SlideApp, ui: &mut Ui) { ui.horizontal(|ui| { ui.label("Duration:"); let d = cur.as_ref().map(|t| t.duration_ms as f64 / 1000.0).unwrap_or(1.0); - if let Some(v) = widgets::spinner(ui, "tdur", d, 0.25, "", 46.0) { + if let Some(v) = widgets::spinner(ui, "tdur", "Duration", d, 0.25, "", 46.0) { run(app, "transition.timing", json!({"duration": (v.max(0.0) * 1000.0).round() as u64})); } }); @@ -1873,7 +1879,7 @@ fn transitions(app: &mut SlideApp, ui: &mut Ui) { if ui.checkbox(&mut on, "After:").changed() { run(app, "transition.timing", json!({"after": if on { json!(after.unwrap_or(0)) } else { Value::Null }})); } - if let Some(v) = widgets::spinner(ui, "tafter", after.unwrap_or(0) as f64 / 1000.0, 1.0, "", 46.0) { + if let Some(v) = widgets::spinner(ui, "tafter", "After", after.unwrap_or(0) as f64 / 1000.0, 1.0, "", 46.0) { run(app, "transition.timing", json!({"after": (v.max(0.0) * 1000.0).round() as u64})); } }); @@ -2142,13 +2148,17 @@ fn animations(app: &mut SlideApp, ui: &mut Ui) { }); ui.horizontal(|ui| { ui.label("Duration:"); - if let Some(v) = widgets::spinner(ui, "adur", cur.as_ref().map(|c| c.duration_ms as f64 / 1000.0).unwrap_or(0.5), 0.25, "", 46.0) { + if let Some(v) = + widgets::spinner(ui, "adur", "Duration", cur.as_ref().map(|c| c.duration_ms as f64 / 1000.0).unwrap_or(0.5), 0.25, "", 46.0) + { run(app, "animation.timing", json!({"duration": (v.max(0.0) * 1000.0).round() as u64})); } }); ui.horizontal(|ui| { ui.label("Delay:"); - if let Some(v) = widgets::spinner(ui, "adelay", cur.as_ref().map(|c| c.delay_ms as f64 / 1000.0).unwrap_or(0.0), 0.25, "", 46.0) { + if let Some(v) = + widgets::spinner(ui, "adelay", "Delay", cur.as_ref().map(|c| c.delay_ms as f64 / 1000.0).unwrap_or(0.0), 0.25, "", 46.0) + { run(app, "animation.timing", json!({"delay": (v.max(0.0) * 1000.0).round() as u64})); } }); @@ -2686,13 +2696,13 @@ fn size_group(app: &mut SlideApp, ui: &mut Ui) { }; ui.horizontal(|ui| { ui.label("Height:"); - if let Some(v) = widgets::spinner(ui, "sh", x.h / 72.0, 0.1, "\"", 54.0) { + if let Some(v) = widgets::spinner(ui, "sh", "Height", x.h / 72.0, 0.1, "\"", 54.0) { run(app, "shape.resize", json!({"h": v.max(0.0) * 72.0})); } }); ui.horizontal(|ui| { ui.label("Width: "); - if let Some(v) = widgets::spinner(ui, "sw", x.w / 72.0, 0.1, "\"", 54.0) { + if let Some(v) = widgets::spinner(ui, "sw", "Width", x.w / 72.0, 0.1, "\"", 54.0) { run(app, "shape.resize", json!({"w": v.max(0.0) * 72.0})); } }); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index ea2aaad6..b4819396 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -510,15 +510,16 @@ pub fn gallery( clicked } -/// A labelled number field with up/down steppers. Returns the new value when committed. -pub fn spinner(ui: &mut Ui, id: &str, value: f64, step: f64, suffix: &str, width: f32) -> Option { +/// A number field with up/down steppers, named by the visible label of its row. Returns the new +/// value when committed. +pub fn spinner(ui: &mut Ui, id: &str, label: &str, value: f64, step: f64, suffix: &str, width: f32) -> Option { let t = Tokens::get(ui.ctx()); let key = ui.scope_id().with(id); let mut text: String = ui.data_mut(|d| d.get_temp::(key)).unwrap_or_else(|| fmt_num(value, suffix)); let mut out = None; ui.horizontal(|ui| { ui.spacing_mut().item_spacing.x = 0.0; - let r = ui.add(egui::TextEdit::singleline(&mut text).desired_width(width).font(theme::font(12.0))).accessible_name(format!("{id} value")); + let r = ui.add(egui::TextEdit::singleline(&mut text).desired_width(width).font(theme::font(12.0))).accessible_name(format!("{label} value")); if r.has_focus() { ui.data_mut(|d| d.insert_temp(key, text.clone())); } else { @@ -532,9 +533,9 @@ pub fn spinner(ui: &mut Ui, id: &str, value: f64, step: f64, suffix: &str, width } let (rect, _) = ui.allocate_exact_size(vec2(12.0, 22.0), Sense::hover()); let up = ui.interact(Rect::from_min_max(rect.min, pos2(rect.max.x, rect.center().y)), key.with("up"), Sense::click()); - up.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, format!("Increase {id}"))); + up.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, format!("Increase {label}"))); let down = ui.interact(Rect::from_min_max(pos2(rect.min.x, rect.center().y), rect.max), key.with("dn"), Sense::click()); - down.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, format!("Decrease {id}"))); + down.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, true, format!("Decrease {label}"))); let p = ui.painter(); let c = rect.center(); p.line_segment([pos2(c.x - 3.0, c.y - 3.0), pos2(c.x, c.y - 6.0)], Stroke::new(1.0, t.text_dim)); diff --git a/docs/egui-037-migration-plan.md b/docs/egui-037-migration-plan.md index 7d7dec48..3dd89c98 100644 --- a/docs/egui-037-migration-plan.md +++ b/docs/egui-037-migration-plan.md @@ -1,9 +1,11 @@ # DeckCraft: egui 0.37 migration plan -Current candidate font pin: `37827adc41fd165f9a9234ac40f94dbad353aa69` restores the complete prior Latin catalog alongside the migration fallbacks. All three workflows now use that revision; it must be published before this app branch. +Current candidate font pin: `37827adc41fd165f9a9234ac40f94dbad353aa69` restores the complete prior Latin catalog alongside the migration fallbacks. All three workflows use that published revision. Status update (2026-10-25): the candidate includes main `80f13011d5a3c353369c6e38250df6e5a260f3e0`, preserving its table-cell selection, text AutoFormat, edit-point history, fill controls, status accessibility and GPU-test serialization. IME deletion's undo entry retains main's new point-type state. Status-control assertions locate the actual laid-out status row, avoiding the identically named ribbon control without assuming fixed UI zoom. Managed run `egui037-deckcraft-status-row-ci-20261025-0753` passed the focused status regression and all eight CI stages: 2,045 passing test executions, zero failures and six ignored across 61 summaries. These are execution counts, including repeated tests. Required font coverage was separately validated in run 0724; the merged tree retains it and its tests. Cargo dependency pins and lockfile are unchanged by this refresh. Native/browser GUI, native IME and paired application performance remain open. Publication requires separate explicit approval; human review and human merge are required. +Review follow-up (2026-10-25): managed run `egui037-deck-review-green-ci-20261025-0810` passed the required-font regression and all eight CI stages, including WASM: 2,058 passing test executions, zero failures and six ignored across 61 summaries. Native builds now require BIZ UDPGothic Bold while web builds require Regular only; the missing-Bold regression failed with the old acceptance behavior and passed with the fix. Accessibility queries now distinguish combo-box purpose from selected value. Complexity refactors preserve the existing behavior. Long-paragraph IME deletion still scans a prefix/suffix; bounded-cost and paired performance qualification remain open. Remote SonarCloud and CodeRabbit must rerun on the eventual published commit. + The original planning record below is historical. ## Earlier qualification before the combined font input — 2026-10-25 @@ -723,7 +725,8 @@ immutable full SHA is obtained, and **all three** pins move together: `.github/workflows/release.yml` (`craft-fonts-ref`). Do not invent or pre-commit a future SHA, and do not advance only one workflow. `crates/fonts/build.rs` now validates required builds after exclusion and, for wasm, the web filter: Inter Regular/Bold (or a supported Variable face) and BIZ UDPGothic -Regular must actually be embedded, so a partial pin fails instead of silently losing the default +Regular must actually be embedded, plus BIZ UDPGothic Bold natively for the weight pair; wasm keeps +Regular only for its size budget. A partial pin fails instead of silently losing the default document/UI face. The required-input gate is explicit, but CI and FreeBSD qualification remain partial: