diff --git a/src/components/compositing/compositor.rs b/src/components/compositing/compositor.rs index 72dc10c3209..f92a2920531 100644 --- a/src/components/compositing/compositor.rs +++ b/src/components/compositing/compositor.rs @@ -348,12 +348,16 @@ impl IOCompositor { fn set_unrendered_color(&mut self, pipeline_id: PipelineId, layer_id: LayerId, color: Color) { match self.scene.root { - Some(ref layer) => CompositorData::set_unrendered_color(layer.clone(), - pipeline_id, - layer_id, - color), - None => false, - }; + Some(ref root_layer) => { + match CompositorData::find_layer_with_pipeline_and_layer_id(root_layer.clone(), + pipeline_id, + layer_id) { + Some(ref layer) => CompositorData::set_unrendered_color(layer.clone(), color), + None => { } + } + } + None => { } + } } fn set_ids(&mut self, @@ -390,7 +394,6 @@ impl IOCompositor { }; if layer_id != root_layer_id { - let root_pipeline_id = root_pipeline.id; let new_compositor_data = CompositorData::new_root(root_pipeline, size, self.opts.cpu_painting); @@ -399,18 +402,16 @@ impl IOCompositor { new_compositor_data)); new_root.extra_data.borrow_mut().unrendered_color = unrendered_color; - let parent_layer_id = new_root.extra_data.borrow().id; - assert!(CompositorData::add_child_if_necessary(new_root.clone(), - root_pipeline_id, - parent_layer_id, - layer_id, - Rect(Point2D(0f32, 0f32), size), - size, - Scrollable)); + CompositorData::add_child_if_necessary(new_root.clone(), + layer_id, + Rect(Point2D(0f32, 0f32), size), + size, + Scrollable); // Release all tiles from the layer before dropping it. - for layer in self.scene.root.mut_iter() { - CompositorData::clear_all_tiles(layer.clone()); + match self.scene.root { + Some(ref mut layer) => CompositorData::clear_all_tiles(layer.clone()), + None => { } } self.scene.root = Some(new_root); } @@ -424,19 +425,26 @@ impl IOCompositor { rect: Rect, scroll_policy: ScrollPolicy) { match self.scene.root { - Some(ref root) => { - let parent_layer_id = root.extra_data.borrow().id; - let page_size = root.extra_data.borrow().page_size.unwrap(); - assert!(CompositorData::add_child_if_necessary(root.clone(), - pipeline_id, - parent_layer_id, + Some(ref root_layer) => { + let parent_layer_id = root_layer.extra_data.borrow().id; + match CompositorData::find_layer_with_pipeline_and_layer_id(root_layer.clone(), + pipeline_id, + parent_layer_id) { + Some(ref mut parent_layer) => { + let page_size = root_layer.extra_data.borrow().page_size.unwrap(); + CompositorData::add_child_if_necessary(parent_layer.clone(), layer_id, rect, page_size, - scroll_policy)) + scroll_policy); + } + None => { + fail!("Compositor: couldn't find parent layer"); + } + } } - None => fail!("Compositor: Received new layer without initialized pipeline"), - }; + None => fail!("Compositor: Received new layer without initialized pipeline") + } self.ask_for_tiles(); } @@ -522,14 +530,24 @@ impl IOCompositor { new_layer_buffer_set.mark_will_leak(); match self.scene.root { - Some(ref layer) => { - assert!(CompositorData::add_buffers(layer.clone(), - &self.graphics_context, - pipeline_id, - layer_id, - new_layer_buffer_set, - epoch).is_none()); - self.recomposite = true; + Some(ref root_layer) => { + match CompositorData::find_layer_with_pipeline_and_layer_id(root_layer.clone(), + pipeline_id, + layer_id) { + Some(ref layer) => { + assert!(CompositorData::add_buffers(layer.clone(), + &self.graphics_context, + new_layer_buffer_set, + epoch)); + self.recomposite = true; + } + None => { + // FIXME: This may potentially be triggered by a race condition where a + // buffers are being rendered but the layer is removed before rendering + // completes. + fail!("compositor given paint command for non-existent layer"); + } + } } None => { fail!("compositor given paint command with no root layer initialized"); @@ -681,11 +699,14 @@ impl IOCompositor { let page_cursor = cursor.as_f32() / scale; let page_window = self.page_window(); let mut scroll = false; - for layer in self.scene.root.mut_iter() { - scroll = CompositorData::handle_scroll_event(layer.clone(), - page_delta, - page_cursor, - page_window) || scroll; + match self.scene.root { + Some(ref mut layer) => { + scroll = CompositorData::handle_scroll_event(layer.clone(), + page_delta, + page_cursor, + page_window) || scroll; + } + None => { } } self.recomposite_if(scroll); self.ask_for_tiles(); @@ -735,11 +756,14 @@ impl IOCompositor { let page_cursor = TypedPoint2D(-1f32, -1f32); // Make sure this hits the base layer let page_window = self.page_window(); - for layer in self.scene.root.mut_iter() { - CompositorData::handle_scroll_event(layer.clone(), - page_delta, - page_cursor, - page_window); + match self.scene.root { + Some(ref mut layer) => { + CompositorData::handle_scroll_event(layer.clone(), + page_delta, + page_cursor, + page_window); + } + None => { } } self.recomposite = true; @@ -758,17 +782,20 @@ impl IOCompositor { fn ask_for_tiles(&mut self) { let scale = self.device_pixels_per_page_px(); let page_window = self.page_window(); - for layer in self.scene.root.mut_iter() { - if !layer.extra_data.borrow().hidden { + match self.scene.root { + Some(ref mut layer) if !layer.extra_data.borrow().hidden => { let rect = Rect(Point2D(0f32, 0f32), page_window.to_untyped()); - let recomposite = CompositorData::get_buffer_request(layer.clone(), + let recomposite = + CompositorData::send_buffer_requests_recursively(layer.clone(), &self.graphics_context, rect, scale.get()); self.recomposite = self.recomposite || recomposite; - } else { + } + Some(_) => { debug!("Compositor: root layer is hidden!"); } + None => { } } } diff --git a/src/components/compositing/compositor_data.rs b/src/components/compositing/compositor_data.rs index e0965aab059..86d4fa2aeb0 100644 --- a/src/components/compositing/compositor_data.rs +++ b/src/components/compositing/compositor_data.rs @@ -146,38 +146,18 @@ impl CompositorData { /// Adds a child layer to the layer with the given ID and the given pipeline, if it doesn't /// exist yet. The child layer will have the same pipeline, tile size, memory limit, and CPU /// painting status as its parent. - /// - /// Returns: - /// * True if the layer was added; - /// * True if the layer was not added because it already existed; - /// * False if the layer could not be added because no suitable parent layer with the given - /// ID and pipeline could be found. pub fn add_child_if_necessary(layer: Rc>, - pipeline_id: PipelineId, - parent_layer_id: LayerId, child_layer_id: LayerId, rect: Rect, page_size: Size2D, - scroll_policy: ScrollPolicy) -> bool { - if layer.extra_data.borrow().pipeline.id != pipeline_id || - layer.extra_data.borrow().id != parent_layer_id { - return layer.children().iter().any(|kid| { - CompositorData::add_child_if_necessary(kid.clone(), - pipeline_id, - parent_layer_id, - child_layer_id, - rect, - page_size, - scroll_policy) - }) - } - + scroll_policy: ScrollPolicy) { // See if we've already made this child layer. + let pipeline_id = layer.extra_data.borrow().pipeline.id; if layer.children().iter().any(|kid| { kid.extra_data.borrow().pipeline.id == pipeline_id && kid.extra_data.borrow().id == child_layer_id }) { - return true + return; } let new_compositor_data = CompositorData::new(layer.extra_data.borrow().pipeline.clone(), @@ -197,8 +177,6 @@ impl CompositorData { // Place the kid's layer in the container passed in. Layer::add_child(layer.clone(), new_kid.clone()); - - true } /// Move the layer's descendants that don't want scroll events and scroll by a relative @@ -338,11 +316,11 @@ 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_request(layer: Rc>, - graphics_context: &NativeCompositingGraphicsContext, - window_rect: Rect, - scale: f32) - -> bool { + pub fn send_buffer_requests_recursively(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 { @@ -366,7 +344,7 @@ impl CompositorData { CompositorData::build_layer_tree(layer.clone(), graphics_context); } - let get_child_buffer_request = |kid: &Rc>| -> bool { + let send_child_buffer_request = |kid: &Rc>| -> bool { match kid.extra_data.borrow().scissor { Some(scissor) => { let mut new_rect = window_rect; @@ -380,10 +358,10 @@ impl CompositorData { // to make the child_rect appear in coordinates local to it. let child_rect = Rect(new_rect.origin.sub(&scissor.origin), new_rect.size); - CompositorData::get_buffer_request(kid.clone(), - graphics_context, - child_rect, - scale) + CompositorData::send_buffer_requests_recursively(kid.clone(), + graphics_context, + child_rect, + scale) } None => { false // Layer is offscreen @@ -395,7 +373,7 @@ impl CompositorData { }; layer.children().iter().filter(|x| !x.extra_data.borrow().hidden) - .map(get_child_buffer_request) + .map(send_child_buffer_request) .any(|b| b) || redisplay } @@ -409,7 +387,7 @@ impl CompositorData { new_rect: Rect) -> bool { debug!("compositor_data: starting set_clipping_rect()"); - match CompositorData::find_child_with_layer_and_pipeline_id(layer.clone(), + match CompositorData::find_child_with_pipeline_and_layer_id(layer.clone(), pipeline_id, layer_id) { Some(child_node) => { @@ -561,7 +539,7 @@ impl CompositorData { - fn find_child_with_layer_and_pipeline_id(layer: Rc>, + fn find_child_with_pipeline_and_layer_id(layer: Rc>, pipeline_id: PipelineId, layer_id: LayerId) -> Option>> { @@ -574,6 +552,27 @@ impl CompositorData { return None } + pub fn find_layer_with_pipeline_and_layer_id(layer: Rc>, + pipeline_id: PipelineId, + layer_id: LayerId) + -> Option>> { + if layer.extra_data.borrow().pipeline.id == pipeline_id && + layer.extra_data.borrow().id == layer_id { + return Some(layer.clone()); + } + + for kid in layer.children().iter() { + match CompositorData::find_layer_with_pipeline_and_layer_id(kid.clone(), + pipeline_id, + layer_id) { + v @ Some(_) => { return v; } + None => { } + } + } + + return None; + } + // A helper method to resize sublayers. fn resize_helper(layer: Rc>, pipeline_id: PipelineId, @@ -583,7 +582,7 @@ impl CompositorData { -> bool { debug!("compositor_data: starting resize_helper()"); - let found = match CompositorData::find_child_with_layer_and_pipeline_id(layer.clone(), + let found = match CompositorData::find_child_with_pipeline_and_layer_id(layer.clone(), pipeline_id, layer_id) { Some(child) => { @@ -676,33 +675,9 @@ impl CompositorData { // layer buffer set is consumed, and None is returned. pub fn add_buffers(layer: Rc>, graphics_context: &NativeCompositingGraphicsContext, - pipeline_id: PipelineId, - layer_id: LayerId, - mut new_buffers: Box, + new_buffers: Box, epoch: Epoch) - -> Option> { - debug!("compositor_data: starting add_buffers()"); - if layer.extra_data.borrow().pipeline.id != pipeline_id || - layer.extra_data.borrow().id != layer_id { - // ID does not match ours, so recurse on descendents (including hidden children). - for child_layer in layer.children().iter() { - match CompositorData::add_buffers(child_layer.clone(), - graphics_context, - pipeline_id, - layer_id, - new_buffers, - epoch) { - None => return None, - Some(buffers) => new_buffers = buffers, - } - } - - // Not found. Give the caller the buffers back. - return Some(new_buffers) - } - - debug!("compositor_data: layers found for add_buffers()"); - + -> bool { if layer.extra_data.borrow().epoch != epoch { debug!("add_buffers: compositor epoch mismatch: {:?} != {:?}, id: {:?}", layer.extra_data.borrow().epoch, @@ -710,7 +685,7 @@ impl CompositorData { layer.extra_data.borrow().pipeline.id); let msg = UnusedBufferMsg(new_buffers.buffers); let _ = layer.extra_data.borrow().pipeline.render_chan.send_opt(msg); - return None + return false; } { @@ -725,7 +700,7 @@ impl CompositorData { } CompositorData::build_layer_tree(layer.clone(), graphics_context); - None + return true; } // Recursively sets occluded portions of quadtrees to Hidden, so that they do not ask for @@ -792,26 +767,8 @@ impl CompositorData { } } - pub fn set_unrendered_color(layer: Rc>, - pipeline_id: PipelineId, - layer_id: LayerId, - color: Color) - -> bool { - if layer.extra_data.borrow().pipeline.id != pipeline_id || - layer.extra_data.borrow().id != layer_id { - for child_layer in layer.children().iter() { - if CompositorData::set_unrendered_color(child_layer.clone(), - pipeline_id, - layer_id, - color) { - return true; - } - } - return false; - } - + pub fn set_unrendered_color(layer: Rc>, color: Color) { layer.extra_data.borrow_mut().unrendered_color = color; - return true; } }