Skip to content

Make the intermediate render targets select the correct size dynamically. #705

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 12, 2017
Merged

Make the intermediate render targets select the correct size dynamically. #705

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
2 changes: 1 addition & 1 deletion webrender/src/render_backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ impl RenderBackend {
}
}
ApiMsg::ExternalEvent(evt) => {
let mut notifier = self.notifier.lock();
let notifier = self.notifier.lock();
notifier.unwrap()
.as_mut()
.unwrap()
Expand Down
20 changes: 10 additions & 10 deletions webrender/src/renderer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use time::precise_time_ns;
use util::TransformedRectKind;
use webrender_traits::{ColorF, Epoch, PipelineId, RenderNotifier, RenderDispatcher};
use webrender_traits::{ExternalImageId, ImageFormat, RenderApiSender, RendererKind};
use webrender_traits::{DeviceIntRect, DeviceSize, DevicePoint, DeviceIntPoint, DeviceIntSize, DeviceUintSize};
use webrender_traits::{DeviceIntRect, DevicePoint, DeviceIntPoint, DeviceIntSize, DeviceUintSize};
use webrender_traits::channel;
use webrender_traits::VRCompositorHandler;

Expand Down Expand Up @@ -1024,14 +1024,14 @@ impl Renderer {
fn draw_target(&mut self,
render_target: Option<(TextureId, i32)>,
target: &RenderTarget,
target_size: &DeviceSize,
target_size: &DeviceUintSize,
cache_texture: Option<TextureId>,
should_clear: bool,
background_color: Option<ColorF>) {
self.device.disable_depth();
self.device.enable_depth_write();

let dimensions = [target_size.width as u32, target_size.height as u32];
let dimensions = [target_size.width, target_size.height];
let projection = {
let _gm = self.gpu_profile.add_marker(GPU_TAG_SETUP_TARGET);
self.device.bind_draw_target(render_target, Some(dimensions));
Expand All @@ -1055,9 +1055,9 @@ impl Renderer {
// for mix-blend-mode etc).
[1.0, 1.0, 1.0, 0.0],
Matrix4D::ortho(0.0,
target_size.width,
target_size.width as f32,
0.0,
target_size.height,
target_size.height as f32,
ORTHO_NEAR_PLANE,
ORTHO_FAR_PLANE)
),
Expand All @@ -1066,8 +1066,8 @@ impl Renderer {
color.to_array()
}),
Matrix4D::ortho(0.0,
target_size.width,
target_size.height,
target_size.width as f32,
target_size.height as f32,
0.0,
ORTHO_NEAR_PLANE,
ORTHO_FAR_PLANE)
Expand Down Expand Up @@ -1347,10 +1347,10 @@ impl Renderer {
for (pass_index, pass) in frame.passes.iter().enumerate() {
let (do_clear, size, target_id) = if pass.is_framebuffer {
(self.clear_framebuffer || needs_clear,
DeviceSize::new(framebuffer_size.width as f32, framebuffer_size.height as f32),
framebuffer_size,
None)
} else {
(true, frame.cache_size, Some(self.render_targets[pass_index]))
(true, &frame.cache_size, Some(self.render_targets[pass_index]))
};

for (target_index, target) in pass.targets.iter().enumerate() {
Expand All @@ -1359,7 +1359,7 @@ impl Renderer {
});
self.draw_target(render_target,
target,
&size,
size,
src_id,
do_clear,
frame.background_color);
Expand Down
63 changes: 40 additions & 23 deletions webrender/src/tiling.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use webrender_traits::{BorderDisplayItem, BorderSide, BorderStyle, YuvColorSpace
use webrender_traits::{AuxiliaryLists, ItemRange, BoxShadowClipMode, ClipRegion};
use webrender_traits::{PipelineId, ScrollLayerId, WebGLContextId, FontRenderMode};
use webrender_traits::{DeviceIntRect, DeviceIntPoint, DeviceIntSize, DeviceIntLength, device_length};
use webrender_traits::{DeviceUintSize, DeviceUintPoint, DeviceSize};
use webrender_traits::{DeviceUintSize, DeviceUintPoint};
use webrender_traits::{LayerRect, LayerPoint, LayerSize};
use webrender_traits::{LayerToScrollTransform, LayerToWorldTransform, WorldToLayerTransform};
use webrender_traits::{WorldPoint4D, ScrollLayerPixel, as_scroll_parent_rect};
Expand Down Expand Up @@ -819,7 +819,7 @@ pub struct RenderTarget {
}

impl RenderTarget {
fn new() -> RenderTarget {
fn new(size: DeviceUintSize) -> RenderTarget {
RenderTarget {
alpha_batcher: AlphaBatcher::new(),
clip_batcher: ClipBatcher::new(),
Expand All @@ -828,9 +828,7 @@ impl RenderTarget {
text_run_textures: BatchTextures::no_texture(),
vertical_blurs: Vec::new(),
horizontal_blurs: Vec::new(),
page_allocator: TexturePage::new(CacheTextureId(0),
DeviceUintSize::new(RENDERABLE_CACHE_SIZE as u32,
RENDERABLE_CACHE_SIZE as u32)),
page_allocator: TexturePage::new(CacheTextureId(0), size),
}
}

Expand Down Expand Up @@ -957,34 +955,40 @@ pub struct RenderPass {
pub is_framebuffer: bool,
tasks: Vec<RenderTask>,
pub targets: Vec<RenderTarget>,
size: DeviceUintSize,
}

impl RenderPass {
fn new(pass_index: isize, is_framebuffer: bool) -> RenderPass {
fn new(pass_index: isize,
is_framebuffer: bool,
size: DeviceUintSize) -> RenderPass {
Copy link
Member

Choose a reason for hiding this comment

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

let's store the size inside a RenderPass in order to avoid asking for it in allocate_target?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed - had to change the code structure slightly to make this work, but it's much cleaner now.

RenderPass {
pass_index: RenderPassIndex(pass_index),
is_framebuffer: is_framebuffer,
targets: vec![ RenderTarget::new() ],
targets: vec![ RenderTarget::new(size) ],
tasks: vec![],
size: size,
}
}

fn add_render_task(&mut self, task: RenderTask) {
self.tasks.push(task);
}

fn allocate_target(targets: &mut Vec<RenderTarget>, size: DeviceUintSize) -> DeviceUintPoint {
let existing_origin = targets.last_mut()
.unwrap()
.page_allocator.allocate(&size);
fn allocate_target(&mut self, alloc_size: DeviceUintSize) -> DeviceUintPoint {
let existing_origin = self.targets
.last_mut()
.unwrap()
.page_allocator
.allocate(&alloc_size);
match existing_origin {
Some(origin) => origin,
None => {
let mut new_target = RenderTarget::new();
let mut new_target = RenderTarget::new(self.size);
let origin = new_target.page_allocator
.allocate(&size)
.expect(&format!("Each render task must allocate <= size of one target! ({:?})", size));
targets.push(new_target);
.allocate(&alloc_size)
.expect(&format!("Each render task must allocate <= size of one target! ({:?})", alloc_size));
self.targets.push(new_target);
origin
}
}
Expand All @@ -995,7 +999,8 @@ impl RenderPass {
ctx: &RenderTargetContext,
render_tasks: &mut RenderTaskCollection) {
// Step through each task, adding to batches as appropriate.
for mut task in self.tasks.drain(..) {
let tasks = mem::replace(&mut self.tasks, Vec::new());
for mut task in tasks {
// Find a target to assign this task to, or create a new
// one if required.
match task.location {
Expand All @@ -1018,7 +1023,7 @@ impl RenderPass {
}

let alloc_size = DeviceUintSize::new(size.width as u32, size.height as u32);
let alloc_origin = Self::allocate_target(&mut self.targets, alloc_size);
let alloc_origin = self.allocate_target(alloc_size);

*origin = Some((DeviceIntPoint::new(alloc_origin.x as i32,
alloc_origin.y as i32),
Expand Down Expand Up @@ -1383,7 +1388,6 @@ impl RenderTask {
}

pub const SCREEN_TILE_SIZE: i32 = 256;
pub const RENDERABLE_CACHE_SIZE: i32 = 2048;

#[derive(Debug, Clone)]
pub struct DebugRect {
Expand Down Expand Up @@ -1802,7 +1806,7 @@ pub struct Frame {
pub background_color: Option<ColorF>,
pub device_pixel_ratio: f32,
pub debug_rects: Vec<DebugRect>,
pub cache_size: DeviceSize,
pub cache_size: DeviceUintSize,
pub passes: Vec<RenderPass>,
pub profile_counters: FrameProfileCounters,

Expand Down Expand Up @@ -2859,7 +2863,6 @@ impl FrameBuilder {
layer_map: &LayerMap,
auxiliary_lists_map: &AuxiliaryListsMap,
device_pixel_ratio: f32) -> Frame {

let mut profile_counters = FrameProfileCounters::new();
profile_counters.total_primitives.set(self.prim_store.prim_count());

Expand All @@ -2872,6 +2875,20 @@ impl FrameBuilder {
device_length(self.screen_rect.size.height as f32,
device_pixel_ratio)));

// Pick a size for the cache render targets to be. The main requirement is that it
// has to be at least as large as the framebuffer size. This ensures that it will
// always be able to allocate the worst case render task (such as a clip mask that
// covers the entire screen).
// However, there are some extremely subtle rounding errors that occur in the
// reftests under OSMesa if the cache targets are exactly the size of the
// framebuffer. To work around this, we'll align the cache size to a multiple
// of the tile size. This can be removed once the tiling code is gone.
// TODO(gw): Remove this hack once the tiling code is sorted out!!
let max_dimension = cmp::max(screen_rect.size.width, screen_rect.size.height);
let aligned_max_dimension = (max_dimension + SCREEN_TILE_SIZE - 1) & !(SCREEN_TILE_SIZE-1);
let cache_size = DeviceUintSize::new(aligned_max_dimension as u32,
Copy link
Member

Choose a reason for hiding this comment

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

if this code wasn't temporary, I'd ask for each of width/height to be rounded up separately

Copy link
Member Author

Choose a reason for hiding this comment

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

It was originally separate - it actually needs to be square to avoid the rounding glitch in OSMesa :|

aligned_max_dimension as u32);

let mut debug_rects = Vec::new();

let (x_tile_count, y_tile_count, mut screen_tiles) = self.create_screen_tiles(device_pixel_ratio);
Expand Down Expand Up @@ -2955,7 +2972,8 @@ impl FrameBuilder {
// pass and target as required.
for index in 0..max_passes_needed {
passes.push(RenderPass::new(index as isize,
index == max_passes_needed-1));
index == max_passes_needed-1,
cache_size));
}

for compiled_screen_tile in compiled_screen_tiles {
Expand All @@ -2979,8 +2997,7 @@ impl FrameBuilder {
debug_rects: debug_rects,
profile_counters: profile_counters,
passes: passes,
cache_size: DeviceSize::new(RENDERABLE_CACHE_SIZE as f32,
RENDERABLE_CACHE_SIZE as f32),
cache_size: cache_size,
layer_texture_data: self.packed_layers.clone(),
render_task_data: render_tasks.render_task_data,
gpu_data16: self.prim_store.gpu_data16.build(),
Expand Down
2 changes: 1 addition & 1 deletion webrender_traits/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#![cfg_attr(feature = "nightly", feature(nonzero))]
#![cfg_attr(feature = "serde_derive", feature(proc_macro, rustc_attrs, structural_match))]
#![cfg_attr(feature = "serde_derive", feature(rustc_attrs, structural_match))]
Copy link
Member

Choose a reason for hiding this comment

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

is this no longer needed for serde? I assume there is a compile warning at the moment then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct


extern crate app_units;
extern crate byteorder;
Expand Down