From b5a3c975a93db26e16a8c3eb0f3d6f8608b967f0 Mon Sep 17 00:00:00 2001 From: batu_hoang Date: Wed, 21 May 2025 21:06:32 +0800 Subject: [PATCH] Add retry for hit testing with expired epoch Signed-off-by: batu_hoang --- components/compositing/compositor.rs | 57 +++++++++++++++----- components/compositing/webview_renderer.rs | 60 +++++++++++++++++++--- 2 files changed, 96 insertions(+), 21 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index a76b0022122..3428454106d 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -281,6 +281,12 @@ impl PipelineDetails { } } +pub enum HitTestError { + EpochMismatch, + #[allow(dead_code)] + Other, +} + impl ServoRenderer { pub fn shutdown_state(&self) -> ShutdownState { self.shutdown_state.get() @@ -290,15 +296,16 @@ impl ServoRenderer { &self, point: DevicePoint, details_for_pipeline: impl Fn(PipelineId) -> Option<&'a PipelineDetails>, - ) -> Option { - self.hit_test_at_point_with_flags_and_pipeline( + ) -> Result, HitTestError> { + match self.hit_test_at_point_with_flags_and_pipeline( point, HitTestFlags::empty(), None, details_for_pipeline, - ) - .first() - .cloned() + ) { + Ok(hit_test_results) => Ok(hit_test_results.first().cloned()), + Err(error) => Err(error), + } } // TODO: split this into first half (global) and second half (one for whole compositor, one for webview) @@ -308,14 +315,15 @@ impl ServoRenderer { flags: HitTestFlags, pipeline_id: Option, details_for_pipeline: impl Fn(PipelineId) -> Option<&'a PipelineDetails>, - ) -> Vec { + ) -> Result, HitTestError> { // DevicePoint and WorldPoint are the same for us. let world_point = WorldPoint::from_untyped(point.to_untyped()); let results = self.webrender_api .hit_test(self.webrender_document, pipeline_id, world_point, flags); - results + let mut epoch_mismatch = false; + let results = results .items .iter() .filter_map(|item| { @@ -323,10 +331,16 @@ impl ServoRenderer { let details = details_for_pipeline(pipeline_id)?; // If the epoch in the tag does not match the current epoch of the pipeline, - // then the hit test is against an old version of the display list and we - // should ignore this hit test for now. + // then the hit test is against an old version of the display list. match details.most_recent_display_list_epoch { - Some(epoch) if epoch.as_u16() == item.tag.1 => {}, + Some(epoch) => { + if epoch.as_u16() != item.tag.1 { + // It's too early to hit test for now. + // New scene building is in progress. + epoch_mismatch = true; + return None; + } + }, _ => return None, } @@ -340,7 +354,13 @@ impl ServoRenderer { scroll_tree_node: info.scroll_tree_node, }) }) - .collect() + .collect(); + + if epoch_mismatch { + return Err(HitTestError::EpochMismatch); + } + + Ok(results) } pub(crate) fn send_transaction(&mut self, transaction: Transaction) { @@ -619,7 +639,7 @@ impl IOCompositor { .global .borrow() .hit_test_at_point(point, details_for_pipeline); - if let Some(result) = result { + if let Ok(Some(result)) = result { self.global.borrow_mut().update_cursor(point, &result); } } @@ -841,7 +861,8 @@ impl IOCompositor { flags, pipeline, details_for_pipeline, - ); + ) + .unwrap_or_default(); let _ = sender.send(result); }, @@ -1629,11 +1650,19 @@ impl IOCompositor { }, CompositorMsg::NewWebRenderFrameReady(..) => { found_recomposite_msg = true; - compositor_messages.push(msg) + compositor_messages.push(msg); }, _ => compositor_messages.push(msg), } } + + if found_recomposite_msg { + // Process all pending events + self.webview_renderers.iter().for_each(|webview| { + webview.dispatch_pending_input_events(); + }); + } + for msg in compositor_messages { self.handle_browser_message(msg); diff --git a/components/compositing/webview_renderer.rs b/components/compositing/webview_renderer.rs index b0e91ccb02e..b2126702370 100644 --- a/components/compositing/webview_renderer.rs +++ b/components/compositing/webview_renderer.rs @@ -3,8 +3,8 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::cell::RefCell; -use std::collections::HashMap; use std::collections::hash_map::Keys; +use std::collections::{HashMap, VecDeque}; use std::rc::Rc; use base::id::{PipelineId, WebViewId}; @@ -25,7 +25,7 @@ use webrender_api::units::{ }; use webrender_api::{ExternalScrollId, HitTestFlags, ScrollLocation}; -use crate::compositor::{PipelineDetails, ServoRenderer}; +use crate::compositor::{HitTestError, PipelineDetails, ServoRenderer}; use crate::touch::{TouchHandler, TouchMoveAction, TouchMoveAllowed, TouchSequenceState}; // Default viewport constraints @@ -98,6 +98,8 @@ pub(crate) struct WebViewRenderer { /// Whether or not this [`WebViewRenderer`] isn't throttled and has a pipeline with /// active animations or animation frame callbacks. animating: bool, + /// Pending input events queue. Priavte and only this thread pushes events to it. + pending_input_events: RefCell>, } impl Drop for WebViewRenderer { @@ -132,6 +134,7 @@ impl WebViewRenderer { max_viewport_zoom: None, hidpi_scale_factor: Scale::new(hidpi_scale_factor.0), animating: false, + pending_input_events: Default::default(), } } @@ -316,14 +319,27 @@ impl WebViewRenderer { return; }; + if self.pending_input_events.borrow().len() > 0 { + self.pending_input_events.borrow_mut().push_back(event); + return; + } + // If we can't find a pipeline to send this event to, we cannot continue. let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id); - let Some(result) = self + let result = match self .global .borrow() .hit_test_at_point(point, get_pipeline_details) - else { - return; + { + Ok(Some(hit_test_results)) => hit_test_results, + Err(HitTestError::EpochMismatch) => { + dbg!("A hit test failed with epoch mismatch. Retrying"); + self.pending_input_events.borrow_mut().push_back(event); + return; + }, + _ => { + return; + }, }; self.global.borrow_mut().update_cursor(point, &result); @@ -335,6 +351,35 @@ impl WebViewRenderer { } } + pub(crate) fn dispatch_pending_input_events(&self) { + while let Some(event) = self.pending_input_events.borrow_mut().pop_front() { + // Events that do not need to do hit testing are sent directly to the + // constellation to filter down. + let Some(point) = event.point() else { + continue; + }; + + // If we can't find a pipeline to send this event to, we cannot continue. + let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id); + let Ok(Some(result)) = self + .global + .borrow() + .hit_test_at_point(point, get_pipeline_details) + else { + continue; + }; + + dbg!("Hit test for pending event succeeded"); + self.global.borrow_mut().update_cursor(point, &result); + + if let Err(error) = self.global.borrow().constellation_sender.send( + EmbedderToConstellationMessage::ForwardInputEvent(self.id, event, Some(result)), + ) { + warn!("Sending event to constellation failed ({error:?})."); + } + } + } + pub(crate) fn notify_input_event(&mut self, event: InputEvent) { if self.global.borrow().shutdown_state() != ShutdownState::NotShuttingDown { return; @@ -406,7 +451,7 @@ impl WebViewRenderer { fn send_touch_event(&self, mut event: TouchEvent) -> bool { let get_pipeline_details = |pipeline_id| self.pipelines.get(&pipeline_id); - let Some(result) = self + let Ok(Some(result)) = self .global .borrow() .hit_test_at_point(event.point, get_pipeline_details) @@ -858,7 +903,8 @@ impl WebViewRenderer { HitTestFlags::FIND_ALL, None, get_pipeline_details, - ); + ) + .unwrap_or_default(); // Iterate through all hit test results, processing only the first node of each pipeline. // This is needed to propagate the scroll events from a pipeline representing an iframe to