Merge pull request #2799 from zwarich/compositor-transactions

Batch buffer rendering requests and their replies in transactions

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;
@@ -45,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;
@@ -301,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)),
+                (Ok(Paint(pipeline_id, epoch, replies)), NotShuttingDown) => {
-                 NotShuttingDown) => {
+                    for (layer_id, new_layer_buffer_set) in replies.move_iter() {
-                    self.paint(pipeline_id, layer_id, new_layer_buffer_set, epoch);
+                        self.paint(pipeline_id, layer_id, new_layer_buffer_set, epoch);
+                    }
                 }
 
                 (Ok(ScrollFragmentPoint(pipeline_id, layer_id, point)), NotShuttingDown) => {
@@ -743,11 +746,16 @@ impl IOCompositor {
         match self.scene.root {
             Some(ref mut layer) => {
                 let rect = Rect(Point2D(0f32, 0f32), page_window.to_untyped());
+                let mut request_map = HashMap::new();
                 let recomposite =
-                    CompositorData::send_buffer_requests_recursively(layer.clone(),
+                    CompositorData::get_buffer_requests_recursively(&mut request_map,
-                                                                     &self.graphics_context,
+                                                                    layer.clone(),
-                                                                     rect,
+                                                                    &self.graphics_context,
-                                                                     scale.get());
+                                                                    rect,
+                                                                    scale.get());
+                for (_pipeline_id, (chan, requests)) in request_map.move_iter() {
+                    let _ = chan.send_opt(ReRenderMsg(requests));
+                }
                 self.recomposite = self.recomposite || recomposite;
             }
             None => { }

src/components/compositing/compositor_data.rs

@@ -11,7 +11,7 @@ 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::{ReRenderRequest, RenderChan, UnusedBufferMsg};
 use layers::layers::{Layer, Flip, LayerBuffer, LayerBufferSet, NoFlip, TextureLayer};
 use layers::quadtree::Tile;
 use layers::platform::surface::{NativeCompositingGraphicsContext, NativeSurfaceMethods};
@@ -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,11 +121,14 @@ 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<Layer<CompositorData>>,
+    pub fn get_buffer_requests_recursively(requests: &mut HashMap<PipelineId,
-                                            graphics_context: &NativeCompositingGraphicsContext,
+                                                                  (RenderChan,
-                                            window_rect: Rect<f32>,
+                                                                   Vec<ReRenderRequest>)>,
-                                            scale: f32)
+                                           layer: Rc<Layer<CompositorData>>,
-                                            -> bool {
+                                           graphics_context: &NativeCompositingGraphicsContext,
+                                           window_rect: Rect<f32>,
+                                           scale: f32)
+                                           -> bool {
         let (request, unused) = Layer::get_tile_rects_page(layer.clone(), window_rect, scale);
         let redisplay = !unused.is_empty();
         if redisplay {
@@ -134,21 +138,24 @@ impl CompositorData {
         }
         if !request.is_empty() {
             // Ask for tiles.
-            //
+            let pipeline_id = layer.extra_data.borrow().pipeline.id;
-            // FIXME(#2003, pcwalton): We may want to batch these up in the case in which
+            let msg = ReRenderRequest {
-            // one page has multiple layers, to avoid the user seeing inconsistent states.
+                buffer_requests: request,
-            let msg = ReRenderMsg(request,
+                scale: scale,
-                                  scale,
+                layer_id: layer.extra_data.borrow().id,
-                                  layer.extra_data.borrow().id,
+                epoch: layer.extra_data.borrow().epoch,
-                                  layer.extra_data.borrow().epoch);
+            };
-            let _ = layer.extra_data.borrow().pipeline.render_chan.send_opt(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 {
             CompositorData::build_layer_tree(layer.clone(), graphics_context);
         }
 
-        let send_child_buffer_request = |kid: &Rc<Layer<CompositorData>>| -> bool {
+        let get_child_buffer_request = |kid: &Rc<Layer<CompositorData>>| -> 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 +167,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(),
+                    CompositorData::get_buffer_requests_recursively(requests,
-                                                                     graphics_context,
+                                                                    kid.clone(),
-                                                                     child_rect,
+                                                                    graphics_context,
-                                                                     scale)
+                                                                    child_rect,
+                                                                    scale)
                 }
                 None => {
                     false // Layer is offscreen
@@ -171,7 +179,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,

src/components/compositing/compositor_task.rs

@@ -96,10 +96,9 @@ impl RenderListener for CompositorChan {
 
     fn paint(&self,
              pipeline_id: PipelineId,
-             layer_id: LayerId,
+             epoch: Epoch,
-             layer_buffer_set: Box<LayerBufferSet>,
+             replies: Vec<(LayerId, Box<LayerBufferSet>)>) {
-             epoch: Epoch) {
+        self.chan.send(Paint(pipeline_id, epoch, replies));
-        self.chan.send(Paint(pipeline_id, layer_id, layer_buffer_set, epoch))
     }
 
     fn initialize_layers_for_pipeline(&self,
@@ -180,7 +179,7 @@ pub enum Msg {
     /// Scroll a page in a window
     ScrollFragmentPoint(PipelineId, LayerId, Point2D<f32>),
     /// Requests that the compositor paint the given layer buffer set for the given page size.
-    Paint(PipelineId, LayerId, Box<LayerBufferSet>, Epoch),
+    Paint(PipelineId, Epoch, Vec<(LayerId, Box<LayerBufferSet>)>),
     /// Alerts the compositor to the current status of page loading.
     ChangeReadyState(ReadyState),
     /// Alerts the compositor to the current status of rendering.

src/components/gfx/render_task.rs

@@ -50,9 +50,16 @@ pub struct RenderLayer {
     pub scroll_policy: ScrollPolicy,
 }
 
+pub struct ReRenderRequest {
+    pub buffer_requests: Vec<BufferRequest>,
+    pub scale: f32,
+    pub layer_id: LayerId,
+    pub epoch: Epoch,
+}
+
 pub enum Msg {
     RenderMsg(SmallVec1<RenderLayer>),
-    ReRenderMsg(Vec<BufferRequest>, f32, LayerId, Epoch),
+    ReRenderMsg(Vec<ReRenderRequest>),
     UnusedBufferMsg(Vec<Box<LayerBuffer>>),
     PaintPermissionGranted,
     PaintPermissionRevoked,
@@ -230,12 +237,26 @@ impl<C:RenderListener + Send> RenderTask<C> {
                                       self.epoch,
                                       self.render_layers.as_slice());
                 }
-                ReRenderMsg(tiles, scale, layer_id, epoch) => {
+                ReRenderMsg(requests) => {
-                    if self.epoch == epoch {
+                    if !self.paint_permission {
-                        self.render(tiles, scale, layer_id);
+                        debug!("render_task: render ready msg");
-                    } else {
+                        let ConstellationChan(ref mut c) = self.constellation_chan;
-                        debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch);
+                        c.send(RendererReadyMsg(self.id));
+                        continue;
                     }
+
+                    let mut replies = Vec::new();
+                    for ReRenderRequest { buffer_requests, scale, layer_id, epoch }
+                          in requests.move_iter() {
+                        if self.epoch == epoch {
+                            self.render(&mut replies, buffer_requests, scale, layer_id);
+                        } else {
+                            debug!("renderer epoch mismatch: {:?} != {:?}", self.epoch, epoch);
+                        }
+                    }
+
+                    debug!("render_task: returning surfaces");
+                    self.compositor.paint(self.id, self.epoch, replies);
                 }
                 UnusedBufferMsg(unused_buffers) => {
                     for buffer in unused_buffers.move_iter().rev() {
@@ -269,10 +290,11 @@ impl<C:RenderListener + Send> RenderTask<C> {
     }
 
     /// Renders one layer and sends the tiles back to the layer.
-    ///
+    fn render(&mut self,
-    /// FIXME(pcwalton): We will probably want to eventually send all layers belonging to a page in
+              replies: &mut Vec<(LayerId, Box<LayerBufferSet>)>,
-    /// one transaction, to avoid the user seeing inconsistent states.
+              tiles: Vec<BufferRequest>,
-    fn render(&mut self, tiles: Vec<BufferRequest>, scale: f32, layer_id: LayerId) {
+              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!();
@@ -417,14 +439,7 @@ impl<C:RenderListener + Send> RenderTask<C> {
                 buffers: new_buffers,
             };
 
-            debug!("render_task: returning surface");
+            replies.push((render_layer.id, layer_buffer_set));
-            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.set_render_state(IdleRenderState);
         })
     }

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,
+             epoch: Epoch,
-             layer_buffer_set: Box<LayerBufferSet>,
+             replies: Vec<(LayerId, Box<LayerBufferSet>)>);
-             epoch: Epoch);
 
     fn set_render_state(&self, render_state: RenderState);
 }