diff --git a/agents.md b/agents.md index 914641c..da735ad 100644 --- a/agents.md +++ b/agents.md @@ -88,6 +88,10 @@ Real and verified: (LOAD 1 → LOAD 2). The webview loading state reconciles through redirects/pushState (`awaiting_url_change`), and `run_dev.sh` resolves the macOS Metal toolchain so `cargo run`'s shader-build failure is fixed. +- Scroll input: consecutive same-document wheel requests coalesce at the + profile worker boundary while preserving total device-pixel distance; + clicks, text, and allow-once permission transfers retain strict ordering. + Verified by a blocked-worker burst regression and the live-site scroll e2e. - Cross-platform: download Open/Reveal use per-OS launchers (macOS `open`, Windows `cmd start`/`explorer /select`, Linux `xdg-open`); the command overlay closes on Escape. diff --git a/crates/ely_app/src/shell/web_surface_worker.rs b/crates/ely_app/src/shell/web_surface_worker.rs index b6e95d6..0e1862d 100644 --- a/crates/ely_app/src/shell/web_surface_worker.rs +++ b/crates/ely_app/src/shell/web_surface_worker.rs @@ -13,8 +13,8 @@ use crate::services::servo_live::{ #[path = "web_surface_worker_dispatch.rs"] mod dispatch; use dispatch::{ - dispatch_result, forward_permission_consumptions, preserve_latest_hover, - request_has_ordered_input, + can_merge_consecutive_scroll, dispatch_result, forward_permission_consumptions, + merge_consecutive_scroll, preserve_latest_hover, request_has_ordered_input, }; /// Blocking transport for one profile-scoped Servo sidecar. @@ -225,11 +225,12 @@ impl LiveRuntimeWorker { } let mut request = WorkerRequest::Ensure { generation, request: Box::new(request) }; if let Some(pending) = q.pending.get_mut(&tab_id) { - let replace_tail = pending.back().is_some_and(|tail| { - matches!(tail, WorkerRequest::Poll { .. }) - || (!request_has_ordered_input(&request) && !request_has_ordered_input(tail)) - }); - if replace_tail && let Some(tail) = pending.back_mut() { + if let Some(tail) = pending.back_mut() + && (can_merge_consecutive_scroll(&request, tail) + || matches!(tail, WorkerRequest::Poll { .. }) + || (!request_has_ordered_input(&request) && !request_has_ordered_input(tail))) + { + merge_consecutive_scroll(&mut request, tail); preserve_latest_hover(&mut request, tail); *tail = request; } else { @@ -241,7 +242,6 @@ impl LiveRuntimeWorker { } cvar.notify_one(); } - pub(super) fn submit_poll(&self, generation: RequestGeneration, tab_id: String) -> bool { let (lock, cvar) = &*self.queue; let mut q = match lock.lock() { diff --git a/crates/ely_app/src/shell/web_surface_worker_dispatch.rs b/crates/ely_app/src/shell/web_surface_worker_dispatch.rs index 92c2cda..b9c4731 100644 --- a/crates/ely_app/src/shell/web_surface_worker_dispatch.rs +++ b/crates/ely_app/src/shell/web_surface_worker_dispatch.rs @@ -6,6 +6,55 @@ use super::{ LiveRuntimeClient, LiveRuntimeClientError, RequestGeneration, WorkerRequest, WorkerResponse, }; +pub(super) fn can_merge_consecutive_scroll( + latest: &WorkerRequest, + previous: &WorkerRequest, +) -> bool { + let ( + WorkerRequest::Ensure { request: latest, .. }, + WorkerRequest::Ensure { request: previous, .. }, + ) = (latest, previous) + else { + return false; + }; + is_scroll_only(latest) + && is_scroll_only(previous) + && latest.profile_id == previous.profile_id + && latest.url == previous.url + && latest.width == previous.width + && latest.height == previous.height + && latest.page_zoom_percent == previous.page_zoom_percent + && latest.device_pixel_ratio == previous.device_pixel_ratio +} + +pub(super) fn merge_consecutive_scroll(latest: &mut WorkerRequest, previous: &WorkerRequest) { + if !can_merge_consecutive_scroll(latest, previous) { + return; + } + let ( + WorkerRequest::Ensure { request: latest, .. }, + WorkerRequest::Ensure { request: previous, .. }, + ) = (latest, previous) + else { + return; + }; + + latest.scroll_delta_x = previous.scroll_delta_x.saturating_add(latest.scroll_delta_x); + latest.scroll_delta_y = previous.scroll_delta_y.saturating_add(latest.scroll_delta_y); + if latest.hover_x.is_none() && latest.hover_y.is_none() { + latest.hover_x = previous.hover_x; + latest.hover_y = previous.hover_y; + } +} + +fn is_scroll_only(request: &crate::services::servo_live::ServoLiveEnsureRequest) -> bool { + (request.scroll_delta_x != 0 || request.scroll_delta_y != 0) + && request.click_x.is_none() + && request.click_y.is_none() + && request.typed_text.is_none() + && !request.site_permissions.iter().any(|permission| permission.state == "allow-once") +} + pub(super) fn forward_permission_consumptions( client: &mut dyn LiveRuntimeClient, response_tx: &mpsc::Sender, diff --git a/crates/ely_app/src/shell/web_surface_worker_scroll_tests.rs b/crates/ely_app/src/shell/web_surface_worker_scroll_tests.rs new file mode 100644 index 0000000..86b3aa4 --- /dev/null +++ b/crates/ely_app/src/shell/web_surface_worker_scroll_tests.rs @@ -0,0 +1,94 @@ +use std::{ + sync::{Arc, Mutex, mpsc}, + time::Duration, +}; + +use crate::services::servo_live::{ServoLiveEnsureRequest, ServoLiveFrame}; + +use super::super::{ + LiveRuntimeClient, LiveRuntimeClientError, LiveRuntimeWorker, RequestGeneration, +}; + +struct BlockingScrollClient { + deltas: Arc>>, + first_started_tx: Option>, + release_first_rx: mpsc::Receiver<()>, +} + +impl LiveRuntimeClient for BlockingScrollClient { + fn ensure( + &mut self, + request: ServoLiveEnsureRequest, + ) -> Result, LiveRuntimeClientError> { + self.deltas + .lock() + .map_err(|_| "scroll recorder lock was poisoned".to_string())? + .push((request.scroll_delta_x, request.scroll_delta_y)); + if let Some(first_started_tx) = self.first_started_tx.take() { + first_started_tx.send(()).map_err(|error| error.to_string())?; + self.release_first_rx.recv().map_err(|error| error.to_string())?; + } + Ok(None) + } + + fn poll(&mut self, _tab_id: String) -> Result, LiveRuntimeClientError> { + Ok(None) + } + + fn close(&mut self, _tab_id: String) -> Result<(), LiveRuntimeClientError> { + Ok(()) + } +} + +#[test] +fn consecutive_scroll_updates_coalesce_without_losing_distance() -> Result<(), String> { + let deltas = Arc::new(Mutex::new(Vec::new())); + let client_deltas = deltas.clone(); + let (first_started_tx, first_started_rx) = mpsc::channel(); + let (release_first_tx, release_first_rx) = mpsc::channel(); + let worker = LiveRuntimeWorker::new(move || { + Ok(Box::new(BlockingScrollClient { + deltas: client_deltas, + first_started_tx: Some(first_started_tx), + release_first_rx, + })) + })?; + + worker.submit_ensure(RequestGeneration::new(1), ensure_request(0)); + first_started_rx.recv_timeout(Duration::from_secs(1)).map_err(|error| error.to_string())?; + for generation in 2..=101 { + worker.submit_ensure(RequestGeneration::new(generation), ensure_request(1)); + } + release_first_tx.send(()).map_err(|error| error.to_string())?; + worker.wait_until_idle(); + + assert_eq!( + *deltas.lock().map_err(|_| "scroll recorder lock was poisoned".to_string())?, + vec![(0, 0), (100, 100)] + ); + Ok(()) +} + +fn ensure_request(scroll_delta: i32) -> ServoLiveEnsureRequest { + ServoLiveEnsureRequest { + tab_id: "tab-a".to_string(), + profile_id: "profile".to_string(), + url: "https://example.com/".to_string(), + width: 640, + height: 480, + page_zoom_percent: 100, + device_pixel_ratio: 1.0, + scroll_delta_x: scroll_delta, + scroll_delta_y: scroll_delta, + scroll_point_x: (scroll_delta != 0).then_some(1), + scroll_point_y: (scroll_delta != 0).then_some(1), + click_x: None, + click_y: None, + hover_x: None, + hover_y: None, + typed_text: None, + site_permission_generation: 1, + site_permissions: Vec::new(), + allow_once_grants: Vec::new(), + } +} diff --git a/crates/ely_app/src/shell/web_surface_worker_tests.rs b/crates/ely_app/src/shell/web_surface_worker_tests.rs index f5d73a4..fbe4678 100644 --- a/crates/ely_app/src/shell/web_surface_worker_tests.rs +++ b/crates/ely_app/src/shell/web_surface_worker_tests.rs @@ -464,3 +464,6 @@ fn ensure_request(tab_id: &str, input: RecordedInput) -> ServoLiveEnsureRequest allow_once_grants: Vec::new(), } } + +#[path = "web_surface_worker_scroll_tests.rs"] +mod scroll_tests;