From c60c72fb15fe7ad81bdfb558c979306d488ec45e Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Mon, 28 Jul 2014 19:49:15 -0700 Subject: [PATCH 1/3] Push get_buffer_requests_recursively into rust-layers Also do some reorganization and abstraction in IOCompositor now that we do not have to borrow self.scene.root. --- src/components/compositing/compositor.rs | 111 ++++++++++-------- src/components/compositing/compositor_data.rs | 40 +------ src/support/layers/rust-layers | 2 +- 3 files changed, 66 insertions(+), 87 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 11173d140f8..445f9e83afc 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -27,7 +27,7 @@ use geom::rect::Rect; use geom::size::TypedSize2D; use geom::scale_factor::ScaleFactor; use gfx::render_task::{RenderChan, ReRenderMsg, ReRenderRequest, UnusedBufferMsg}; -use layers::layers::LayerBufferSet; +use layers::layers::{BufferRequest, Layer, LayerBufferSet}; use layers::rendergl; use layers::rendergl::RenderContext; use layers::scene::Scene; @@ -806,61 +806,78 @@ impl IOCompositor { chan.send(NavigateMsg(direction)) } - /// Get BufferRequests from each layer. - fn ask_for_tiles(&mut self) { + fn convert_buffer_requests_to_pipeline_requests_map(&self, + requests: Vec<(Rc>, + Vec)>) -> + HashMap)> { let scale = self.device_pixels_per_page_px(); - let mut num_rerendermsgs_sent = 0; - let window_rect_in_device_pixels = Rect(Point2D(0f32, 0f32), - self.window_size.as_f32().to_untyped()); + let mut results: + HashMap)> = HashMap::new(); - match self.scene.root { - Some(ref mut layer) => { - let mut layers_and_requests = Vec::new(); - let mut unused_buffers = Vec::new(); - CompositorData::get_buffer_requests_recursively(&mut layers_and_requests, - &mut unused_buffers, - layer.clone(), - window_rect_in_device_pixels); + for (layer, mut layer_requests) in requests.move_iter() { + let pipeline_id = layer.extra_data.borrow().pipeline.id; + let &(_, ref mut vec) = results.find_or_insert_with(pipeline_id, |_| { + (layer.extra_data.borrow().pipeline.render_chan.clone(), Vec::new()) + }); - // Return unused tiles first, so that they can be reused by any new BufferRequests. + // All the BufferRequests are in layer/device coordinates, but the render task + // wants to know the page coordinates. We scale them before sending them. + for request in layer_requests.mut_iter() { + request.page_rect = request.page_rect / scale.get(); + } + + vec.push(ReRenderRequest { + buffer_requests: layer_requests, + scale: scale.get(), + layer_id: layer.extra_data.borrow().id, + epoch: layer.extra_data.borrow().epoch, + }); + } + + return results; + } + + fn send_back_unused_buffers(&mut self) { + match self.root_pipeline { + Some(ref pipeline) => { + let unused_buffers = self.scene.collect_unused_buffers(); let have_unused_buffers = unused_buffers.len() > 0; self.recomposite = self.recomposite || have_unused_buffers; if have_unused_buffers { let message = UnusedBufferMsg(unused_buffers); - let _ = layer.extra_data.borrow().pipeline.render_chan.send_opt(message); + let _ = pipeline.render_chan.send_opt(message); } - - // We want to batch requests for each pipeline to avoid race conditions - // when handling the resulting BufferRequest responses. - let mut pipeline_requests: - HashMap)> = HashMap::new(); - for (layer, mut requests) in layers_and_requests.move_iter() { - let pipeline_id = layer.extra_data.borrow().pipeline.id; - let &(_, ref mut vec) = pipeline_requests.find_or_insert_with(pipeline_id, |_| { - (layer.extra_data.borrow().pipeline.render_chan.clone(), Vec::new()) - }); - - // All the BufferRequests are in layer/device coordinates, but the render task - // wants to know the page coordinates. We scale them before sending them. - for request in requests.mut_iter() { - request.page_rect = request.page_rect / scale.get(); - } - - vec.push(ReRenderRequest { - buffer_requests: requests, - scale: scale.get(), - layer_id: layer.extra_data.borrow().id, - epoch: layer.extra_data.borrow().epoch, - }); - } - - for (_pipeline_id, (chan, requests)) in pipeline_requests.move_iter() { - num_rerendermsgs_sent += 1; - let _ = chan.send_opt(ReRenderMsg(requests)); - } - } - None => { } + }, + None => {} } + } + + /// Get BufferRequests from each layer. + fn ask_for_tiles(&mut self) { + let mut layers_and_requests = Vec::new(); + self.scene.get_buffer_requests(&mut layers_and_requests, + Rect(Point2D(0f32, 0f32), + self.window_size.as_f32().to_untyped())); + + // Return unused tiles first, so that they can be reused by any new BufferRequests. + self.send_back_unused_buffers(); + + if layers_and_requests.len() == 0 { + return; + } + + // We want to batch requests for each pipeline to avoid race conditions + // when handling the resulting BufferRequest responses. + let pipeline_requests = + self.convert_buffer_requests_to_pipeline_requests_map(layers_and_requests); + + let mut num_rerendermsgs_sent = 0; + for (_pipeline_id, (chan, requests)) in pipeline_requests.move_iter() { + num_rerendermsgs_sent += 1; + let _ = chan.send_opt(ReRenderMsg(requests)); + } + self.add_outstanding_rerendermsg(num_rerendermsgs_sent); } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index e08694443b9..801fcecc9e9 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -8,11 +8,10 @@ use pipeline::CompositionPipeline; use azure::azure_hl::Color; use geom::point::TypedPoint2D; -use geom::rect::Rect; use geom::scale_factor::ScaleFactor; use geom::size::{Size2D, TypedSize2D}; use gfx::render_task::UnusedBufferMsg; -use layers::layers::{BufferRequest, Layer, LayerBuffer, LayerBufferSet}; +use layers::layers::{Layer, LayerBufferSet}; use layers::platform::surface::NativeSurfaceMethods; use servo_msg::compositor_msg::{Epoch, LayerId}; use servo_msg::compositor_msg::ScrollPolicy; @@ -81,43 +80,6 @@ impl CompositorData { layer.add_child(new_kid.clone()); } - // Given the current window size, determine which tiles need to be (re-)rendered and sends them - // off the the appropriate renderer. Returns true if and only if the scene should be repainted. - pub fn get_buffer_requests_recursively(requests: &mut Vec<(Rc>, - Vec)>, - unused_buffers: &mut Vec>, - layer: Rc>, - window_rect_in_device_pixels: Rect) { - let (request, unused) = layer.get_tile_rects_page(window_rect_in_device_pixels); - unused_buffers.push_all_move(unused); - - if !request.is_empty() { - requests.push((layer.clone(), request)); - } - - for kid in layer.children().iter() { - let mut new_rect = window_rect_in_device_pixels; - let content_offset = kid.content_offset.borrow(); - new_rect.origin.x = new_rect.origin.x - content_offset.x; - new_rect.origin.y = new_rect.origin.y - content_offset.y; - - match new_rect.intersection(&*kid.bounds.borrow()) { - Some(new_rect) => { - // Child layers act as if they are rendered at (0,0), so we - // subtract the layer's (x,y) coords in its containing page - // to make the child_rect appear in coordinates local to it. - let child_rect = Rect(new_rect.origin.sub(&kid.bounds.borrow().origin), - new_rect.size); - CompositorData::get_buffer_requests_recursively(requests, - unused_buffers, - kid.clone(), - child_rect); - } - None => {} - } - }; - } - pub fn update_layer(layer: Rc>, layer_properties: LayerProperties) { layer.extra_data.borrow_mut().epoch = layer_properties.epoch; layer.extra_data.borrow_mut().background_color = layer_properties.background_color; diff --git a/src/support/layers/rust-layers b/src/support/layers/rust-layers index f5f5a8ba195..b8a9223648f 160000 --- a/src/support/layers/rust-layers +++ b/src/support/layers/rust-layers @@ -1 +1 @@ -Subproject commit f5f5a8ba195921b2d786cbaf98c32087edafb0fe +Subproject commit b8a9223648fcd2b8f379eb6a29f2769e82dc7967 From d3c0d8d6f8da6f4bdc69810f1acea3417818b2c3 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 29 Jul 2014 18:10:09 -0700 Subject: [PATCH 2/3] Rename Compositor::ask_for_tiles to send_buffer_requests_for_all_layers This name better reflects what the method does. --- src/components/compositing/compositor.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 445f9e83afc..8953de073dc 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -223,7 +223,7 @@ impl IOCompositor { // If a pinch-zoom happened recently, ask for tiles at the new resolution if self.zoom_action && precise_time_s() - self.zoom_time > 0.3 { self.zoom_action = false; - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } } @@ -438,7 +438,7 @@ impl IOCompositor { self.scroll_layer_to_fragment_point_if_necessary(layer_properties.pipeline_id, layer_properties.id); - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } fn create_or_update_descendant_layer(&mut self, mut layer_properties: LayerProperties) { @@ -448,7 +448,7 @@ impl IOCompositor { } self.scroll_layer_to_fragment_point_if_necessary(layer_properties.pipeline_id, layer_properties.id); - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } fn create_descendant_layer(&self, layer_properties: LayerProperties) { @@ -534,7 +534,7 @@ impl IOCompositor { }; if should_ask_for_tiles { - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } } @@ -602,7 +602,7 @@ impl IOCompositor { if ask { self.recomposite_if(move); - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } } @@ -735,7 +735,7 @@ impl IOCompositor { None => { } } self.recomposite_if(scroll); - self.ask_for_tiles(); + self.send_buffer_requests_for_all_layers(); } fn device_pixels_per_screen_px(&self) -> ScaleFactor { @@ -853,8 +853,7 @@ impl IOCompositor { } } - /// Get BufferRequests from each layer. - fn ask_for_tiles(&mut self) { + fn send_buffer_requests_for_all_layers(&mut self) { let mut layers_and_requests = Vec::new(); self.scene.get_buffer_requests(&mut layers_and_requests, Rect(Point2D(0f32, 0f32), From 9edaff4304c19a34dcb4df798ab90e6503d64d43 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 29 Jul 2014 18:10:49 -0700 Subject: [PATCH 3/3] Remove CompositorData::add_child The contents of this method have dwindled to two statements. Just remove the method now. --- src/components/compositing/compositor.rs | 18 +++++++++++++----- src/components/compositing/compositor_data.rs | 12 ------------ 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 8953de073dc..54f0c637f2f 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use compositor_data::{CompositorData, WantsScrollEvents}; +use compositor_data::{CompositorData, DoesntWantScrollEvents, WantsScrollEvents}; use compositor_task::{Msg, CompositorTask, Exit, ChangeReadyState, SetIds, LayerProperties}; use compositor_task::{GetGraphicsMetadata, CreateOrUpdateRootLayer, CreateOrUpdateDescendantLayer}; use compositor_task::{SetLayerClipRect, Paint, ScrollFragmentPoint, LoadComplete}; @@ -421,12 +421,15 @@ impl IOCompositor { background_color: layer_properties.background_color, scroll_policy: FixedPosition, }; - let new_root = CompositorData::new_layer(root_pipeline, + let new_root = CompositorData::new_layer(root_pipeline.clone(), root_properties, WantsScrollEvents, self.opts.tile_size); - - CompositorData::add_child(new_root.clone(), layer_properties); + let first_chid = CompositorData::new_layer(root_pipeline.clone(), + layer_properties, + DoesntWantScrollEvents, + self.opts.tile_size); + new_root.add_child(first_chid); // Release all tiles from the layer before dropping it. match self.scene.root { @@ -459,7 +462,12 @@ impl IOCompositor { layer_properties.pipeline_id, parent_layer_id) { Some(ref mut parent_layer) => { - CompositorData::add_child(parent_layer.clone(), layer_properties); + let pipeline = parent_layer.extra_data.borrow().pipeline.clone(); + let new_layer = CompositorData::new_layer(pipeline, + layer_properties, + DoesntWantScrollEvents, + parent_layer.tile_size); + parent_layer.add_child(new_layer); } None => { fail!("Compositor: couldn't find parent layer"); diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 801fcecc9e9..577afb3851f 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -68,18 +68,6 @@ impl CompositorData { Rc::new(Layer::new(layer_properties.rect, tile_size, new_compositor_data)) } - /// Adds a child layer to the layer with the given ID and the given pipeline, if it doesn't - /// exist yet. The child layer will have the same pipeline, tile size, memory limit, and CPU - /// painting status as its parent. - pub fn add_child(layer: Rc>, - layer_properties: LayerProperties) { - let new_kid = CompositorData::new_layer(layer.extra_data.borrow().pipeline.clone(), - layer_properties, - DoesntWantScrollEvents, - layer.tile_size); - layer.add_child(new_kid.clone()); - } - pub fn update_layer(layer: Rc>, layer_properties: LayerProperties) { layer.extra_data.borrow_mut().epoch = layer_properties.epoch; layer.extra_data.borrow_mut().background_color = layer_properties.background_color;