Address review comments

Author: gw3583

webrender/src/clip.rs

@@ -321,9 +321,13 @@ pub struct ClipStore {
 #[derive(Debug)]
 pub struct ClipChainInstance {
     pub clips_range: ClipNodeRange,
+    // Combined clip rect for clips that are in the
+    // same coordinate system as the primitive.
     pub local_clip_rect: LayoutRect,
     pub has_non_root_coord_system: bool,
     pub has_non_local_clips: bool,
+    // Combined clip rect in picture space (may
+    // be more conservative that local_clip_rect).
     pub pic_clip_rect: PictureRect,
 }
 

webrender/src/frame_builder.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use api::{ColorF, DeviceIntPoint, DeviceIntRect, DevicePixelScale, LayoutPixel, PicturePixel};
+use api::{ColorF, DeviceIntPoint, DevicePixelScale, LayoutPixel, PicturePixel};
 use api::{DeviceUintPoint, DeviceUintRect, DeviceUintSize, DocumentLayer, FontRenderMode, PictureRect};
 use api::{LayoutPoint, LayoutRect, LayoutSize, PipelineId, WorldPoint, WorldRect, WorldPixel};
 use clip::{ClipStore};
@@ -71,7 +71,6 @@ pub struct FrameBuildingContext<'a> {
     pub device_pixel_scale: DevicePixelScale,
     pub scene_properties: &'a SceneProperties,
     pub pipelines: &'a FastHashMap<PipelineId, Arc<ScenePipeline>>,
-    pub screen_rect: DeviceIntRect,
     pub world_rect: WorldRect,
     pub clip_scroll_tree: &'a ClipScrollTree,
     pub transforms: &'a TransformPalette,
@@ -226,7 +225,6 @@ impl FrameBuilder {
             device_pixel_scale,
             scene_properties,
             pipelines,
-            screen_rect: self.screen_rect.to_i32(),
             world_rect,
             clip_scroll_tree,
             transforms: transform_palette,
@@ -283,8 +281,8 @@ impl FrameBuilder {
         let pic_state = pic.take_state();
 
         let root_render_task = RenderTask::new_picture(
-            RenderTaskLocation::Fixed(frame_context.screen_rect),
-            frame_context.screen_rect.size.to_f32(),
+            RenderTaskLocation::Fixed(self.screen_rect.to_i32()),
+            self.screen_rect.size.to_f32(),
             root_prim_index,
             DeviceIntPoint::zero(),
             pic_state.tasks,

webrender/src/prim_store.rs

@@ -2263,6 +2263,9 @@ impl Primitive {
         true
     }
 
+    // Returns true if the primitive *might* need a clip mask. If
+    // false, there is no need to even check for clip masks for
+    // this primitive.
     fn reset_clip_task(&mut self) -> bool {
         self.metadata.clip_task_id = None;
         match self.details {
@@ -2741,6 +2744,7 @@ impl Primitive {
             println!("\tupdating clip task with pic rect {:?}", clip_chain.pic_clip_rect);
         }
         // Reset clips from previous frames since we may clip differently each frame.
+        // If this primitive never needs clip masks, just return straight away.
         if !self.reset_clip_task() {
             return;
         }