Bug 1632409 - Separate image mask API from the generic clip API in WR. r=kats

Separating out the generic clip definition API into image masks
(and other types in future) makes the serialized data smaller. More
importantly, it will allow us to simplify some of the WR clip internals
in future and optimize the performance spent in clip chain handling.

Differential Revision: https://phabricator.services.mozilla.com/D72125
This commit is contained in:
Glenn Watson 2020-04-23 20:44:21 +00:00
parent 3d7e214963
commit 31dabce5a8
14 changed files with 155 additions and 84 deletions

View File

@ -1021,8 +1021,7 @@ wr::WrClipChainId DisplayListBuilder::DefineClipChain(
wr::WrClipId DisplayListBuilder::DefineClip(
const Maybe<wr::WrSpaceAndClip>& aParent, const wr::LayoutRect& aClipRect,
const nsTArray<wr::ComplexClipRegion>* aComplex,
const wr::ImageMask* aMask) {
const nsTArray<wr::ComplexClipRegion>* aComplex) {
CancelGroup();
WrClipId clipId;
@ -1030,19 +1029,27 @@ wr::WrClipId DisplayListBuilder::DefineClip(
clipId = wr_dp_define_clip_with_parent_clip(
mWrState, aParent.ptr(), aClipRect,
aComplex ? aComplex->Elements() : nullptr,
aComplex ? aComplex->Length() : 0, aMask);
aComplex ? aComplex->Length() : 0);
} else {
clipId = wr_dp_define_clip_with_parent_clip_chain(
mWrState, &mCurrentSpaceAndClipChain, aClipRect,
aComplex ? aComplex->Elements() : nullptr,
aComplex ? aComplex->Length() : 0, aMask);
aComplex ? aComplex->Length() : 0);
}
WRDL_LOG("DefineClip id=%zu p=%s r=%s m=%p b=%s complex=%zu\n", mWrState,
clipId.id, aParent ? Stringify(aParent->clip.id).c_str() : "(nil)",
Stringify(aClipRect).c_str(), aMask,
aMask ? Stringify(aMask->rect).c_str() : "none",
aComplex ? aComplex->Length() : 0);
WRDL_LOG("DefineClip id=%zu p=%s r=%s complex=%zu\n", mWrState, clipId.id,
aParent ? Stringify(aParent->clip.id).c_str() : "(nil)",
Stringify(aClipRect).c_str(), aComplex ? aComplex->Length() : 0);
return clipId;
}
wr::WrClipId DisplayListBuilder::DefineImageMaskClip(
const wr::ImageMask& aMask) {
CancelGroup();
WrClipId clipId = wr_dp_define_image_mask_clip_with_parent_clip_chain(
mWrState, &mCurrentSpaceAndClipChain, aMask);
return clipId;
}
@ -1136,7 +1143,7 @@ void DisplayListBuilder::PushRoundedRect(const wr::LayoutRect& aBounds,
AutoTArray<wr::ComplexClipRegion, 1> clips;
clips.AppendElement(wr::SimpleRadii(aBounds, aBounds.size.width / 2));
// TODO: use `mCurrentSpaceAndClipChain.clip_chain` as a parent?
auto clipId = DefineClip(Nothing(), aBounds, &clips, nullptr);
auto clipId = DefineClip(Nothing(), aBounds, &clips);
auto spaceAndClip = WrSpaceAndClip{mCurrentSpaceAndClipChain.space, clipId};
wr_dp_push_rect_with_parent_clip(mWrState, aBounds, clip, aIsBackfaceVisible,
@ -1181,7 +1188,7 @@ void DisplayListBuilder::PushClearRectWithComplexRegion(
Stringify(aBounds).c_str(), Stringify(clip).c_str());
AutoTArray<wr::ComplexClipRegion, 1> clips;
auto clipId = DefineClip(Nothing(), aBounds, &clips, nullptr);
auto clipId = DefineClip(Nothing(), aBounds, &clips);
auto spaceAndClip = WrSpaceAndClip{mCurrentSpaceAndClipChain.space, clipId};
wr_dp_push_clear_rect_with_parent_clip(mWrState, aBounds, clip,
@ -1198,7 +1205,7 @@ void DisplayListBuilder::PushBackdropFilter(
AutoTArray<wr::ComplexClipRegion, 1> clips;
clips.AppendElement(aRegion);
auto clipId = DefineClip(Nothing(), aBounds, &clips, nullptr);
auto clipId = DefineClip(Nothing(), aBounds, &clips);
auto spaceAndClip = WrSpaceAndClip{mCurrentSpaceAndClipChain.space, clipId};
wr_dp_push_backdrop_filter_with_parent_clip(

View File

@ -438,8 +438,9 @@ class DisplayListBuilder final {
wr::WrClipId DefineClip(
const Maybe<wr::WrSpaceAndClip>& aParent, const wr::LayoutRect& aClipRect,
const nsTArray<wr::ComplexClipRegion>* aComplex = nullptr,
const wr::ImageMask* aMask = nullptr);
const nsTArray<wr::ComplexClipRegion>* aComplex = nullptr);
wr::WrClipId DefineImageMaskClip(const wr::ImageMask& aMask);
wr::WrSpatialId DefineStickyFrame(const wr::LayoutRect& aContentRect,
const float* aTopMargin,

View File

@ -2433,14 +2433,12 @@ pub extern "C" fn wr_dp_define_clip_with_parent_clip(
clip_rect: LayoutRect,
complex: *const ComplexClipRegion,
complex_count: usize,
mask: *const ImageMask,
) -> WrClipId {
wr_dp_define_clip_impl(
&mut state.frame_builder,
parent.to_webrender(state.pipeline_id),
clip_rect,
unsafe { make_slice(complex, complex_count) },
unsafe { mask.as_ref() }.map(|m| *m),
)
}
@ -2451,14 +2449,12 @@ pub extern "C" fn wr_dp_define_clip_with_parent_clip_chain(
clip_rect: LayoutRect,
complex: *const ComplexClipRegion,
complex_count: usize,
mask: *const ImageMask,
) -> WrClipId {
wr_dp_define_clip_impl(
&mut state.frame_builder,
parent.to_webrender(state.pipeline_id),
clip_rect,
unsafe { make_slice(complex, complex_count) },
unsafe { mask.as_ref() }.map(|m| *m),
)
}
@ -2467,12 +2463,29 @@ fn wr_dp_define_clip_impl(
parent: SpaceAndClipInfo,
clip_rect: LayoutRect,
complex_regions: &[ComplexClipRegion],
mask: Option<ImageMask>,
) -> WrClipId {
debug_assert!(unsafe { is_in_main_thread() });
let clip_id = frame_builder
.dl_builder
.define_clip(&parent, clip_rect, complex_regions.iter().cloned(), mask);
.define_clip(&parent, clip_rect, complex_regions.iter().cloned());
WrClipId::from_webrender(clip_id)
}
#[no_mangle]
pub extern "C" fn wr_dp_define_image_mask_clip_with_parent_clip_chain(
state: &mut WrState,
parent: &WrSpaceAndClipChain,
mask: ImageMask,
) -> WrClipId {
debug_assert!(unsafe { is_in_main_thread() });
let clip_id = state
.frame_builder
.dl_builder
.define_clip_image_mask(
&parent.to_webrender(state.pipeline_id),
mask,
);
WrClipId::from_webrender(clip_id)
}

View File

@ -147,7 +147,6 @@ impl Rectangle {
&api::SpaceAndClipInfo::root_scroll(pipeline_id),
rect,
vec![region],
None,
);
builder.push_rect(

View File

@ -84,7 +84,7 @@ impl App {
radii: BorderRadius::uniform(30.0),
mode: ClipMode::Clip,
};
let clip_id = builder.define_clip(&space_and_clip, clip_bounds, vec![complex_clip], None);
let clip_id = builder.define_clip(&space_and_clip, clip_bounds, vec![complex_clip]);
// Fill it with a white rect
builder.push_rect(

View File

@ -217,11 +217,17 @@ impl Example for App {
BorderRadius::uniform(20.0),
ClipMode::Clip
);
let clip_id = builder.define_clip(
let mask_clip_id = builder.define_clip_image_mask(
&root_space_and_clip,
mask,
);
let clip_id = builder.define_clip(
&SpaceAndClipInfo {
spatial_id: root_space_and_clip.spatial_id,
clip_id: mask_clip_id,
},
content_bounds,
vec![complex],
Some(mask)
);
builder.push_rect(

View File

@ -969,7 +969,6 @@ impl<I: Iterator<Item = ComplexClipRegion>> Iterator for ComplexTranslateIter<I>
#[derive(Clone, Debug)]
pub struct ClipRegion<I> {
pub main: LayoutRect,
pub image_mask: Option<ImageMask>,
pub complex_clips: I,
}
@ -977,19 +976,13 @@ impl<J> ClipRegion<ComplexTranslateIter<J>> {
pub fn create_for_clip_node(
rect: LayoutRect,
complex_clips: J,
mut image_mask: Option<ImageMask>,
reference_frame_relative_offset: &LayoutVector2D,
) -> Self
where
J: Iterator<Item = ComplexClipRegion>
{
if let Some(ref mut image_mask) = image_mask {
image_mask.rect = image_mask.rect.translate(*reference_frame_relative_offset);
}
ClipRegion {
main: rect.translate(*reference_frame_relative_offset),
image_mask,
complex_clips: ComplexTranslateIter {
source: complex_clips,
offset: *reference_frame_relative_offset,
@ -1005,7 +998,6 @@ impl ClipRegion<Option<ComplexClipRegion>> {
) -> Self {
ClipRegion {
main: local_clip.translate(*reference_frame_relative_offset),
image_mask: None,
complex_clips: None,
}
}

View File

@ -9,7 +9,7 @@ use api::{FilterOp, FilterPrimitive, FontInstanceKey, GlyphInstance, GlyphOption
use api::{IframeDisplayItem, ImageKey, ImageRendering, ItemRange, ColorDepth, QualitySettings};
use api::{LineOrientation, LineStyle, NinePatchBorderSource, PipelineId, MixBlendMode};
use api::{PropertyBinding, ReferenceFrame, ReferenceFrameKind, ScrollFrameDisplayItem, ScrollSensitivity};
use api::{Shadow, SpaceAndClipInfo, SpatialId, StackingContext, StickyFrameDisplayItem};
use api::{Shadow, SpaceAndClipInfo, SpatialId, StackingContext, StickyFrameDisplayItem, ImageMask};
use api::{ClipMode, PrimitiveKeyKind, TransformStyle, YuvColorSpace, ColorRange, YuvData, TempFilterData};
use api::units::*;
use crate::clip::{ClipChainId, ClipRegion, ClipItemKey, ClipStore, ClipItemKeyKind};
@ -810,7 +810,6 @@ impl<'a> SceneBuilder<'a> {
let clip_region = ClipRegion::create_for_clip_node(
info.clip_rect,
item.complex_clip().iter(),
None,
&current_offset,
);
// Just use clip rectangle as the frame rect for this scroll frame.
@ -1385,13 +1384,27 @@ impl<'a> SceneBuilder<'a> {
space,
);
}
DisplayItem::ImageMaskClip(ref info) => {
let parent_space = self.get_space(&info.parent_space_and_clip.spatial_id);
let current_offset = self.current_offset(parent_space);
let image_mask = ImageMask {
rect: info.image_mask.rect.translate(current_offset),
..info.image_mask
};
self.add_image_mask_clip_node(
info.id,
&info.parent_space_and_clip,
&image_mask,
);
}
DisplayItem::Clip(ref info) => {
let parent_space = self.get_space(&info.parent_space_and_clip.spatial_id);
let current_offset = self.current_offset(parent_space);
let clip_region = ClipRegion::create_for_clip_node(
info.clip_rect,
item.complex_clip().iter(),
info.image_mask,
&current_offset,
);
self.add_clip_node(info.id, &info.parent_space_and_clip, clip_region);
@ -2305,6 +2318,56 @@ impl<'a> SceneBuilder<'a> {
);
}
fn add_image_mask_clip_node(
&mut self,
new_node_id: ClipId,
space_and_clip: &SpaceAndClipInfo,
image_mask: &ImageMask,
) -> ClipChainId {
// Map from parent ClipId to existing clip-chain.
let parent_clip_chain_index = self.id_to_index_mapper.get_clip_chain_id(space_and_clip.clip_id);
// Map the ClipId for the positioning node to a spatial node index.
let spatial_node_index = self.id_to_index_mapper.get_spatial_node_index(space_and_clip.spatial_id);
let snap_to_device = &mut self.sc_stack.last_mut().unwrap().snap_to_device;
snap_to_device.set_target_spatial_node(
spatial_node_index,
&self.spatial_tree,
);
let snapped_mask_rect = snap_to_device.snap_rect(&image_mask.rect);
let item = ClipItemKey {
kind: ClipItemKeyKind::image_mask(image_mask, snapped_mask_rect),
spatial_node_index,
};
let handle = self
.interners
.clip
.intern(&item, || {
ClipInternData {
clip_node_kind: ClipNodeKind::Complex,
spatial_node_index,
}
});
let clip_chain_index = self.clip_store
.add_clip_chain_node(
handle,
parent_clip_chain_index,
);
// Map the supplied ClipId -> clip chain id.
self.id_to_index_mapper.add_clip_chain(
new_node_id,
clip_chain_index,
1,
);
clip_chain_index
}
pub fn add_clip_node<I>(
&mut self,
new_node_id: ClipId,
@ -2360,32 +2423,6 @@ impl<'a> SceneBuilder<'a> {
);
clip_count += 1;
if let Some(ref image_mask) = clip_region.image_mask {
let snapped_mask_rect = snap_to_device.snap_rect(&image_mask.rect);
let item = ClipItemKey {
kind: ClipItemKeyKind::image_mask(image_mask, snapped_mask_rect),
spatial_node_index,
};
let handle = self
.interners
.clip
.intern(&item, || {
ClipInternData {
clip_node_kind: ClipNodeKind::Complex,
spatial_node_index,
}
});
parent_clip_chain_index = self
.clip_store
.add_clip_chain_node(
handle,
parent_clip_chain_index,
);
clip_count += 1;
}
for region in clip_region.complex_clips {
let snapped_region_rect = snap_to_device.snap_rect(&region.rect);
let item = ClipItemKey {

View File

@ -136,6 +136,7 @@ pub enum DisplayItem {
BackdropFilter(BackdropFilterDisplayItem),
// Clips
ImageMaskClip(ImageMaskClipDisplayItem),
Clip(ClipDisplayItem),
ClipChain(ClipChainItem),
@ -184,6 +185,7 @@ pub enum DebugDisplayItem {
YuvImage(YuvImageDisplayItem),
BackdropFilter(BackdropFilterDisplayItem),
ImageMaskClip(ImageMaskClipDisplayItem),
Clip(ClipDisplayItem, Vec<ComplexClipRegion>),
ClipChain(ClipChainItem, Vec<ClipId>),
@ -203,12 +205,18 @@ pub enum DebugDisplayItem {
PopAllShadows,
}
#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)]
pub struct ImageMaskClipDisplayItem {
pub id: ClipId,
pub parent_space_and_clip: SpaceAndClipInfo,
pub image_mask: ImageMask,
}
#[derive(Clone, Copy, Debug, Default, Deserialize, PartialEq, Serialize, PeekPoke)]
pub struct ClipDisplayItem {
pub id: ClipId,
pub parent_space_and_clip: SpaceAndClipInfo,
pub clip_rect: LayoutRect,
pub image_mask: Option<ImageMask>,
} // IMPLICIT: complex_clips: Vec<ComplexClipRegion>
/// The minimum and maximum allowable offset for a sticky frame in a single dimension.
@ -1465,6 +1473,7 @@ impl DisplayItem {
DisplayItem::BoxShadow(..) => "box_shadow",
DisplayItem::ClearRectangle(..) => "clear_rectangle",
DisplayItem::HitTest(..) => "hit_test",
DisplayItem::ImageMaskClip(..) => "image_mask_clip",
DisplayItem::Clip(..) => "clip",
DisplayItem::ClipChain(..) => "clip_chain",
DisplayItem::ConicGradient(..) => "conic_gradient",

View File

@ -472,6 +472,7 @@ impl BuiltDisplayList {
Real::SetGradientStops => Debug::SetGradientStops(
item.iter.cur_stops.iter().collect()
),
Real::ImageMaskClip(v) => Debug::ImageMaskClip(v),
Real::StickyFrame(v) => Debug::StickyFrame(v),
Real::Rectangle(v) => Debug::Rectangle(v),
Real::ClearRectangle(v) => Debug::ClearRectangle(v),
@ -897,7 +898,7 @@ impl<'de> Deserialize<'de> for BuiltDisplayList {
DisplayListBuilder::push_iter_impl(&mut temp, stops);
Real::SetGradientStops
},
Debug::ImageMaskClip(v) => Real::ImageMaskClip(v),
Debug::Rectangle(v) => Real::Rectangle(v),
Debug::ClearRectangle(v) => Real::ClearRectangle(v),
Debug::HitTest(v) => Real::HitTest(v),
@ -1748,12 +1749,27 @@ impl DisplayListBuilder {
id
}
pub fn define_clip_image_mask(
&mut self,
parent_space_and_clip: &di::SpaceAndClipInfo,
image_mask: di::ImageMask,
) -> di::ClipId {
let id = self.generate_clip_index();
let item = di::DisplayItem::ImageMaskClip(di::ImageMaskClipDisplayItem {
id,
parent_space_and_clip: *parent_space_and_clip,
image_mask,
});
self.push_item(&item);
id
}
pub fn define_clip<I>(
&mut self,
parent_space_and_clip: &di::SpaceAndClipInfo,
clip_rect: LayoutRect,
complex_clips: I,
image_mask: Option<di::ImageMask>,
) -> di::ClipId
where
I: IntoIterator<Item = di::ComplexClipRegion>,
@ -1764,7 +1780,6 @@ impl DisplayListBuilder {
id,
parent_space_and_clip: *parent_space_and_clip,
clip_rect,
image_mask,
});
self.push_item(&item);

View File

@ -316,7 +316,6 @@ impl<'a> RawtestHarness<'a> {
&root_space_and_clip,
rect(40., 41., 200., 201.),
vec![],
None,
);
let info = CommonItemProperties {
@ -404,7 +403,6 @@ impl<'a> RawtestHarness<'a> {
&root_space_and_clip,
rect(-1000.0, -1000.0, 2000.0, 2000.0),
vec![],
None,
);
let info = CommonItemProperties {
@ -501,7 +499,6 @@ impl<'a> RawtestHarness<'a> {
&root_space_and_clip,
rect(-1000.0, -1000.0, 2000.0, 2000.0),
vec![],
None,
);
let info = CommonItemProperties {
@ -548,7 +545,6 @@ impl<'a> RawtestHarness<'a> {
&root_space_and_clip,
rect(-1000.0, -1000.0, 2000.0, 2000.0),
vec![],
None,
);
let info = CommonItemProperties {
@ -597,7 +593,6 @@ impl<'a> RawtestHarness<'a> {
&root_space_and_clip,
rect(-1000.0, -1000.0, 2000.0, 2000.0),
vec![],
None,
);
let info = CommonItemProperties {
@ -1080,7 +1075,6 @@ impl<'a> RawtestHarness<'a> {
&SpaceAndClipInfo::root_scroll(self.wrench.root_pipeline_id),
rect(110., 120., 200., 200.),
None::<ComplexClipRegion>,
None
);
builder.push_rect(
&self.make_common_properties_with_clip_and_spatial(
@ -1097,7 +1091,6 @@ impl<'a> RawtestHarness<'a> {
&SpaceAndClipInfo { spatial_id, clip_id },
rect(80., 80., 90., 90.),
None::<ComplexClipRegion>,
None
);
let space_and_clip = SpaceAndClipInfo {
spatial_id,
@ -1143,7 +1136,6 @@ impl<'a> RawtestHarness<'a> {
&SpaceAndClipInfo { spatial_id, clip_id },
rect(80., 80., 100., 100.),
None::<ComplexClipRegion>,
None
);
builder.push_rect(
&self.make_common_properties_with_clip_and_spatial(
@ -1373,7 +1365,6 @@ impl<'a> RawtestHarness<'a> {
&space_and_clip,
rect,
vec![make_rounded_complex_clip(&rect, 20.)],
None,
);
builder.push_rect(
&CommonItemProperties {
@ -1393,7 +1384,6 @@ impl<'a> RawtestHarness<'a> {
&space_and_clip,
rect,
vec![make_rounded_complex_clip(&rect, 20.)],
None,
);
let clip_chain_id = builder.define_clip_chain(None, vec![clip_id]);
builder.push_rect(

View File

@ -1709,7 +1709,6 @@ impl YamlFrameReader {
&self.top_space_and_clip(),
clip_rect,
vec![complex_clip],
None,
);
self.clip_id_stack.push(id);
pushed_clip = true;
@ -1965,14 +1964,19 @@ impl YamlFrameReader {
let clip_rect = yaml["bounds"].as_rect().expect("clip must have a bounds");
let numeric_id = yaml["id"].as_i64();
let complex_clips = self.to_complex_clip_regions(&yaml["complex"]);
let image_mask = self.to_image_mask(&yaml["image-mask"], wrench);
let mut space_and_clip = self.top_space_and_clip();
if let Some(image_mask) = self.to_image_mask(&yaml["image-mask"], wrench) {
space_and_clip.clip_id = dl.define_clip_image_mask(
&space_and_clip,
image_mask,
);
}
let space_and_clip = self.top_space_and_clip();
let real_id = dl.define_clip(
&space_and_clip,
clip_rect,
complex_clips,
image_mask,
);
if let Some(numeric_id) = numeric_id {
self.add_clip_id_mapping(numeric_id as u64, real_id);

View File

@ -197,7 +197,7 @@ bool nsDisplayFieldSetBorder::CreateWebRenderCommands(
region.radii = wr::EmptyBorderRadius();
nsTArray<mozilla::wr::ComplexClipRegion> array{region};
auto clip = aBuilder.DefineClip(Nothing(), layoutRect, &array, nullptr);
auto clip = aBuilder.DefineClip(Nothing(), layoutRect, &array);
auto clipChain = aBuilder.DefineClipChain({clip}, true);
clipOut.emplace(aBuilder, clipChain);
}

View File

@ -10102,8 +10102,7 @@ static Maybe<wr::WrClipId> CreateSimpleClipRegion(
MOZ_ASSERT_UNREACHABLE("Unhandled shape id?");
return Nothing();
}
wr::WrClipId clipId =
aBuilder.DefineClip(Nothing(), rect, &clipRegions, nullptr);
wr::WrClipId clipId = aBuilder.DefineClip(Nothing(), rect, &clipRegions);
return Some(clipId);
}
@ -10122,8 +10121,7 @@ static Maybe<wr::WrClipId> CreateWRClipPathAndMasks(
return Nothing();
}
wr::WrClipId clipId = aBuilder.DefineClip(
Nothing(), wr::ToLayoutRect(aBounds), nullptr, mask.ptr());
wr::WrClipId clipId = aBuilder.DefineImageMaskClip(mask.ref());
return Some(clipId);
}