diff --git a/gfx/layers/wr/ScrollingLayersHelper.cpp b/gfx/layers/wr/ScrollingLayersHelper.cpp index bfa77e89eef2..dd0ac0125822 100644 --- a/gfx/layers/wr/ScrollingLayersHelper.cpp +++ b/gfx/layers/wr/ScrollingLayersHelper.cpp @@ -177,7 +177,7 @@ ScrollingLayersHelper::DefineAndPushScrollLayers(nsDisplayItem* aItem, DefineAndPushChain(asrClippedBy, aBuilder, aStackingContext, aAppUnitsPerDevPixel, aCache); // Finally, push the ASR itself as a scroll layer. Note that the - // implementation of wr_push_scroll_layer in bindings.rs makes sure the + // implementation of PushScrollLayer in DisplayListBuilder makes sure the // scroll layer doesn't get defined multiple times so we don't need to worry // about that here. if (PushScrollLayer(metadata->GetMetrics(), aStackingContext)) { diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index f81fcc0b1ff5..95f398d93979 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -685,14 +685,21 @@ DisplayListBuilder::PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollI { WRDL_LOG("PushScrollLayer id=%" PRIu64 " co=%s cl=%s\n", mWrState, aScrollId, Stringify(aContentRect).c_str(), Stringify(aClipRect).c_str()); - wr_dp_push_scroll_layer(mWrState, aScrollId, aContentRect, aClipRect); - if (!mScrollIdStack.empty()) { - auto it = mScrollParents.insert({aScrollId, mScrollIdStack.back()}); - if (!it.second) { // aScrollId was already a key in mScrollParents - // so check that the parent value is the same. - MOZ_ASSERT(it.first->second == mScrollIdStack.back()); - } + + Maybe parent = + mScrollIdStack.empty() ? Nothing() : Some(mScrollIdStack.back()); + auto it = mScrollParents.insert({aScrollId, parent}); + if (it.second) { + // An insertion took place, which means we haven't defined aScrollId before. + // So let's define it now. + wr_dp_define_scroll_layer(mWrState, aScrollId, aContentRect, aClipRect); + } else { + // aScrollId was already a key in mScrollParents so check that the parent + // value is the same. + MOZ_ASSERT(it.first->second == parent); } + + wr_dp_push_scroll_layer(mWrState, aScrollId); mScrollIdStack.push_back(aScrollId); } @@ -1011,7 +1018,7 @@ Maybe DisplayListBuilder::ParentScrollIdFor(layers::FrameMetrics::ViewID aScrollId) { auto it = mScrollParents.find(aScrollId); - return (it == mScrollParents.end() ? Nothing() : Some(it->second)); + return (it == mScrollParents.end() ? Nothing() : it->second); } } // namespace wr diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index 32a0543f6604..be653b1062dd 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -348,8 +348,10 @@ protected: std::vector mClipIdStack; std::vector mScrollIdStack; - // Track the parent scroll id of each scroll id that we encountered. - std::unordered_map mScrollParents; + // Track the parent scroll id of each scroll id that we encountered. A + // Nothing() value indicates a root scroll id. We also use this structure to + // ensure that we don't define a particular scroll layer multiple times. + std::unordered_map> mScrollParents; friend class WebRenderAPI; }; diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs index abcfb638aaac..6adff1246f05 100644 --- a/gfx/webrender_bindings/src/bindings.rs +++ b/gfx/webrender_bindings/src/bindings.rs @@ -1,4 +1,3 @@ -use std::collections::HashSet; use std::ffi::CString; use std::{mem, slice}; use std::path::PathBuf; @@ -926,7 +925,6 @@ pub unsafe extern "C" fn wr_api_get_namespace(dh: &mut DocumentHandle) -> WrIdNa pub struct WebRenderFrameBuilder { pub root_pipeline_id: WrPipelineId, pub dl_builder: webrender_api::DisplayListBuilder, - pub scroll_clips_defined: HashSet, } impl WebRenderFrameBuilder { @@ -935,7 +933,6 @@ impl WebRenderFrameBuilder { WebRenderFrameBuilder { root_pipeline_id: root_pipeline_id, dl_builder: webrender_api::DisplayListBuilder::new(root_pipeline_id, content_size), - scroll_clips_defined: HashSet::new(), } } } @@ -1102,21 +1099,22 @@ pub extern "C" fn wr_dp_pop_clip(state: &mut WrState) { } #[no_mangle] -pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState, - scroll_id: u64, - content_rect: LayoutRect, - clip_rect: LayoutRect) { +pub extern "C" fn wr_dp_define_scroll_layer(state: &mut WrState, + scroll_id: u64, + content_rect: LayoutRect, + clip_rect: LayoutRect) { assert!(unsafe { is_in_main_thread() }); let clip_id = ClipId::new(scroll_id, state.pipeline_id); - // Avoid defining multiple scroll clips with the same clip id, as that - // results in undefined behaviour or assertion failures. - if !state.frame_builder.scroll_clips_defined.contains(&clip_id) { + state.frame_builder.dl_builder.define_scroll_frame( + Some(clip_id), content_rect, clip_rect, vec![], None, + ScrollSensitivity::Script); +} - state.frame_builder.dl_builder.define_scroll_frame( - Some(clip_id), content_rect, clip_rect, vec![], None, - ScrollSensitivity::Script); - state.frame_builder.scroll_clips_defined.insert(clip_id); - } +#[no_mangle] +pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState, + scroll_id: u64) { + assert!(unsafe { is_in_main_thread() }); + let clip_id = ClipId::new(scroll_id, state.pipeline_id); state.frame_builder.dl_builder.push_clip_id(clip_id); } diff --git a/gfx/webrender_bindings/webrender_ffi_generated.h b/gfx/webrender_bindings/webrender_ffi_generated.h index c5bcbfa6d3fa..a6c3d94ebb43 100644 --- a/gfx/webrender_bindings/webrender_ffi_generated.h +++ b/gfx/webrender_bindings/webrender_ffi_generated.h @@ -860,6 +860,13 @@ uint64_t wr_dp_define_clip(WrState *aState, const WrImageMask *aMask) WR_FUNC; +WR_INLINE +void wr_dp_define_scroll_layer(WrState *aState, + uint64_t aScrollId, + LayoutRect aContentRect, + LayoutRect aClipRect) +WR_FUNC; + WR_INLINE void wr_dp_end(WrState *aState) WR_FUNC; @@ -1027,9 +1034,7 @@ WR_FUNC; WR_INLINE void wr_dp_push_scroll_layer(WrState *aState, - uint64_t aScrollId, - LayoutRect aContentRect, - LayoutRect aClipRect) + uint64_t aScrollId) WR_FUNC; WR_INLINE