Skip to content

Commit

Permalink
compositor: Improve the way we wait for frames
Browse files Browse the repository at this point in the history 
In the newest version of WebRender it will be harder to make the
distinction between frame queued for scrolling and other kinds of
pending frames. This change makes it so that we queue frames for both
kinds of changes the same way and keeps a counting of pending frames.
This is conceptually a lot simpler.

In addition, do queue a composite even when recomposite isn't necessary
for a WebRender frame when there are active requestAnimationFrame
callbacks. Doing a composite is what triggers the callbacks to actually
run in the script thread! I believe this was a bug, but the WebRender
upgrade made it much more obvious.

These changes are in preparation for the WebRender upgrade.
  • Loading branch information
@mrobinson
mrobinson committed Mar 6, 2024
1 parent 24a088d commit c4122ea
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 96 deletions.
106 changes: 64 additions & 42 deletions components/compositing/compositor.rs
View file
Expand Up @@ -13,7 +13,7 @@ use std::time::{SystemTime, UNIX_EPOCH};

use canvas::canvas_paint_thread::ImageUpdate;
use compositing_traits::{
CanvasToCompositorMsg, CompositingReason, CompositionPipeline, CompositorMsg,
CanvasToCompositorMsg, CompositionPipeline, CompositorMsg,
CompositorReceiver, ConstellationMsg, FontToCompositorMsg, ForwardedToCompositorMsg,
SendableFrameTree,
};
Expand Down Expand Up @@ -53,7 +53,7 @@ use webrender_api::{
self, BuiltDisplayList, ClipId, DirtyRect, DocumentId, Epoch as WebRenderEpoch,
ExternalScrollId, HitTestFlags, PipelineId as WebRenderPipelineId, PropertyBinding,
ReferenceFrameKind, ScrollClamping, ScrollLocation, SpaceAndClipInfo, SpatialId,
TransformStyle, ZoomFactor,
TransformStyle, ZoomFactor
};

use crate::gl::RenderTargetInfo;
Expand Down Expand Up @@ -182,9 +182,6 @@ pub struct IOCompositor<Window: WindowMethods + ?Sized> {
/// Pending scroll/zoom events.
pending_scroll_zoom_events: Vec<ScrollZoomEvent>,

/// Whether we're waiting on a recomposite after dispatching a scroll.
waiting_for_results_of_scroll: bool,

/// Used by the logic that determines when it is safe to output an
/// image for the reftest framework.
ready_to_save_state: ReadyState,
Expand Down Expand Up @@ -244,10 +241,8 @@ pub struct IOCompositor<Window: WindowMethods + ?Sized> {
/// True to translate mouse input into touch events.
convert_mouse_to_touch: bool,

/// True if a WR frame render has been requested. Screenshots
/// taken before the render is complete will not reflect the
/// most up to date rendering.
waiting_on_pending_frame: bool,
/// The number of frames pending to receive from WebRender.
pending_frames: usize,

/// Waiting for external code to call present.
waiting_on_present: bool,
Expand All @@ -272,6 +267,23 @@ enum ScrollZoomEvent {
Scroll(ScrollEvent),
}

/// Why we performed a composite. This is used for debugging.
///
/// TODO: It would be good to have a bit more precision here about why a composite
/// was originally triggered, but that would require tracking the reason when a
/// frame is queued in WebRender and then remembering when the frame is ready.
#[derive(Clone, Copy, Debug, PartialEq)]
enum CompositingReason {
/// We're performing the single composite in headless mode.
Headless,
/// We're performing a composite to run an animation.
Animation,
/// A new WebRender frame has arrived.
NewWebRenderFrame,
/// The window has been resized and will need to be synchronously repainted.
Resize,
}

#[derive(Debug, PartialEq)]
enum CompositionRequest {
NoCompositingNecessary,
Expand Down Expand Up @@ -387,7 +399,6 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
composition_request: CompositionRequest::NoCompositingNecessary,
touch_handler: TouchHandler::new(),
pending_scroll_zoom_events: Vec::new(),
waiting_for_results_of_scroll: false,
composite_target,
shutdown_state: ShutdownState::NotShuttingDown,
page_zoom: Scale::new(1.0),
Expand Down Expand Up @@ -415,7 +426,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
is_running_problem_test,
exit_after_load,
convert_mouse_to_touch,
waiting_on_pending_frame: false,
pending_frames: 0,
waiting_on_present: false,
}
}
Expand Down Expand Up @@ -529,6 +540,10 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}

fn handle_browser_message(&mut self, msg: CompositorMsg) -> bool {
if matches!(msg, CompositorMsg::NewWebRenderFrameReady(..)) {
self.pending_frames -= 1;
}

match (msg, self.shutdown_state) {
(_, ShutdownState::FinishedShuttingDown) => {
error!("compositor shouldn't be handling messages after shutting down");
Expand All @@ -552,11 +567,6 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.send_scroll_positions_to_layout_for_pipeline(&frame_tree.pipeline.id);
},

(CompositorMsg::Recomposite(reason), ShutdownState::NotShuttingDown) => {
self.waiting_on_pending_frame = false;
self.composition_request = CompositionRequest::CompositeNow(reason)
},

(CompositorMsg::TouchEventProcessed(result), ShutdownState::NotShuttingDown) => {
self.touch_handler.on_event_processed(result);
},
Expand All @@ -577,8 +587,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.ready_to_save_state,
ReadyState::WaitingForConstellationReply
);
if is_ready && !self.waiting_on_pending_frame && !self.waiting_for_results_of_scroll
{
if is_ready && self.pending_frames == 0 {
self.ready_to_save_state = ReadyState::ReadyToSaveImage;
if self.is_running_problem_test {
println!("ready to save image!");
Expand Down Expand Up @@ -607,17 +616,19 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
},

(
CompositorMsg::NewScrollFrameReady(recomposite_needed),
CompositorMsg::NewWebRenderFrameReady(recomposite_needed),
ShutdownState::NotShuttingDown,
) => {
self.waiting_for_results_of_scroll = false;
if let Some(result) = self.hit_test_at_device_point(self.cursor_pos) {
self.update_cursor(result);
}
if recomposite_needed {
self.composition_request = CompositionRequest::CompositeNow(
CompositingReason::NewWebRenderScrollFrame,
);
if let Some(result) = self.hit_test_at_device_point(self.cursor_pos) {
self.update_cursor(result);
}
self.composition_request =
CompositionRequest::CompositeNow(CompositingReason::NewWebRenderFrame);
}

if recomposite_needed || self.animation_callbacks_active() {
self.composite_if_necessary(CompositingReason::NewWebRenderFrame)
}
},

Expand Down Expand Up @@ -700,7 +711,6 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
ForwardedToCompositorMsg::Layout(
script_traits::ScriptToCompositorMsg::SendInitialTransaction(pipeline),
) => {
self.waiting_on_pending_frame = true;
let mut txn = Transaction::new();
txn.set_display_list(
WebRenderEpoch(0),
Expand All @@ -710,18 +720,17 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
false,
);

self.generate_frame(&mut txn);
self.webrender_api
.send_transaction(self.webrender_document, txn);
},

ForwardedToCompositorMsg::Layout(
script_traits::ScriptToCompositorMsg::SendScrollNode(point, scroll_id),
) => {
self.waiting_for_results_of_scroll = true;

let mut txn = Transaction::new();
txn.scroll_node_with_id(point, scroll_id, ScrollClamping::NoClamping);
txn.generate_frame(0);
self.generate_frame(&mut txn);
self.webrender_api
.send_transaction(self.webrender_document, txn);
},
Expand All @@ -738,8 +747,6 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
_ => return warn!("Could not recieve WebRender display list."),
};

self.waiting_on_pending_frame = true;

let pipeline_id = display_list_info.pipeline_id;
let details = self.pipeline_details(PipelineId::from_webrender(pipeline_id));
details.most_recent_display_list_epoch = Some(display_list_info.epoch);
Expand All @@ -757,7 +764,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
),
true,
);
txn.generate_frame(0);
self.generate_frame(&mut txn);
self.webrender_api
.send_transaction(self.webrender_document, txn);
},
Expand Down Expand Up @@ -879,6 +886,12 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
}
}

/// Queue a new frame in the transaction and increase the pending frames count.
fn generate_frame(&mut self, transaction: &mut Transaction) {
self.pending_frames += 1;
transaction.generate_frame(0);
}

/// Sets or unsets the animations-running flag for the given pipeline, and schedules a
/// recomposite if necessary.
fn change_running_animations_state(
Expand Down Expand Up @@ -1011,7 +1024,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {

let mut txn = Transaction::new();
self.set_root_content_pipeline_handling_pinch_zoom(&mut txn);
txn.generate_frame(0);
self.generate_frame(&mut txn);
self.webrender_api
.send_transaction(self.webrender_document, txn);

Expand Down Expand Up @@ -1449,10 +1462,9 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
let scroll_origin = LayoutPoint::new(-offset.x, -offset.y);
transaction.scroll_node_with_id(scroll_origin, external_id, ScrollClamping::NoClamping);
self.send_scroll_positions_to_layout_for_pipeline(&pipeline_id);
self.waiting_for_results_of_scroll = true
}

transaction.generate_frame(0);
self.generate_frame(&mut transaction);
self.webrender_api
.send_transaction(self.webrender_document, transaction);
}
Expand Down Expand Up @@ -1630,6 +1642,13 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
false
}

/// Returns true if any animation callbacks (ie `requestAnimationFrame`) are waiting for a response.
fn animation_callbacks_active(&self) -> bool {
self.pipeline_details
.values()
.any(|details| details.animation_callbacks_running)
}

/// Query the constellation to see if the current compositor
/// output matches the current frame tree output, and if the
/// associated script threads are idle.
Expand Down Expand Up @@ -1930,7 +1949,6 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.composition_request = CompositionRequest::NoCompositingNecessary;

self.process_animations();
self.waiting_for_results_of_scroll = false;

Ok(rv)
}
Expand Down Expand Up @@ -2013,8 +2031,12 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
let mut found_recomposite_msg = false;
while let Some(msg) = self.port.try_recv_compositor_msg() {
match msg {
CompositorMsg::Recomposite(_) if found_recomposite_msg => {},
CompositorMsg::Recomposite(_) => {
CompositorMsg::NewWebRenderFrameReady(_) if found_recomposite_msg => {
// Only take one of duplicate NewWebRendeFrameReady messages, but do subtract
// one frame from the pending frames.
self.pending_frames -= 1;
},
CompositorMsg::NewWebRenderFrameReady(_) => {
found_recomposite_msg = true;
compositor_messages.push(msg)
},
Expand Down Expand Up @@ -2054,7 +2076,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
// The WebXR thread may make a different context current
let _ = self.rendering_context.make_gl_context_current();

if !self.pending_scroll_zoom_events.is_empty() && !self.waiting_for_results_of_scroll {
if !self.pending_scroll_zoom_events.is_empty() {
self.process_pending_scroll_events()
}
self.shutdown_state != ShutdownState::FinishedShuttingDown
Expand All @@ -2068,7 +2090,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
while self.shutdown_state != ShutdownState::ShuttingDown {
let msg = self.port.recv_compositor_msg();
let need_recomposite = match msg {
CompositorMsg::Recomposite(_) => true,
CompositorMsg::NewWebRenderFrameReady(_) => true,
_ => false,
};
let keep_going = self.handle_browser_message(msg);
Expand Down Expand Up @@ -2113,7 +2135,7 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.webrender.set_debug_flags(flags);

let mut txn = Transaction::new();
txn.generate_frame(0);
self.generate_frame(&mut txn);
self.webrender_api
.send_transaction(self.webrender_document, txn);
}
Expand Down
20 changes: 6 additions & 14 deletions components/servo/lib.rs
View file
Expand Up @@ -33,7 +33,7 @@ use canvas_traits::webgl::WebGLThreads;
use compositing::windowing::{EmbedderEvent, EmbedderMethods, WindowMethods};
use compositing::{CompositeTarget, IOCompositor, InitialCompositorState, ShutdownState};
use compositing_traits::{
CanvasToCompositorMsg, CompositingReason, CompositorMsg, CompositorProxy, CompositorReceiver,
CanvasToCompositorMsg, CompositorMsg, CompositorProxy, CompositorReceiver,
ConstellationMsg, FontToCompositorMsg, ForwardedToCompositorMsg,
};
#[cfg(all(
Expand Down Expand Up @@ -198,26 +198,18 @@ impl webrender_api::RenderNotifier for RenderNotifier {
Box::new(RenderNotifier::new(self.compositor_proxy.clone()))
}

fn wake_up(&self, composite_needed: bool) {
if composite_needed {
self.compositor_proxy
.recomposite(CompositingReason::NewWebRenderFrame);
}
}
fn wake_up(&self, _composite_needed: bool) { }

fn new_frame_ready(
&self,
_document_id: DocumentId,
scrolled: bool,
_scrolled: bool,
composite_needed: bool,
_render_time_ns: Option<u64>,
) {
if scrolled {
self.compositor_proxy
.send(CompositorMsg::NewScrollFrameReady(composite_needed));
} else {
self.wake_up(true);
}
println!("FRAME READY");
self.compositor_proxy
.send(CompositorMsg::NewWebRenderFrameReady(composite_needed));
}
}

Expand Down

0 comments on commit c4122ea

Please sign in to comment.