Avoid redundant frame builds (take two).

webrender/src/clip_scroll_tree.rs

@@ -253,10 +253,14 @@ impl ClipScrollTree {
         &mut self,
         pan: WorldPoint,
         scene_properties: &SceneProperties,
-    ) -> TransformPalette {
-        let mut transform_palette = TransformPalette::new(self.spatial_nodes.len());
+        mut transform_palette: Option<&mut TransformPalette>,
+    ) {
         if self.spatial_nodes.is_empty() {
-            return transform_palette;
+            return;
+        }
+
+        if let Some(ref mut palette) = transform_palette {
+            palette.allocate(self.spatial_nodes.len());
         }
 
         self.coord_systems.clear();
@@ -282,7 +286,9 @@ impl ClipScrollTree {
             };
 
             node.update(&mut state, &mut self.coord_systems, scene_properties);
-            node.push_gpu_data(&mut transform_palette, node_index);
+            if let Some(ref mut palette) = transform_palette {
+                node.push_gpu_data(palette, node_index);
+            }
 
             if !node.children.is_empty() {
                 node.prepare_state_for_children(&mut state);
@@ -293,8 +299,6 @@ impl ClipScrollTree {
                 );
             }
         }
-
-        transform_palette
     }
 
     pub fn finalize_and_apply_pending_scroll_offsets(&mut self, old_states: ScrollStates) {
@@ -506,7 +510,7 @@ fn test_cst_simple_translation() {
         LayoutVector2D::zero(),
     );
 
-    cst.update_tree(WorldPoint::zero(), &SceneProperties::new());
+    cst.update_tree(WorldPoint::zero(), &SceneProperties::new(), None);
 
     test_pt(100.0, 100.0, &cst, child1, root, 200.0, 100.0);
     test_pt(100.0, 100.0, &cst, root, child1, 0.0, 100.0);
@@ -551,7 +555,7 @@ fn test_cst_simple_scale() {
         LayoutVector2D::zero(),
     );
 
-    cst.update_tree(WorldPoint::zero(), &SceneProperties::new());
+    cst.update_tree(WorldPoint::zero(), &SceneProperties::new(), None);
 
     test_pt(100.0, 100.0, &cst, child1, root, 400.0, 100.0);
     test_pt(100.0, 100.0, &cst, root, child1, 25.0, 100.0);
@@ -605,7 +609,7 @@ fn test_cst_scale_translation() {
         LayoutVector2D::zero(),
     );
 
-    cst.update_tree(WorldPoint::zero(), &SceneProperties::new());
+    cst.update_tree(WorldPoint::zero(), &SceneProperties::new(), None);
 
     test_pt(100.0, 100.0, &cst, child1, root, 200.0, 150.0);
     test_pt(100.0, 100.0, &cst, child2, root, 300.0, 450.0);
@@ -644,7 +648,7 @@ fn test_cst_translation_rotate() {
         LayoutVector2D::zero(),
     );
 
-    cst.update_tree(WorldPoint::zero(), &SceneProperties::new());
+    cst.update_tree(WorldPoint::zero(), &SceneProperties::new(), None);
 
     test_pt(100.0, 0.0, &cst, child1, root, 0.0, -100.0);
 }

webrender/src/frame_builder.rs

@@ -343,9 +343,11 @@ impl FrameBuilder {
         resource_cache.begin_frame(frame_id);
         gpu_cache.begin_frame();
 
-        let mut transform_palette = clip_scroll_tree.update_tree(
+        let mut transform_palette = TransformPalette::new();
+        clip_scroll_tree.update_tree(
             pan,
             scene_properties,
+            Some(&mut transform_palette),
         );
 
         self.update_scroll_bars(clip_scroll_tree, gpu_cache);

webrender/src/gpu_types.rs

@@ -415,14 +415,19 @@ pub struct TransformPalette {
 }
 
 impl TransformPalette {
-    pub fn new(spatial_node_count: usize) -> Self {
+    pub fn new() -> Self {
         TransformPalette {
-            transforms: vec![TransformData::invalid(); spatial_node_count],
-            metadata: vec![TransformMetadata::invalid(); spatial_node_count],
+            transforms: Vec::new(),
+            metadata: Vec::new(),
             map: FastHashMap::default(),
         }
     }
 
+    pub fn allocate(&mut self, count: usize) {
+        self.transforms = vec![TransformData::invalid(); count];
+        self.metadata = vec![TransformMetadata::invalid(); count];
+    }
+
     pub fn set_world_transform(
         &mut self,
         index: SpatialNodeIndex,

webrender/src/render_backend.rs

@@ -206,6 +206,9 @@ impl Document {
                 };
             }
             FrameMsg::HitTest(pipeline_id, point, flags, tx) => {
+                if !self.hit_tester_is_valid {
+                    self.rebuild_hit_tester();
+                }
 
                 let result = match self.hit_tester {
                     Some(ref hit_tester) => {
@@ -293,6 +296,24 @@ impl Document {
         }
     }
 
+    fn rebuild_hit_tester(&mut self) {
+        if let Some(ref mut frame_builder) = self.frame_builder {
+            let accumulated_scale_factor = self.view.accumulated_scale_factor();
+            let pan = self.view.pan.to_f32() / accumulated_scale_factor;
+
+            self.clip_scroll_tree.update_tree(
+                pan,
+                &self.dynamic_properties,
+                None,
+            );
+
+            self.hit_tester = Some(frame_builder.create_hit_tester(
+                &self.clip_scroll_tree,
+                &self.clip_data_store,
+            ));
+        }
+    }
+
     pub fn updated_pipeline_info(&mut self) -> PipelineInfo {
         let removed_pipelines = replace(&mut self.removed_pipelines, Vec::new());
         PipelineInfo {
@@ -347,14 +368,12 @@ impl Document {
 
 struct DocumentOps {
     scroll: bool,
-    build_frame: bool,
 }
 
 impl DocumentOps {
     fn nop() -> Self {
         DocumentOps {
             scroll: false,
-            build_frame: false,
         }
     }
 }
@@ -615,7 +634,6 @@ impl RenderBackend {
                             txn.clip_updates.take(),
                             replace(&mut txn.frame_ops, Vec::new()),
                             replace(&mut txn.notifications, Vec::new()),
-                            txn.build_frame,
                             txn.render_frame,
                             &mut frame_counter,
                             &mut profile_counters,
@@ -625,6 +643,9 @@ impl RenderBackend {
                     SceneBuilderResult::FlushComplete(tx) => {
                         tx.send(()).ok();
                     }
+                    SceneBuilderResult::ExternalEvent(evt) => {
+                        self.notifier.external_event(evt);
+                    }
                     SceneBuilderResult::Stopped => {
                         panic!("We haven't sent a Stop yet, how did we get a Stopped back?");
                     }
@@ -727,7 +748,7 @@ impl RenderBackend {
                 ).unwrap();
             }
             ApiMsg::ExternalEvent(evt) => {
-                self.notifier.external_event(evt);
+                self.low_priority_scene_tx.send(SceneBuilderRequest::ExternalEvent(evt)).unwrap();
             }
             ApiMsg::ClearNamespace(namespace_id) => {
                 self.resource_cache.clear_namespace(namespace_id);
@@ -885,7 +906,6 @@ impl RenderBackend {
             rasterized_blobs: Vec::new(),
             notifications: transaction_msg.notifications,
             set_root_pipeline: None,
-            build_frame: transaction_msg.generate_frame,
             render_frame: transaction_msg.generate_frame,
         });
 
@@ -921,7 +941,6 @@ impl RenderBackend {
                 None,
                 replace(&mut txn.frame_ops, Vec::new()),
                 replace(&mut txn.notifications, Vec::new()),
-                txn.build_frame,
                 txn.render_frame,
                 frame_counter,
                 profile_counters,
@@ -959,7 +978,6 @@ impl RenderBackend {
         clip_updates: Option<ClipDataUpdateList>,
         mut frame_ops: Vec<FrameMsg>,
         mut notifications: Vec<NotificationRequest>,
-        mut build_frame: bool,
         mut render_frame: bool,
         frame_counter: &mut u32,
         profile_counters: &mut BackendProfileCounters,
@@ -972,7 +990,7 @@ impl RenderBackend {
         // fiddle with things after a potentially long scene build, but just
         // before rendering. This is useful for rendering with the latest
         // async transforms.
-        if build_frame {
+        if requested_frame || has_built_scene {
             if let Some(ref sampler) = self.sampler {
                 frame_ops.append(&mut sampler.sample());
             }
@@ -992,7 +1010,6 @@ impl RenderBackend {
         for frame_msg in frame_ops {
             let _timer = profile_counters.total_time.timer();
             let op = doc.process_frame_msg(frame_msg);
-            build_frame |= op.build_frame;
             scroll |= op.scroll;
         }
 
@@ -1007,31 +1024,20 @@ impl RenderBackend {
             &mut profile_counters.resources,
         );
 
-        // After applying the new scene we need to
-        // rebuild the hit-tester, so we trigger a frame generation
-        // step.
-        //
-        // TODO: We could avoid some the cost of building the frame by only
-        // building the information required for hit-testing (See #2807).
-        build_frame |= has_built_scene;
-
         if doc.dynamic_properties.flush_pending_updates() {
             doc.frame_is_valid = false;
             doc.hit_tester_is_valid = false;
-            build_frame = true;
         }
 
         if !doc.can_render() {
             // TODO: this happens if we are building the first scene asynchronously and
             // scroll at the same time. we should keep track of the fact that we skipped
             // composition here and do it as soon as we receive the scene.
-            build_frame = false;
             render_frame = false;
         }
 
-        if doc.frame_is_valid {
-            build_frame = false;
-        }
+        // Avoid re-building the frame if the current built frame is still valid.
+        let build_frame = render_frame && !doc.frame_is_valid;
 
         let mut frame_build_time = None;
         if build_frame && doc.has_pixels() {
@@ -1096,6 +1102,10 @@ impl RenderBackend {
         if requested_frame {
             self.notifier.new_frame_ready(document_id, scroll, render_frame, frame_build_time);
         }
+
+        if !doc.hit_tester_is_valid {
+            doc.rebuild_hit_tester();
+        }
     }
 
     #[cfg(not(feature = "debugger"))]

webrender/src/scene_builder.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use api::{AsyncBlobImageRasterizer, BlobImageRequest, BlobImageParams, BlobImageResult};
-use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, ResourceUpdate, Epoch};
+use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, ResourceUpdate, ExternalEvent, Epoch};
 use api::{BuiltDisplayList, ColorF, LayoutSize, NotificationRequest, Checkpoint};
 use api::channel::MsgSender;
 #[cfg(feature = "capture")]
@@ -39,7 +39,6 @@ pub struct Transaction {
     pub frame_ops: Vec<FrameMsg>,
     pub notifications: Vec<NotificationRequest>,
     pub set_root_pipeline: Option<PipelineId>,
-    pub build_frame: bool,
     pub render_frame: bool,
 }
 
@@ -73,7 +72,6 @@ pub struct BuiltTransaction {
     pub clip_updates: Option<ClipDataUpdateList>,
     pub scene_build_start_time: u64,
     pub scene_build_end_time: u64,
-    pub build_frame: bool,
     pub render_frame: bool,
 }
 
@@ -116,6 +114,7 @@ pub struct BuiltScene {
 // Message from render backend to scene builder.
 pub enum SceneBuilderRequest {
     Transaction(Box<Transaction>),
+    ExternalEvent(ExternalEvent),
     DeleteDocument(DocumentId),
     WakeUp,
     Flush(MsgSender<()>),
@@ -132,6 +131,7 @@ pub enum SceneBuilderRequest {
 // Message from scene builder to render backend.
 pub enum SceneBuilderResult {
     Transaction(Box<BuiltTransaction>, Option<Sender<SceneSwapResult>>),
+    ExternalEvent(ExternalEvent),
     FlushComplete(MsgSender<()>),
     Stopped,
 }
@@ -228,6 +228,10 @@ impl SceneBuilder {
                 Ok(SceneBuilderRequest::SaveScene(config)) => {
                     self.save_scene(config);
                 }
+                Ok(SceneBuilderRequest::ExternalEvent(evt)) => {
+                    self.tx.send(SceneBuilderResult::ExternalEvent(evt)).unwrap();
+                    self.api_tx.send(ApiMsg::WakeUp).unwrap();
+                }
                 Ok(SceneBuilderRequest::Stop) => {
                     self.tx.send(SceneBuilderResult::Stopped).unwrap();
                     // We don't need to send a WakeUp to api_tx because we only
@@ -307,7 +311,6 @@ impl SceneBuilder {
 
             let txn = Box::new(BuiltTransaction {
                 document_id: item.document_id,
-                build_frame: true,
                 render_frame: item.build_frame,
                 built_scene,
                 resource_updates: Vec::new(),
@@ -408,7 +411,6 @@ impl SceneBuilder {
 
         Box::new(BuiltTransaction {
             document_id: txn.document_id,
-            build_frame: txn.build_frame || built_scene.is_some(),
             render_frame: txn.render_frame,
             built_scene,
             rasterized_blobs,