Skip to content

Commit

Permalink
Bug 1580178 - Allow hit-testing without synchronous messaging. r=boto…
Browse files Browse the repository at this point in the history
…nd,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
  • Loading branch information
nical committed Mar 2, 2020
1 parent d60d832 commit 4b16ff3
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 33 deletions.
5 changes: 3 additions & 2 deletions gfx/layers/apz/src/APZCTreeManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
36 changes: 35 additions & 1 deletion gfx/webrender_bindings/WebRenderAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions gfx/webrender_bindings/WebRenderAPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -327,6 +340,8 @@ class WebRenderAPI final {
RefPtr<wr::WebRenderAPI> mRootApi;
RefPtr<wr::WebRenderAPI> mRootDocumentApi;

WrHitTester* mFastHitTester;

friend class DisplayListBuilder;
friend class layers::WebRenderBridgeParent;
};
Expand Down
59 changes: 59 additions & 0 deletions gfx/webrender_bindings/src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion gfx/wr/webrender/src/clip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
49 changes: 47 additions & 2 deletions gfx/wr/webrender/src/hit_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions gfx/wr/webrender/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion gfx/wr/webrender/src/prim_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 4b16ff3

Please sign in to comment.