From ab6774087b835046a0039c48df325229853754ef Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 29 Jun 2016 16:45:00 -0700 Subject: [PATCH 1/3] Make the epsilon for the spring simulation larger. 0.001 is way too small and results in lots of unnecessary frames of bounce animation. --- src/spring.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spring.rs b/src/spring.rs index 5e655aca97..1fcbdfec4b 100644 --- a/src/spring.rs +++ b/src/spring.rs @@ -5,7 +5,7 @@ use euclid::Point2D; /// Some arbitrarily small positive number used as threshold value. -pub const EPSILON: f32 = 0.001; +pub const EPSILON: f32 = 0.1; /// The default stiffness factor. pub const STIFFNESS: f32 = 0.2; From 6e9d1e3bff25a94ec2c55bdc2c83c560fd501ad9 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 29 Jun 2016 16:46:08 -0700 Subject: [PATCH 2/3] Don't process fling events after the bounce animation has started. Before, the animation could incorrectly restart if a fling lasted longer than the bounce animation took to run. --- src/frame.rs | 5 +++-- src/layer.rs | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/frame.rs b/src/frame.rs index be449021d3..a137d9541c 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -768,7 +768,8 @@ impl Frame { layer.scrolling.started_bouncing_back = false } else if overscrolling && ((delta.x < 1.0 && delta.y < 1.0) || phase == ScrollEventPhase::End) { - layer.scrolling.started_bouncing_back = true + layer.scrolling.started_bouncing_back = true; + layer.scrolling.bouncing_back = true } layer.scrolling.offset.x = layer.scrolling.offset.x.round(); @@ -1479,7 +1480,7 @@ impl Frame { -> HashSet> { let mut layers_bouncing_back = HashSet::with_hasher(Default::default()); for (scroll_layer_id, layer) in &self.layers { - if layer.scrolling.started_bouncing_back { + if layer.scrolling.bouncing_back { layers_bouncing_back.insert(*scroll_layer_id); } } diff --git a/src/layer.rs b/src/layer.rs index f5c8637fe0..4816e31f2f 100644 --- a/src/layer.rs +++ b/src/layer.rs @@ -138,7 +138,7 @@ impl Layer { let finished = self.scrolling.spring.animate(); self.scrolling.offset = self.scrolling.spring.current(); if finished { - self.scrolling.started_bouncing_back = false; + self.scrolling.bouncing_back = false } } } @@ -148,6 +148,7 @@ pub struct ScrollingState { pub offset: Point2D, pub spring: Spring, pub started_bouncing_back: bool, + pub bouncing_back: bool, } impl ScrollingState { @@ -156,6 +157,7 @@ impl ScrollingState { offset: Point2D::new(0.0, 0.0), spring: Spring::at(Point2D::new(0.0, 0.0), STIFFNESS, DAMPING), started_bouncing_back: false, + bouncing_back: false, } } } From 3aec09d1af971b1a3a009e13a3b4f32998712b7f Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 29 Jun 2016 16:47:19 -0700 Subject: [PATCH 3/3] Only send new scroll frames to the compositor if a scroll event actually resulted in motion. Requires servo/webrender_traits#64. --- src/frame.rs | 15 +++++++++++---- src/render_backend.rs | 34 +++++++++++++++++++++++++++++----- 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/frame.rs b/src/frame.rs index a137d9541c..d02b1f867c 100644 --- a/src/frame.rs +++ b/src/frame.rs @@ -710,23 +710,25 @@ impl Frame { result } + /// Returns true if any layers actually changed position or false otherwise. pub fn scroll(&mut self, mut delta: Point2D, cursor: Point2D, - phase: ScrollEventPhase) { + phase: ScrollEventPhase) + -> bool { let root_scroll_layer_id = match self.root_scroll_layer_id { Some(root_scroll_layer_id) => root_scroll_layer_id, - None => return, + None => return false, }; let scroll_layer_id = match self.get_scroll_layer(&cursor, root_scroll_layer_id) { Some(scroll_layer_id) => scroll_layer_id, - None => return, + None => return false, }; let layer = self.layers.get_mut(&scroll_layer_id).unwrap(); if layer.scrolling.started_bouncing_back && phase == ScrollEventPhase::Move(false) { - return + return false } let overscroll_amount = layer.overscroll_amount(); @@ -744,6 +746,8 @@ impl Frame { let is_unscrollable = layer.layer_size.width <= layer.viewport_rect.size.width && layer.layer_size.height <= layer.viewport_rect.size.height; + let original_layer_scroll_offset = layer.scrolling.offset; + if layer.layer_size.width > layer.viewport_rect.size.width { layer.scrolling.offset.x = layer.scrolling.offset.x + delta.x; if is_unscrollable || !CAN_OVERSCROLL { @@ -778,6 +782,9 @@ impl Frame { if CAN_OVERSCROLL { layer.stretch_overscroll_spring(); } + + layer.scrolling.offset != original_layer_scroll_offset || layer.scrolling + .started_bouncing_back } pub fn tick_scrolling_bounce_animations(&mut self) { diff --git a/src/render_backend.rs b/src/render_backend.rs index 4e376666d4..5bec879514 100644 --- a/src/render_backend.rs +++ b/src/render_backend.rs @@ -229,7 +229,7 @@ impl RenderBackend { self.render() }); - self.publish_frame(frame, &mut profile_counters); + self.publish_frame_and_notify_compositor(frame, &mut profile_counters); } ApiMsg::SetRootPipeline(pipeline_id) => { let frame = profile_counters.total_time.profile(|| { @@ -243,11 +243,20 @@ impl RenderBackend { } ApiMsg::Scroll(delta, cursor, move_phase) => { let frame = profile_counters.total_time.profile(|| { - self.frame.scroll(delta, cursor, move_phase); - self.render() + if self.frame.scroll(delta, cursor, move_phase) { + Some(self.render()) + } else { + None + } }); - self.publish_frame(frame, &mut profile_counters); + match frame { + Some(frame) => { + self.publish_frame(frame, &mut profile_counters); + self.notify_compositor_of_new_scroll_frame(true) + } + None => self.notify_compositor_of_new_scroll_frame(false), + } } ApiMsg::TickScrollingBounce => { let frame = profile_counters.total_time.profile(|| { @@ -255,7 +264,7 @@ impl RenderBackend { self.render() }); - self.publish_frame(frame, &mut profile_counters); + self.publish_frame_and_notify_compositor(frame, &mut profile_counters); } ApiMsg::TranslatePointToLayerSpace(point, tx) => { // First, find the specific layer that contains the point. @@ -425,6 +434,12 @@ impl RenderBackend { let msg = ResultMsg::NewFrame(frame, pending_updates, profile_counters.clone()); self.result_tx.send(msg).unwrap(); profile_counters.reset(); + } + + fn publish_frame_and_notify_compositor(&mut self, + frame: RendererFrame, + profile_counters: &mut BackendProfileCounters) { + self.publish_frame(frame, profile_counters); // TODO(gw): This is kindof bogus to have to lock the notifier // each time it's used. This is due to some nastiness @@ -433,5 +448,14 @@ impl RenderBackend { let mut notifier = self.notifier.lock(); notifier.as_mut().unwrap().as_mut().unwrap().new_frame_ready(); } + + fn notify_compositor_of_new_scroll_frame(&mut self, composite_needed: bool) { + // TODO(gw): This is kindof bogus to have to lock the notifier + // each time it's used. This is due to some nastiness + // in initialization order for Servo. Perhaps find a + // cleaner way to do this, or use the OnceMutex on crates.io? + let mut notifier = self.notifier.lock(); + notifier.as_mut().unwrap().as_mut().unwrap().new_scroll_frame_ready(composite_needed); + } }