From c1823083504b91e909dfccc4cc87cb4ab90238db Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Thu, 28 Aug 2014 13:56:01 -0700 Subject: [PATCH 1/6] Store Compositor ReadyState per-pipeline Instead of storing a single ReadyState, store one per pipeline and track the earliest one. --- components/compositing/compositor.rs | 29 +++++++++++++++++------ components/compositing/compositor_task.rs | 6 ++--- components/msg/compositor_msg.rs | 4 ++-- components/script/page.rs | 2 +- components/script/script_task.rs | 5 ++-- 5 files changed, 31 insertions(+), 15 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 087e9b4773c..e098de519bb 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -21,6 +21,7 @@ use windowing::PinchZoomWindowEvent; use azure::azure_hl::SourceSurfaceMethods; use azure::azure_hl; +use std::cmp; use geom::matrix::identity; use geom::point::{Point2D, TypedPoint2D}; use geom::rect::Rect; @@ -100,8 +101,8 @@ pub struct IOCompositor { /// The time of the last zoom action has started. zoom_time: f64, - /// Current display/reflow status of the page - ready_state: ReadyState, + /// Current display/reflow status of each pipeline. + ready_states: HashMap, /// Whether the page being rendered has loaded completely. /// Differs from ReadyState because we can finish loading (ready) @@ -165,7 +166,7 @@ impl IOCompositor { viewport_zoom: ScaleFactor(1.0), zoom_action: false, zoom_time: 0f64, - ready_state: Blank, + ready_states: HashMap::new(), load_complete: false, constellation_chan: constellation_chan, time_profiler_chan: time_profiler_chan, @@ -275,9 +276,8 @@ impl IOCompositor { break; } - (Ok(ChangeReadyState(ready_state)), NotShuttingDown) => { - self.window.set_ready_state(ready_state); - self.ready_state = ready_state; + (Ok(ChangeReadyState(pipeline_id, ready_state)), NotShuttingDown) => { + self.change_ready_state(pipeline_id, ready_state); } (Ok(ChangeRenderState(render_state)), NotShuttingDown) => { @@ -333,6 +333,21 @@ impl IOCompositor { } } + fn change_ready_state(&mut self, pipeline_id: PipelineId, ready_state: ReadyState) { + self.ready_states.insert_or_update_with(pipeline_id, + ready_state, + |_key, value| *value = ready_state); + self.window.set_ready_state(self.get_earliest_pipeline_ready_state()); + } + + fn get_earliest_pipeline_ready_state(&self) -> ReadyState { + if self.ready_states.len() == 0 { + return Blank; + } + return self.ready_states.values().fold(FinishedLoading, |a, &b| cmp::min(a, b)); + + } + fn change_render_state(&mut self, render_state: RenderState) { self.window.set_render_state(render_state); if render_state == IdleRenderState { @@ -873,7 +888,7 @@ impl IOCompositor { // Render to PNG. We must read from the back buffer (ie, before // self.window.present()) as OpenGL ES 2 does not have glReadBuffer(). - if self.load_complete && self.ready_state == FinishedLoading + if self.load_complete && self.get_earliest_pipeline_ready_state() == FinishedLoading && self.opts.output_file.is_some() && !self.has_outstanding_render_msgs() { let (width, height) = (self.window_size.width.get(), self.window_size.height.get()); let path = from_str::(self.opts.output_file.get_ref().as_slice()).unwrap(); diff --git a/components/compositing/compositor_task.rs b/components/compositing/compositor_task.rs index be87ce9462e..dd98381ee3b 100644 --- a/components/compositing/compositor_task.rs +++ b/components/compositing/compositor_task.rs @@ -38,8 +38,8 @@ pub struct CompositorChan { /// Implementation of the abstract `ScriptListener` interface. impl ScriptListener for CompositorChan { - fn set_ready_state(&self, ready_state: ReadyState) { - let msg = ChangeReadyState(ready_state); + fn set_ready_state(&self, pipeline_id: PipelineId, ready_state: ReadyState) { + let msg = ChangeReadyState(pipeline_id, ready_state); self.chan.send(msg); } @@ -174,7 +174,7 @@ pub enum Msg { /// Requests that the compositor paint the given layer buffer set for the given page size. Paint(PipelineId, Epoch, Vec<(LayerId, Box)>), /// Alerts the compositor to the current status of page loading. - ChangeReadyState(ReadyState), + ChangeReadyState(PipelineId, ReadyState), /// Alerts the compositor to the current status of rendering. ChangeRenderState(RenderState), /// Alerts the compositor that the RenderMsg has been discarded. diff --git a/components/msg/compositor_msg.rs b/components/msg/compositor_msg.rs index 6008e4fca23..650c7fae71a 100644 --- a/components/msg/compositor_msg.rs +++ b/components/msg/compositor_msg.rs @@ -20,7 +20,7 @@ pub enum RenderState { RenderingRenderState, } -#[deriving(PartialEq, Clone)] +#[deriving(Eq, Ord, PartialEq, PartialOrd, Clone)] pub enum ReadyState { /// Informs the compositor that nothing has been done yet. Used for setting status Blank, @@ -107,7 +107,7 @@ pub trait RenderListener { /// The interface used by the script task to tell the compositor to update its ready state, /// which is used in displaying the appropriate message in the window's title. pub trait ScriptListener : Clone { - fn set_ready_state(&self, ReadyState); + fn set_ready_state(&self, PipelineId, ReadyState); fn scroll_fragment_point(&self, pipeline_id: PipelineId, layer_id: LayerId, diff --git a/components/script/page.rs b/components/script/page.rs index e61c3cd1a58..cf532783c8b 100644 --- a/components/script/page.rs +++ b/components/script/page.rs @@ -328,7 +328,7 @@ impl Page { self.join_layout(); // Tell the user that we're performing layout. - compositor.set_ready_state(PerformingLayout); + compositor.set_ready_state(self.id, PerformingLayout); // Layout will let us know when it's done. let (join_chan, join_port) = channel(); diff --git a/components/script/script_task.rs b/components/script/script_task.rs index e49dc600211..e743b9a098e 100644 --- a/components/script/script_task.rs +++ b/components/script/script_task.rs @@ -512,7 +512,8 @@ impl ScriptTask { let mut layout_join_port = page.layout_join_port.deref().borrow_mut(); *layout_join_port = None; } - self.compositor.set_ready_state(FinishedLoading); + + self.compositor.set_ready_state(pipeline_id, FinishedLoading); if page.pending_reflows.get() > 0 { page.pending_reflows.set(0); @@ -616,7 +617,7 @@ impl ScriptTask { let document = Document::new(&*window, Some(url.clone()), HTMLDocument, None).root(); window.deref().init_browser_context(&*document); - self.compositor.set_ready_state(Loading); + self.compositor.set_ready_state(pipeline_id, Loading); // Parse HTML. // // Note: We can parse the next document in parallel with any previous documents. From 0e8667946496b3a0a3d7c18f28c33371001a2de5 Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Thu, 28 Aug 2014 15:02:23 -0700 Subject: [PATCH 2/6] Store one RenderState per-pipeline in the Compositor This can later be used to decide whether the entire pipeline is ready for rendering. --- components/compositing/compositor.rs | 13 ++++++++++--- components/compositing/compositor_task.rs | 6 +++--- components/gfx/render_task.rs | 5 ++--- components/msg/compositor_msg.rs | 2 +- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index e098de519bb..da3ac32b189 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -104,6 +104,9 @@ pub struct IOCompositor { /// Current display/reflow status of each pipeline. ready_states: HashMap, + /// Current render status of each pipeline. + render_states: HashMap, + /// Whether the page being rendered has loaded completely. /// Differs from ReadyState because we can finish loading (ready) /// many times for a single page. @@ -167,6 +170,7 @@ impl IOCompositor { zoom_action: false, zoom_time: 0f64, ready_states: HashMap::new(), + render_states: HashMap::new(), load_complete: false, constellation_chan: constellation_chan, time_profiler_chan: time_profiler_chan, @@ -280,8 +284,8 @@ impl IOCompositor { self.change_ready_state(pipeline_id, ready_state); } - (Ok(ChangeRenderState(render_state)), NotShuttingDown) => { - self.change_render_state(render_state); + (Ok(ChangeRenderState(pipeline_id, render_state)), NotShuttingDown) => { + self.change_render_state(pipeline_id, render_state); } (Ok(RenderMsgDiscarded), NotShuttingDown) => { @@ -348,7 +352,10 @@ impl IOCompositor { } - fn change_render_state(&mut self, render_state: RenderState) { + fn change_render_state(&mut self, pipeline_id: PipelineId, render_state: RenderState) { + self.render_states.insert_or_update_with(pipeline_id, + render_state, + |_key, value| *value = render_state); self.window.set_render_state(render_state); if render_state == IdleRenderState { self.composite_ready = true; diff --git a/components/compositing/compositor_task.rs b/components/compositing/compositor_task.rs index dd98381ee3b..0f63b11d4b0 100644 --- a/components/compositing/compositor_task.rs +++ b/components/compositing/compositor_task.rs @@ -126,8 +126,8 @@ impl RenderListener for CompositorChan { self.chan.send(RenderMsgDiscarded); } - fn set_render_state(&self, render_state: RenderState) { - self.chan.send(ChangeRenderState(render_state)) + fn set_render_state(&self, pipeline_id: PipelineId, render_state: RenderState) { + self.chan.send(ChangeRenderState(pipeline_id, render_state)) } } @@ -176,7 +176,7 @@ pub enum Msg { /// Alerts the compositor to the current status of page loading. ChangeReadyState(PipelineId, ReadyState), /// Alerts the compositor to the current status of rendering. - ChangeRenderState(RenderState), + ChangeRenderState(PipelineId, RenderState), /// Alerts the compositor that the RenderMsg has been discarded. RenderMsgDiscarded, /// Sets the channel to the current layout and render tasks, along with their id diff --git a/components/gfx/render_task.rs b/components/gfx/render_task.rs index 2d7b8b5e2d7..8c41d8d1d26 100644 --- a/components/gfx/render_task.rs +++ b/components/gfx/render_task.rs @@ -234,9 +234,8 @@ impl RenderTask { continue; } - self.compositor.set_render_state(RenderingRenderState); - let mut replies = Vec::new(); + self.compositor.set_render_state(self.id, RenderingRenderState); for RenderRequest { buffer_requests, scale, layer_id, epoch } in requests.move_iter() { if self.epoch == epoch { @@ -246,7 +245,7 @@ impl RenderTask { } } - self.compositor.set_render_state(IdleRenderState); + self.compositor.set_render_state(self.id, IdleRenderState); debug!("render_task: returning surfaces"); self.compositor.paint(self.id, self.epoch, replies); diff --git a/components/msg/compositor_msg.rs b/components/msg/compositor_msg.rs index 650c7fae71a..30a882c2f0a 100644 --- a/components/msg/compositor_msg.rs +++ b/components/msg/compositor_msg.rs @@ -101,7 +101,7 @@ pub trait RenderListener { replies: Vec<(LayerId, Box)>); fn render_msg_discarded(&self); - fn set_render_state(&self, render_state: RenderState); + fn set_render_state(&self, PipelineId, RenderState); } /// The interface used by the script task to tell the compositor to update its ready state, From e17cc2d5e24555e961c0d71fd08d740bb2492cf3 Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Fri, 29 Aug 2014 16:49:08 -0700 Subject: [PATCH 3/6] Create root compositor layers for all frames eagerly Instead of waiting to create the root layer, create them as soon as the Compositor receives the frame tree. This allows the compositor to create a layer tree skeleton of which to hang the base layers when they are ready. --- components/compositing/compositor.rs | 115 ++++++++++++++++----------- 1 file changed, 67 insertions(+), 48 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index da3ac32b189..fe3092a5017 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -36,7 +36,7 @@ use layers::scene::Scene; use opengles::gl2; use png; use servo_msg::compositor_msg::{Blank, Epoch, FixedPosition, FinishedLoading, IdleRenderState}; -use servo_msg::compositor_msg::{LayerId, ReadyState, RenderState}; +use servo_msg::compositor_msg::{LayerId, ReadyState, RenderingRenderState, RenderState}; use servo_msg::constellation_msg::{ConstellationChan, ExitMsg, LoadUrlMsg, NavigateMsg}; use servo_msg::constellation_msg::{PipelineId, ResizedWindowMsg, WindowSizeData}; use servo_msg::constellation_msg; @@ -293,7 +293,9 @@ impl IOCompositor { } (Ok(SetIds(frame_tree, response_chan, new_constellation_chan)), _) => { - self.set_ids(frame_tree, response_chan, new_constellation_chan); + self.set_frame_tree(&frame_tree, + response_chan, + new_constellation_chan); } (Ok(GetGraphicsMetadata(chan)), NotShuttingDown) => { @@ -391,19 +393,52 @@ impl IOCompositor { } } - fn set_ids(&mut self, - frame_tree: SendableFrameTree, - response_chan: Sender<()>, - new_constellation_chan: ConstellationChan) { + fn set_frame_tree(&mut self, + frame_tree: &SendableFrameTree, + response_chan: Sender<()>, + new_constellation_chan: ConstellationChan) { response_chan.send(()); self.root_pipeline = Some(frame_tree.pipeline.clone()); + // If we have an old root layer, release all old tiles before replacing it. + match self.scene.root { + Some(ref mut layer) => CompositorData::clear_all_tiles(layer.clone()), + None => { } + } + self.scene.root = Some(self.create_frame_tree_root_layers(frame_tree)); + // Initialize the new constellation channel by sending it the root window size. self.constellation_chan = new_constellation_chan; self.send_window_size(); } + fn create_frame_tree_root_layers(&mut self, + frame_tree: &SendableFrameTree) + -> Rc> { + // Initialize the ReadyState and RenderState for this pipeline. + self.ready_states.insert(frame_tree.pipeline.id, Blank); + self.render_states.insert(frame_tree.pipeline.id, RenderingRenderState); + + let layer_properties = LayerProperties { + pipeline_id: frame_tree.pipeline.id, + epoch: Epoch(0), + id: LayerId::null(), + rect: Rect::zero(), + background_color: azure_hl::Color::new(0., 0., 0., 0.), + scroll_policy: FixedPosition, + }; + let root_layer = CompositorData::new_layer(frame_tree.pipeline.clone(), + layer_properties, + WantsScrollEvents, + self.opts.tile_size); + + for kid in frame_tree.children.iter() { + root_layer.add_child(self.create_frame_tree_root_layers(&kid.frame_tree)); + } + return root_layer; + } + fn find_layer_with_pipeline_and_layer_id(&self, pipeline_id: PipelineId, layer_id: LayerId) @@ -419,6 +454,13 @@ impl IOCompositor { } + fn find_pipeline_root_layer(&self, pipeline_id: PipelineId) -> Rc> { + match self.find_layer_with_pipeline_and_layer_id(pipeline_id, LayerId::null()) { + Some(ref layer) => layer.clone(), + None => fail!("Tried to create or update layer for unknown pipeline"), + } + } + fn update_layer_if_exists(&mut self, properties: LayerProperties) -> bool { match self.find_layer_with_pipeline_and_layer_id(properties.pipeline_id, properties.id) { Some(existing_layer) => { @@ -440,35 +482,20 @@ impl IOCompositor { let need_new_root_layer = !self.update_layer_if_exists(layer_properties); if need_new_root_layer { - let root_pipeline = match self.root_pipeline { - Some(ref root_pipeline) => root_pipeline.clone(), - None => fail!("Compositor: Making new layer without initialized pipeline"), - }; + let root_layer = self.find_pipeline_root_layer(layer_properties.pipeline_id); + CompositorData::update_layer(root_layer.clone(), layer_properties); - let root_properties = LayerProperties { - pipeline_id: root_pipeline.id, - epoch: layer_properties.epoch, - id: LayerId::null(), - rect: layer_properties.rect, - background_color: layer_properties.background_color, - scroll_policy: FixedPosition, - }; - let new_root = CompositorData::new_layer(root_pipeline.clone(), - root_properties, - WantsScrollEvents, - self.opts.tile_size); - let first_chid = CompositorData::new_layer(root_pipeline.clone(), - layer_properties, - DoesntWantScrollEvents, - self.opts.tile_size); - new_root.add_child(first_chid); + let root_layer_pipeline = root_layer.extra_data.borrow().pipeline.clone(); + let first_child = CompositorData::new_layer(root_layer_pipeline.clone(), + layer_properties, + DoesntWantScrollEvents, + self.opts.tile_size); - // Release all tiles from the layer before dropping it. - match self.scene.root { - Some(ref mut layer) => CompositorData::clear_all_tiles(layer.clone()), - None => { } - } - self.scene.root = Some(new_root); + // Add the first child / base layer to the front of the child list, so that + // child iframe layers are rendered on top of the base layer. These iframe + // layers were added previously when creating the layer tree skeleton in + // create_frame_tree_root_layers. + root_layer.children().insert(0, first_child); } self.scroll_layer_to_fragment_point_if_necessary(layer_properties.pipeline_id, @@ -487,21 +514,13 @@ impl IOCompositor { } fn create_descendant_layer(&self, layer_properties: LayerProperties) { - match self.scene.root { - Some(ref root_layer) => { - let root_layer_pipeline = root_layer.extra_data.borrow().pipeline.clone(); - if root_layer_pipeline.id != layer_properties.pipeline_id { - fail!("Compositor: New layer pipeline does not match root layer pipeline"); - } - - let new_layer = CompositorData::new_layer(root_layer_pipeline, - layer_properties, - DoesntWantScrollEvents, - root_layer.tile_size); - root_layer.add_child(new_layer); - } - None => fail!("Compositor: Received new layer without root layer") - } + let root_layer = self.find_pipeline_root_layer(layer_properties.pipeline_id); + let root_layer_pipeline = root_layer.extra_data.borrow().pipeline.clone(); + let new_layer = CompositorData::new_layer(root_layer_pipeline, + layer_properties, + DoesntWantScrollEvents, + root_layer.tile_size); + root_layer.add_child(new_layer); } fn send_window_size(&self) { From ff71c32218462f868cb5f7a07ad322418695c6db Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Fri, 29 Aug 2014 17:30:07 -0700 Subject: [PATCH 4/6] Remove SetLayerClipRect and add SetLayerOrigin The constallation has accurate information about iframe layer origins, but not their size. --- components/compositing/compositor.rs | 26 +++++++++++------------ components/compositing/compositor_task.rs | 6 ++---- components/compositing/constellation.rs | 8 +++---- components/compositing/headless.rs | 4 ++-- 4 files changed, 21 insertions(+), 23 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index fe3092a5017..02d1f9fc825 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -5,7 +5,7 @@ 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}; +use compositor_task::{SetLayerOrigin, Paint, ScrollFragmentPoint, LoadComplete}; use compositor_task::{ShutdownComplete, ChangeRenderState, RenderMsgDiscarded}; use constellation::SendableFrameTree; use events; @@ -312,8 +312,8 @@ impl IOCompositor { self.create_or_update_descendant_layer(layer_properties); } - (Ok(SetLayerClipRect(pipeline_id, layer_id, new_rect)), NotShuttingDown) => { - self.set_layer_clip_rect(pipeline_id, layer_id, new_rect); + (Ok(SetLayerOrigin(pipeline_id, layer_id, origin)), NotShuttingDown) => { + self.set_layer_origin(pipeline_id, layer_id, origin); } (Ok(Paint(pipeline_id, epoch, replies)), NotShuttingDown) => { @@ -558,17 +558,17 @@ impl IOCompositor { self.recomposite_if(needs_recomposite); } - fn set_layer_clip_rect(&mut self, - pipeline_id: PipelineId, - layer_id: LayerId, - new_rect_in_page_coordinates: Rect) { - let new_rect_in_layer_coordinates = - self.convert_page_rect_to_layer_coordinates(new_rect_in_page_coordinates); - let new_rect_in_layer_coordinates = Rect::from_untyped(&new_rect_in_layer_coordinates); - + fn set_layer_origin(&mut self, + pipeline_id: PipelineId, + layer_id: LayerId, + new_origin: Point2D) { + let new_origin_in_device_coordinates = new_origin * self.device_pixels_per_page_px().get(); match self.find_layer_with_pipeline_and_layer_id(pipeline_id, layer_id) { - Some(ref layer) => *layer.bounds.borrow_mut() = new_rect_in_layer_coordinates, - None => fail!("compositor received SetLayerClipRect for nonexistent layer"), + Some(ref layer) => { + layer.bounds.borrow_mut().origin = + Point2D::from_untyped(&new_origin_in_device_coordinates) + } + None => fail!("Compositor received SetLayerOrigin for nonexistent layer"), }; self.send_buffer_requests_for_all_layers(); diff --git a/components/compositing/compositor_task.rs b/components/compositing/compositor_task.rs index 0f63b11d4b0..14cf50bea75 100644 --- a/components/compositing/compositor_task.rs +++ b/components/compositing/compositor_task.rs @@ -117,8 +117,6 @@ impl RenderListener for CompositorChan { } else { self.chan.send(CreateOrUpdateDescendantLayer(layer_properties)); } - - self.chan.send(SetLayerClipRect(pipeline_id, metadata.id, layer_properties.rect)); } } @@ -167,8 +165,8 @@ pub enum Msg { /// Tells the compositor to create a descendant layer for a pipeline if necessary (i.e. if no /// layer with that ID exists). CreateOrUpdateDescendantLayer(LayerProperties), - /// Alerts the compositor that the specified layer's clipping rect has changed. - SetLayerClipRect(PipelineId, LayerId, Rect), + /// Alerts the compositor that the specified layer's origin has changed. + SetLayerOrigin(PipelineId, LayerId, Point2D), /// 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. diff --git a/components/compositing/constellation.rs b/components/compositing/constellation.rs index 0cf02f2ce35..add70aaf39c 100644 --- a/components/compositing/constellation.rs +++ b/components/compositing/constellation.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_task::{CompositorChan, LoadComplete, ShutdownComplete, SetLayerClipRect, SetIds}; +use compositor_task::{CompositorChan, LoadComplete, ShutdownComplete, SetLayerOrigin, SetIds}; use std::collections::hashmap::{HashMap, HashSet}; use geom::rect::{Rect, TypedRect}; use geom::scale_factor::ScaleFactor; @@ -507,9 +507,9 @@ impl Constellation { initial_viewport: rect.size * ScaleFactor(1.0), device_pixel_ratio: self.window_size.device_pixel_ratio, })); - self.compositor_chan.send(SetLayerClipRect(pipeline.id, - LayerId::null(), - rect.to_untyped())); + self.compositor_chan.send(SetLayerOrigin(pipeline.id, + LayerId::null(), + rect.to_untyped().origin)); } else { already_sent.insert(pipeline.id); } diff --git a/components/compositing/headless.rs b/components/compositing/headless.rs index d8d56e64d00..a1817560670 100644 --- a/components/compositing/headless.rs +++ b/components/compositing/headless.rs @@ -4,7 +4,7 @@ use compositor_task::{Msg, Exit, ChangeReadyState, SetIds}; use compositor_task::{GetGraphicsMetadata, CreateOrUpdateRootLayer, CreateOrUpdateDescendantLayer}; -use compositor_task::{SetLayerClipRect, Paint, ScrollFragmentPoint, LoadComplete}; +use compositor_task::{SetLayerOrigin, Paint, ScrollFragmentPoint, LoadComplete}; use compositor_task::{ShutdownComplete, ChangeRenderState, RenderMsgDiscarded}; use geom::scale_factor::ScaleFactor; @@ -90,7 +90,7 @@ impl NullCompositor { CreateOrUpdateRootLayer(..) | CreateOrUpdateDescendantLayer(..) | - SetLayerClipRect(..) | Paint(..) | + SetLayerOrigin(..) | Paint(..) | ChangeReadyState(..) | ChangeRenderState(..) | ScrollFragmentPoint(..) | LoadComplete(..) | RenderMsgDiscarded(..) => () } From 39e3a4af70efb236f9ef8797f586f3c95b66c02b Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 2 Sep 2014 12:49:57 -0700 Subject: [PATCH 5/6] Don't produce image output until all pipelines rendered something Instead of producing image output as soon as the first pipeline is ready, we wait to produce the output until all pipelines are in the idle RenderState. This should remove a race condition when running reference tests. --- components/compositing/compositor.rs | 46 +++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/components/compositing/compositor.rs b/components/compositing/compositor.rs index 02d1f9fc825..ea8bf447257 100644 --- a/components/compositing/compositor.rs +++ b/components/compositing/compositor.rs @@ -110,7 +110,7 @@ pub struct IOCompositor { /// Whether the page being rendered has loaded completely. /// Differs from ReadyState because we can finish loading (ready) /// many times for a single page. - load_complete: bool, + got_load_complete_message: bool, /// The command line option flags. opts: Opts, @@ -171,7 +171,7 @@ impl IOCompositor { zoom_time: 0f64, ready_states: HashMap::new(), render_states: HashMap::new(), - load_complete: false, + got_load_complete_message: false, constellation_chan: constellation_chan, time_profiler_chan: time_profiler_chan, memory_profiler_chan: memory_profiler_chan, @@ -328,7 +328,7 @@ impl IOCompositor { } (Ok(LoadComplete(..)), NotShuttingDown) => { - self.load_complete = true; + self.got_load_complete_message = true; } // When we are shutting_down, we need to avoid performing operations @@ -364,6 +364,13 @@ impl IOCompositor { } } + fn all_pipelines_in_idle_render_state(&self) -> bool { + if self.ready_states.len() == 0 { + return false; + } + return self.render_states.values().all(|&value| value == IdleRenderState); + } + fn has_render_msg_tracking(&self) -> bool { // only track RenderMsg's if the compositor outputs to a file. self.opts.output_file.is_some() @@ -707,7 +714,7 @@ impl IOCompositor { fn on_load_url_window_event(&mut self, url_string: String) { debug!("osmain: loading URL `{:s}`", url_string); - self.load_complete = false; + self.got_load_complete_message = false; let root_pipeline_id = match self.scene.root { Some(ref layer) => layer.extra_data.borrow().pipeline.id.clone(), None => fail!("Compositor: Received LoadUrlWindowEvent without initialized compositor \ @@ -894,6 +901,25 @@ impl IOCompositor { self.add_outstanding_render_msg(num_render_msgs_sent); } + fn is_ready_to_render_image_output(&self) -> bool { + if !self.got_load_complete_message { + return false; + } + + if self.get_earliest_pipeline_ready_state() != FinishedLoading { + return false; + } + + if self.has_outstanding_render_msgs() { + return false; + } + + if !self.all_pipelines_in_idle_render_state() { + return false; + } + return true; + } + fn composite(&mut self) { profile(time::CompositingCategory, self.time_profiler_chan.clone(), || { debug!("compositor: compositing"); @@ -912,10 +938,14 @@ impl IOCompositor { } }); - // Render to PNG. We must read from the back buffer (ie, before - // self.window.present()) as OpenGL ES 2 does not have glReadBuffer(). - if self.load_complete && self.get_earliest_pipeline_ready_state() == FinishedLoading - && self.opts.output_file.is_some() && !self.has_outstanding_render_msgs() { + if self.opts.output_file.is_some() { + // If we aren't ready to produce image output, we will wait until the next composite. + if !self.is_ready_to_render_image_output() { + return; + } + + // We must read from the back buffer (ie, before self.window.present()) as + // OpenGL ES 2 does not have glReadBuffer(). let (width, height) = (self.window_size.width.get(), self.window_size.height.get()); let path = from_str::(self.opts.output_file.get_ref().as_slice()).unwrap(); let mut pixels = gl2::read_pixels(0, 0, From 6d0e103cdefd8a7b08e7f7f87731493db7b4703c Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Fri, 29 Aug 2014 17:35:30 -0700 Subject: [PATCH 6/6] Update and expand iframe tests Add iframe tests to their own subdirectory and add another test case that used to trigger a fatal error. The new test case uses the "allow-scripts" sandbox attribute to work around a script task failure caused by the child frame sharing the same script task as the parent. --- tests/ref/basic.list | 4 +++- tests/ref/iframe/multiple_external.html | 17 +++++++++++++++++ tests/ref/iframe/multiple_external_child.html | 5 +++++ tests/ref/iframe/multiple_external_ref.html | 5 +++++ tests/ref/iframe/simple.html | 9 +++++++++ .../simple_ref.html} | 0 tests/ref/simple_iframe.html | 7 ------- 7 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 tests/ref/iframe/multiple_external.html create mode 100644 tests/ref/iframe/multiple_external_child.html create mode 100644 tests/ref/iframe/multiple_external_ref.html create mode 100644 tests/ref/iframe/simple.html rename tests/ref/{simple_iframe_ref.html => iframe/simple_ref.html} (100%) delete mode 100644 tests/ref/simple_iframe.html diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 40df098bd79..cfb9e4d8f8a 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -36,7 +36,6 @@ == background_external_stylesheet.html background_ref.html == block_image.html 500x300_green.html != block_image.html noteq_500x300_white.html -# == simple_iframe.html simple_iframe_ref.html -- disabled due to iframe crashiness == object_element_a.html object_element_b.html == append_style_a.html append_style_b.html == height_compute_reset.html height_compute.html @@ -116,3 +115,6 @@ flaky_gpu,flaky_linux == acid2_noscroll.html acid2_ref_broken.html # The following tests fails the ref-tests #== multiple_css_class_a.html multiple_css_class_b.html + +== iframe/simple.html iframe/simple_ref.html +== iframe/multiple_external.html iframe/multiple_external_ref.html diff --git a/tests/ref/iframe/multiple_external.html b/tests/ref/iframe/multiple_external.html new file mode 100644 index 00000000000..870bef5ba2d --- /dev/null +++ b/tests/ref/iframe/multiple_external.html @@ -0,0 +1,17 @@ + + + + + + + + + + diff --git a/tests/ref/iframe/multiple_external_child.html b/tests/ref/iframe/multiple_external_child.html new file mode 100644 index 00000000000..8e1b304226d --- /dev/null +++ b/tests/ref/iframe/multiple_external_child.html @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/ref/iframe/multiple_external_ref.html b/tests/ref/iframe/multiple_external_ref.html new file mode 100644 index 00000000000..8e1b304226d --- /dev/null +++ b/tests/ref/iframe/multiple_external_ref.html @@ -0,0 +1,5 @@ + + + + + diff --git a/tests/ref/iframe/simple.html b/tests/ref/iframe/simple.html new file mode 100644 index 00000000000..3625f863b0b --- /dev/null +++ b/tests/ref/iframe/simple.html @@ -0,0 +1,9 @@ + + +
+ +
+ + diff --git a/tests/ref/simple_iframe_ref.html b/tests/ref/iframe/simple_ref.html similarity index 100% rename from tests/ref/simple_iframe_ref.html rename to tests/ref/iframe/simple_ref.html diff --git a/tests/ref/simple_iframe.html b/tests/ref/simple_iframe.html deleted file mode 100644 index 08c7de8847f..00000000000 --- a/tests/ref/simple_iframe.html +++ /dev/null @@ -1,7 +0,0 @@ - - - - -