mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-03 15:11:15 -06:00
Merge PR #3408: Control: pointer and scroll gestures refuse numbers past any screen instead of queueing NaN positions
docs/whats-new.md: both entries kept, newest first.
This commit is contained in:
@@ -54,6 +54,8 @@ impl LightcraftApp {
|
||||
if self.pending_gestures.len() >= MAX_PENDING_GESTURES {
|
||||
return Err(format!("{}: {MAX_PENDING_GESTURES} control gestures are already pending", method));
|
||||
}
|
||||
// refused at once, without waiting for a layout or taking a pending slot
|
||||
crate::control::check_gesture_numbers(&method, ¶ms)?;
|
||||
let now = self.tick_now_ms();
|
||||
self.pending_gestures.push(PendingGesture {
|
||||
method,
|
||||
|
||||
@@ -616,6 +616,30 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result<Value, String
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
/// The largest coordinate or offset a gesture takes, in points (far past any screen).
|
||||
const MAX_GESTURE_NUMBER: f64 = 1e6;
|
||||
|
||||
/// Every number of a gesture's parameters (and of `ui.pointer`'s events) is finite and within
|
||||
/// ±[`MAX_GESTURE_NUMBER`]. A larger one turns into an infinite or NaN `f32` position or scroll
|
||||
/// delta: egui asserts on NaN positions, and a release build would hand them to every widget.
|
||||
/// `count` and `steps` are clamped where they are read instead, and `ui.zoom`'s `factor` (a
|
||||
/// multiplier, not a position) is checked and clamped by its own branch. Checked when a request
|
||||
/// is admitted and again here, for callers that resolve a gesture directly.
|
||||
pub(crate) fn check_gesture_numbers(method: &str, p: &Value) -> Result<(), String> {
|
||||
fn check(method: &str, key: &str, v: &Value) -> Result<(), String> {
|
||||
match v {
|
||||
Value::Number(n) if !matches!(key, "count" | "steps" | "factor") => match n.as_f64() {
|
||||
Some(x) if x.abs() <= MAX_GESTURE_NUMBER => Ok(()),
|
||||
_ => Err(format!("{method}: `{key}` must be a number between -1000000 and 1000000")),
|
||||
},
|
||||
Value::Array(a) => a.iter().try_for_each(|v| check(method, key, v)),
|
||||
Value::Object(o) => o.iter().try_for_each(|(k, v)| check(method, k, v)),
|
||||
_ => Ok(()),
|
||||
}
|
||||
}
|
||||
check(method, "", p)
|
||||
}
|
||||
|
||||
/// Resolve one queued synthetic-input control request against the **current** layout.
|
||||
///
|
||||
/// Called by the app only after a completed UI pass newer than admission (plan §4.5). Widget rects
|
||||
@@ -623,6 +647,7 @@ pub fn export_active(app: &mut LightcraftApp, p: &Value) -> Result<Value, String
|
||||
/// has pushed the events onto `app.synthetic`; the app answers the request once the UI consumed
|
||||
/// them.
|
||||
pub(crate) fn gesture_events(app: &mut LightcraftApp, ctx: &egui::Context, method: &str, p: &Value) -> Result<GestureOutcome, String> {
|
||||
check_gesture_numbers(method, p)?;
|
||||
let s = |k: &str| p.get(k).and_then(Value::as_str);
|
||||
let f = |k: &str| p.get(k).and_then(Value::as_f64);
|
||||
match method {
|
||||
|
||||
@@ -564,7 +564,7 @@ fn one_synthetic_batch_until_a_completed_pass_and_drops_wait_for_the_ui_phase()
|
||||
|
||||
#[test]
|
||||
fn first_headless_frame_runs_logic_and_uses_its_simulated_clock() {
|
||||
let mut h = Headless::new(app(), [800.0, 600.0], 1.0);
|
||||
let mut h = Headless::new(self::app(), [800.0, 600.0], 1.0);
|
||||
assert_eq!(h.time(), 0.0);
|
||||
h.step();
|
||||
assert!(h.app.styled, "the first frame at t=0 runs logic and installs fonts");
|
||||
@@ -903,6 +903,48 @@ fn ui_scroll_carries_validated_unit_phase_and_source() {
|
||||
assert!(app.synthetic.is_empty());
|
||||
}
|
||||
|
||||
/// A coordinate or delta too large for an `f32` became an infinite or NaN pointer position or
|
||||
/// scroll delta: egui panicked on the NaN in debug builds ("Comparing NaN vectors"), and a release
|
||||
/// build handed it to every widget. Such a gesture is refused and queues nothing.
|
||||
#[test]
|
||||
fn gestures_refuse_numbers_that_are_not_screen_positions() {
|
||||
let (mut app, ctx) = (app(), egui::Context::default());
|
||||
app.widgets.push(("finite:test".into(), egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(20.0, 20.0))));
|
||||
app.image_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(100.0, 100.0)));
|
||||
let refused = [
|
||||
("ui.click", serde_json::json!({"x": 1e308})),
|
||||
("ui.click", serde_json::json!({"y": -1e308})),
|
||||
("ui.move", serde_json::json!({"x": 1e300, "y": 1.0})),
|
||||
("ui.drag", serde_json::json!({"x": 1.0, "toY": 1e308})),
|
||||
("ui.dragWidget", serde_json::json!({"id": "finite:test", "dx": -1e308})),
|
||||
("ui.scroll", serde_json::json!({"dy": 1e308})),
|
||||
("ui.pointer", serde_json::json!({"events": [{"kind": "down", "x": 0.5}, {"kind": "move", "x": 1e308}]})),
|
||||
];
|
||||
for (method, p) in refused {
|
||||
let e = crate::control::gesture_events(&mut app, &ctx, method, &p).err().unwrap_or_else(|| panic!("{method} {p} was accepted"));
|
||||
assert!(e.starts_with(method) && e.contains("between"), "{e}");
|
||||
assert!(app.synthetic.is_empty(), "{method} {p} queued events");
|
||||
}
|
||||
// ordinary values still work, and count / steps are still clamped rather than refused
|
||||
crate::control::gesture_events(&mut app, &ctx, "ui.click", &serde_json::json!({"x": 10.0, "y": -5.0, "count": u64::MAX})).unwrap();
|
||||
crate::control::gesture_events(&mut app, &ctx, "ui.drag", &serde_json::json!({"x": 1.0, "toX": 999_999.0, "steps": 1e12})).unwrap();
|
||||
assert!(!app.synthetic.is_empty());
|
||||
|
||||
// refused when the request is admitted: no pending slot, no wait for a layout
|
||||
let (tx, rx) = reply_channel();
|
||||
let e = app.admit_gesture(&ctx, "ui.click".into(), serde_json::json!({"x": 1e308}), tx).expect_err("refused at admission");
|
||||
assert!(e.contains("between"), "{e}");
|
||||
assert!(app.pending_gestures.is_empty());
|
||||
drop(rx);
|
||||
|
||||
// through the whole control channel, as an agent sends it: an error, not a panic
|
||||
let mut h = Headless::new(self::app(), [800.0, 600.0], 1.0);
|
||||
h.step();
|
||||
let r = h.request("ui.click", serde_json::json!({"x": 1e308}), Duration::from_secs(5));
|
||||
assert_eq!(r["ok"], false, "{r}");
|
||||
h.step();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn synthetic_pointer_requests_bound_clicks_and_drag_steps() {
|
||||
let (mut app, ctx) = (app(), egui::Context::default());
|
||||
|
||||
@@ -92,7 +92,7 @@ filmstrip is visible.
|
||||
| `ui.inspect` | — | UI state, window, `loupe` (`source`, `pending`, `failed` = why its render failed, else `null`, and `region: {full, window, pending}` when a zoomed view also renders the window on screen at no more than 100 % — `full` is the frame it is cut from, `window` its `[x, y, w, h]` — and `regionBefore` for the Before side of a Before/After view), canvas/image rects, `scroll: {grid, filmstrip}` (scroll offsets in points, `null` until drawn), active photo, selection, perf (`frameMs` = layout, `logicMs` = per-frame logic before it, `updateMs` = both, `maxUpdateMs`, `fps`, render queue …, `gpu` = adapter in use, `gpuReason` = why renders don't use the GPU, `gpuFallback` = latest render redone on the CPU and why — see `docs/gpu-pipeline.md`), status, memory (bytes per cache, see `library.memory`; plus stage caches — `budgetBytes`, `trimmed`, `sharedSourceBytes` (the one device copy of a big original) — and textures), `export: {running: {total, done, current} \| null, last}`, `notices` (warnings waiting to be shown, e.g. a damaged settings file; OK = `button:noticeOk`), `quitPrompt` (why quitting was stopped: tasks that would be cut short are running — `button:quitAnyway` cancels them and quits, `button:quitCancel` keeps working — or unsaved changes: `button:quitRetry` / `button:quitAnyway` / `button:quitCancel`), `import: {done, total, imported, cancelled} \| null` (an import runs on a worker thread), `tasks` (other background work: `Find Missing Photos`, `Auto Import`), `activity` (every long-running task in flight as the activity stack in the top-left corner shows it, same shape as `activity.list`; a row appears once its task is 0.5 s old, ids `activity:row:<id>`, ✕ = `activity:cancel:<id>`, `activity:more` expands past 3 rows), `fileDialogs` (commands waiting on a native file dialog, which runs off the UI thread; they run again with the answer when it closes), `copied` (the text the UI last put on the clipboard, `null` until something is copied) |
|
||||
| `ui.widgets` | `{filter?}` | On-screen widgets `{id, rect: [x, y, w, h]}` (screen points) |
|
||||
| `ui.clickWidget` / `ui.dragWidget` / `ui.hoverWidget` | `{id, count?, button?, fx?, fy?}` (`button: "right"` right-clicks; `count` is capped at 16) / `{id, toX?, toY?, dx?, dy?, steps?}` (steps are capped at 1000) / `{id, fx?, fy?}` | Real egui input on a widget (hover: the pointer rests on it, e.g. for preset/profile previews). `fx`/`fy` are fractions of the registered rect and may lie outside `[0,1]`; the resulting point must still be inside egui's clipped interaction rect, and a non-finite fraction or resolved point is rejected before any input is queued. Geometry comes from the latest completed layout; if the widget moves before the first event or before a press, or its latest clipped rect no longer contains the point, the request fails with an error instead of clicking another widget or retargeting. A target below an ordinary modal dialog may succeed as a no-op or a backdrop click, because egui routes that input to the dialog; a target covered by a non-modal overlay fails. The blocking library-problem/launch-list window is explicit: its dim takes the modal layer, but a background target fails instead of reporting a no-op success, and only that window's own controls (`button:library…`) pass. Its creation frame is shielded and redrawn with empty input like a prompt's, so background text/pointer input cannot act while the new modal areas settle. A quit prompt or settings notice is a real egui modal scope with a foreground backdrop. Only its own buttons (`button:quit…`, `button:noticeOk`) accept a `ui.clickWidget`; background targets fail, and background keystrokes, text/clipboard input, wheel/zoom input and AccessKit actions are dropped before panels see them. The prompt itself takes Tab/Shift+Tab, Enter/Space on the focused button, and Escape/Enter for the safe default Cancel; AccessKit Focus/Click works when targeted at a prompt button. Its first creation frame (including a Tasks→Unsaved or notice→next-notice transition) is shielded and redrawn with empty input, so a press/release in that frame, or a held press from before it, cannot act underneath it; a synthetic drag already in progress is canceled. Once settled, raw `ui.click`/`ui.drag`/`ui.pointer` and backdrop clicks are still delivered to the prompt backdrop as no-ops, and the prompt's own button clicks work normally. The area/layer check cannot see a different widget covering the point in the same layer. A `ui.clickWidget` whose widget disappears before its release also fails, after canceling the pointer press without a click or drag (the same rules apply when a click spans several host frames) |
|
||||
| `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points (the same click and step caps apply) |
|
||||
| `ui.move` / `ui.click` / `ui.drag` | `{x, y, count?, button?}` / `{x, y, toX, toY, steps?}` | Raw pointer input, screen points (the same click and step caps apply). In every pointer and scroll gesture a number other than `count` and `steps` must lie between −1,000,000 and 1,000,000, or the request is refused and queues nothing |
|
||||
| `ui.pointer` | `{events: [{kind: down\|drag\|up, x, y}], alt?, shift?, cmd?}` | Gesture in normalized image coordinates (Detail view) |
|
||||
| `ui.key` | `{key, cmd?, shift?, alt?, ctrl?}` | Key press; while a prompt is up only its Tab/Shift+Tab, Enter, Space and Escape are delivered, every other key is dropped |
|
||||
| `ui.text` | `{text}` | Text input |
|
||||
|
||||
@@ -1,5 +1,12 @@
|
||||
# What's new in LightCraft
|
||||
|
||||
## 2026-10-30
|
||||
|
||||
### The control channel refuses pointer positions that aren't on any screen
|
||||
- `ui.click`, `ui.move`, `ui.drag`, `ui.dragWidget`, `ui.pointer` and `ui.scroll` with a coordinate or delta far past
|
||||
any screen (such as `1e308`) used to queue an infinite or NaN position. A debug build panicked on it and a release
|
||||
build handed it to every widget. Numbers outside ±1,000,000 are now an error, and nothing is queued.
|
||||
|
||||
## 2026-10-27
|
||||
|
||||
### The Edit menu groups Select Only Active Photo and Paste Settings from Previous with their kin
|
||||
|
||||
Reference in New Issue
Block a user