From 4870b4bc1cb972217b36011ac1245cbe5614a41e Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sat, 11 Apr 2015 11:02:32 -0500 Subject: [PATCH] servo: Merge #5634 - Improve scrolling performance (from pcwalton:fix-scrolling-perf); r=metajack This goes hand in hand with servo/glutin#18 to get our scrolling performance back on Mac. r? @glennw Source-Repo: https://github.com/servo/servo Source-Revision: f017a4459c4d1067dec74ce1bf00cc6ce0612859 --- servo/components/compositing/compositor.rs | 126 +++++++++++------- .../components/compositing/compositor_task.rs | 2 + servo/components/compositing/headless.rs | 1 + servo/ports/glutin/window.rs | 11 +- 4 files changed, 89 insertions(+), 51 deletions(-) diff --git a/servo/components/compositing/compositor.rs b/servo/components/compositing/compositor.rs index 387a98e35eaa..7ddfd045b783 100644 --- a/servo/components/compositing/compositor.rs +++ b/servo/components/compositing/compositor.rs @@ -82,6 +82,8 @@ pub struct IOCompositor { /// The device pixel ratio for this window. hidpi_factor: ScaleFactor, + channel_to_self: Box, + /// A handle to the scrolling timer. scrolling_timer: ScrollingTimerProxy, @@ -140,7 +142,7 @@ pub struct ScrollEvent { enum CompositionRequest { NoCompositingNecessary, CompositeOnScrollTimeout(u64), - CompositeNow, + CompositeNow(CompositingReason), } #[derive(Copy, PartialEq, Debug)] @@ -206,6 +208,7 @@ impl IOCompositor { }), window_size: window_size, hidpi_factor: hidpi_factor, + channel_to_self: sender.clone_compositor_proxy(), scrolling_timer: ScrollingTimerProxy::new(sender), composition_request: CompositionRequest::NoCompositingNecessary, pending_scroll_events: Vec::new(), @@ -295,9 +298,7 @@ impl IOCompositor { (Msg::SetFrameTree(frame_tree, response_chan, new_constellation_chan), ShutdownState::NotShuttingDown) => { - self.set_frame_tree(&frame_tree, - response_chan, - new_constellation_chan); + self.set_frame_tree(&frame_tree, response_chan, new_constellation_chan); self.send_viewport_rects_for_all_layers(); self.get_title_for_main_frame(); } @@ -323,7 +324,10 @@ impl IOCompositor { (Msg::AssignPaintedBuffers(pipeline_id, epoch, replies), ShutdownState::NotShuttingDown) => { for (layer_id, new_layer_buffer_set) in replies.into_iter() { - self.assign_painted_buffers(pipeline_id, layer_id, new_layer_buffer_set, epoch); + self.assign_painted_buffers(pipeline_id, + layer_id, + new_layer_buffer_set, + epoch); } self.remove_outstanding_paint_msg(); } @@ -338,7 +342,7 @@ impl IOCompositor { // If we're painting in headless mode, schedule a recomposite. if opts::get().output_file.is_some() { - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::Headless); } // Inform the embedder that the load has finished. @@ -352,13 +356,19 @@ impl IOCompositor { match self.composition_request { CompositionRequest::CompositeOnScrollTimeout(this_timestamp) => { if timestamp == this_timestamp { - self.composition_request = CompositionRequest::CompositeNow + self.composition_request = CompositionRequest::CompositeNow( + CompositingReason::HitScrollTimeout) } } _ => {} } } + (Msg::RecompositeAfterScroll, ShutdownState::NotShuttingDown) => { + self.composition_request = + CompositionRequest::CompositeNow(CompositingReason::ContinueScroll) + } + (Msg::KeyEvent(key, state, modified), ShutdownState::NotShuttingDown) => { if state == KeyState::Pressed { self.window.handle_key(key, modified); @@ -390,7 +400,7 @@ impl IOCompositor { // If we're painting in headless mode, schedule a recomposite. if opts::get().output_file.is_some() { - self.composite_if_necessary() + self.composite_if_necessary(CompositingReason::Headless) } } @@ -417,7 +427,7 @@ impl IOCompositor { self.get_or_create_pipeline_details(pipeline_id).animations_running = animations_running; if animations_running { - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::Animation); } } @@ -526,7 +536,7 @@ impl IOCompositor { self.send_window_size(); self.got_set_frame_tree_message = true; - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::NewFrameTree); } fn create_root_layer_for_pipeline_and_rect(&mut self, @@ -571,7 +581,8 @@ impl IOCompositor { return root_layer; } - fn find_pipeline_root_layer(&self, pipeline_id: PipelineId) -> Option>> { + fn find_pipeline_root_layer(&self, pipeline_id: PipelineId) + -> Option>> { if !self.pipeline_details.contains_key(&pipeline_id) { panic!("Tried to create or update layer for unknown pipeline") } @@ -674,22 +685,19 @@ impl IOCompositor { fn scroll_layer_to_fragment_point_if_necessary(&mut self, pipeline_id: PipelineId, layer_id: LayerId) { - match self.fragment_point.take() { - Some(point) => { - if !self.move_layer(pipeline_id, layer_id, Point2D::from_untyped(&point)) { - panic!("Compositor: Tried to scroll to fragment with unknown layer."); - } - - self.start_scrolling_timer_if_necessary(); + if let Some(point) = self.fragment_point.take() { + if !self.move_layer(pipeline_id, layer_id, Point2D::from_untyped(&point)) { + panic!("Compositor: Tried to scroll to fragment with unknown layer."); } - None => {} + + self.start_scrolling_timer_if_necessary(); } } fn start_scrolling_timer_if_necessary(&mut self) { match self.composition_request { - CompositionRequest::CompositeNow | CompositionRequest::CompositeOnScrollTimeout(_) => - return, + CompositionRequest::CompositeNow(_) | + CompositionRequest::CompositeOnScrollTimeout(_) => return, CompositionRequest::NoCompositingNecessary => {} } @@ -743,7 +751,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(self, new_layer_buffer_set, epoch)); - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::NewPaintedBuffers); } fn scroll_fragment_to_point(&mut self, @@ -751,11 +759,9 @@ impl IOCompositor { layer_id: LayerId, point: Point2D) { if self.move_layer(pipeline_id, layer_id, Point2D::from_untyped(&point)) { - if self.send_buffer_requests_for_all_layers() { - self.start_scrolling_timer_if_necessary(); - } + self.perform_updates_after_scroll() } else { - self.fragment_point = Some(point); + self.fragment_point = Some(point) } } @@ -881,7 +887,7 @@ impl IOCompositor { cursor: cursor, }); - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::Scroll); } fn process_pending_scroll_events(&mut self) { @@ -895,8 +901,7 @@ impl IOCompositor { layer.handle_scroll_event(delta, cursor); } - self.start_scrolling_timer_if_necessary(); - self.send_buffer_requests_for_all_layers(); + self.perform_updates_after_scroll(); } if had_scroll_events { @@ -904,6 +909,16 @@ impl IOCompositor { } } + /// Performs buffer requests and starts the scrolling timer or schedules a recomposite as + /// necessary. + fn perform_updates_after_scroll(&mut self) { + if self.send_buffer_requests_for_all_layers() { + self.start_scrolling_timer_if_necessary(); + } else { + self.channel_to_self.send(Msg::RecompositeAfterScroll); + } + } + /// If there are any animations running, dispatches appropriate messages to the constellation. fn process_animations(&mut self) { for (pipeline_id, pipeline_details) in self.pipeline_details.iter() { @@ -969,7 +984,7 @@ impl IOCompositor { } self.send_viewport_rects_for_all_layers(); - self.composite_if_necessary(); + self.composite_if_necessary(CompositingReason::Zoom); } fn on_navigation_window_event(&self, direction: WindowNavigateMsg) { @@ -1143,19 +1158,15 @@ impl IOCompositor { origin: Point2D::zero(), size: self.window_size.as_f32(), }; - // paint the scene. - match self.scene.root { - Some(ref layer) => { - match self.context { - None => { - debug!("compositor: not compositing because context not yet set up") - } - Some(context) => { - rendergl::render_scene(layer.clone(), context, &self.scene); - } + + // Paint the scene. + if let Some(ref layer) = self.scene.root { + match self.context { + Some(context) => rendergl::render_scene(layer.clone(), context, &self.scene), + None => { + debug!("compositor: not compositing because context not yet set up") } } - None => {} } }); @@ -1205,9 +1216,9 @@ impl IOCompositor { self.process_animations(); } - fn composite_if_necessary(&mut self) { + fn composite_if_necessary(&mut self, reason: CompositingReason) { if self.composition_request == CompositionRequest::NoCompositingNecessary { - self.composition_request = CompositionRequest::CompositeNow + self.composition_request = CompositionRequest::CompositeNow(reason) } } @@ -1342,8 +1353,11 @@ impl CompositorEventListener for IOCompositor where Window: Wind } match self.composition_request { - CompositionRequest::NoCompositingNecessary | CompositionRequest::CompositeOnScrollTimeout(_) => {} - CompositionRequest::CompositeNow => self.composite(), + CompositionRequest::NoCompositingNecessary | + CompositionRequest::CompositeOnScrollTimeout(_) => {} + CompositionRequest::CompositeNow(_) => { + self.composite() + } } self.shutdown_state != ShutdownState::FinishedShuttingDown @@ -1401,3 +1415,25 @@ impl CompositorEventListener for IOCompositor where Window: Wind chan.send(ConstellationMsg::GetPipelineTitle(root_pipeline_id)).unwrap(); } } + +/// Why we performed a composite. This is used for debugging. +#[derive(Copy, Clone, PartialEq)] +pub enum CompositingReason { + /// We hit the scroll timeout and are therefore drawing unrendered content. + HitScrollTimeout, + /// The window has been scrolled and we're starting the first recomposite. + Scroll, + /// A scroll has continued and we need to recomposite again. + ContinueScroll, + /// We're performing the single composite in headless mode. + Headless, + /// We're performing a composite to run an animation. + Animation, + /// A new frame tree has been loaded. + NewFrameTree, + /// New painted buffers have been received. + NewPaintedBuffers, + /// The window has been zoomed. + Zoom, +} + diff --git a/servo/components/compositing/compositor_task.rs b/servo/components/compositing/compositor_task.rs index c951f589965a..7b08b2cb249a 100644 --- a/servo/components/compositing/compositor_task.rs +++ b/servo/components/compositing/compositor_task.rs @@ -212,6 +212,7 @@ pub enum Msg { /// Indicates that the scrolling timeout with the given starting timestamp has happened and a /// composite should happen. (See the `scrolling` module.) ScrollTimeout(u64), + RecompositeAfterScroll, /// Sends an unconsumed key event back to the compositor. KeyEvent(Key, KeyState, KeyModifiers), /// Changes the cursor. @@ -240,6 +241,7 @@ impl Debug for Msg { Msg::SetFrameTree(..) => write!(f, "SetFrameTree"), Msg::LoadComplete => write!(f, "LoadComplete"), Msg::ScrollTimeout(..) => write!(f, "ScrollTimeout"), + Msg::RecompositeAfterScroll => write!(f, "RecompositeAfterScroll"), Msg::KeyEvent(..) => write!(f, "KeyEvent"), Msg::SetCursor(..) => write!(f, "SetCursor"), Msg::PaintTaskExited(..) => write!(f, "PaintTaskExited"), diff --git a/servo/components/compositing/headless.rs b/servo/components/compositing/headless.rs index 72a5b0c961e7..46f8351ba21e 100644 --- a/servo/components/compositing/headless.rs +++ b/servo/components/compositing/headless.rs @@ -103,6 +103,7 @@ impl CompositorEventListener for NullCompositor { Msg::LoadComplete | Msg::PaintMsgDiscarded(..) | Msg::ScrollTimeout(..) | + Msg::RecompositeAfterScroll | Msg::ChangePageTitle(..) | Msg::ChangePageUrl(..) | Msg::KeyEvent(..) | diff --git a/servo/ports/glutin/window.rs b/servo/ports/glutin/window.rs index ea4858de7b71..1732833e2460 100644 --- a/servo/ports/glutin/window.rs +++ b/servo/ports/glutin/window.rs @@ -190,11 +190,11 @@ impl Window { } } else { let dx = 0.0; - let dy = (delta as f32) * 30.0; + let dy = delta as f32; self.scroll_window(dx, dy); } }, - Event::Refresh | Event::Awakened => { + Event::Refresh => { self.event_queue.borrow_mut().push(WindowEvent::Refresh); } _ => {} @@ -218,7 +218,7 @@ impl Window { fn scroll_window(&self, dx: f32, dy: f32) { let mouse_pos = self.mouse_pos.get(); let event = WindowEvent::Scroll(TypedPoint2D(dx as f32, dy as f32), - TypedPoint2D(mouse_pos.x as i32, mouse_pos.y as i32)); + TypedPoint2D(mouse_pos.x as i32, mouse_pos.y as i32)); self.event_queue.borrow_mut().push(event); } @@ -683,9 +683,8 @@ impl CompositorProxy for GlutinCompositorProxy { fn send(&mut self, msg: compositor_task::Msg) { // Send a message and kick the OS event loop awake. self.sender.send(msg).unwrap(); - match self.window_proxy { - Some(ref window_proxy) => window_proxy.wakeup_event_loop(), - None => {} + if let Some(ref window_proxy) = self.window_proxy { + window_proxy.wakeup_event_loop() } } fn clone_compositor_proxy(&self) -> Box {