Skip to content

Avoid picture primitive copies via VecHelper #3362

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Nov 28, 2018
Merged

Avoid picture primitive copies via VecHelper #3362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
216 changes: 108 additions & 108 deletions webrender/src/display_list_flattener.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use spatial_node::{StickyFrameInfo};
use std::{f32, mem};
use std::collections::vec_deque::VecDeque;
use tiling::{CompositeOps};
use util::{MaxRect};
use util::{MaxRect, VecHelper};

#[derive(Debug, Copy, Clone)]
struct ClipNode {
Expand Down Expand Up @@ -1114,23 +1114,24 @@ impl<'a> DisplayListFlattener<'a> {
};

// Add picture for this actual stacking context contents to render into.
let prim_list = PrimitiveList::new(
stacking_context.primitives,
&self.resources.prim_interner,
);
let leaf_picture = PicturePrimitive::new_image(
leaf_composite_mode,
leaf_context_3d,
stacking_context.pipeline_id,
leaf_output_pipeline_id,
true,
stacking_context.requested_raster_space,
prim_list,
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
let leaf_pic_index = PictureIndex(self.prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
leaf_composite_mode,
leaf_context_3d,
stacking_context.pipeline_id,
leaf_output_pipeline_id,
true,
stacking_context.requested_raster_space,
PrimitiveList::new(
stacking_context.primitives,
&self.resources.prim_interner,
),
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
))
);
let leaf_pic_index = self.prim_store.create_picture(leaf_picture);

// Create a chain of pictures based on presence of filters,
// mix-blend-mode and/or 3d rendering context containers.
Expand All @@ -1153,56 +1154,56 @@ impl<'a> DisplayListFlattener<'a> {
if let Picture3DContext::In { root_data: Some(mut prims), ancestor_index } = stacking_context.context_3d {
prims.push(cur_instance.clone());

let prim_list = PrimitiveList::new(
prims,
&self.resources.prim_interner,
);

// This is the acttual picture representing our 3D hierarchy root.
let container_picture = PicturePrimitive::new_image(
None,
Picture3DContext::In {
root_data: Some(Vec::new()),
ancestor_index,
},
stacking_context.pipeline_id,
stacking_context.frame_output_pipeline_id,
true,
stacking_context.requested_raster_space,
prim_list,
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
current_pic_index = PictureIndex(self.prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
None,
Picture3DContext::In {
root_data: Some(Vec::new()),
ancestor_index,
},
stacking_context.pipeline_id,
stacking_context.frame_output_pipeline_id,
true,
stacking_context.requested_raster_space,
PrimitiveList::new(
prims,
&self.resources.prim_interner,
),
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
))
);

current_pic_index = self.prim_store.create_picture(container_picture);

cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: current_pic_index };
}

// For each filter, create a new image with that composite mode.
for filter in &stacking_context.composite_ops.filters {
let filter = filter.sanitize();
let prim_list = PrimitiveList::new(
vec![cur_instance.clone()],
&self.resources.prim_interner,
);

let filter_picture = PicturePrimitive::new_image(
Some(PictureCompositeMode::Filter(filter)),
Picture3DContext::Out,
stacking_context.pipeline_id,
None,
true,
stacking_context.requested_raster_space,
prim_list,
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
let filter_pic_index = PictureIndex(self.prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
Some(PictureCompositeMode::Filter(filter)),
Picture3DContext::Out,
stacking_context.pipeline_id,
None,
true,
stacking_context.requested_raster_space,
PrimitiveList::new(
vec![cur_instance.clone()],
&self.resources.prim_interner,
),
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
))
);
let filter_pic_index = self.prim_store.create_picture(filter_picture);
current_pic_index = filter_pic_index;

current_pic_index = filter_pic_index;
cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: current_pic_index };

if cur_instance.is_chased() {
Expand All @@ -1216,26 +1217,26 @@ impl<'a> DisplayListFlattener<'a> {

// Same for mix-blend-mode.
if let Some(mix_blend_mode) = stacking_context.composite_ops.mix_blend_mode {
let prim_list = PrimitiveList::new(
vec![cur_instance.clone()],
&self.resources.prim_interner,
let blend_pic_index = PictureIndex(self.prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
Some(PictureCompositeMode::MixBlend(mix_blend_mode)),
Picture3DContext::Out,
stacking_context.pipeline_id,
None,
true,
stacking_context.requested_raster_space,
PrimitiveList::new(
vec![cur_instance.clone()],
&self.resources.prim_interner,
),
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
))
);

let blend_picture = PicturePrimitive::new_image(
Some(PictureCompositeMode::MixBlend(mix_blend_mode)),
Picture3DContext::Out,
stacking_context.pipeline_id,
None,
true,
stacking_context.requested_raster_space,
prim_list,
stacking_context.spatial_node_index,
max_clip,
&self.clip_store,
);
let blend_pic_index = self.prim_store.create_picture(blend_picture);
current_pic_index = blend_pic_index;

cur_instance.kind = PrimitiveInstanceKind::Picture { pic_index: blend_pic_index };

if cur_instance.is_chased() {
Expand Down Expand Up @@ -1553,31 +1554,31 @@ impl<'a> DisplayListFlattener<'a> {
// No point in adding a shadow here if there were no primitives
// added to the shadow.
if !prims.is_empty() {
let prim_list = PrimitiveList::new(
prims,
&self.resources.prim_interner,
);

// Create a picture that the shadow primitives will be added to. If the
// blur radius is 0, the code in Picture::prepare_for_render will
// detect this and mark the picture to be drawn directly into the
// parent picture, which avoids an intermediate surface and blur.
let blur_filter = FilterOp::Blur(std_deviation).sanitize();
let mut shadow_pic = PicturePrimitive::new_image(
Some(PictureCompositeMode::Filter(blur_filter)),
Picture3DContext::Out,
pipeline_id,
None,
is_passthrough,
raster_space,
prim_list,
pending_shadow.clip_and_scroll.spatial_node_index,
max_clip,
&self.clip_store,
);

// Create the primitive to draw the shadow picture into the scene.
let shadow_pic_index = self.prim_store.create_picture(shadow_pic);
let shadow_pic_index = PictureIndex(self.prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
Some(PictureCompositeMode::Filter(blur_filter)),
Picture3DContext::Out,
pipeline_id,
None,
is_passthrough,
raster_space,
PrimitiveList::new(
prims,
&self.resources.prim_interner,
),
pending_shadow.clip_and_scroll.spatial_node_index,
max_clip,
&self.clip_store,
))
);

let shadow_prim_key = PrimitiveKey::new(
true,
Expand Down Expand Up @@ -2203,26 +2204,25 @@ impl FlattenedStackingContext {
Picture3DContext::Out => panic!("Unexpected out of 3D context"),
};

let prim_list = PrimitiveList::new(
mem::replace(&mut self.primitives, Vec::new()),
prim_interner,
);

let container_picture = PicturePrimitive::new_image(
Some(PictureCompositeMode::Blit),
flat_items_context_3d,
self.pipeline_id,
None,
true,
self.requested_raster_space,
prim_list,
self.spatial_node_index,
LayoutRect::max_rect(),
clip_store,
let pic_index = PictureIndex(prim_store.pictures
.alloc()
.init(PicturePrimitive::new_image(
Some(PictureCompositeMode::Blit),
flat_items_context_3d,
self.pipeline_id,
None,
true,
self.requested_raster_space,
PrimitiveList::new(
mem::replace(&mut self.primitives, Vec::new()),
prim_interner,
),
self.spatial_node_index,
LayoutRect::max_rect(),
clip_store,
))
);

let pic_index = prim_store.create_picture(container_picture);

Some(PrimitiveInstance::new(
PrimitiveInstanceKind::Picture { pic_index },
self.primitive_data_handle,
Expand Down
2 changes: 1 addition & 1 deletion webrender/src/frame_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ impl FrameBuilder {
let mut profile_counters = FrameProfileCounters::new();
profile_counters
.total_primitives
.set(self.prim_store.prim_count);
.set(self.prim_store.prim_count());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if this only ran if profiling was on.


resource_cache.begin_frame(stamp);
gpu_cache.begin_frame(stamp.frame_id());
Expand Down
19 changes: 6 additions & 13 deletions webrender/src/prim_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2587,10 +2587,6 @@ pub struct PrimitiveStore {

/// List of animated opacity bindings for a primitive.
pub opacity_bindings: OpacityBindingStorage,

/// Total count of primitive instances contained in pictures.
/// This is used for profile counters only.
pub prim_count: usize,
}

impl PrimitiveStore {
Expand All @@ -2600,7 +2596,6 @@ impl PrimitiveStore {
text_runs: TextRunStorage::new(stats.text_run_count),
images: ImageInstanceStorage::new(stats.image_count),
opacity_bindings: OpacityBindingStorage::new(stats.opacity_binding_count),
prim_count: 0,
}
}

Expand All @@ -2626,14 +2621,12 @@ impl PrimitiveStore {
}
}

pub fn create_picture(
&mut self,
prim: PicturePrimitive,
) -> PictureIndex {
let index = PictureIndex(self.pictures.len());
self.prim_count += prim.prim_list.prim_instances.len();
self.pictures.push(prim);
index
/// Returns the total count of primitive instances contained in pictures.
pub fn prim_count(&self) -> usize {
self.pictures
.iter()
.map(|p| p.prim_list.prim_instances.len())
.sum()
}

/// Update a picture, determining surface configuration,
Expand Down
43 changes: 42 additions & 1 deletion webrender/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,53 @@ use euclid::{Point2D, Rect, Size2D, TypedPoint2D, TypedRect, TypedSize2D, Vector
use euclid::{TypedTransform2D, TypedTransform3D, TypedVector2D, TypedScale};
use num_traits::Zero;
use plane_split::{Clipper, Polygon};
use std::{i32, f32, fmt};
use std::{i32, f32, fmt, ptr};
use std::borrow::Cow;


// Matches the definition of SK_ScalarNearlyZero in Skia.
const NEARLY_ZERO: f32 = 1.0 / 4096.0;

/// A typesafe helper that separates new value construction from
/// vector growing, allowing LLVM to ideally construct the element in place.
#[must_use]
pub struct Allocation<'a, T: 'a> {
vec: &'a mut Vec<T>,
index: usize,
}

impl<'a, T> Allocation<'a, T> {
// writing is safe because alloc() ensured enough capacity
// and `Allocation` holds a mutable borrow to prevent anyone else
// from breaking this invariant.
#[inline(always)]
pub fn init(self, value: T) -> usize {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add:
// writing is safe because alloc() ensured enough capacity and Allocation holds a mutable borrow to prevent anyone else from breaking this invariant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this actually avoids the memcpy / memmove? At least for self arguments a bit back we couldn't work around it like this, see rust-lang/rust#42763.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed in playground. In fn foo() changing the push(xxx) to alloc().init(xxx) removes the memcpy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed rust-lang/rust#56333 for a problem contributing to the first example not working well in the playground.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like SmallVec also causes this to not work. I filed rust-lang/rust#56356 about that.

unsafe {
ptr::write(self.vec.as_mut_ptr().add(self.index), value);
self.vec.set_len(self.index + 1);
}
self.index
}
}

pub trait VecHelper<T> {
fn alloc(&mut self) -> Allocation<T>;
}

impl<T> VecHelper<T> for Vec<T> {
fn alloc(&mut self) -> Allocation<T> {
let index = self.len();
if self.capacity() == index {
self.reserve(1);
}
Allocation {
vec: self,
index,
}
}
}


// Represents an optimized transform where there is only
// a scale and translation (which are guaranteed to maintain
// an axis align rectangle under transformation). The
Expand Down
2 changes: 1 addition & 1 deletion webrender_api/src/display_list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ impl DisplayListBuilder {
let mut iter = BuiltDisplayListIter::new(&temp);
while let Some(item) = iter.next_raw() {
if index >= range.start.unwrap_or(0) && range.end.map_or(true, |e| index < e) {
writeln!(sink, "{}{:?}", " ".repeat(indent), item.display_item());
writeln!(sink, "{}{:?}", " ".repeat(indent), item.display_item()).unwrap();
}
index += 1;
}
Expand Down