From b60f3544050a22a830418cc7f8e4342705dbad04 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 6 Jan 2015 15:23:53 -0800 Subject: [PATCH] Store pipelines in the Compositor Instead of cloning pipelines and storing them once per layer, store them globally in the compositor and access them via id. This trades lots of unnecessary duplication for a HashMap lookup. --- components/compositing/compositor.rs | 97 ++++++++++------------ components/compositing/compositor_layer.rs | 81 +++++++++++------- 2 files changed, 96 insertions(+), 82 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index fc31b9e80a2..a2d86d9d5a5 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -19,7 +19,7 @@ use geom::rect::{Rect, TypedRect}; use geom::size::TypedSize2D; use geom::scale_factor::ScaleFactor; use gfx::paint_task::Msg as PaintMsg; -use gfx::paint_task::{PaintChan, PaintRequest}; +use gfx::paint_task::PaintRequest; use layers::geometry::{DevicePixel, LayerPixel}; use layers::layers::{BufferRequest, Layer, LayerBufferSet}; use layers::rendergl; @@ -154,22 +154,22 @@ struct HitTestResult { } struct PipelineDetails { + /// The pipeline associated with this PipelineDetails object. + pipeline: Option, + /// The status of this pipeline's ScriptTask. ready_state: ReadyState, /// The status of this pipeline's PaintTask. paint_state: PaintState, - - /// The channel to this pipeline's PaintTask, - paint_task_channel: Option, } impl PipelineDetails { fn new() -> PipelineDetails { PipelineDetails { + pipeline: None, ready_state: ReadyState::Blank, paint_state: PaintState::Painting, - paint_task_channel: None, } } } @@ -408,6 +408,20 @@ impl IOCompositor { return self.pipeline_details.get_mut(&pipeline_id).unwrap(); } + pub fn get_pipeline<'a>(&'a self, pipeline_id: PipelineId) -> &'a CompositionPipeline { + match self.pipeline_details.get(&pipeline_id) { + Some(ref details) => { + match details.pipeline { + Some(ref pipeline) => pipeline, + None => panic!("Compositor layer has an unitialized pipeline ({}).", + pipeline_id), + + } + } + None => panic!("Compositor layer has an unknown pipeline ({}).", pipeline_id), + } + } + fn change_page_title(&mut self, _: PipelineId, title: Option) { self.window.set_page_title(title); } @@ -464,7 +478,7 @@ impl IOCompositor { // If we have an old root layer, release all old tiles before replacing it. match self.scene.root { - Some(ref mut layer) => layer.clear_all_tiles(), + Some(ref layer) => layer.clear_all_tiles(self), None => { } } self.scene.root = Some(self.create_frame_tree_root_layers(frame_tree, None)); @@ -491,13 +505,11 @@ impl IOCompositor { scroll_policy: ScrollPolicy::Scrollable, }; - let root_layer = CompositorData::new_layer(pipeline.clone(), - layer_properties, + let root_layer = CompositorData::new_layer(layer_properties, WantsScrollEventsFlag::WantsScrollEvents, opts::get().tile_size); - self.get_or_create_pipeline_details(pipeline.id).paint_task_channel = - Some(pipeline.paint_chan.clone()); + self.get_or_create_pipeline_details(pipeline.id).pipeline = Some(pipeline.clone()); // All root layers mask to bounds. *root_layer.masks_to_bounds.borrow_mut() = true; @@ -569,9 +581,7 @@ impl IOCompositor { if need_new_base_layer { root_layer.update_layer_except_bounds(layer_properties); - let root_layer_pipeline = root_layer.extra_data.borrow().pipeline.clone(); let base_layer = CompositorData::new_layer( - root_layer_pipeline.clone(), layer_properties, WantsScrollEventsFlag::DoesntWantScrollEvents, opts::get().tile_size); @@ -603,9 +613,7 @@ impl IOCompositor { None => return, // This pipeline is in the process of shutting down. }; - let root_layer_pipeline = root_layer.extra_data.borrow().pipeline.clone(); - let new_layer = CompositorData::new_layer(root_layer_pipeline, - layer_properties, + let new_layer = CompositorData::new_layer(layer_properties, WantsScrollEventsFlag::DoesntWantScrollEvents, root_layer.tile_size); root_layer.add_child(new_layer); @@ -694,18 +702,9 @@ impl IOCompositor { None => {} } - match self.pipeline_details.entry(pipeline_id) { - Occupied(entry) => { - match entry.get().paint_task_channel { - Some(ref channel) => { - let message = PaintMsg::UnusedBuffer(new_layer_buffer_set.buffers); - let _ = channel.send_opt(message); - } - None => panic!("Received buffers for an uninitialized pipeline!"), - } - } - Vacant(_) => panic!("Received buffers for an unknown pipeline!"), - } + let pipeline = self.get_pipeline(pipeline_id); + let message = PaintMsg::UnusedBuffer(new_layer_buffer_set.buffers); + let _ = pipeline.paint_chan.send_opt(message); } fn assign_painted_buffers_to_layer(&mut self, @@ -722,7 +721,7 @@ impl IOCompositor { // FIXME(pcwalton): This is going to cause problems with inconsistent frames since // we only composite one layer at a time. - assert!(layer.add_buffers(new_layer_buffer_set, epoch)); + assert!(layer.add_buffers(self, new_layer_buffer_set, epoch)); self.composite_if_necessary(); } @@ -820,7 +819,7 @@ impl IOCompositor { debug!("osmain: loading URL `{}`", url_string); self.got_load_complete_message = false; let root_pipeline_id = match self.scene.root { - Some(ref layer) => layer.extra_data.borrow().pipeline.id.clone(), + Some(ref layer) => layer.get_pipeline_id(), None => panic!("Compositor: Received WindowEvent::LoadUrl without initialized compositor \ layers"), }; @@ -838,14 +837,14 @@ impl IOCompositor { MouseWindowEvent::MouseUp(_, p) => p, }; match self.find_topmost_layer_at_point(point / self.scene.scale) { - Some(result) => result.layer.send_mouse_event(mouse_window_event, result.point), + Some(result) => result.layer.send_mouse_event(self, mouse_window_event, result.point), None => {}, } } fn on_mouse_window_move_event_class(&self, cursor: TypedPoint2D) { match self.find_topmost_layer_at_point(cursor / self.scene.scale) { - Some(result) => result.layer.send_mouse_move_event(result.point), + Some(result) => result.layer.send_mouse_move_event(self, result.point), None => {}, } } @@ -957,25 +956,21 @@ impl IOCompositor { fn convert_buffer_requests_to_pipeline_requests_map(&self, requests: Vec<(Rc>, - Vec)>) -> - HashMap)> { + Vec)>) + -> HashMap> { let scale = self.device_pixels_per_page_px(); - let mut results: - HashMap)> = HashMap::new(); + let mut results: HashMap> = HashMap::new(); for (layer, mut layer_requests) in requests.into_iter() { - let &(_, ref mut vec) = - match results.entry(layer.extra_data.borrow().pipeline.id) { - Occupied(mut entry) => { - *entry.get_mut() = - (layer.extra_data.borrow().pipeline.paint_chan.clone(), vec!()); - entry.into_mut() - } - Vacant(entry) => { - entry.set((layer.extra_data.borrow().pipeline.paint_chan.clone(), vec!())) - } - }; + let vec = match results.entry(layer.get_pipeline_id()) { + Occupied(mut entry) => { + *entry.get_mut() = Vec::new(); + entry.into_mut() + } + Vacant(entry) => { + entry.set(Vec::new()) + } + }; // All the BufferRequests are in layer/device coordinates, but the paint task // wants to know the page coordinates. We scale them before sending them. @@ -1011,7 +1006,7 @@ impl IOCompositor { if layer.extra_data.borrow().id == LayerId::null() { let layer_rect = Rect(-layer.extra_data.borrow().scroll_offset.to_untyped(), layer.bounds.borrow().size.to_untyped()); - let pipeline = &layer.extra_data.borrow().pipeline; + let pipeline = self.get_pipeline(layer.get_pipeline_id()); let ScriptControlChan(ref chan) = pipeline.script_chan; chan.send(ConstellationControlMsg::Viewport(pipeline.id.clone(), layer_rect)); } @@ -1047,9 +1042,9 @@ impl IOCompositor { self.convert_buffer_requests_to_pipeline_requests_map(layers_and_requests); let mut num_paint_msgs_sent = 0; - for (_pipeline_id, (chan, requests)) in pipeline_requests.into_iter() { + for (pipeline_id, requests) in pipeline_requests.into_iter() { num_paint_msgs_sent += 1; - let _ = chan.send_opt(PaintMsg::Paint(requests)); + let _ = self.get_pipeline(pipeline_id).paint_chan.send_opt(PaintMsg::Paint(requests)); } self.add_outstanding_paint_msg(num_paint_msgs_sent); @@ -1267,7 +1262,7 @@ fn find_layer_with_pipeline_and_layer_id_for_layer(layer: Rc Option>> { - if layer.extra_data.borrow().pipeline.id == pipeline_id && + if layer.extra_data.borrow().pipeline_id == pipeline_id && layer.extra_data.borrow().id == layer_id { return Some(layer); } diff --git a/components/compositing/compositor_layer.rs b/components/compositing/compositor_layer.rs index ba6d969c0e1..9afb50cb005 100644 --- a/components/compositing/compositor_layer.rs +++ b/components/compositing/compositor_layer.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use compositor_task::LayerProperties; -use pipeline::CompositionPipeline; +use compositor::IOCompositor; use windowing::{MouseWindowEvent, WindowMethods}; use azure::azure_hl; @@ -20,13 +20,15 @@ use layers::platform::surface::NativeSurfaceMethods; use script_traits::CompositorEvent::{ClickEvent, MouseDownEvent, MouseMoveEvent, MouseUpEvent}; use script_traits::{ScriptControlChan, ConstellationControlMsg}; use servo_msg::compositor_msg::{Epoch, LayerId, ScrollPolicy}; +use servo_msg::constellation_msg::PipelineId; use std::num::Float; use std::num::FloatMath; use std::rc::Rc; pub struct CompositorData { - /// This layer's pipeline. BufferRequests and mouse events will be sent through this. - pub pipeline: CompositionPipeline, + /// This layer's pipeline id. The compositor can associate this id with an + /// actual CompositionPipeline. + pub pipeline_id: PipelineId, /// The ID of this layer within the pipeline. pub id: LayerId, @@ -47,13 +49,12 @@ pub struct CompositorData { } impl CompositorData { - pub fn new_layer(pipeline: CompositionPipeline, - layer_properties: LayerProperties, + pub fn new_layer(layer_properties: LayerProperties, wants_scroll_events: WantsScrollEventsFlag, tile_size: uint) -> Rc> { let new_compositor_data = CompositorData { - pipeline: pipeline, + pipeline_id: layer_properties.pipeline_id, id: layer_properties.id, wants_scroll_events: wants_scroll_events, scroll_policy: layer_properties.scroll_policy, @@ -68,20 +69,24 @@ impl CompositorData { } } -pub trait CompositorLayer { +pub trait CompositorLayer { fn update_layer_except_bounds(&self, layer_properties: LayerProperties); fn update_layer(&self, layer_properties: LayerProperties); - fn add_buffers(&self, new_buffers: Box, epoch: Epoch) -> bool; + fn add_buffers(&self, + compositor: &IOCompositor, + new_buffers: Box, + epoch: Epoch) + -> bool; /// Destroys all layer tiles, sending the buffers back to the painter to be destroyed or /// reused. - fn clear(&self); + fn clear(&self, compositor: &IOCompositor); /// Destroys tiles for this layer and all descendent layers, sending the buffers back to the /// painter to be destroyed or reused. - fn clear_all_tiles(&self); + fn clear_all_tiles(&self, compositor: &IOCompositor); /// Destroys all tiles of all layers, including children, *without* sending them back to the /// painter. You must call this only when the paint task is destined to be going down; @@ -104,10 +109,12 @@ pub trait CompositorLayer { // sends the event off to the appropriate pipeline. NB: the cursor position is in // page coordinates. fn send_mouse_event(&self, + compositor: &IOCompositor, event: MouseWindowEvent, cursor: TypedPoint2D); fn send_mouse_move_event(&self, + compositor: &IOCompositor, cursor: TypedPoint2D); fn clamp_scroll_offset_and_scroll_layer(&self, @@ -120,6 +127,9 @@ pub trait CompositorLayer { /// Return a flag describing how this layer deals with scroll events. fn wants_scroll_events(&self) -> WantsScrollEventsFlag; + + /// Return the pipeline id associated with this layer. + fn get_pipeline_id(&self) -> PipelineId; } #[deriving(Copy, PartialEq, Clone)] @@ -165,7 +175,7 @@ pub enum ScrollEventResult { ScrollPositionUnchanged, } -impl CompositorLayer for Layer { +impl CompositorLayer for Layer { fn update_layer_except_bounds(&self, layer_properties: LayerProperties) { self.extra_data.borrow_mut().epoch = layer_properties.epoch; self.extra_data.borrow_mut().scroll_policy = layer_properties.scroll_policy; @@ -190,33 +200,35 @@ impl CompositorLayer for Layer { // // If the epoch of the message does not match the layer's epoch, the message is ignored, the // layer buffer set is consumed, and None is returned. - fn add_buffers(&self, new_buffers: Box, epoch: Epoch) -> bool { + fn add_buffers(&self, + compositor: &IOCompositor, + new_buffers: Box, + epoch: Epoch) + -> bool { if self.extra_data.borrow().epoch != epoch { debug!("add_buffers: compositor epoch mismatch: {} != {}, id: {}", self.extra_data.borrow().epoch, epoch, - self.extra_data.borrow().pipeline.id); - let msg = PaintMsg::UnusedBuffer(new_buffers.buffers); - let _ = self.extra_data.borrow().pipeline.paint_chan.send_opt(msg); + self.get_pipeline_id()); + let pipeline = compositor.get_pipeline(self.get_pipeline_id()); + let _ = pipeline.paint_chan.send_opt(PaintMsg::UnusedBuffer(new_buffers.buffers)); return false; } - { - for buffer in new_buffers.buffers.into_iter().rev() { - self.add_buffer(buffer); - } + for buffer in new_buffers.buffers.into_iter().rev() { + self.add_buffer(buffer); + } - let unused_buffers = self.collect_unused_buffers(); - if !unused_buffers.is_empty() { // send back unused buffers - let msg = PaintMsg::UnusedBuffer(unused_buffers); - let _ = self.extra_data.borrow().pipeline.paint_chan.send_opt(msg); - } + let unused_buffers = self.collect_unused_buffers(); + if !unused_buffers.is_empty() { // send back unused buffers + let pipeline = compositor.get_pipeline(self.get_pipeline_id()); + let _ = pipeline.paint_chan.send_opt(PaintMsg::UnusedBuffer(unused_buffers)); } return true; } - fn clear(&self) { + fn clear(&self, compositor: &IOCompositor) { let mut buffers = self.collect_buffers(); if !buffers.is_empty() { @@ -227,16 +239,17 @@ impl CompositorLayer for Layer { buffer.mark_wont_leak() } - let _ = self.extra_data.borrow().pipeline.paint_chan.send_opt(PaintMsg::UnusedBuffer(buffers)); + let pipeline = compositor.get_pipeline(self.get_pipeline_id()); + let _ = pipeline.paint_chan.send_opt(PaintMsg::UnusedBuffer(buffers)); } } /// Destroys tiles for this layer and all descendent layers, sending the buffers back to the /// painter to be destroyed or reused. - fn clear_all_tiles(&self) { - self.clear(); + fn clear_all_tiles(&self, compositor: &IOCompositor) { + self.clear(compositor); for kid in self.children().iter() { - kid.clear_all_tiles(); + kid.clear_all_tiles(compositor); } } @@ -314,6 +327,7 @@ impl CompositorLayer for Layer { } fn send_mouse_event(&self, + compositor: &IOCompositor, event: MouseWindowEvent, cursor: TypedPoint2D) { let event_point = cursor.to_untyped(); @@ -325,15 +339,17 @@ impl CompositorLayer for Layer { MouseWindowEvent::MouseUp(button, _) => MouseUpEvent(button, event_point), }; - let pipeline = &self.extra_data.borrow().pipeline; + + let pipeline = compositor.get_pipeline(self.get_pipeline_id()); let ScriptControlChan(ref chan) = pipeline.script_chan; let _ = chan.send_opt(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message)); } fn send_mouse_move_event(&self, + compositor: &IOCompositor, cursor: TypedPoint2D) { let message = MouseMoveEvent(cursor.to_untyped()); - let pipeline = &self.extra_data.borrow().pipeline; + let pipeline = compositor.get_pipeline(self.get_pipeline_id()); let ScriptControlChan(ref chan) = pipeline.script_chan; let _ = chan.send_opt(ConstellationControlMsg::SendEvent(pipeline.id.clone(), message)); } @@ -362,4 +378,7 @@ impl CompositorLayer for Layer { self.extra_data.borrow().wants_scroll_events } + fn get_pipeline_id(&self) -> PipelineId { + self.extra_data.borrow().pipeline_id + } }