From b21331e134d5bac17c80290cd010dfe59babc7c7 Mon Sep 17 00:00:00 2001 From: Glenn Watson Date: Tue, 24 Jul 2018 10:38:33 +1000 Subject: [PATCH] Only trigger a frame build if scene properties have changed. In many important use cases (e.g. video playback on youtube) the display list doesn't change very often. Typically, the only thing changing between frames are the natively decoded video texture surfaces. In these cases, we can save a significant amount of CPU usage and battery power by not building a frame at all. Previously, any call to set/update dynamic properties would trigger a frame rebuild. However, it's often the case that Gecko sends dynamic properties updates that are either empty or exactly the same as the previous frame. With this change, WR will compare the scene properties and only trigger a frame rebuild if they have actually changed. This is a partial step towards fixing #2917 - a couple of other changes are also needed to completely avoid a frame rebuiild in this use case. --- webrender/src/render_backend.rs | 193 ++++++++++++++++---------------- webrender/src/scene.rs | 56 +++++++-- webrender_api/src/api.rs | 4 +- 3 files changed, 144 insertions(+), 109 deletions(-) diff --git a/webrender/src/render_backend.rs b/webrender/src/render_backend.rs index a410a9e1c8..c9fb077f37 100644 --- a/webrender/src/render_backend.rs +++ b/webrender/src/render_backend.rs @@ -169,6 +169,97 @@ impl Document { !self.view.window_size.is_empty_or_negative() } + fn process_frame_msg( + &mut self, + message: FrameMsg, + ) -> DocumentOps { + match message { + FrameMsg::UpdateEpoch(pipeline_id, epoch) => { + self.current.scene.update_epoch(pipeline_id, epoch); + + DocumentOps::nop() + } + FrameMsg::EnableFrameOutput(pipeline_id, enable) => { + if enable { + self.output_pipelines.insert(pipeline_id); + } else { + self.output_pipelines.remove(&pipeline_id); + } + DocumentOps::nop() + } + FrameMsg::Scroll(delta, cursor) => { + profile_scope!("Scroll"); + + let mut should_render = true; + let node_index = match self.hit_tester { + Some(ref hit_tester) => { + // Ideally we would call self.scroll_nearest_scrolling_ancestor here, but + // we need have to avoid a double-borrow. + let test = HitTest::new(None, cursor, HitTestFlags::empty()); + hit_tester.find_node_under_point(test) + } + None => { + should_render = false; + None + } + }; + + let should_render = + should_render && + self.scroll_nearest_scrolling_ancestor(delta, node_index) && + self.render_on_scroll == Some(true); + DocumentOps { + scroll: true, + render: should_render, + composite: should_render, + ..DocumentOps::nop() + } + } + FrameMsg::HitTest(pipeline_id, point, flags, tx) => { + + let result = match self.hit_tester { + Some(ref hit_tester) => { + hit_tester.hit_test(HitTest::new(pipeline_id, point, flags)) + } + None => HitTestResult { items: Vec::new() }, + }; + + tx.send(result).unwrap(); + DocumentOps::nop() + } + FrameMsg::SetPan(pan) => { + self.view.pan = pan; + DocumentOps::nop() + } + FrameMsg::ScrollNodeWithId(origin, id, clamp) => { + profile_scope!("ScrollNodeWithScrollId"); + + let should_render = self.scroll_node(origin, id, clamp) + && self.render_on_scroll == Some(true); + + DocumentOps { + scroll: true, + render: should_render, + composite: should_render, + ..DocumentOps::nop() + } + } + FrameMsg::GetScrollNodeState(tx) => { + profile_scope!("GetScrollNodeState"); + tx.send(self.get_scroll_node_state()).unwrap(); + DocumentOps::nop() + } + FrameMsg::UpdateDynamicProperties(property_bindings) => { + self.dynamic_properties.set_properties(property_bindings); + DocumentOps::nop() + } + FrameMsg::AppendDynamicProperties(property_bindings) => { + self.dynamic_properties.add_properties(property_bindings); + DocumentOps::nop() + } + } + } + // TODO: We will probably get rid of this soon and always forward to the scene building thread. fn build_scene(&mut self, resource_cache: &mut ResourceCache, scene_id: u64) { let max_texture_size = resource_cache.max_texture_size(); @@ -601,100 +692,6 @@ impl RenderBackend { } } - fn process_frame_msg( - &mut self, - document_id: DocumentId, - message: FrameMsg, - ) -> DocumentOps { - let doc = self.documents.get_mut(&document_id).expect("No document?"); - - match message { - FrameMsg::UpdateEpoch(pipeline_id, epoch) => { - doc.current.scene.update_epoch(pipeline_id, epoch); - - DocumentOps::nop() - } - FrameMsg::EnableFrameOutput(pipeline_id, enable) => { - if enable { - doc.output_pipelines.insert(pipeline_id); - } else { - doc.output_pipelines.remove(&pipeline_id); - } - DocumentOps::nop() - } - FrameMsg::Scroll(delta, cursor) => { - profile_scope!("Scroll"); - - let mut should_render = true; - let node_index = match doc.hit_tester { - Some(ref hit_tester) => { - // Ideally we would call doc.scroll_nearest_scrolling_ancestor here, but - // we need have to avoid a double-borrow. - let test = HitTest::new(None, cursor, HitTestFlags::empty()); - hit_tester.find_node_under_point(test) - } - None => { - should_render = false; - None - } - }; - - let should_render = - should_render && - doc.scroll_nearest_scrolling_ancestor(delta, node_index) && - doc.render_on_scroll == Some(true); - DocumentOps { - scroll: true, - render: should_render, - composite: should_render, - ..DocumentOps::nop() - } - } - FrameMsg::HitTest(pipeline_id, point, flags, tx) => { - - let result = match doc.hit_tester { - Some(ref hit_tester) => { - hit_tester.hit_test(HitTest::new(pipeline_id, point, flags)) - } - None => HitTestResult { items: Vec::new() }, - }; - - tx.send(result).unwrap(); - DocumentOps::nop() - } - FrameMsg::SetPan(pan) => { - doc.view.pan = pan; - DocumentOps::nop() - } - FrameMsg::ScrollNodeWithId(origin, id, clamp) => { - profile_scope!("ScrollNodeWithScrollId"); - - let should_render = doc.scroll_node(origin, id, clamp) - && doc.render_on_scroll == Some(true); - - DocumentOps { - scroll: true, - render: should_render, - composite: should_render, - ..DocumentOps::nop() - } - } - FrameMsg::GetScrollNodeState(tx) => { - profile_scope!("GetScrollNodeState"); - tx.send(doc.get_scroll_node_state()).unwrap(); - DocumentOps::nop() - } - FrameMsg::UpdateDynamicProperties(property_bindings) => { - doc.dynamic_properties.set_properties(property_bindings); - DocumentOps::render() - } - FrameMsg::AppendDynamicProperties(property_bindings) => { - doc.dynamic_properties.add_properties(property_bindings); - DocumentOps::render() - } - } - } - fn next_namespace_id(&self) -> IdNamespace { IdNamespace(NEXT_NAMESPACE_ID.fetch_add(1, Ordering::Relaxed) as u32) } @@ -1070,12 +1067,16 @@ impl RenderBackend { } } + let doc = self.documents.get_mut(&document_id).unwrap(); + for frame_msg in transaction_msg.frame_ops { let _timer = profile_counters.total_time.timer(); - op.combine(self.process_frame_msg(document_id, frame_msg)); + op.combine(doc.process_frame_msg(frame_msg)); } - let doc = self.documents.get_mut(&document_id).unwrap(); + if doc.dynamic_properties.flush_pending_updates() { + op.render = true; + } if transaction_msg.generate_frame { if let Some(ref mut ros) = doc.render_on_scroll { diff --git a/webrender/src/scene.rs b/webrender/src/scene.rs index 27b9cc3b69..ef55211cab 100644 --- a/webrender/src/scene.rs +++ b/webrender/src/scene.rs @@ -13,10 +13,11 @@ use std::sync::Arc; /// re-submitting the display list itself. #[cfg_attr(feature = "capture", derive(Serialize))] #[cfg_attr(feature = "replay", derive(Deserialize))] -#[derive(Clone)] pub struct SceneProperties { transform_properties: FastHashMap, float_properties: FastHashMap, + current_properties: DynamicProperties, + pending_properties: Option, } impl SceneProperties { @@ -24,27 +25,60 @@ impl SceneProperties { SceneProperties { transform_properties: FastHashMap::default(), float_properties: FastHashMap::default(), + current_properties: DynamicProperties::default(), + pending_properties: None, } } /// Set the current property list for this display list. pub fn set_properties(&mut self, properties: DynamicProperties) { - self.transform_properties.clear(); - self.float_properties.clear(); - self.add_properties(properties); + self.pending_properties = Some(properties); } /// Add to the current property list for this display list. pub fn add_properties(&mut self, properties: DynamicProperties) { - for property in properties.transforms { - self.transform_properties - .insert(property.key.id, property.value); - } + let mut pending_properties = self.pending_properties + .take() + .unwrap_or_default(); + + pending_properties.transforms.extend(properties.transforms); + pending_properties.floats.extend(properties.floats); + + self.pending_properties = Some(pending_properties); + } + + /// Flush any pending updates to the scene properties. Returns + /// true if the properties have changed since the last flush + /// was called. This code allows properties to be changed by + /// multiple set_properties and add_properties calls during a + /// single transaction, and still correctly determine if any + /// properties have changed. This can have significant power + /// saving implications, allowing a frame build to be skipped + /// if the properties haven't changed in many cases. + pub fn flush_pending_updates(&mut self) -> bool { + let mut properties_changed = false; - for property in properties.floats { - self.float_properties - .insert(property.key.id, property.value); + if let Some(pending_properties) = self.pending_properties.take() { + if pending_properties != self.current_properties { + self.transform_properties.clear(); + self.float_properties.clear(); + + for property in &pending_properties.transforms { + self.transform_properties + .insert(property.key.id, property.value); + } + + for property in &pending_properties.floats { + self.float_properties + .insert(property.key.id, property.value); + } + + self.current_properties = pending_properties; + properties_changed = true; + } } + + properties_changed } /// Get the current value for a transform property. diff --git a/webrender_api/src/api.rs b/webrender_api/src/api.rs index 1be9edd0ea..9aa604b17e 100644 --- a/webrender_api/src/api.rs +++ b/webrender_api/src/api.rs @@ -1090,7 +1090,7 @@ impl From for PropertyBinding { /// The current value of an animated property. This is /// supplied by the calling code. -#[derive(Clone, Copy, Debug, Deserialize, Serialize)] +#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq)] pub struct PropertyValue { pub key: PropertyBindingKey, pub value: T, @@ -1099,7 +1099,7 @@ pub struct PropertyValue { /// When using `generate_frame()`, a list of `PropertyValue` structures /// can optionally be supplied to provide the current value of any /// animated properties. -#[derive(Clone, Deserialize, Serialize, Debug)] +#[derive(Clone, Deserialize, Serialize, Debug, PartialEq, Default)] pub struct DynamicProperties { pub transforms: Vec>, pub floats: Vec>,