Reduce the number of messages needed to exchange buffers from 3 to 2

src/components/servo-gfx/compositor.rs

@@ -24,7 +24,6 @@ pub struct LayerBufferSet {
 /// The interface used to by the renderer to acquire draw targets for each rendered frame and
 /// submit them to be drawn to the display.
 pub trait Compositor {
-    fn begin_drawing(&self, next_dt: comm::Chan<LayerBufferSet>);
-    fn draw(&self, next_dt: comm::Chan<LayerBufferSet>, draw_me: LayerBufferSet);
+    fn paint(&self, layer_buffer_set: LayerBufferSet);
 }
 

src/components/servo-gfx/render_layers.rs

@@ -134,6 +134,8 @@ pub fn render_layers(layer_ref: *RenderLayer,
         }
     }
 
-    return LayerBufferSet { buffers: new_buffers };
+    LayerBufferSet {
+        buffers: new_buffers,
+    }
 }
 

src/components/servo-gfx/render_task.rs

@@ -5,7 +5,7 @@
 // The task that handles all rendering/painting.
 
 use azure::AzFloat;
-use compositor::{Compositor, LayerBufferSet};
+use compositor::Compositor;
 use font_context::FontContext;
 use geom::matrix2d::Matrix2D;
 use opts::Opts;
@@ -13,67 +13,70 @@ use render_context::RenderContext;
 use render_layers::{RenderLayer, render_layers};
 
 use core::cell::Cell;
-use core::comm::{Port, SharedChan};
+use core::comm::{Chan, Port, SharedChan};
 use core::task::SingleThreaded;
 use std::task_pool::TaskPool;
 use servo_net::util::spawn_listener;
 
-use servo_util::time;
-use servo_util::time::time;
-use servo_util::time::profile;
 use servo_util::time::ProfilerChan;
+use servo_util::time::profile;
+use servo_util::time::time;
+use servo_util::time;
 
 pub enum Msg {
     RenderMsg(RenderLayer),
-    ExitMsg(comm::Chan<()>)
+    ExitMsg(Chan<()>),
 }
 
-pub type RenderTask = SharedChan<Msg>;
+#[deriving(Clone)]
+pub struct RenderTask {
+    channel: SharedChan<Msg>,
+}
 
-pub fn RenderTask<C:Compositor + Owned>(compositor: C,
-                                        opts: Opts,
-                                        prof_chan: ProfilerChan) -> RenderTask {
-    let compositor_cell = Cell(compositor);
-    let opts_cell = Cell(opts);
-    let render_task = do spawn_listener |po: Port<Msg>| {
-        let (layer_buffer_set_port, layer_buffer_channel) = comm::stream();
+impl RenderTask {
+    pub fn new<C:Compositor + Owned>(compositor: C, opts: Opts) -> RenderTask {
+        let compositor_cell = Cell(compositor);
+        let opts_cell = Cell(opts);
+        let (port, chan) = comm::stream();
+        let port = Cell(port);
 
-        let compositor = compositor_cell.take();
-        compositor.begin_drawing(layer_buffer_channel);
+        do spawn {
+            let compositor = compositor_cell.take();
 
-        // FIXME: Annoying three-cell dance here. We need one-shot closures.
-        let opts = opts_cell.with_ref(|o| copy *o);
-        let n_threads = opts.n_render_threads;
-        let new_opts_cell = Cell(opts);
-        let prof_chan2 = prof_chan.clone();
+            // FIXME: Annoying three-cell dance here. We need one-shot closures.
+            let opts = opts_cell.with_ref(|o| copy *o);
+            let n_threads = opts.n_render_threads;
+            let new_opts_cell = Cell(opts);
 
-        let thread_pool = do TaskPool::new(n_threads, Some(SingleThreaded)) {
-            let opts_cell = Cell(new_opts_cell.with_ref(|o| copy *o));
-            let prof_chan = prof_chan2.clone();
-            let f: ~fn(uint) -> ThreadRenderContext = |thread_index| {
-                ThreadRenderContext {
-                    thread_index: thread_index,
-                    font_ctx: @mut FontContext::new(opts_cell.with_ref(|o| o.render_backend),
-                                                    false,
-                                                    prof_chan.clone()),
-                    opts: opts_cell.with_ref(|o| copy *o),
-                }
+            let thread_pool = do TaskPool::new(n_threads, Some(SingleThreaded)) {
+                let opts_cell = Cell(new_opts_cell.with_ref(|o| copy *o));
+                let f: ~fn(uint) -> ThreadRenderContext = |thread_index| {
+                    let opts = opts_cell.with_ref(|opts| copy *opts);
+
+                    ThreadRenderContext {
+                        thread_index: thread_index,
+                        font_ctx: @mut FontContext::new(opts.render_backend, false),
+                        opts: opts,
+                    }
+                };
+                f
             };
-            f
-        };
 
-        // FIXME: rust/#5967
-        let mut r = Renderer {
-            port: po,
-            compositor: compositor,
-            layer_buffer_set_port: Cell(layer_buffer_set_port),
-            thread_pool: thread_pool,
-            opts: opts_cell.take(),
-            prof_chan: prof_chan.clone()
-        };
-        r.start();
-    };
-    SharedChan::new(render_task)
+            // FIXME: rust/#5967
+            let mut renderer = Renderer {
+                port: port.take(),
+                compositor: compositor,
+                thread_pool: thread_pool,
+                opts: opts_cell.take()
+            };
+
+            renderer.start();
+        }
+
+        RenderTask {
+            channel: SharedChan::new(chan),
+        }
+    }
 }
 
 /// Data that needs to be kept around for each render thread.
@@ -86,7 +89,6 @@ priv struct ThreadRenderContext {
 priv struct Renderer<C> {
     port: Port<Msg>,
     compositor: C,
-    layer_buffer_set_port: Cell<comm::Port<LayerBufferSet>>,
     thread_pool: TaskPool<ThreadRenderContext>,
     opts: Opts,
     prof_chan: ProfilerChan,
@@ -108,25 +110,9 @@ impl<C: Compositor + Owned> Renderer<C> {
     }
 
     fn render(&mut self, render_layer: RenderLayer) {
-        debug!("renderer: got render request");
-
-        let layer_buffer_set_port = self.layer_buffer_set_port.take();
-
-        if !layer_buffer_set_port.peek() {
-            warn!("renderer: waiting on layer buffer");
-        }
-
-        let (new_layer_buffer_set_port, layer_buffer_set_channel) = comm::stream();
-        self.layer_buffer_set_port.put_back(new_layer_buffer_set_port);
-
-        let layer_buffer_set_channel_cell = Cell(layer_buffer_set_channel);
-
         debug!("renderer: rendering");
-
-        do profile(time::RenderingCategory, self.prof_chan.clone()) {
-            let layer_buffer_set_channel = layer_buffer_set_channel_cell.take();
-
-            let layer_buffer_set = do render_layers(&render_layer, &self.opts, self.prof_chan.clone())
+        do time("rendering") {
+            let layer_buffer_set = do render_layers(&render_layer, &self.opts)
                     |render_layer_ref, layer_buffer, buffer_chan| {
                 let layer_buffer_cell = Cell(layer_buffer);
                 do self.thread_pool.execute |thread_render_context| {
@@ -160,7 +146,8 @@ impl<C: Compositor + Owned> Renderer<C> {
             };
 
             debug!("renderer: returning surface");
-            self.compositor.draw(layer_buffer_set_channel, layer_buffer_set);
+            self.compositor.paint(layer_buffer_set);
         }
     }
 }
+

src/components/servo/compositing/mod.rs

@@ -7,16 +7,14 @@ use platform::{Application, Window};
 use scripting::script_task::{LoadMsg, ScriptMsg};
 use windowing::{ApplicationMethods, WindowMethods};
 
-use azure::azure_hl::{BackendType, B8G8R8A8, DataSourceSurface, DrawTarget, SourceSurfaceMethods};
+use azure::azure_hl::{DataSourceSurface, DrawTarget, SourceSurfaceMethods};
 use core::cell::Cell;
 use core::comm::{Chan, SharedChan, Port};
 use core::util;
 use geom::matrix::identity;
 use geom::point::Point2D;
-use geom::rect::Rect;
 use geom::size::Size2D;
-use gfx::compositor::{Compositor, LayerBuffer, LayerBufferSet};
-use gfx::opts::Opts;
+use gfx::compositor::{Compositor, LayerBufferSet};
 use layers::layers::{ARGB32Format, BasicImageData, ContainerLayer, ContainerLayerKind, Format};
 use layers::layers::{Image, ImageData, ImageLayer, ImageLayerKind, RGB24Format, WithDataFn};
 use layers::rendergl;
@@ -29,33 +27,39 @@ mod resize_rate_limiter;
 
 /// The implementation of the layers-based compositor.
 #[deriving(Clone)]
-pub struct CompositorImpl {
-    chan: SharedChan<Msg>
+pub struct CompositorTask {
+    /// A channel on which messages can be sent to the compositor.
+    chan: SharedChan<Msg>,
 }
 
-impl CompositorImpl {
-    /// Creates a new compositor instance.
+impl CompositorTask {
+    /// Starts the compositor. Returns an interface that can be used to communicate with the
+    /// compositor and a port which allows notification when the compositor shuts down.
     pub fn new(script_chan: SharedChan<ScriptMsg>,
-               opts: Opts,
-               prof_chan: ProfilerChan)
-               -> CompositorImpl {
+               profiler_chan: ProfilerChan)
+               -> (CompositorTask, Port<()>) {
         let script_chan = Cell(script_chan);
+        let (shutdown_port, shutdown_chan) = stream();
+        let shutdown_chan = Cell(shutdown_chan);
+
         let chan: Chan<Msg> = do on_osmain |port| {
             debug!("preparing to enter main loop");
-            run_main_loop(port, script_chan.take(), &opts, prof_chan.clone());
+            run_main_loop(port,
+                          script_chan.take(),
+                          shutdown_chan.take(),
+                          profiler_chan.clone());
         };
 
-        CompositorImpl {
-            chan: SharedChan::new(chan)
-        }
+        let task = CompositorTask {
+            chan: SharedChan::new(chan),
+        };
+        (task, shutdown_port)
     }
 }
 
 /// Messages to the compositor.
 pub enum Msg {
-    BeginDrawing(Chan<LayerBufferSet>),
-    Draw(Chan<LayerBufferSet>, LayerBufferSet),
-    AddKeyHandler(Chan<()>),
+    Paint(LayerBufferSet),
     Exit
 }
 
@@ -84,18 +88,19 @@ impl ImageData for AzureDrawTargetImageData {
     }
 }
 
-fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts, prof_chan:ProfilerChan) {
+fn run_main_loop(port: Port<Msg>,
+                 script_chan: SharedChan<ScriptMsg>,
+                 shutdown_chan: Chan<()>,
+                 profiler_chan: ProfilerChan) {
     let app: Application = ApplicationMethods::new();
     let window: @mut Window = WindowMethods::new(&app);
     let resize_rate_limiter = @mut ResizeRateLimiter(script_chan.clone());
 
-    let surfaces = @mut SurfaceSet::new(opts.render_backend);
-    let context = rendergl::init_render_context();
-
     // Create an initial layer tree.
     //
     // TODO: There should be no initial layer tree until the renderer creates one from the display
     // list. This is only here because we don't have that logic in the renderer yet.
+    let context = rendergl::init_render_context();
     let root_layer = @mut ContainerLayer();
     let original_layer_transform;
     {
@@ -108,7 +113,6 @@ fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts,
     }
 
     let scene = @mut Scene(ContainerLayerKind(root_layer), Size2D(800.0, 600.0), identity());
-    let key_handlers: @mut ~[Chan<()>] = @mut ~[];
     let done = @mut false;
 
     // FIXME: This should not be a separate offset applied after the fact but rather should be
@@ -121,24 +125,19 @@ fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts,
         resize_rate_limiter.check_resize_response();
 
         // Handle messages
-        while po.peek() {
-            match po.recv() {
-                AddKeyHandler(key_ch) => key_handlers.push(key_ch),
-                BeginDrawing(sender) => surfaces.lend(sender),
+        while port.peek() {
+            match port.recv() {
                 Exit => *done = true,
 
-                Draw(sender, draw_target) => {
+                Paint(new_layer_buffer_set) => {
                     debug!("osmain: received new frame");
-
-                    // Perform a buffer swap.
-                    surfaces.put_back(draw_target);
-                    surfaces.lend(sender);
+                    let mut new_layer_buffer_set = new_layer_buffer_set;
 
                     // Iterate over the children of the container layer.
                     let mut current_layer_child = root_layer.first_child;
 
                     // Replace the image layer data with the buffer data.
-                    let buffers = util::replace(&mut surfaces.front.layer_buffer_set.buffers, ~[]);
+                    let buffers = util::replace(&mut new_layer_buffer_set.buffers, ~[]);
                     for buffers.each |buffer| {
                         let width = buffer.rect.size.width as uint;
                         let height = buffer.rect.size.height as uint;
@@ -183,7 +182,8 @@ fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts,
                         image_layer.common.set_transform(transform)
                     }
 
-                    surfaces.front.layer_buffer_set.buffers = buffers
+                    // TODO: Recycle the old buffers; send them back to the renderer to reuse if
+                    // it wishes.
                 }
             }
         }
@@ -216,7 +216,7 @@ fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts,
 
     // When the user scrolls, move the layer around.
     do window.set_scroll_callback |delta| {
-        // FIXME: Can't use `+=` due to a Rust bug.
+        // FIXME (Rust #2528): Can't use `+=`.
         let world_offset_copy = *world_offset;
         *world_offset = world_offset_copy + delta;
 
@@ -235,104 +235,28 @@ fn run_main_loop(po: Port<Msg>, script_chan: SharedChan<ScriptMsg>, opts: &Opts,
         // Check for messages coming from the windowing system.
         window.check_loop();
     }
+
+    shutdown_chan.send(())
 }
 
 /// Implementation of the abstract `Compositor` interface.
-impl Compositor for CompositorImpl {
-    fn begin_drawing(&self, next_dt: Chan<LayerBufferSet>) {
-        self.chan.send(BeginDrawing(next_dt))
-    }
-    fn draw(&self, next_dt: Chan<LayerBufferSet>, draw_me: LayerBufferSet) {
-        self.chan.send(Draw(next_dt, draw_me))
-    }
-}
-
-struct SurfaceSet {
-    front: Surface,
-    back: Surface,
-}
-
-impl SurfaceSet {
-    /// Creates a new surface set.
-    fn new(backend: BackendType) -> SurfaceSet {
-        SurfaceSet {
-            front: Surface::new(backend),
-            back: Surface::new(backend),
-        }
-    }
-
-    fn lend(&mut self, receiver: Chan<LayerBufferSet>) {
-        // We are in a position to lend out the surface?
-        assert!(self.front.have);
-        // Ok then take it
-        let old_layer_buffers = util::replace(&mut self.front.layer_buffer_set.buffers, ~[]);
-        let new_layer_buffers = do old_layer_buffers.map |layer_buffer| {
-            let draw_target_ref = &layer_buffer.draw_target;
-            let layer_buffer = LayerBuffer {
-                draw_target: draw_target_ref.clone(),
-                rect: copy layer_buffer.rect,
-                stride: layer_buffer.stride
-            };
-            debug!("osmain: lending surface %?", layer_buffer);
-            layer_buffer
-        };
-        self.front.layer_buffer_set.buffers = old_layer_buffers;
-
-        let new_layer_buffer_set = LayerBufferSet { buffers: new_layer_buffers };
-        receiver.send(new_layer_buffer_set);
-        // Now we don't have it
-        self.front.have = false;
-        // But we (hopefully) have another!
-        util::swap(&mut self.front, &mut self.back);
-        // Let's look
-        assert!(self.front.have);
-    }
-
-    fn put_back(&mut self, layer_buffer_set: LayerBufferSet) {
-        // We have room for a return
-        assert!(self.front.have);
-        assert!(!self.back.have);
-
-        self.back.layer_buffer_set = layer_buffer_set;
-
-        // Now we have it again
-        self.back.have = true;
-    }
-}
-
-struct Surface {
-    layer_buffer_set: LayerBufferSet,
-    have: bool,
-}
-
-impl Surface {
-    fn new(backend: BackendType) -> Surface {
-        let layer_buffer = LayerBuffer {
-            draw_target: DrawTarget::new(backend, Size2D(800, 600), B8G8R8A8),
-            rect: Rect(Point2D(0u, 0u), Size2D(800u, 600u)),
-            stride: 800 * 4
-        };
-        let layer_buffer_set = LayerBufferSet {
-            buffers: ~[ layer_buffer ]
-        };
-        Surface {
-            layer_buffer_set: layer_buffer_set,
-            have: true
-        }
+impl Compositor for CompositorTask {
+    fn paint(&self, layer_buffer_set: LayerBufferSet) {
+        self.chan.send(Paint(layer_buffer_set))
     }
 }
 
 /// A function for spawning into the platform's main thread.
-fn on_osmain<T: Owned>(f: ~fn(po: Port<T>)) -> Chan<T> {
-    let (setup_po, setup_ch) = comm::stream();
+fn on_osmain<T: Owned>(f: ~fn(port: Port<T>)) -> Chan<T> {
+    let (setup_port, setup_chan) = comm::stream();
     // FIXME: rust#6399
     let mut main_task = task::task();
     main_task.sched_mode(task::PlatformThread);
     do main_task.spawn {
-        let (po, ch) = comm::stream();
-        setup_ch.send(ch);
-        f(po);
+        let (port, chan) = comm::stream();
+        setup_chan.send(chan);
+        f(port);
     }
-    setup_po.recv()
+    setup_port.recv()
 }
 

src/components/servo/engine.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 compositing::CompositorImpl;
+use compositing::CompositorTask;
 use layout::layout_task::LayoutTask;
 use layout::layout_task;
 use scripting::script_task::{ExecuteMsg, LoadMsg, ScriptMsg, ScriptTask};
@@ -27,12 +27,12 @@ pub type EngineTask = Chan<Msg>;
 
 pub enum Msg {
     LoadUrlMsg(Url),
-    ExitMsg(Chan<()>)
+    ExitMsg(Chan<()>),
 }
 
 pub struct Engine {
     request_port: Port<Msg>,
-    compositor: CompositorImpl,
+    compositor: CompositorTask,
     render_task: RenderTask,
     resource_task: ResourceTask,
     image_cache_task: ImageCacheTask,
@@ -42,24 +42,23 @@ pub struct Engine {
 }
 
 impl Engine {
-    pub fn start(compositor: CompositorImpl,
+    pub fn start(compositor: CompositorTask,
                  opts: &Opts,
                  script_port: Port<ScriptMsg>,
                  script_chan: SharedChan<ScriptMsg>,
                  resource_task: ResourceTask,
                  image_cache_task: ImageCacheTask,
-                 prof_port: ProfilerPort,
-                 prof_chan: ProfilerChan)
+                 profiler_port: ProfilerPort,
+                 profiler_chan: ProfilerChan)
                  -> EngineTask {
         let (script_port, script_chan) = (Cell(script_port), Cell(script_chan));
-        let prof_port = Cell(prof_port);
+        let profiler_port = Cell(profiler_port);
         let opts = Cell(copy *opts);
 
         do spawn_listener::<Msg> |request| {
-            let profiler_task = time::ProfilerTask::new(prof_port.take(), prof_chan.clone());
-            let render_task = RenderTask(compositor.clone(),
-                                         opts.with_ref(|o| copy *o),
-                                         prof_chan.clone());
+            let render_task = RenderTask::new(compositor.clone(),
+                                              opts.with_ref(|o| copy *o),
+                                              profiler_chan.clone());
 
             let opts = opts.take();
             let layout_task = LayoutTask(render_task.clone(),
@@ -110,7 +109,7 @@ impl Engine {
 
                 let (response_port, response_chan) = comm::stream();
 
-                self.render_task.send(render_task::ExitMsg(response_chan));
+                self.render_task.channel.send(render_task::ExitMsg(response_chan));
                 response_port.recv();
 
                 self.image_cache_task.exit();

src/components/servo/layout/layout_task.rs

@@ -267,7 +267,7 @@ impl Layout {
                 size: Size2D(screen_size.width.to_px() as uint, screen_size.height.to_px() as uint)
             };
 
-            self.render_task.send(RenderMsg(render_layer));
+            self.render_task.channel.send(RenderMsg(render_layer));
         } // time(layout: display list building)
 
         // Tell script that we're done.

src/components/servo/servo.rc

@@ -33,7 +33,7 @@ extern mod core_graphics;
 #[cfg(target_os="macos")]
 extern mod core_text;
 
-use compositing::{AddKeyHandler, CompositorImpl};
+use compositing::CompositorTask;
 use engine::{Engine, LoadUrlMsg};
 
 use core::comm::SharedChan;
@@ -134,20 +134,19 @@ fn main() {
 }
 
 fn run(opts: &Opts) {
+    // Create the script channel.
     let (script_port, script_chan) = comm::stream();
     let script_chan = SharedChan::new(script_chan);
 
-    let (prof_port, prof_chan) = comm::stream();
-    let prof_chan = SharedChan::new(prof_chan);
+    // Create the profiler channel.
+    let (profiler_port, profiler_chan) = comm::stream();
+    let profiler_chan = SharedChan::new(profiler_chan);
 
-    // The platform event handler thread
-    let compositor = CompositorImpl::new(script_chan.clone(), copy *opts, prof_chan.clone());
+    // Create the compositor.
+    let (compositor, shutdown_port) = CompositorTask::new(script_chan.clone(),
+                                                          profiler_chan.clone());
 
-    // Send each file to render then wait for keypress
-    let (keypress_from_compositor, keypress_to_engine) = comm::stream();
-    compositor.chan.send(AddKeyHandler(keypress_to_engine));
-
-    // Create a servo instance
+    // Create a Servo instance.
     let resource_task = ResourceTask();
     let image_cache_task = ImageCacheTask(resource_task.clone());
     let engine_task = Engine::start(compositor.clone(),
@@ -159,25 +158,18 @@ fn run(opts: &Opts) {
                                     prof_port,
                                     prof_chan);
 
+    // Send the URL command to the engine task.
     for opts.urls.each |filename| {
-        let url = make_url(copy *filename, None);
-
-        debug!("master: Sending url `%s`", url.to_str());
-        engine_task.send(LoadUrlMsg(url));
-
-        debug!("master: Waiting for keypress");
-        match keypress_from_compositor.try_recv() {
-            Some(*) => {}
-            None => error!("keypress stream closed unexpectedly"),
-        }
+        engine_task.send(LoadUrlMsg(make_url(copy *filename, None)))
     }
 
-    // Shut everything down
+    // Wait for the compositor to shut down.
+    shutdown_port.recv();
+
+    // Shut the engine down.
     debug!("master: Shut down");
     let (exit_response_from_engine, exit_chan) = comm::stream();
     engine_task.send(engine::ExitMsg(exit_chan));
     exit_response_from_engine.recv();
-
-    compositor.chan.send(compositing::Exit);
 }