From 0b1b6dd087280582012c1fc21ecdb933938b13fc Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 9 Mar 2020 22:00:29 +0000 Subject: [PATCH] Bug 1580178 - Allow hit-testing without synchronous messaging. r=botond,kats This patch adds an asynchronous hit tester that can perform hit testing queries without blocking on a synchronous message to the render backend thread, which is often busy building frames. This is done by having a shared immutable hit tester readable by any thread, atomically swapped each time the render backend processes a new scene or frame. In order to asynchronously hit test without causing race conditions with APZ intenral state, the hit tester has to be built while the APZ lock is held. Differential Revision: https://phabricator.services.mozilla.com/D45345 [ghsync] From https://hg.mozilla.org/mozilla-central/rev/ed2d1ba57e2f0cb9072af5c6e9ef17a9bbdc18a0 --- webrender/src/clip.rs | 2 +- webrender/src/hit_test.rs | 49 +++++++++++++++++++++- webrender/src/lib.rs | 1 + webrender/src/prim_store/mod.rs | 2 +- webrender/src/render_backend.rs | 72 +++++++++++++++++++++------------ webrender_api/src/api.rs | 25 ++++++++++++ 6 files changed, 121 insertions(+), 30 deletions(-) diff --git a/webrender/src/clip.rs b/webrender/src/clip.rs index 5963d72eff..59e6f0d326 100644 --- a/webrender/src/clip.rs +++ b/webrender/src/clip.rs @@ -753,7 +753,7 @@ impl ClipStore { spatial_node_index: SpatialNodeIndex, clip_chains: &[ClipChainId], spatial_tree: &SpatialTree, - clip_data_store: &mut ClipDataStore, + clip_data_store: &ClipDataStore, ) { self.active_clip_node_info.clear(); self.active_local_clip_rect = None; diff --git a/webrender/src/hit_test.rs b/webrender/src/hit_test.rs index f3df10e28d..35d84dc6cd 100644 --- a/webrender/src/hit_test.rs +++ b/webrender/src/hit_test.rs @@ -3,16 +3,52 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use api::{BorderRadius, ClipMode, HitTestFlags, HitTestItem, HitTestResult, ItemTag, PrimitiveFlags}; -use api::PipelineId; +use api::{PipelineId, ApiHitTester}; use api::units::*; use crate::clip::{ClipChainId, ClipDataStore, ClipNode, ClipItemKind, ClipStore}; use crate::clip::{rounded_rectangle_contains_point}; use crate::spatial_tree::{SpatialNodeIndex, SpatialTree}; use crate::internal_types::{FastHashMap, LayoutPrimitiveInfo}; use std::{ops, u32}; -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use crate::util::LayoutToWorldFastTransform; +pub struct SharedHitTester { + // We don't really need a mutex here. We could do with some sort of + // atomic-atomic-ref-counted pointer (an Arc which would let the pointer + // be swapped atomically like an AtomicPtr). + // In practive this shouldn't cause performance issues, though. + hit_tester: Mutex>, +} + +impl SharedHitTester { + pub fn new() -> Self { + SharedHitTester { + hit_tester: Mutex::new(Arc::new(HitTester::empty())), + } + } + + pub fn get_ref(&self) -> Arc { + let guard = self.hit_tester.lock().unwrap(); + Arc::clone(&*guard) + } + + pub(crate) fn update(&self, new_hit_tester: Arc) { + let mut guard = self.hit_tester.lock().unwrap(); + *guard = new_hit_tester; + } +} + +impl ApiHitTester for SharedHitTester { + fn hit_test(&self, + pipeline_id: Option, + point: WorldPoint, + flags: HitTestFlags + ) -> HitTestResult { + self.get_ref().hit_test(HitTest::new(pipeline_id, point, flags)) + } +} + /// A copy of important spatial node data to use during hit testing. This a copy of /// data from the SpatialTree that will persist as a new frame is under construction, /// allowing hit tests consistent with the currently rendered frame. @@ -216,6 +252,15 @@ pub struct HitTester { } impl HitTester { + pub fn empty() -> Self { + HitTester { + scene: Arc::new(HitTestingScene::new(&HitTestingSceneStats::empty())), + spatial_nodes: Vec::new(), + clip_chains: Vec::new(), + pipeline_root_nodes: FastHashMap::default(), + } + } + pub fn new( scene: Arc, spatial_tree: &SpatialTree, diff --git a/webrender/src/lib.rs b/webrender/src/lib.rs index 98c11cb92a..f5b4e1194f 100644 --- a/webrender/src/lib.rs +++ b/webrender/src/lib.rs @@ -217,6 +217,7 @@ pub use crate::renderer::{ RendererStats, SceneBuilderHooks, ThreadListener, ShaderPrecacheFlags, MAX_VERTEX_TEXTURE_WIDTH, }; +pub use crate::hit_test::SharedHitTester; pub use crate::screen_capture::{AsyncScreenshotHandle, RecordedFrameHandle}; pub use crate::shade::{Shaders, WrShaders}; pub use api as webrender_api; diff --git a/webrender/src/prim_store/mod.rs b/webrender/src/prim_store/mod.rs index ef50dfbf71..72e47fe458 100644 --- a/webrender/src/prim_store/mod.rs +++ b/webrender/src/prim_store/mod.rs @@ -2137,7 +2137,7 @@ impl PrimitiveStore { cluster.spatial_node_index, frame_state.clip_chain_stack.current_clips_array(), &frame_context.spatial_tree, - &mut frame_state.data_stores.clip, + &frame_state.data_stores.clip, ); let clip_chain = frame_state diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index 333712c76d..420e0a069a 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -27,7 +27,7 @@ use crate::debug_server; use crate::frame_builder::{FrameBuilder, FrameBuilderConfig}; use crate::glyph_rasterizer::{FontInstance}; use crate::gpu_cache::GpuCache; -use crate::hit_test::{HitTest, HitTester}; +use crate::hit_test::{HitTest, HitTester, SharedHitTester}; use crate::intern::DataStore; use crate::internal_types::{DebugOutput, FastHashMap, FastHashSet, RenderedDocument, ResultMsg}; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; @@ -359,7 +359,10 @@ struct Document { /// A data structure to allow hit testing against rendered frames. This is updated /// every time we produce a fully rendered frame. - hit_tester: Option, + hit_tester: Option>, + /// To avoid synchronous messaging we update a shared hit-tester that other threads + /// can query. + shared_hit_tester: Arc, /// Properties that are resolved during frame building and can be changed at any time /// without requiring the scene to be re-built. @@ -415,6 +418,7 @@ impl Document { frame_builder: FrameBuilder::new(), output_pipelines: FastHashSet::default(), hit_tester: None, + shared_hit_tester: Arc::new(SharedHitTester::new()), dynamic_properties: SceneProperties::new(), frame_is_valid: false, hit_tester_is_valid: false, @@ -487,6 +491,9 @@ impl Document { tx.send(result).unwrap(); } + FrameMsg::RequestHitTester(tx) => { + tx.send(self.shared_hit_tester.clone()).unwrap(); + } FrameMsg::SetPan(pan) => { if self.view.pan != pan { self.view.pan = pan; @@ -573,7 +580,11 @@ impl Document { debug_flags, tile_cache_logger, ); - self.hit_tester = Some(self.scene.create_hit_tester(&self.data_stores.clip)); + + let hit_tester = Arc::new(self.scene.create_hit_tester(&self.data_stores.clip)); + self.hit_tester = Some(Arc::clone(&hit_tester)); + self.shared_hit_tester.update(hit_tester); + frame }; @@ -593,13 +604,15 @@ impl Document { let accumulated_scale_factor = self.view.accumulated_scale_factor(); let pan = self.view.pan.to_f32() / accumulated_scale_factor; - self.scene.spatial_tree.update_tree( - pan, - accumulated_scale_factor, - &self.dynamic_properties, - ); + self.scene.spatial_tree.update_tree( + pan, + accumulated_scale_factor, + &self.dynamic_properties, + ); - self.hit_tester = Some(self.scene.create_hit_tester(&self.data_stores.clip)); + let hit_tester = Arc::new(self.scene.create_hit_tester(&self.data_stores.clip)); + self.hit_tester = Some(Arc::clone(&hit_tester)); + self.shared_hit_tester.update(hit_tester); self.hit_tester_is_valid = true; } @@ -926,6 +939,25 @@ impl RenderBackend { ); } + // If there are any additions or removals of clip modes + // during the scene build, apply them to the data store now. + // This needs to happen before we build the hit tester. + if let Some(updates) = txn.interner_updates.take() { + #[cfg(feature = "capture")] + { + if self.debug_flags.contains(DebugFlags::TILE_CACHE_LOGGING_DBG) { + self.tile_cache_logger.serialize_updates(&updates); + } + } + doc.data_stores.apply_updates(updates, &mut profile_counters); + } + + + // Build the hit tester while the APZ lock is held so that its content + // is in sync with the gecko APZ tree. + if !doc.hit_tester_is_valid { + doc.rebuild_hit_tester(); + } if let Some(ref tx) = result_tx { let (resume_tx, resume_rx) = channel(); tx.send(SceneSwapResult::Complete(resume_tx)).unwrap(); @@ -956,7 +988,6 @@ impl RenderBackend { self.update_document( txn.document_id, txn.resource_updates.take(), - txn.interner_updates.take(), txn.frame_ops.take(), txn.notifications.take(), txn.render_frame, @@ -1418,7 +1449,6 @@ impl RenderBackend { self.update_document( txn.document_id, txn.resource_updates.take(), - None, txn.frame_ops.take(), txn.notifications.take(), txn.render_frame, @@ -1462,7 +1492,6 @@ impl RenderBackend { self.update_document( document_id, Vec::default(), - None, Vec::default(), Vec::default(), false, @@ -1478,7 +1507,6 @@ impl RenderBackend { &mut self, document_id: DocumentId, resource_updates: Vec, - interner_updates: Option, mut frame_ops: Vec, mut notifications: Vec, mut render_frame: bool, @@ -1504,18 +1532,6 @@ impl RenderBackend { let doc = self.documents.get_mut(&document_id).unwrap(); doc.has_built_scene |= has_built_scene; - // If there are any additions or removals of clip modes - // during the scene build, apply them to the data store now. - if let Some(updates) = interner_updates { - #[cfg(feature = "capture")] - { - if self.debug_flags.contains(DebugFlags::TILE_CACHE_LOGGING_DBG) { - self.tile_cache_logger.serialize_updates(&updates); - } - } - doc.data_stores.apply_updates(updates, profile_counters); - } - // TODO: this scroll variable doesn't necessarily mean we scrolled. It is only used // for something wrench specific and we should remove it. let mut scroll = false; @@ -1698,7 +1714,10 @@ impl RenderBackend { report.gpu_cache_metadata = self.gpu_cache.size_of(ops); for doc in self.documents.values() { report.clip_stores += doc.scene.clip_store.size_of(ops); - report.hit_testers += doc.hit_tester.size_of(ops); + report.hit_testers += match &doc.hit_tester { + Some(hit_tester) => hit_tester.size_of(ops), + None => 0, + }; doc.data_stores.report_memory(ops, &mut report) } @@ -1910,6 +1929,7 @@ impl RenderBackend { output_pipelines: FastHashSet::default(), dynamic_properties: SceneProperties::new(), hit_tester: None, + shared_hit_tester: Arc::new(SharedHitTester::new()), frame_is_valid: false, hit_tester_is_valid: false, rendered_frame_is_valid: false, diff --git a/webrender_api/src/api.rs b/webrender_api/src/api.rs index 3a2d5b6d61..c2bf8c4faf 100644 --- a/webrender_api/src/api.rs +++ b/webrender_api/src/api.rs @@ -852,6 +852,8 @@ pub enum FrameMsg { /// HitTest(Option, WorldPoint, HitTestFlags, MsgSender), /// + RequestHitTester(MsgSender>), + /// SetPan(DeviceIntPoint), /// Scroll(ScrollLocation, WorldPoint), @@ -889,6 +891,7 @@ impl fmt::Debug for FrameMsg { f.write_str(match *self { FrameMsg::UpdateEpoch(..) => "FrameMsg::UpdateEpoch", FrameMsg::HitTest(..) => "FrameMsg::HitTest", + FrameMsg::RequestHitTester(..) => "FrameMsg::RequestHitTester", FrameMsg::SetPan(..) => "FrameMsg::SetPan", FrameMsg::Scroll(..) => "FrameMsg::Scroll", FrameMsg::ScrollNodeWithId(..) => "FrameMsg::ScrollNodeWithId", @@ -1680,6 +1683,16 @@ impl RenderApi { rx.recv().unwrap() } + /// Synchronously request an object that can perform fast hit testing queries. + pub fn request_hit_tester(&self, document_id: DocumentId) -> Arc { + let (tx, rx) = channel::msg_channel().unwrap(); + self.send_frame_msg( + document_id, + FrameMsg::RequestHitTester(tx) + ); + rx.recv().unwrap() + } + /// Setup the output region in the framebuffer for a given document. pub fn set_document_view( &self, @@ -2014,6 +2027,18 @@ impl NotificationRequest { } } +/// An object that can perform hit-testing without doing synchronous queries to +/// the RenderBackendThread. +pub trait ApiHitTester: Send + Sync { + /// Does a hit test on display items in the specified document, at the given + /// point. If a pipeline_id is specified, it is used to further restrict the + /// hit results so that only items inside that pipeline are matched. If the + /// HitTestFlags argument contains the FIND_ALL flag, then the vector of hit + /// results will contain all display items that match, ordered from front + /// to back. + fn hit_test(&self, pipeline_id: Option, point: WorldPoint, flags: HitTestFlags) -> HitTestResult; +} + impl Drop for NotificationRequest { fn drop(&mut self) { if let Some(ref mut handler) = self.handler {