Skip to content

Commit

Permalink
Auto merge of #302 - pcwalton:scroll-perf-fixes, r=glennw
Browse files Browse the repository at this point in the history
 	Only send new scroll frames to the compositor if a scroll event actually resulted in motion, and fix some overscroll problems.

Requires servo/webrender_traits#64.

r? @glennw
  • Loading branch information
bors-servo committed Jun 30, 2016
2 parents 1ce30fe + 3aec09d commit b324744
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 13 deletions.
20 changes: 14 additions & 6 deletions src/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -717,23 +717,25 @@ impl Frame {
result
}

/// Returns true if any layers actually changed position or false otherwise.
pub fn scroll(&mut self,
mut delta: Point2D<f32>,
cursor: Point2D<f32>,
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();
Expand All @@ -751,6 +753,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 {
Expand All @@ -775,7 +779,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();
Expand All @@ -784,6 +789,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) {
Expand Down Expand Up @@ -1487,7 +1495,7 @@ impl Frame {
-> HashSet<ScrollLayerId, BuildHasherDefault<FnvHasher>> {
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);
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand All @@ -148,6 +148,7 @@ pub struct ScrollingState {
pub offset: Point2D<f32>,
pub spring: Spring,
pub started_bouncing_back: bool,
pub bouncing_back: bool,
}

impl ScrollingState {
Expand All @@ -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,
}
}
}
Expand Down
34 changes: 29 additions & 5 deletions src/render_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand All @@ -243,19 +243,28 @@ 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(|| {
self.frame.tick_scrolling_bounce_animations();
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.
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}

2 changes: 1 addition & 1 deletion src/spring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit b324744

Please sign in to comment.