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

--HG--
extra : moz-landing-system : lando

gfx/layers/apz/src/APZCTreeManager.cpp

@@ -2749,8 +2749,9 @@ APZCTreeManager::HitTestResult APZCTreeManager::GetAPZCAtPointWR(
   SideBits sideBits = SideBits::eNone;
   APZCTM_LOG("Hit-testing point %s with WR\n",
              Stringify(aHitTestPoint).c_str());
-  bool hitSomething = wr->HitTest(wr::ToWorldPoint(aHitTestPoint), pipelineId,
-                                  scrollId, hitInfo, sideBits);
+  bool hitSomething = wr->FastHitTest(
+    wr::ToWorldPoint(aHitTestPoint), pipelineId,
+    scrollId, hitInfo, sideBits);
   if (!hitSomething) {
     return hit;
   }

gfx/webrender_bindings/WebRenderAPI.cpp

@@ -330,6 +330,10 @@ already_AddRefed<WebRenderAPI> WebRenderAPI::Clone() {
                        mUseTripleBuffering, mSyncHandle, mRenderRoot);
   renderApi->mRootApi = this;  // Hold root api
   renderApi->mRootDocumentApi = this;
+  if (mFastHitTester) {
+    renderApi->mFastHitTester = wr_hit_tester_clone(mFastHitTester);
+  }
+
   return renderApi.forget();
 }
 
@@ -366,13 +370,19 @@ WebRenderAPI::WebRenderAPI(wr::DocumentHandle* aHandle, wr::WindowId aId,
       mUseDComp(aUseDComp),
       mUseTripleBuffering(aUseTripleBuffering),
       mSyncHandle(aSyncHandle),
-      mRenderRoot(aRenderRoot) {}
+      mRenderRoot(aRenderRoot),
+      mFastHitTester(nullptr) {}
 
 WebRenderAPI::~WebRenderAPI() {
   if (!mRootDocumentApi) {
     wr_api_delete_document(mDocHandle);
   }
 
+  if (mFastHitTester) {
+    wr_hit_tester_delete(mFastHitTester);
+    mFastHitTester = nullptr;
+  }
+
   if (!mRootApi) {
     RenderThread::Get()->SetDestroyed(GetId());
 
@@ -473,6 +483,30 @@ bool WebRenderAPI::HitTest(const wr::WorldPoint& aPoint,
   return result;
 }
 
+bool WebRenderAPI::FastHitTest(const wr::WorldPoint& aPoint,
+                               wr::WrPipelineId& aOutPipelineId,
+                               layers::ScrollableLayerGuid::ViewID& aOutScrollId,
+                               gfx::CompositorHitTestInfo& aOutHitInfo,
+                               SideBits& aOutSideBits) {
+  static_assert(DoesCompositorHitTestInfoFitIntoBits<16>(),
+                "CompositorHitTestFlags MAX value has to be less than number "
+                "of bits in uint16_t");
+
+  if (!mFastHitTester) {
+    mFastHitTester = wr_api_request_hit_tester(mDocHandle);
+  }
+
+  uint16_t serialized = static_cast<uint16_t>(aOutHitInfo.serialize());
+  const bool result = wr_hit_tester_hit_test(mFastHitTester, aPoint, &aOutPipelineId,
+                                             &aOutScrollId, &serialized);
+
+  if (result) {
+    aOutSideBits = ExtractSideBitsFromHitInfoBits(serialized);
+    aOutHitInfo.deserialize(serialized);
+  }
+  return result;
+}
+
 void WebRenderAPI::Readback(const TimeStamp& aStartTime, gfx::IntSize size,
                             const gfx::SurfaceFormat& aFormat,
                             const Range<uint8_t>& buffer) {

gfx/webrender_bindings/WebRenderAPI.h

@@ -233,10 +233,23 @@ class WebRenderAPI final {
 
   wr::WindowId GetId() const { return mId; }
 
+  /// Send a blocking hit-testing query to the render backend thread.
   bool HitTest(const wr::WorldPoint& aPoint, wr::WrPipelineId& aOutPipelineId,
                layers::ScrollableLayerGuid::ViewID& aOutScrollId,
                gfx::CompositorHitTestInfo& aOutHitInfo, SideBits& aOutSideBits);
 
+  /// Do a non-blocking hit-testing query on a shared version of the hit testing
+  /// information.
+  ///
+  /// This does not guarantee ordering between the query and in-flight transactions,
+  /// but only blocks on a synchronous message to the render backend thread the first
+  /// time the function is called.
+  /// This generally returns much faster than `HitTest`.
+  bool FastHitTest(const wr::WorldPoint& aPoint, wr::WrPipelineId& aOutPipelineId,
+                   layers::ScrollableLayerGuid::ViewID& aOutScrollId,
+                   gfx::CompositorHitTestInfo& aOutHitInfo,
+                   SideBits& aOutSideBits);
+
   void SendTransaction(TransactionBuilder& aTxn);
 
   void SetFrameStartTime(const TimeStamp& aTime);
@@ -327,6 +340,8 @@ class WebRenderAPI final {
   RefPtr<wr::WebRenderAPI> mRootApi;
   RefPtr<wr::WebRenderAPI> mRootDocumentApi;
 
+  WrHitTester* mFastHitTester;
+
   friend class DisplayListBuilder;
   friend class layers::WebRenderBridgeParent;
 };

gfx/webrender_bindings/src/bindings.rs

@@ -972,6 +972,65 @@ impl AsyncPropertySampler for SamplerCallback {
     }
 }
 
+// cbindgen's parser currently does not handle the dyn keyword.
+// We work around it by wrapping the referece counted pointer into
+// a struct and boxing it.
+//
+// See https://github.com/eqrion/cbindgen/issues/385
+//
+// Once this is fixed we should be able to pass `*mut dyn ApiHitTester`
+// and avoid the extra indirection.
+pub struct WrHitTester {
+    ptr: Arc<dyn ApiHitTester>,
+}
+
+#[no_mangle]
+pub extern "C" fn wr_api_request_hit_tester(
+    dh: &DocumentHandle,
+) -> *mut WrHitTester {
+    let hit_tester = dh.api.request_hit_tester(dh.document_id);
+    Box::into_raw(Box::new(WrHitTester { ptr: hit_tester }))
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn wr_hit_tester_clone(hit_tester: *mut WrHitTester) -> *mut WrHitTester {
+    let new_ref = Arc::clone(&(*hit_tester).ptr);
+    Box::into_raw(Box::new(WrHitTester { ptr: new_ref }))
+}
+
+
+#[no_mangle]
+pub extern "C" fn wr_hit_tester_hit_test(
+    hit_tester: &WrHitTester,
+    point: WorldPoint,
+    out_pipeline_id: &mut WrPipelineId,
+    out_scroll_id: &mut u64,
+    out_hit_info: &mut u16
+) -> bool {
+    let result = hit_tester.ptr.hit_test(
+        None,
+        point,
+        HitTestFlags::empty()
+    );
+
+    for item in &result.items {
+        // For now we should never be getting results back for which the tag is
+        // 0 (== CompositorHitTestInvisibleToHit). In the future if we allow this,
+        // we'll want to |continue| on the loop in this scenario.
+        debug_assert!(item.tag.1 != 0);
+        *out_pipeline_id = item.pipeline;
+        *out_scroll_id = item.tag.0;
+        *out_hit_info = item.tag.1;
+        return true;
+    }
+    return false;
+}
+
+#[no_mangle]
+pub unsafe extern "C" fn wr_hit_tester_delete(hit_tester: *mut WrHitTester) {
+    let _ = Box::from_raw(hit_tester);
+}
+
 extern "C" {
     fn gecko_profiler_register_thread(name: *const ::std::os::raw::c_char);
     fn gecko_profiler_unregister_thread();

gfx/wr/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;

gfx/wr/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<Arc<HitTester>>,
+}
+
+impl SharedHitTester {
+    pub fn new() -> Self {
+        SharedHitTester {
+            hit_tester: Mutex::new(Arc::new(HitTester::empty())),
+        }
+    }
+
+    pub fn get_ref(&self) -> Arc<HitTester> {
+        let guard = self.hit_tester.lock().unwrap();
+        Arc::clone(&*guard)
+    }
+
+    pub(crate) fn update(&self, new_hit_tester: Arc<HitTester>) {
+        let mut guard = self.hit_tester.lock().unwrap();
+        *guard = new_hit_tester;
+    }
+}
+
+impl ApiHitTester for SharedHitTester {
+    fn hit_test(&self,
+        pipeline_id: Option<PipelineId>,
+        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<HitTestingScene>,
         spatial_tree: &SpatialTree,

gfx/wr/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;

gfx/wr/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

gfx/wr/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<HitTester>,
+    hit_tester: Option<Arc<HitTester>>,
+    /// To avoid synchronous messaging we update a shared hit-tester that other threads
+    /// can query.
+    shared_hit_tester: Arc<SharedHitTester>,
 
     /// 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<ResourceUpdate>,
-        interner_updates: Option<InternerUpdates>,
         mut frame_ops: Vec<FrameMsg>,
         mut notifications: Vec<NotificationRequest>,
         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,

gfx/wr/webrender_api/src/api.rs

@@ -852,6 +852,8 @@ pub enum FrameMsg {
     ///
     HitTest(Option<PipelineId>, WorldPoint, HitTestFlags, MsgSender<HitTestResult>),
     ///
+    RequestHitTester(MsgSender<Arc<dyn ApiHitTester>>),
+    ///
     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<dyn ApiHitTester> {
+        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<PipelineId>, point: WorldPoint, flags: HitTestFlags) -> HitTestResult;
+}
+
 impl Drop for NotificationRequest {
     fn drop(&mut self) {
         if let Some(ref mut handler) = self.handler {