Skip to content

Commit d150e48

Browse files
author
bors-servo
authored
Auto merge of #17398 - glennw:opt-composite, r=pcwalton
Improve decisions in compositor over when to draw a frame. This patch fixes a couple of issues in the compositor: 1) Remove the delayed composition code. Previously, this would schedule a composite for 12ms in the future. This doesn't really make any sense with WR. There's no point in doing a composite unless WR has provided a new frame to be drawn. This fixes issues in several benchmarks where we were doing multiple composite / renders per rAF, which is a waste of CPU time. This *does* make the framerate slower in some cases (such as a slow rAF callback) but it's more correct - otherwise we were just compositing the same frame multiple times for no real benefit. 2) Inform the window of the current animation state of the compositor. Specifically, if an animation (or rAF) is currently active, the window system switches to use event polling, and does not block on the OS-level event loop. In the case of active animation, we just assume that we want to be running as the vsync interval and not blocking. This means the compositor thread only sleeps on vsync during animation, which reduces OS scheduling and results in much smoother animation. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17398) <!-- Reviewable:end -->
2 parents 5f10a25 + 36829da commit d150e48

File tree

6 files changed

+32
-154
lines changed

6 files changed

+32
-154
lines changed

components/compositing/compositor.rs

Lines changed: 7 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use CompositionPipeline;
66
use SendableFrameTree;
77
use compositor_thread::{CompositorProxy, CompositorReceiver};
88
use compositor_thread::{InitialCompositorState, Msg, RenderListener};
9-
use delayed_composition::DelayedCompositionTimerProxy;
109
use euclid::{Point2D, TypedPoint2D, TypedVector2D, TypedRect, ScaleFactor, TypedSize2D};
1110
use gfx_traits::Epoch;
1211
use gleam::gl;
@@ -131,9 +130,6 @@ pub struct IOCompositor<Window: WindowMethods> {
131130

132131
channel_to_self: CompositorProxy,
133132

134-
/// A handle to the delayed composition timer.
135-
delayed_composition_timer: DelayedCompositionTimerProxy,
136-
137133
/// The type of composition to perform
138134
composite_target: CompositeTarget,
139135

@@ -207,7 +203,6 @@ struct ScrollZoomEvent {
207203
#[derive(PartialEq, Debug)]
208204
enum CompositionRequest {
209205
NoCompositingNecessary,
210-
DelayedComposite(u64),
211206
CompositeNow(CompositingReason),
212207
}
213208

@@ -363,7 +358,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
363358
scale: ScaleFactor::new(1.0),
364359
scale_factor: scale_factor,
365360
channel_to_self: state.sender.clone_compositor_proxy(),
366-
delayed_composition_timer: DelayedCompositionTimerProxy::new(state.sender),
367361
composition_request: CompositionRequest::NoCompositingNecessary,
368362
touch_handler: TouchHandler::new(),
369363
pending_scroll_zoom_events: Vec::new(),
@@ -437,8 +431,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
437431
let _ = receiver.recv();
438432
}
439433

440-
self.delayed_composition_timer.shutdown();
441-
442434
self.shutdown_state = ShutdownState::FinishedShuttingDown;
443435
}
444436

@@ -524,16 +516,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
524516
}
525517
}
526518

527-
(Msg::DelayedCompositionTimeout(timestamp), ShutdownState::NotShuttingDown) => {
528-
if let CompositionRequest::DelayedComposite(this_timestamp) =
529-
self.composition_request {
530-
if timestamp == this_timestamp {
531-
self.composition_request = CompositionRequest::CompositeNow(
532-
CompositingReason::DelayedCompositeTimeout)
533-
}
534-
}
535-
}
536-
537519
(Msg::Recomposite(reason), ShutdownState::NotShuttingDown) => {
538520
self.composition_request = CompositionRequest::CompositeNow(reason)
539521
}
@@ -753,18 +735,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
753735
}
754736
}
755737

756-
fn schedule_delayed_composite_if_necessary(&mut self) {
757-
match self.composition_request {
758-
CompositionRequest::CompositeNow(_) => return,
759-
CompositionRequest::DelayedComposite(_) |
760-
CompositionRequest::NoCompositingNecessary => {}
761-
}
762-
763-
let timestamp = precise_time_ns();
764-
self.delayed_composition_timer.schedule_composite(timestamp);
765-
self.composition_request = CompositionRequest::DelayedComposite(timestamp);
766-
}
767-
768738
fn scroll_fragment_to_point(&mut self, id: ClipId, point: Point2D<f32>) {
769739
self.webrender_api.scroll_node_with_id(LayoutPoint::from_untyped(&point), id,
770740
ScrollClamping::ToContentBounds);
@@ -1235,13 +1205,18 @@ impl<Window: WindowMethods> IOCompositor<Window> {
12351205
pipeline_ids.push(*pipeline_id);
12361206
}
12371207
}
1208+
let animation_state = if pipeline_ids.is_empty() {
1209+
windowing::AnimationState::Idle
1210+
} else {
1211+
windowing::AnimationState::Animating
1212+
};
1213+
self.window.set_animation_state(animation_state);
12381214
for pipeline_id in &pipeline_ids {
12391215
self.tick_animations_for_pipeline(*pipeline_id)
12401216
}
12411217
}
12421218

12431219
fn tick_animations_for_pipeline(&mut self, pipeline_id: PipelineId) {
1244-
self.schedule_delayed_composite_if_necessary();
12451220
let animation_callbacks_running = self.pipeline_details(pipeline_id).animation_callbacks_running;
12461221
if animation_callbacks_running {
12471222
let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Script);
@@ -1635,14 +1610,6 @@ impl<Window: WindowMethods> IOCompositor<Window> {
16351610
_ => compositor_messages.push(msg),
16361611
}
16371612
}
1638-
if found_recomposite_msg {
1639-
compositor_messages.retain(|msg| {
1640-
match *msg {
1641-
Msg::DelayedCompositionTimeout(_) => false,
1642-
_ => true,
1643-
}
1644-
})
1645-
}
16461613
for msg in compositor_messages {
16471614
if !self.handle_browser_message(msg) {
16481615
break
@@ -1664,8 +1631,7 @@ impl<Window: WindowMethods> IOCompositor<Window> {
16641631
}
16651632

16661633
match self.composition_request {
1667-
CompositionRequest::NoCompositingNecessary |
1668-
CompositionRequest::DelayedComposite(_) => {}
1634+
CompositionRequest::NoCompositingNecessary => {}
16691635
CompositionRequest::CompositeNow(_) => {
16701636
self.composite()
16711637
}

components/compositing/compositor_thread.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,6 @@ pub enum Msg {
100100
HistoryChanged(Vec<LoadData>, usize),
101101
/// Wether or not to follow a link
102102
AllowNavigation(ServoUrl, IpcSender<bool>),
103-
/// We hit the delayed composition timeout. (See `delayed_composition.rs`.)
104-
DelayedCompositionTimeout(u64),
105103
/// Composite.
106104
Recomposite(CompositingReason),
107105
/// Sends an unconsumed key event back to the compositor.
@@ -160,7 +158,6 @@ impl Debug for Msg {
160158
Msg::AllowNavigation(..) => write!(f, "AllowNavigation"),
161159
Msg::LoadStart => write!(f, "LoadStart"),
162160
Msg::HistoryChanged(..) => write!(f, "HistoryChanged"),
163-
Msg::DelayedCompositionTimeout(..) => write!(f, "DelayedCompositionTimeout"),
164161
Msg::Recomposite(..) => write!(f, "Recomposite"),
165162
Msg::KeyEvent(..) => write!(f, "KeyEvent"),
166163
Msg::TouchEventProcessed(..) => write!(f, "TouchEventProcessed"),

components/compositing/delayed_composition.rs

Lines changed: 0 additions & 107 deletions
This file was deleted.

components/compositing/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ use style_traits::CSSPixel;
3535

3636
mod compositor;
3737
pub mod compositor_thread;
38-
mod delayed_composition;
3938
mod touch;
4039
pub mod windowing;
4140

components/compositing/windowing.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,12 @@ impl Debug for WindowEvent {
102102
}
103103
}
104104

105+
#[derive(Debug, Copy, Clone, PartialEq)]
106+
pub enum AnimationState {
107+
Idle,
108+
Animating,
109+
}
110+
105111
pub trait WindowMethods {
106112
/// Returns the rendering area size in hardware pixels.
107113
fn framebuffer_size(&self) -> TypedSize2D<u32, DevicePixel>;
@@ -163,4 +169,10 @@ pub trait WindowMethods {
163169

164170
/// Return the GL function pointer trait.
165171
fn gl(&self) -> Rc<gl::Gl>;
172+
173+
/// Set whether the application is currently animating.
174+
/// Typically, when animations are active, the window
175+
/// will want to avoid blocking on UI events, and just
176+
/// run the event loop at the vsync interval.
177+
fn set_animation_state(&self, _state: AnimationState) {}
166178
}

ports/glutin/window.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
77
use NestedEventLoopListener;
88
use compositing::compositor_thread::EventLoopWaker;
9-
use compositing::windowing::{MouseWindowEvent, WindowNavigateMsg};
9+
use compositing::windowing::{AnimationState, MouseWindowEvent, WindowNavigateMsg};
1010
use compositing::windowing::{WindowEvent, WindowMethods};
1111
use euclid::{Point2D, Size2D, TypedPoint2D, TypedVector2D, TypedRect, ScaleFactor, TypedSize2D};
1212
#[cfg(target_os = "windows")]
@@ -196,6 +196,8 @@ pub struct Window {
196196
#[cfg(not(target_os = "windows"))]
197197
pressed_key_map: RefCell<Vec<(ScanCode, char)>>,
198198

199+
animation_state: Cell<AnimationState>,
200+
199201
gl: Rc<gl::Gl>,
200202
}
201203

@@ -316,6 +318,7 @@ impl Window {
316318
#[cfg(target_os = "windows")]
317319
last_pressed_key: Cell::new(None),
318320
gl: gl.clone(),
321+
animation_state: Cell::new(AnimationState::Idle),
319322
};
320323

321324
window.present();
@@ -655,10 +658,14 @@ impl Window {
655658
let mut events = mem::replace(&mut *self.event_queue.borrow_mut(), Vec::new());
656659
let mut close_event = false;
657660

661+
let poll = self.animation_state.get() == AnimationState::Animating ||
662+
opts::get().output_file.is_some() ||
663+
opts::get().exit_after_load ||
664+
opts::get().headless;
658665
// When writing to a file then exiting, use event
659666
// polling so that we don't block on a GUI event
660667
// such as mouse click.
661-
if opts::get().output_file.is_some() || opts::get().exit_after_load || opts::get().headless {
668+
if poll {
662669
match self.kind {
663670
WindowKind::Window(ref window) => {
664671
while let Some(event) = window.poll_events().next() {
@@ -1005,6 +1012,10 @@ impl WindowMethods for Window {
10051012

10061013
}
10071014

1015+
fn set_animation_state(&self, state: AnimationState) {
1016+
self.animation_state.set(state);
1017+
}
1018+
10081019
fn set_inner_size(&self, size: Size2D<u32>) {
10091020
match self.kind {
10101021
WindowKind::Window(ref window) => {

0 commit comments

Comments
 (0)