From adfd6492ad1441b4c03dcafd07611cdabb6e5941 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:47 -0700 Subject: [PATCH 01/11] Make send_buffer_requests_recursively batch messages Also, rename it to get_buffer_requests_recursively to reflect that the caller has to send all of the batched messages itself. --- src/components/compositing/compositor.rs | 13 ++++++--- src/components/compositing/compositor_data.rs | 29 ++++++++++--------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 5762277b4d1..fc689e12a5e 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -743,11 +743,16 @@ impl IOCompositor { match self.scene.root { Some(ref mut layer) => { let rect = Rect(Point2D(0f32, 0f32), page_window.to_untyped()); + let mut requests = Vec::new(); let recomposite = - CompositorData::send_buffer_requests_recursively(layer.clone(), - &self.graphics_context, - rect, - scale.get()); + CompositorData::get_buffer_requests_recursively(&mut requests, + layer.clone(), + &self.graphics_context, + rect, + scale.get()); + for (chan, msg) in requests.move_iter() { + let _ = chan.send_opt(msg); + } self.recomposite = self.recomposite || recomposite; } None => { } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 5eca6289b4c..f5e8c1540d2 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -11,7 +11,8 @@ use geom::matrix::identity; use geom::point::TypedPoint2D; use geom::rect::Rect; use geom::size::{Size2D, TypedSize2D}; -use gfx::render_task::{ReRenderMsg, UnusedBufferMsg}; +use gfx::render_task; +use gfx::render_task::{ReRenderMsg, RenderChan, UnusedBufferMsg}; use layers::layers::{Layer, Flip, LayerBuffer, LayerBufferSet, NoFlip, TextureLayer}; use layers::quadtree::Tile; use layers::platform::surface::{NativeCompositingGraphicsContext, NativeSurfaceMethods}; @@ -120,11 +121,12 @@ impl CompositorData { // 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 send_buffer_requests_recursively(layer: Rc>, - graphics_context: &NativeCompositingGraphicsContext, - window_rect: Rect, - scale: f32) - -> bool { + pub fn get_buffer_requests_recursively(requests: &mut Vec<(RenderChan, render_task::Msg)>, + layer: Rc>, + graphics_context: &NativeCompositingGraphicsContext, + window_rect: Rect, + scale: f32) + -> bool { let (request, unused) = Layer::get_tile_rects_page(layer.clone(), window_rect, scale); let redisplay = !unused.is_empty(); if redisplay { @@ -141,14 +143,14 @@ impl CompositorData { scale, layer.extra_data.borrow().id, layer.extra_data.borrow().epoch); - let _ = layer.extra_data.borrow().pipeline.render_chan.send_opt(msg); + requests.push((layer.extra_data.borrow().pipeline.render_chan.clone(), msg)); } if redisplay { CompositorData::build_layer_tree(layer.clone(), graphics_context); } - let send_child_buffer_request = |kid: &Rc>| -> bool { + let get_child_buffer_request = |kid: &Rc>| -> bool { let mut new_rect = window_rect; let offset = kid.extra_data.borrow().scroll_offset.to_untyped(); new_rect.origin.x = new_rect.origin.x - offset.x; @@ -160,10 +162,11 @@ impl CompositorData { // 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::send_buffer_requests_recursively(kid.clone(), - graphics_context, - child_rect, - scale) + CompositorData::get_buffer_requests_recursively(requests, + kid.clone(), + graphics_context, + child_rect, + scale) } None => { false // Layer is offscreen @@ -171,7 +174,7 @@ impl CompositorData { } }; - layer.children().iter().map(send_child_buffer_request).any(|b| b) || redisplay + layer.children().iter().map(get_child_buffer_request).any(|b| b) || redisplay } // Move the sublayer to an absolute position in page coordinates relative to its parent, From 5611e34379e0d1a5baf49935156b69cadb4cc4f1 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:47 -0700 Subject: [PATCH 02/11] Separate the contents of ReRenderMsg into its own type Make a ReRenderRequest type to represent the contents of a ReRenderMsg. Currently, ReRenderMsg just takes a single one, but it in the future it will take multiple requests. --- src/components/compositing/compositor_data.rs | 12 +++++++----- src/components/gfx/render_task.rs | 13 ++++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index f5e8c1540d2..95b89b7a579 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -12,7 +12,7 @@ use geom::point::TypedPoint2D; use geom::rect::Rect; use geom::size::{Size2D, TypedSize2D}; use gfx::render_task; -use gfx::render_task::{ReRenderMsg, RenderChan, UnusedBufferMsg}; +use gfx::render_task::{ReRenderMsg, ReRenderRequest, RenderChan, UnusedBufferMsg}; use layers::layers::{Layer, Flip, LayerBuffer, LayerBufferSet, NoFlip, TextureLayer}; use layers::quadtree::Tile; use layers::platform::surface::{NativeCompositingGraphicsContext, NativeSurfaceMethods}; @@ -139,10 +139,12 @@ impl CompositorData { // // FIXME(#2003, pcwalton): We may want to batch these up in the case in which // one page has multiple layers, to avoid the user seeing inconsistent states. - let msg = ReRenderMsg(request, - scale, - layer.extra_data.borrow().id, - layer.extra_data.borrow().epoch); + let msg = ReRenderMsg(ReRenderRequest { + buffer_requests: request, + scale: scale, + layer_id: layer.extra_data.borrow().id, + epoch: layer.extra_data.borrow().epoch, + }); requests.push((layer.extra_data.borrow().pipeline.render_chan.clone(), msg)); } diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index d6c31bce048..a87a6738607 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -50,9 +50,16 @@ pub struct RenderLayer { pub scroll_policy: ScrollPolicy, } +pub struct ReRenderRequest { + pub buffer_requests: Vec, + pub scale: f32, + pub layer_id: LayerId, + pub epoch: Epoch, +} + pub enum Msg { RenderMsg(SmallVec1), - ReRenderMsg(Vec, f32, LayerId, Epoch), + ReRenderMsg(ReRenderRequest), UnusedBufferMsg(Vec>), PaintPermissionGranted, PaintPermissionRevoked, @@ -230,9 +237,9 @@ impl RenderTask { self.epoch, self.render_layers.as_slice()); } - ReRenderMsg(tiles, scale, layer_id, epoch) => { + ReRenderMsg(ReRenderRequest { buffer_requests, scale, layer_id, epoch }) => { if self.epoch == epoch { - self.render(tiles, scale, layer_id); + self.render(buffer_requests, scale, layer_id); } else { debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch); } From 25e8077681cd7991832eeb749ef9a1d5d87aaaaf Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:47 -0700 Subject: [PATCH 03/11] Batch ReRenderRequests rather than render_task::Msg Make get_buffer_requests_recursively create a ReRenderRequest rather than a ReRenderMsg, and have its caller create the ReRenderMsg from the ReRenderRequest. --- src/components/compositing/compositor.rs | 5 +++-- src/components/compositing/compositor_data.rs | 9 ++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index fc689e12a5e..4c23043175c 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -26,6 +26,7 @@ use geom::point::{Point2D, TypedPoint2D}; use geom::rect::Rect; use geom::size::TypedSize2D; use geom::scale_factor::ScaleFactor; +use gfx::render_task::ReRenderMsg; use layers::layers::LayerBufferSet; use layers::platform::surface::NativeCompositingGraphicsContext; use layers::rendergl; @@ -750,8 +751,8 @@ impl IOCompositor { &self.graphics_context, rect, scale.get()); - for (chan, msg) in requests.move_iter() { - let _ = chan.send_opt(msg); + for (chan, request) in requests.move_iter() { + let _ = chan.send_opt(ReRenderMsg(request)); } self.recomposite = self.recomposite || recomposite; } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 95b89b7a579..1e8f195a775 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -11,8 +11,7 @@ use geom::matrix::identity; use geom::point::TypedPoint2D; use geom::rect::Rect; use geom::size::{Size2D, TypedSize2D}; -use gfx::render_task; -use gfx::render_task::{ReRenderMsg, ReRenderRequest, RenderChan, UnusedBufferMsg}; +use gfx::render_task::{ReRenderRequest, RenderChan, UnusedBufferMsg}; use layers::layers::{Layer, Flip, LayerBuffer, LayerBufferSet, NoFlip, TextureLayer}; use layers::quadtree::Tile; use layers::platform::surface::{NativeCompositingGraphicsContext, NativeSurfaceMethods}; @@ -121,7 +120,7 @@ impl CompositorData { // 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<(RenderChan, render_task::Msg)>, + pub fn get_buffer_requests_recursively(requests: &mut Vec<(RenderChan, ReRenderRequest)>, layer: Rc>, graphics_context: &NativeCompositingGraphicsContext, window_rect: Rect, @@ -139,12 +138,12 @@ impl CompositorData { // // FIXME(#2003, pcwalton): We may want to batch these up in the case in which // one page has multiple layers, to avoid the user seeing inconsistent states. - let msg = ReRenderMsg(ReRenderRequest { + let msg = ReRenderRequest { buffer_requests: request, scale: scale, layer_id: layer.extra_data.borrow().id, epoch: layer.extra_data.borrow().epoch, - }); + }; requests.push((layer.extra_data.borrow().pipeline.render_chan.clone(), msg)); } From a378f3e6800d1bd3e1e2418222038453e65f9d91 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 04/11] Group ReRenderRequests by PipelineId --- src/components/compositing/compositor.rs | 11 +++++++---- src/components/compositing/compositor_data.rs | 10 ++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 4c23043175c..722d6c3d629 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -46,6 +46,7 @@ use servo_util::opts::Opts; use servo_util::time::{profile, TimeProfilerChan}; use servo_util::{memory, time, url}; use std::io::timer::sleep; +use std::collections::hashmap::HashMap; use std::path::Path; use std::rc::Rc; use time::precise_time_s; @@ -744,15 +745,17 @@ impl IOCompositor { match self.scene.root { Some(ref mut layer) => { let rect = Rect(Point2D(0f32, 0f32), page_window.to_untyped()); - let mut requests = Vec::new(); + let mut request_map = HashMap::new(); let recomposite = - CompositorData::get_buffer_requests_recursively(&mut requests, + CompositorData::get_buffer_requests_recursively(&mut request_map, layer.clone(), &self.graphics_context, rect, scale.get()); - for (chan, request) in requests.move_iter() { - let _ = chan.send_opt(ReRenderMsg(request)); + for (_pipeline_id, requests) in request_map.move_iter() { + for (chan, request) in requests.move_iter() { + let _ = chan.send_opt(ReRenderMsg(request)); + } } self.recomposite = self.recomposite || recomposite; } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 1e8f195a775..609845782e5 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -20,6 +20,7 @@ use servo_msg::compositor_msg::{Epoch, FixedPosition, LayerId}; use servo_msg::compositor_msg::ScrollPolicy; use servo_msg::constellation_msg::PipelineId; use servo_util::geometry::PagePx; +use std::collections::hashmap::HashMap; use std::rc::Rc; #[cfg(target_os="macos")] @@ -120,7 +121,9 @@ impl CompositorData { // 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<(RenderChan, ReRenderRequest)>, + pub fn get_buffer_requests_recursively(requests: &mut HashMap>, layer: Rc>, graphics_context: &NativeCompositingGraphicsContext, window_rect: Rect, @@ -138,13 +141,16 @@ impl CompositorData { // // FIXME(#2003, pcwalton): We may want to batch these up in the case in which // one page has multiple layers, to avoid the user seeing inconsistent states. + let pipeline_id = layer.extra_data.borrow().pipeline.id; + let chan = layer.extra_data.borrow().pipeline.render_chan.clone(); let msg = ReRenderRequest { buffer_requests: request, scale: scale, layer_id: layer.extra_data.borrow().id, epoch: layer.extra_data.borrow().epoch, }; - requests.push((layer.extra_data.borrow().pipeline.render_chan.clone(), msg)); + let vec = requests.find_or_insert(pipeline_id, Vec::new()); + vec.push((chan, msg)); } if redisplay { From 0f21d6be1259a3cbadd42fad0b1dba6d4a7084c6 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 05/11] Only clone the RenderChan once per Pipeline --- src/components/compositing/compositor.rs | 4 ++-- src/components/compositing/compositor_data.rs | 11 ++++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 722d6c3d629..2bd4900735a 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -752,8 +752,8 @@ impl IOCompositor { &self.graphics_context, rect, scale.get()); - for (_pipeline_id, requests) in request_map.move_iter() { - for (chan, request) in requests.move_iter() { + for (_pipeline_id, (chan, requests)) in request_map.move_iter() { + for request in requests.move_iter() { let _ = chan.send_opt(ReRenderMsg(request)); } } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 609845782e5..975047ffd2c 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -122,8 +122,8 @@ impl CompositorData { // 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 HashMap>, + (RenderChan, + Vec)>, layer: Rc>, graphics_context: &NativeCompositingGraphicsContext, window_rect: Rect, @@ -142,15 +142,16 @@ impl CompositorData { // FIXME(#2003, pcwalton): We may want to batch these up in the case in which // one page has multiple layers, to avoid the user seeing inconsistent states. let pipeline_id = layer.extra_data.borrow().pipeline.id; - let chan = layer.extra_data.borrow().pipeline.render_chan.clone(); let msg = ReRenderRequest { buffer_requests: request, scale: scale, layer_id: layer.extra_data.borrow().id, epoch: layer.extra_data.borrow().epoch, }; - let vec = requests.find_or_insert(pipeline_id, Vec::new()); - vec.push((chan, msg)); + let &(_, ref mut vec) = requests.find_or_insert_with(pipeline_id, |_| { + (layer.extra_data.borrow().pipeline.render_chan.clone(), Vec::new()) + }); + vec.push(msg); } if redisplay { From 8d56fa37be3a81d5cff818a99ed0407ef2e65446 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 06/11] Make ReRenderMsg take multiple ReRenderRequests --- src/components/compositing/compositor.rs | 4 +--- src/components/gfx/render_task.rs | 15 +++++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 2bd4900735a..38cfdf5e46f 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -753,9 +753,7 @@ impl IOCompositor { rect, scale.get()); for (_pipeline_id, (chan, requests)) in request_map.move_iter() { - for request in requests.move_iter() { - let _ = chan.send_opt(ReRenderMsg(request)); - } + let _ = chan.send_opt(ReRenderMsg(requests)); } self.recomposite = self.recomposite || recomposite; } diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index a87a6738607..c2dffd8273b 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -59,7 +59,7 @@ pub struct ReRenderRequest { pub enum Msg { RenderMsg(SmallVec1), - ReRenderMsg(ReRenderRequest), + ReRenderMsg(Vec), UnusedBufferMsg(Vec>), PaintPermissionGranted, PaintPermissionRevoked, @@ -237,11 +237,14 @@ impl RenderTask { self.epoch, self.render_layers.as_slice()); } - ReRenderMsg(ReRenderRequest { buffer_requests, scale, layer_id, epoch }) => { - if self.epoch == epoch { - self.render(buffer_requests, scale, layer_id); - } else { - debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch); + ReRenderMsg(requests) => { + for ReRenderRequest { buffer_requests, scale, layer_id, epoch } + in requests.move_iter() { + if self.epoch == epoch { + self.render(buffer_requests, scale, layer_id); + } else { + debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch); + } } } UnusedBufferMsg(unused_buffers) => { From ec8f71d4b55b756047efd298658e26532d716c5e Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 07/11] Check whether the render task has permission to paint before painting This matches the behavior of RenderMsg. It doesn't make sense to go to the trouble of rendering if the render task doesn't have permission to paint in the first place. --- src/components/gfx/render_task.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index c2dffd8273b..4c720653e81 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -238,6 +238,13 @@ impl RenderTask { self.render_layers.as_slice()); } ReRenderMsg(requests) => { + if !self.paint_permission { + debug!("render_task: render ready msg"); + let ConstellationChan(ref mut c) = self.constellation_chan; + c.send(RendererReadyMsg(self.id)); + continue; + } + for ReRenderRequest { buffer_requests, scale, layer_id, epoch } in requests.move_iter() { if self.epoch == epoch { @@ -428,13 +435,7 @@ impl RenderTask { }; debug!("render_task: returning surface"); - if self.paint_permission { - self.compositor.paint(self.id, render_layer.id, layer_buffer_set, self.epoch); - } else { - debug!("render_task: RendererReadyMsg send"); - let ConstellationChan(ref mut c) = self.constellation_chan; - c.send(RendererReadyMsg(self.id)); - } + self.compositor.paint(self.id, render_layer.id, layer_buffer_set, self.epoch); self.compositor.set_render_state(IdleRenderState); }) } From a832acc4c8fb7db55fe077892cf51e49c914bfd8 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 08/11] Batch replies in RenderTask::render --- src/components/gfx/render_task.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index 4c720653e81..36c990a57f9 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -245,14 +245,20 @@ impl RenderTask { continue; } + let mut replies = Vec::new(); for ReRenderRequest { buffer_requests, scale, layer_id, epoch } in requests.move_iter() { if self.epoch == epoch { - self.render(buffer_requests, scale, layer_id); + self.render(&mut replies, buffer_requests, scale, layer_id); } else { debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch); } } + + debug!("render_task: returning surfaces"); + for (layer_id, layer_buffer_set) in replies.move_iter() { + self.compositor.paint(self.id, layer_id, layer_buffer_set, self.epoch); + } } UnusedBufferMsg(unused_buffers) => { for buffer in unused_buffers.move_iter().rev() { @@ -289,7 +295,11 @@ impl RenderTask { /// /// FIXME(pcwalton): We will probably want to eventually send all layers belonging to a page in /// one transaction, to avoid the user seeing inconsistent states. - fn render(&mut self, tiles: Vec, scale: f32, layer_id: LayerId) { + fn render(&mut self, + replies: &mut Vec<(LayerId, Box)>, + tiles: Vec, + scale: f32, + layer_id: LayerId) { time::profile(time::RenderingCategory, self.time_profiler_chan.clone(), || { // FIXME: Try not to create a new array here. let mut new_buffers = vec!(); @@ -434,8 +444,7 @@ impl RenderTask { buffers: new_buffers, }; - debug!("render_task: returning surface"); - self.compositor.paint(self.id, render_layer.id, layer_buffer_set, self.epoch); + replies.push((render_layer.id, layer_buffer_set)); self.compositor.set_render_state(IdleRenderState); }) } From 340d9caba5cbe32595792177eefefe52c6ba9bf9 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 09/11] Make RenderListener::paint take multiple replies --- src/components/compositing/compositor_task.rs | 9 +++++---- src/components/gfx/render_task.rs | 4 +--- src/components/msg/compositor_msg.rs | 5 ++--- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/components/compositing/compositor_task.rs b/src/components/compositing/compositor_task.rs index 7534131a935..888fc650767 100644 --- a/src/components/compositing/compositor_task.rs +++ b/src/components/compositing/compositor_task.rs @@ -96,10 +96,11 @@ impl RenderListener for CompositorChan { fn paint(&self, pipeline_id: PipelineId, - layer_id: LayerId, - layer_buffer_set: Box, - epoch: Epoch) { - self.chan.send(Paint(pipeline_id, layer_id, layer_buffer_set, epoch)) + epoch: Epoch, + replies: Vec<(LayerId, Box)>) { + for (layer_id, layer_buffer_set) in replies.move_iter() { + self.chan.send(Paint(pipeline_id, layer_id, layer_buffer_set, epoch)) + } } fn initialize_layers_for_pipeline(&self, diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index 36c990a57f9..6d5e45ebbe9 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -256,9 +256,7 @@ impl RenderTask { } debug!("render_task: returning surfaces"); - for (layer_id, layer_buffer_set) in replies.move_iter() { - self.compositor.paint(self.id, layer_id, layer_buffer_set, self.epoch); - } + self.compositor.paint(self.id, self.epoch, replies); } UnusedBufferMsg(unused_buffers) => { for buffer in unused_buffers.move_iter().rev() { diff --git a/src/components/msg/compositor_msg.rs b/src/components/msg/compositor_msg.rs index b1e4ae210a0..48d7600a0ae 100644 --- a/src/components/msg/compositor_msg.rs +++ b/src/components/msg/compositor_msg.rs @@ -102,9 +102,8 @@ pub trait RenderListener { /// Sends new tiles for the given layer to the compositor. fn paint(&self, pipeline_id: PipelineId, - layer_id: LayerId, - layer_buffer_set: Box, - epoch: Epoch); + epoch: Epoch, + replies: Vec<(LayerId, Box)>); fn set_render_state(&self, render_state: RenderState); } From acc0daa519ce3737624d9e047634feec61379c09 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 10/11] Make the CompositorTask Paint message take multiple replies --- src/components/compositing/compositor.rs | 7 ++++--- src/components/compositing/compositor_task.rs | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 38cfdf5e46f..c9aa14cee14 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -303,9 +303,10 @@ impl IOCompositor { self.set_layer_clip_rect(pipeline_id, layer_id, new_rect); } - (Ok(Paint(pipeline_id, layer_id, new_layer_buffer_set, epoch)), - NotShuttingDown) => { - self.paint(pipeline_id, layer_id, new_layer_buffer_set, epoch); + (Ok(Paint(pipeline_id, epoch, replies)), NotShuttingDown) => { + for (layer_id, new_layer_buffer_set) in replies.move_iter() { + self.paint(pipeline_id, layer_id, new_layer_buffer_set, epoch); + } } (Ok(ScrollFragmentPoint(pipeline_id, layer_id, point)), NotShuttingDown) => { diff --git a/src/components/compositing/compositor_task.rs b/src/components/compositing/compositor_task.rs index 888fc650767..b4ffaa6ba81 100644 --- a/src/components/compositing/compositor_task.rs +++ b/src/components/compositing/compositor_task.rs @@ -98,9 +98,7 @@ impl RenderListener for CompositorChan { pipeline_id: PipelineId, epoch: Epoch, replies: Vec<(LayerId, Box)>) { - for (layer_id, layer_buffer_set) in replies.move_iter() { - self.chan.send(Paint(pipeline_id, layer_id, layer_buffer_set, epoch)) - } + self.chan.send(Paint(pipeline_id, epoch, replies)); } fn initialize_layers_for_pipeline(&self, @@ -181,7 +179,7 @@ pub enum Msg { /// Scroll a page in a window ScrollFragmentPoint(PipelineId, LayerId, Point2D), /// Requests that the compositor paint the given layer buffer set for the given page size. - Paint(PipelineId, LayerId, Box, Epoch), + Paint(PipelineId, Epoch, Vec<(LayerId, Box)>), /// Alerts the compositor to the current status of page loading. ChangeReadyState(ReadyState), /// Alerts the compositor to the current status of rendering. From fbd2b588ed5d08616a324ce3895ad0899c72e626 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Thu, 10 Jul 2014 01:31:48 -0700 Subject: [PATCH 11/11] Remove FIXMEs related to batching buffer requests --- src/components/compositing/compositor_data.rs | 3 --- src/components/gfx/render_task.rs | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index 975047ffd2c..c68a2709d1b 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -138,9 +138,6 @@ impl CompositorData { } if !request.is_empty() { // Ask for tiles. - // - // FIXME(#2003, pcwalton): We may want to batch these up in the case in which - // one page has multiple layers, to avoid the user seeing inconsistent states. let pipeline_id = layer.extra_data.borrow().pipeline.id; let msg = ReRenderRequest { buffer_requests: request, diff --git a/src/components/gfx/render_task.rs b/src/components/gfx/render_task.rs index 6d5e45ebbe9..5677ecb629e 100644 --- a/src/components/gfx/render_task.rs +++ b/src/components/gfx/render_task.rs @@ -290,9 +290,6 @@ impl RenderTask { } /// Renders one layer and sends the tiles back to the layer. - /// - /// FIXME(pcwalton): We will probably want to eventually send all layers belonging to a page in - /// one transaction, to avoid the user seeing inconsistent states. fn render(&mut self, replies: &mut Vec<(LayerId, Box)>, tiles: Vec,