Skip to content

Commit

Permalink
Simplify determining if a batch can be merged / needs scissor.
Browse files Browse the repository at this point in the history 
Previously, we checked if the bounding box of any child primitives
extended outside the rectangle of the allocated task rect. However,
there is a simpler way to calculate this.

If the allocated size of the render task is >= the unclipped size
of the picture bounding rect, no scissor is needed, since we know
that local clip rects will take care of ensuring nothing is
drawn outside the task boundary.

This is an optimization, but the main benefit is removing one more
piece of code that relies on knowledge of screen / device rects,
which simplifies the ongoing work to be able to rasterize in other
coordinate systems.
  • Loading branch information
@gw3583
gw3583 committed Aug 16, 2018
1 parent e33d0de commit 8ca21b0
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 51 deletions.
28 changes: 4 additions & 24 deletions webrender/src/batch.rs
View file
Expand Up @@ -263,7 +263,6 @@ impl OpaqueBatchList {
pub struct BatchList {
pub alpha_batch_list: AlphaBatchList,
pub opaque_batch_list: OpaqueBatchList,
pub combined_bounding_rect: DeviceIntRect,
}

impl BatchList {
Expand All @@ -275,24 +274,14 @@ impl BatchList {
BatchList {
alpha_batch_list: AlphaBatchList::new(),
opaque_batch_list: OpaqueBatchList::new(batch_area_threshold),
combined_bounding_rect: DeviceIntRect::zero(),
}
}

fn add_bounding_rect(
&mut self,
task_relative_bounding_rect: &DeviceIntRect,
) {
self.combined_bounding_rect = self.combined_bounding_rect.union(task_relative_bounding_rect);
}

pub fn get_suitable_batch(
&mut self,
key: BatchKey,
task_relative_bounding_rect: &DeviceIntRect,
) -> &mut Vec<PrimitiveInstance> {
self.add_bounding_rect(task_relative_bounding_rect);

match key.blend_mode {
BlendMode::None => {
self.opaque_batch_list
Expand Down Expand Up @@ -409,31 +398,27 @@ pub struct AlphaBatchBuilder {
pub batch_list: BatchList,
glyph_fetch_buffer: Vec<GlyphFetchResult>,
target_rect: DeviceIntRect,
can_merge: bool,
}

impl AlphaBatchBuilder {
pub fn new(
screen_size: DeviceIntSize,
target_rect: DeviceIntRect,
can_merge: bool,
) -> Self {
AlphaBatchBuilder {
batch_list: BatchList::new(screen_size),
glyph_fetch_buffer: Vec::new(),
target_rect,
can_merge,
}
}

pub fn build(mut self, merged_batches: &mut AlphaBatchContainer) -> Option<AlphaBatchContainer> {
self.batch_list.finalize();

let task_relative_target_rect = DeviceIntRect::new(
DeviceIntPoint::zero(),
self.target_rect.size,
);

let can_merge = task_relative_target_rect.contains_rect(&self.batch_list.combined_bounding_rect);

if can_merge {
if self.can_merge {
merged_batches.merge(self);
None
} else {
Expand Down Expand Up @@ -1203,8 +1188,6 @@ impl AlphaBatchBuilder {
brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION,
};

self.batch_list.add_bounding_rect(task_relative_bounding_rect);

let batch_key = BatchKey {
blend_mode,
kind: BatchKind::Brush(batch_kind),
Expand Down Expand Up @@ -1236,8 +1219,6 @@ impl AlphaBatchBuilder {
brush_flags: BrushFlags::PERSPECTIVE_INTERPOLATION,
};

self.batch_list.add_bounding_rect(task_relative_bounding_rect);

match brush.segment_desc {
Some(ref segment_desc) => {
let alpha_batch_key = BatchKey {
Expand Down Expand Up @@ -1317,7 +1298,6 @@ fn add_gradient_tiles(
base_prim_header: &PrimitiveHeader,
prim_headers: &mut PrimitiveHeaders,
) {
batch_list.add_bounding_rect(task_relative_bounding_rect);
let batch = batch_list.get_suitable_batch(
BatchKey {
blend_mode: blend_mode,
Expand Down
1 change: 1 addition & 0 deletions webrender/src/frame_builder.rs
View file
Expand Up @@ -250,6 +250,7 @@ impl FrameBuilder {

let root_render_task = RenderTask::new_picture(
RenderTaskLocation::Fixed(frame_context.screen_rect),
frame_context.screen_rect.size,
root_prim_index,
DeviceIntPoint::zero(),
pic_state.tasks,
Expand Down
2 changes: 1 addition & 1 deletion webrender/src/glyph_rasterizer/pathfinder.rs
View file
Expand Up @@ -301,7 +301,7 @@ fn request_render_task_from_pathfinder(glyph_key: &GlyphKey,
let subpixel_offset = TypedPoint2D::new(glyph_subpixel_offset as f32, 0.0);
let embolden_amount = compute_embolden_amount(size.to_f32_px());

let location = RenderTaskLocation::Dynamic(None, Some(*glyph_size));
let location = RenderTaskLocation::Dynamic(None, *glyph_size);
let glyph_render_task = RenderTask::new_glyph(location,
mesh,
&glyph_origin,
Expand Down
18 changes: 12 additions & 6 deletions webrender/src/picture.rs
View file
Expand Up @@ -372,7 +372,8 @@ impl PicturePrimitive {
// relevant transforms haven't changed from frame to frame.
let surface = if pic_state_for_children.has_non_root_coord_system {
let picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
RenderTaskLocation::Dynamic(None, device_rect.size),
prim_screen_rect.unclipped.size,
prim_index,
device_rect.origin,
pic_state_for_children.tasks,
Expand Down Expand Up @@ -428,7 +429,8 @@ impl PicturePrimitive {
let child_tasks = mem::replace(&mut pic_state_for_children.tasks, Vec::new());

let picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
RenderTaskLocation::Dynamic(None, device_rect.size),
prim_screen_rect.unclipped.size,
prim_index,
device_rect.origin,
child_tasks,
Expand Down Expand Up @@ -484,7 +486,8 @@ impl PicturePrimitive {
);

let mut picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(device_rect.size)),
RenderTaskLocation::Dynamic(None, device_rect.size),
prim_screen_rect.unclipped.size,
prim_index,
device_rect.origin,
pic_state_for_children.tasks,
Expand Down Expand Up @@ -553,7 +556,8 @@ impl PicturePrimitive {
);

let picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
prim_screen_rect.unclipped.size,
prim_index,
prim_screen_rect.clipped.origin,
pic_state_for_children.tasks,
Expand Down Expand Up @@ -588,7 +592,8 @@ impl PicturePrimitive {
);

let picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
prim_screen_rect.unclipped.size,
prim_index,
prim_screen_rect.clipped.origin,
pic_state_for_children.tasks,
Expand All @@ -608,7 +613,8 @@ impl PicturePrimitive {
);

let picture_task = RenderTask::new_picture(
RenderTaskLocation::Dynamic(None, Some(prim_screen_rect.clipped.size)),
RenderTaskLocation::Dynamic(None, prim_screen_rect.clipped.size),
prim_screen_rect.unclipped.size,
prim_index,
prim_screen_rect.clipped.origin,
pic_state_for_children.tasks,
Expand Down
32 changes: 14 additions & 18 deletions webrender/src/render_task.rs
View file
Expand Up @@ -178,7 +178,7 @@ impl ops::IndexMut<RenderTaskId> for RenderTaskTree {
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub enum RenderTaskLocation {
Fixed(DeviceIntRect),
Dynamic(Option<(DeviceIntPoint, RenderTargetIndex)>, Option<DeviceIntSize>),
Dynamic(Option<(DeviceIntPoint, RenderTargetIndex)>, DeviceIntSize),
TextureCache(SourceTexture, i32, DeviceIntRect),
}

Expand All @@ -202,6 +202,7 @@ pub struct ClipRegionTask {
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct PictureTask {
pub prim_index: PrimitiveIndex,
pub can_merge: bool,
pub content_origin: DeviceIntPoint,
pub uv_rect_handle: GpuCacheHandle,
uv_rect_kind: UvRectKind,
Expand Down Expand Up @@ -330,7 +331,7 @@ impl RenderTask {
render_task_sanity_check(&size);

RenderTask {
location: RenderTaskLocation::Dynamic(None, Some(size)),
location: RenderTaskLocation::Dynamic(None, size),
children,
kind,
clear_mode,
Expand All @@ -340,28 +341,30 @@ impl RenderTask {

pub fn new_picture(
location: RenderTaskLocation,
unclipped_size: DeviceIntSize,
prim_index: PrimitiveIndex,
content_origin: DeviceIntPoint,
children: Vec<RenderTaskId>,
uv_rect_kind: UvRectKind,
) -> Self {
let size = match location {
RenderTaskLocation::Dynamic(_, Some(size)) => Some(size),
RenderTaskLocation::Fixed(rect) => Some(rect.size),
RenderTaskLocation::TextureCache(_, _, rect) => Some(rect.size),
_ => None,
RenderTaskLocation::Dynamic(_, size) => size,
RenderTaskLocation::Fixed(rect) => rect.size,
RenderTaskLocation::TextureCache(_, _, rect) => rect.size,
};

if let Some(size) = size {
render_task_sanity_check(&size);
}
render_task_sanity_check(&size);

let can_merge = size.width >= unclipped_size.width &&
size.height >= unclipped_size.height;

RenderTask {
location,
children,
kind: RenderTaskKind::Picture(PictureTask {
prim_index,
content_origin,
can_merge,
uv_rect_handle: GpuCacheHandle::new(),
uv_rect_kind,
}),
Expand Down Expand Up @@ -770,10 +773,7 @@ impl RenderTask {
pub fn get_dynamic_size(&self) -> DeviceIntSize {
match self.location {
RenderTaskLocation::Fixed(..) => DeviceIntSize::zero(),
RenderTaskLocation::Dynamic(_, Some(size)) => size,
RenderTaskLocation::Dynamic(_, None) => {
panic!("bug: render task must have assigned size by now");
}
RenderTaskLocation::Dynamic(_, size) => size,
RenderTaskLocation::TextureCache(_, _, rect) => rect.size,
}
}
Expand All @@ -798,7 +798,6 @@ impl RenderTask {
// to mark a task as unused explicitly. This
// would allow us to restore this debug check.
RenderTaskLocation::Dynamic(Some((origin, target_index)), size) => {
let size = size.expect("bug: must be assigned a size by now");
(DeviceIntRect::new(origin, size), target_index)
}
RenderTaskLocation::Dynamic(None, _) => {
Expand Down Expand Up @@ -1096,10 +1095,7 @@ impl RenderTaskCache {
RenderTaskLocation::TextureCache(..) => {
panic!("BUG: dynamic task was expected");
}
RenderTaskLocation::Dynamic(_, None) => {
panic!("BUG: must have assigned size by now");
}
RenderTaskLocation::Dynamic(_, Some(size)) => size,
RenderTaskLocation::Dynamic(_, size) => size,
};

// Select the right texture page to allocate from.
Expand Down
7 changes: 5 additions & 2 deletions webrender/src/tiling.rs
View file
Expand Up @@ -351,7 +351,11 @@ impl RenderTarget for ColorRenderTarget {

let (target_rect, _) = task.get_target_rect();

let mut batch_builder = AlphaBatchBuilder::new(self.screen_size, target_rect);
let mut batch_builder = AlphaBatchBuilder::new(
self.screen_size,
target_rect,
pic_task.can_merge,
);

batch_builder.add_pic_to_batch(
pic,
Expand Down Expand Up @@ -845,7 +849,6 @@ impl RenderPass {
None
}
RenderTaskLocation::Dynamic(ref mut origin, size) => {
let size = size.expect("bug: size must be assigned by now");
let alloc_size = DeviceUintSize::new(size.width as u32, size.height as u32);
let (alloc_origin, target_index) = match target_kind {
RenderTargetKind::Color => color.allocate(alloc_size),
Expand Down

0 comments on commit 8ca21b0

Please sign in to comment.