Bug 1667918 - Pt 4 - Remove clipped_world_rect from primitive vis. r=nical

This patch removes the last use of clipped_world_rect, and removes
it from the PrimitiveVisibility struct.

Instead, the already calculated tile rect is used for visibility
determination when picture caching is enabled. When picture caching
is disabled, the clipped world rect is calculated on demand but
not stored.

This patch also introduces an enum for current visibility state,
which gives better control over accessing visibility information
depending on the current pass of frame building.

Differential Revision: https://phabricator.services.mozilla.com/D92545

gfx/wr/webrender/src/batch.rs

@@ -30,7 +30,7 @@ use crate::renderer::{BlendMode, ImageBufferKind, ShaderColorMode};
 use crate::renderer::{BLOCKS_PER_UV_RECT, MAX_VERTEX_TEXTURE_WIDTH};
 use crate::resource_cache::{CacheItem, GlyphFetchResult, ImageProperties, ImageRequest, ResourceCache};
 use crate::space::SpaceMapper;
-use crate::visibility::{PrimitiveVisibilityMask, PrimitiveVisibility, PrimitiveVisibilityFlags};
+use crate::visibility::{PrimitiveVisibilityMask, PrimitiveVisibility, PrimitiveVisibilityFlags, VisibilityState};
 use smallvec::SmallVec;
 use std::{f32, i32, usize};
 use crate::util::{project_rect, MaxRect, TransformedRectKind};
@@ -750,10 +750,6 @@ impl BatchBuilder {
                 continue;
             }
             for prim_instance in &pic.prim_list.prim_instances[cluster.prim_range()] {
-                if !prim_instance.is_visible() {
-                    continue;
-                }
-
                 // Add each run in this picture to the batch.
                 self.add_prim_to_batch(
                     prim_instance,
@@ -786,6 +782,7 @@ impl BatchBuilder {
         &mut self,
         prim_rect: LayoutRect,
         prim_info: &PrimitiveVisibility,
+        prim_vis_mask: PrimitiveVisibilityMask,
         z_id: ZBufferId,
         transform_id: TransformPaletteId,
         batch_features: BatchFeatures,
@@ -820,7 +817,6 @@ impl BatchBuilder {
 
         let bounding_rect = &prim_info.clip_chain.pic_clip_rect;
         let transform_kind = transform_id.transform_kind();
-        let prim_vis_mask = prim_info.visibility_mask;
 
         self.add_segmented_prim_to_batch(
             None,
@@ -859,11 +855,21 @@ impl BatchBuilder {
         z_generator: &mut ZBufferIdGenerator,
         composite_state: &mut CompositeState,
     ) {
+        let prim_vis_mask = match prim_instance.vis.state {
+            VisibilityState::Culled => {
+                return;
+            }
+            VisibilityState::Unset | VisibilityState::Coarse { .. } => {
+                panic!("bug: invalid visibility state");
+            }
+            VisibilityState::Detailed { visibility_mask } => {
+                visibility_mask
+            }
+        };
+
         #[cfg(debug_assertions)] //TODO: why is this needed?
         debug_assert_eq!(prim_instance.prepared_frame_id, render_tasks.frame_id());
 
-        assert!(prim_instance.is_visible(),
-            "Invisible primitive {:?} shouldn't be added to the batch", prim_instance);
         let is_chased = prim_instance.is_chased();
 
         let transform_id = transforms
@@ -906,7 +912,6 @@ impl BatchBuilder {
             batch_features |= BatchFeatures::ANTIALIASING;
         }
 
-        let prim_vis_mask = prim_info.visibility_mask;
         let clip_task_address = ctx.get_prim_clip_task_address(
             prim_info.clip_task_index,
             render_tasks,
@@ -1375,6 +1380,11 @@ impl BatchBuilder {
                             };
                             let pic = &ctx.prim_store.pictures[child_pic_index.0];
 
+                            let child_visibility_mask = match child_prim_info.state {
+                                VisibilityState::Detailed { visibility_mask } => visibility_mask,
+                                _ => panic!("bug: culled prim should not be in child list"),
+                            };
+
                             // Get clip task, if set, for the picture primitive.
                             let child_clip_task_address = ctx.get_prim_clip_task_address(
                                 child_prim_info.clip_task_index,
@@ -1428,7 +1438,7 @@ impl BatchBuilder {
                                 z_id,
                                 prim_header_index,
                                 child.gpu_address,
-                                child_prim_info.visibility_mask,
+                                child_visibility_mask,
                             );
                         }
                     }
@@ -2135,6 +2145,7 @@ impl BatchBuilder {
                 if is_compositor_surface {
                     self.emit_placeholder(prim_rect,
                                           prim_info,
+                                          prim_vis_mask,
                                           z_id,
                                           transform_id,
                                           batch_features,
@@ -2258,6 +2269,7 @@ impl BatchBuilder {
                 if is_compositor_surface {
                     self.emit_placeholder(prim_rect,
                                           prim_info,
+                                          prim_vis_mask,
                                           z_id,
                                           transform_id,
                                           batch_features,

gfx/wr/webrender/src/frame_builder.rs

@@ -398,9 +398,12 @@ impl FrameBuilder {
         // Push a default dirty region which culls primitives
         // against the screen world rect, in absence of any
         // other dirty regions.
-        let mut default_dirty_region = DirtyRegion::new();
+        let mut default_dirty_region = DirtyRegion::new(
+            ROOT_SPATIAL_NODE_INDEX,
+        );
         default_dirty_region.add_dirty_region(
-            frame_context.global_screen_world_rect,
+            frame_context.global_screen_world_rect.cast_unit(),
+            frame_context.spatial_tree,
         );
         frame_state.push_dirty_region(default_dirty_region);
 

gfx/wr/webrender/src/picture.rs

@@ -134,7 +134,8 @@ use std::ops::Range;
 use crate::texture_cache::TextureCacheHandle;
 use crate::util::{MaxRect, VecHelper, MatrixHelpers, Recycler, raster_rect_to_device_pixels, ScaleOffset};
 use crate::filterdata::{FilterDataHandle};
-use crate::visibility::{PrimitiveVisibilityMask, PrimitiveVisibilityFlags, FrameVisibilityContext, FrameVisibilityState};
+use crate::visibility::{PrimitiveVisibilityMask, PrimitiveVisibilityFlags, FrameVisibilityContext};
+use crate::visibility::{VisibilityState, FrameVisibilityState};
 #[cfg(any(feature = "capture", feature = "replay"))]
 use ron;
 #[cfg(feature = "capture")]
@@ -1733,10 +1734,10 @@ impl TileDescriptor {
 /// Stores both the world and devices rects for a single dirty rect.
 #[derive(Debug, Clone)]
 pub struct DirtyRegionRect {
-    /// World rect of this dirty region
-    pub world_rect: WorldRect,
     /// Bitfield for picture render tasks that draw this dirty region.
     pub visibility_mask: PrimitiveVisibilityMask,
+    /// The dirty region in space of the picture cache
+    pub rect_in_pic_space: PictureRect,
 }
 
 /// Represents the dirty region of a tile cache picture.
@@ -1747,30 +1748,51 @@ pub struct DirtyRegion {
 
     /// The overall dirty rect, a combination of dirty_rects
     pub combined: WorldRect,
+
+    /// Spatial node of the picture cache this region represents
+    spatial_node_index: SpatialNodeIndex,
 }
 
 impl DirtyRegion {
     /// Construct a new dirty region tracker.
     pub fn new(
+        spatial_node_index: SpatialNodeIndex,
     ) -> Self {
         DirtyRegion {
             dirty_rects: Vec::with_capacity(PrimitiveVisibilityMask::MAX_DIRTY_REGIONS),
             combined: WorldRect::zero(),
+            spatial_node_index,
         }
     }
 
     /// Reset the dirty regions back to empty
-    pub fn clear(&mut self) {
+    pub fn reset(
+        &mut self,
+        spatial_node_index: SpatialNodeIndex,
+    ) {
         self.dirty_rects.clear();
         self.combined = WorldRect::zero();
+        self.spatial_node_index = spatial_node_index;
     }
 
     /// Add a dirty region to the tracker. Returns the visibility mask that corresponds to
     /// this region in the tracker.
     pub fn add_dirty_region(
         &mut self,
-        world_rect: WorldRect,
+        rect_in_pic_space: PictureRect,
+        spatial_tree: &SpatialTree,
     ) -> PrimitiveVisibilityMask {
+        let map_pic_to_world = SpaceMapper::new_with_target(
+            ROOT_SPATIAL_NODE_INDEX,
+            self.spatial_node_index,
+            WorldRect::max_rect(),
+            spatial_tree,
+        );
+
+        let world_rect = map_pic_to_world
+            .map(&rect_in_pic_space)
+            .expect("bug");
+
         // Include this in the overall dirty rect
         self.combined = self.combined.union(&world_rect);
 
@@ -1781,8 +1803,8 @@ impl DirtyRegion {
             visibility_mask.set_visible(dirty_region_index);
 
             self.dirty_rects.push(DirtyRegionRect {
-                world_rect,
                 visibility_mask,
+                rect_in_pic_space,
             });
         } else {
             // If we run out of dirty regions, then force the last dirty region to
@@ -1793,7 +1815,7 @@ impl DirtyRegion {
             visibility_mask.set_visible(PrimitiveVisibilityMask::MAX_DIRTY_REGIONS - 1);
 
             let combined_region = self.dirty_rects.last_mut().unwrap();
-            combined_region.world_rect = combined_region.world_rect.union(&world_rect);
+            combined_region.rect_in_pic_space = combined_region.rect_in_pic_space.union(&rect_in_pic_space);
         }
 
         visibility_mask
@@ -1804,29 +1826,43 @@ impl DirtyRegion {
     pub fn inflate(
         &self,
         inflate_amount: f32,
+        spatial_tree: &SpatialTree,
     ) -> DirtyRegion {
+        let map_pic_to_world = SpaceMapper::new_with_target(
+            ROOT_SPATIAL_NODE_INDEX,
+            self.spatial_node_index,
+            WorldRect::max_rect(),
+            spatial_tree,
+        );
+
         let mut dirty_rects = Vec::with_capacity(self.dirty_rects.len());
         let mut combined = WorldRect::zero();
 
         for rect in &self.dirty_rects {
-            let world_rect = rect.world_rect.inflate(inflate_amount, inflate_amount);
+            let rect_in_pic_space = rect.rect_in_pic_space.inflate(inflate_amount, inflate_amount);
+
+            let world_rect = map_pic_to_world
+                .map(&rect_in_pic_space)
+                .expect("bug");
+
             combined = combined.union(&world_rect);
             dirty_rects.push(DirtyRegionRect {
-                world_rect,
                 visibility_mask: rect.visibility_mask,
+                rect_in_pic_space,
             });
         }
 
         DirtyRegion {
             dirty_rects,
             combined,
+            spatial_node_index: self.spatial_node_index,
         }
     }
 
     /// Creates a record of this dirty region for exporting to test infrastructure.
     pub fn record(&self) -> RecordedDirtyRegion {
-        let mut rects: Vec<WorldRect> =
-            self.dirty_rects.iter().map(|r| r.world_rect).collect();
+        let mut rects: Vec<PictureRect> =
+            self.dirty_rects.iter().map(|r| r.rect_in_pic_space).collect();
         rects.sort_unstable_by_key(|r| (r.origin.y as usize, r.origin.x as usize));
         RecordedDirtyRegion { rects }
     }
@@ -1835,7 +1871,7 @@ impl DirtyRegion {
 /// A recorded copy of the dirty region for exporting to test infrastructure.
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub struct RecordedDirtyRegion {
-    pub rects: Vec<WorldRect>,
+    pub rects: Vec<PictureRect>,
 }
 
 impl ::std::fmt::Display for RecordedDirtyRegion {
@@ -2357,7 +2393,7 @@ impl TileCacheInstance {
             spatial_node_comparer: SpatialNodeComparer::new(),
             color_bindings: FastHashMap::default(),
             old_color_bindings: FastHashMap::default(),
-            dirty_region: DirtyRegion::new(),
+            dirty_region: DirtyRegion::new(params.spatial_node_index),
             tile_size: PictureSize::zero(),
             tile_rect: TileRect::zero(),
             tile_bounds_p0: TileOffset::zero(),
@@ -3205,7 +3241,6 @@ impl TileCacheInstance {
         &mut self,
         prim_instance: &mut PrimitiveInstance,
         prim_spatial_node_index: SpatialNodeIndex,
-        prim_clip_chain: &ClipChainInstance,
         local_prim_rect: LayoutRect,
         frame_context: &FrameVisibilityContext,
         data_stores: &DataStores,
@@ -3215,10 +3250,11 @@ impl TileCacheInstance {
         color_bindings: &ColorBindingStorage,
         surface_stack: &[SurfaceIndex],
         composite_state: &mut CompositeState,
-    ) -> Option<PrimitiveVisibilityFlags> {
+    ) {
         // This primitive exists on the last element on the current surface stack.
         profile_scope!("update_prim_dependencies");
         let prim_surface_index = *surface_stack.last().unwrap();
+        let prim_clip_chain = &prim_instance.vis.clip_chain;
 
         self.map_local_to_surface.set_target_spatial_node(
             prim_spatial_node_index,
@@ -3228,12 +3264,12 @@ impl TileCacheInstance {
         // Map the primitive local rect into picture space.
         let prim_rect = match self.map_local_to_surface.map(&local_prim_rect) {
             Some(rect) => rect,
-            None => return None,
+            None => return,
         };
 
         // If the rect is invalid, no need to create dependencies.
         if prim_rect.size.is_empty() {
-            return None;
+            return;
         }
 
         // If the primitive is directly drawn onto this picture cache surface, then
@@ -3273,7 +3309,7 @@ impl TileCacheInstance {
                         rect.inflate(surface.inflation_factor, surface.inflation_factor)
                     }
                     None => {
-                        return None;
+                        return;
                     }
                 };
 
@@ -3289,7 +3325,7 @@ impl TileCacheInstance {
         // If the primitive is outside the tiling rects, it's known to not
         // be visible.
         if p0.x == p1.x || p0.y == p1.y {
-            return None;
+            return;
         }
 
         // Build the list of resources that this primitive has dependencies on.
@@ -3631,7 +3667,10 @@ impl TileCacheInstance {
             }
         }
 
-        Some(vis_flags)
+        prim_instance.vis.flags = vis_flags;
+        prim_instance.vis.state = VisibilityState::Coarse {
+            rect_in_pic_space: pic_clip_rect,
+        };
     }
 
     /// Print debug information about this picture cache to a tree printer.
@@ -3708,7 +3747,7 @@ impl TileCacheInstance {
         frame_context: &FrameVisibilityContext,
         frame_state: &mut FrameVisibilityState,
     ) {
-        self.dirty_region.clear();
+        self.dirty_region.reset(self.spatial_node_index);
         self.subpixel_mode = self.calculate_subpixel_mode();
 
         let map_pic_to_world = SpaceMapper::new_with_target(
@@ -5298,7 +5337,10 @@ impl PicturePrimitive {
                             // Get the visibility mask bit(s) for this tile from the dirty region tracker. This must be done
                             // outside the if statement below, so that we include in the dirty region tiles that are handled
                             // by a background color only (no surface allocation).
-                            let tile_vis_mask = tile_cache.dirty_region.add_dirty_region(world_dirty_rect);
+                            let tile_vis_mask = tile_cache.dirty_region.add_dirty_region(
+                                tile.local_dirty_rect,
+                                frame_context.spatial_tree,
+                            );
 
                             // Ensure that this texture is allocated.
                             if let TileSurface::Texture { ref mut descriptor, ref mut visibility_mask } = tile.surface.as_mut().unwrap() {
@@ -5581,7 +5623,10 @@ impl PicturePrimitive {
         }
 
         if inflation_factor > 0.0 {
-            let inflated_region = frame_state.current_dirty_region().inflate(inflation_factor);
+            let inflated_region = frame_state.current_dirty_region().inflate(
+                inflation_factor,
+                frame_context.spatial_tree,
+            );
             frame_state.push_dirty_region(inflated_region);
             dirty_region_count += 1;
         }

gfx/wr/webrender/src/prepare.rs

@@ -35,7 +35,7 @@ use crate::segment::SegmentBuilder;
 use crate::space::SpaceMapper;
 use crate::texture_cache::TEXTURE_REGION_DIMENSIONS;
 use crate::util::{clamp_to_scale_factor, pack_as_float, raster_rect_to_device_pixels};
-use crate::visibility::{compute_conservative_visible_rect, PrimitiveVisibility};
+use crate::visibility::{compute_conservative_visible_rect, PrimitiveVisibility, VisibilityState, PrimitiveVisibilityMask};
 
 
 const MAX_MASK_SIZE: f32 = 4096.0;
@@ -72,32 +72,44 @@ pub fn prepare_primitives(
             let prim_instance_index = cluster.prim_range.start + idx;
 
             // First check for coarse visibility (if this primitive was completely off-screen)
-            // TODO(gw): Remove the coarse calculations based on world rect - we can use the
-            //           tile bounds calculated during prim deps instead.
-            if !prim_instance.is_visible() {
-                continue;
-            }
-
-            // The original clipped world rect was calculated during the initial visibility pass.
-            // However, it's possible that the dirty rect has got smaller, if tiles were not
-            // dirty. Intersecting with the dirty rect here eliminates preparing any primitives
-            // outside the dirty rect, and reduces the size of any off-screen surface allocations
-            // for clip masks / render tasks that we make.
-
-            // Clear the current visibiilty mask, and build a more detailed one based on the dirty rect
-            // regions below.
-            prim_instance.clear_visibility();
-            let dirty_region = frame_state.current_dirty_region();
-
-            for dirty_region in &dirty_region.dirty_rects {
-                if prim_instance.vis.clipped_world_rect.intersects(&dirty_region.world_rect) {
-                    prim_instance.vis.visibility_mask.include(dirty_region.visibility_mask);
+            match prim_instance.vis.state {
+                VisibilityState::Unset => {
+                    panic!("bug: invalid vis state");
                 }
-            }
+                VisibilityState::Culled => {
+                    continue;
+                }
+                VisibilityState::Coarse { ref rect_in_pic_space } => {
+                    // The original coarse state was calculated during the initial visibility pass.
+                    // However, it's possible that the dirty rect has got smaller, if tiles were not
+                    // dirty. Intersecting with the dirty rect here eliminates preparing any primitives
+                    // outside the dirty rect, and reduces the size of any off-screen surface allocations
+                    // for clip masks / render tasks that we make.
 
-            // Check again if the prim is now visible after considering the current dirty regions.
-            if !prim_instance.is_visible() {
-                continue;
+                    // Clear the current visibiilty mask, and build a more detailed one based on the dirty rect
+                    // regions below.
+                    let dirty_region = frame_state.current_dirty_region();
+                    let mut visibility_mask = PrimitiveVisibilityMask::empty();
+
+                    for dirty_region in &dirty_region.dirty_rects {
+                        if rect_in_pic_space.intersects(&dirty_region.rect_in_pic_space) {
+                            visibility_mask.include(dirty_region.visibility_mask);
+                        }
+                    }
+
+                    // Check again if the prim is now visible after considering the current dirty regions.
+                    if visibility_mask.is_empty() {
+                        prim_instance.clear_visibility();
+                        continue;
+                    } else {
+                        prim_instance.vis.state = VisibilityState::Detailed {
+                            visibility_mask,
+                        }
+                    }
+                }
+                VisibilityState::Detailed { .. } => {
+                    // Was already set to detailed (picture caching disabled or a root element)
+                }
             }
 
             let plane_split_anchor = PlaneSplitAnchor::new(cluster_index, prim_instance_index);

gfx/wr/webrender/src/prim_store/mod.rs

@@ -39,7 +39,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
 use crate::storage;
 use crate::util::Recycler;
 use crate::internal_types::LayoutPrimitiveInfo;
-use crate::visibility::{PrimitiveVisibility, PrimitiveVisibilityMask};
+use crate::visibility::PrimitiveVisibility;
 
 pub mod backdrop;
 pub mod borders;
@@ -1056,19 +1056,11 @@ impl PrimitiveInstance {
 
     // Reset any pre-frame state for this primitive.
     pub fn reset(&mut self) {
-        self.clear_visibility();
-    }
-
-    pub fn is_visible(&self) -> bool {
-        !self.vis.visibility_mask.is_empty()
+        self.vis.reset();
     }
 
     pub fn clear_visibility(&mut self) {
-        self.set_visibility(PrimitiveVisibilityMask::empty());
-    }
-
-    pub fn set_visibility(&mut self, mask: PrimitiveVisibilityMask) {
-        self.vis.visibility_mask = mask;
+        self.vis.reset();
     }
 
     #[cfg(debug_assertions)]

gfx/wr/webrender/src/visibility.rs

@@ -13,7 +13,7 @@ use euclid::Scale;
 use std::{usize, mem};
 use crate::image_tiling;
 use crate::segment::EdgeAaSegmentMask;
-use crate::clip::{ClipStore, ClipChainStack, ClipNodeRange};
+use crate::clip::{ClipStore, ClipChainStack};
 use crate::composite::CompositeState;
 use crate::spatial_tree::{ROOT_SPATIAL_NODE_INDEX, SpatialTree, SpatialNodeIndex};
 use crate::clip::{ClipInstance, ClipChainInstance};
@@ -137,6 +137,28 @@ bitflags! {
     }
 }
 
+/// Contains the current state of the primitive's visibility.
+#[derive(Debug)]
+#[cfg_attr(feature = "capture", derive(Serialize))]
+pub enum VisibilityState {
+    /// Uninitialized - this should never be encountered after prim reset
+    Unset,
+    /// Culled for being off-screen, or not possible to render (e.g. missing image resource)
+    Culled,
+    /// During picture cache dependency update, was found to be intersecting with one
+    /// or more visible tiles. The rect in picture cache space is stored here to allow
+    /// the detailed calculations below.
+    Coarse {
+        rect_in_pic_space: PictureRect,
+    },
+    /// Once coarse visibility is resolved, this provides a bitmask of which dirty tiles
+    /// this primitive should be rasterized into.
+    Detailed {
+        /// A mask defining which of the dirty regions this primitive is visible in.
+        visibility_mask: PrimitiveVisibilityMask,
+    },
+}
+
 /// Information stored for a visible primitive about the visible
 /// rect and associated clip information.
 #[derive(Debug)]
@@ -145,11 +167,10 @@ pub struct PrimitiveVisibility {
     /// The clip chain instance that was built for this primitive.
     pub clip_chain: ClipChainInstance,
 
-    /// The current world rect, clipped to screen / dirty rect boundaries.
-    // TODO(gw): This is only used by a small number of primitives.
-    //           It's probably faster to not store this and recalculate
-    //           on demand in those cases?
-    pub clipped_world_rect: WorldRect,
+    /// Current visibility state of the primitive.
+    // TODO(gw): Move more of the fields from this struct into
+    //           the state enum.
+    pub state: VisibilityState,
 
     /// An index into the clip task instances array in the primitive
     /// store. If this is ClipTaskIndex::INVALID, then the primitive
@@ -162,9 +183,6 @@ pub struct PrimitiveVisibility {
     /// during batching of visibile primitives.
     pub flags: PrimitiveVisibilityFlags,
 
-    /// A mask defining which of the dirty regions this primitive is visible in.
-    pub visibility_mask: PrimitiveVisibilityMask,
-
     /// The current combined local clip for this primitive, from
     /// the primitive local clip above and the current clip chain.
     pub combined_local_clip_rect: LayoutRect,
@@ -173,24 +191,19 @@ pub struct PrimitiveVisibility {
 impl PrimitiveVisibility {
     pub fn new() -> Self {
         PrimitiveVisibility {
-            clip_chain: ClipChainInstance {
-                clips_range: ClipNodeRange {
-                    first: 0,
-                    count: 0,
-                },
-                local_clip_rect: LayoutRect::zero(),
-                has_non_local_clips: false,
-                needs_mask: false,
-                pic_clip_rect: PictureRect::zero(),
-                pic_spatial_node_index: ROOT_SPATIAL_NODE_INDEX,
-            },
-            clipped_world_rect: WorldRect::zero(),
+            state: VisibilityState::Unset,
+            clip_chain: ClipChainInstance::empty(),
             clip_task_index: ClipTaskIndex::INVALID,
             flags: PrimitiveVisibilityFlags::empty(),
-            visibility_mask: PrimitiveVisibilityMask::empty(),
             combined_local_clip_rect: LayoutRect::zero(),
         }
     }
+
+    pub fn reset(&mut self) {
+        self.state = VisibilityState::Culled;
+        self.clip_task_index = ClipTaskIndex::INVALID;
+        self.flags = PrimitiveVisibilityFlags::empty();
+    }
 }
 
 /// Update visibility pass - update each primitive visibility struct, and
@@ -375,13 +388,9 @@ pub fn update_primitive_visibility(
             };
 
             if is_passthrough {
-                prim_instance.vis = PrimitiveVisibility {
-                    clipped_world_rect: WorldRect::max_rect(),
-                    clip_chain: ClipChainInstance::empty(),
-                    clip_task_index: ClipTaskIndex::INVALID,
-                    combined_local_clip_rect: LayoutRect::zero(),
+                // Pass through pictures are always considered visible in all dirty tiles.
+                prim_instance.vis.state = VisibilityState::Detailed {
                     visibility_mask: PrimitiveVisibilityMask::all(),
-                    flags: PrimitiveVisibilityFlags::empty(),
                 };
             } else {
                 if prim_local_rect.size.width <= 0.0 || prim_local_rect.size.height <= 0.0 {
@@ -443,61 +452,43 @@ pub fn update_primitive_visibility(
                 // Ensure the primitive clip is popped
                 frame_state.clip_chain_stack.pop_clip();
 
-                let clip_chain = match clip_chain {
+                prim_instance.vis.clip_chain = match clip_chain {
                     Some(clip_chain) => clip_chain,
                     None => {
                         if prim_instance.is_chased() {
                             println!("\tunable to build the clip chain, skipping");
                         }
-                        prim_instance.clear_visibility();
                         continue;
                     }
                 };
 
                 if prim_instance.is_chased() {
                     println!("\teffective clip chain from {:?} {}",
-                             clip_chain.clips_range,
+                             prim_instance.vis.clip_chain.clips_range,
                              if apply_local_clip_rect { "(applied)" } else { "" },
                     );
                     println!("\tpicture rect {:?} @{:?}",
-                             clip_chain.pic_clip_rect,
-                             clip_chain.pic_spatial_node_index,
+                             prim_instance.vis.clip_chain.pic_clip_rect,
+                             prim_instance.vis.clip_chain.pic_spatial_node_index,
                     );
                 }
 
-                // Check if the clip bounding rect (in pic space) is visible on screen
-                // This includes both the prim bounding rect + local prim clip rect!
-                let world_rect = match map_surface_to_world.map(&clip_chain.pic_clip_rect) {
-                    Some(world_rect) => world_rect,
-                    None => {
-                        continue;
-                    }
-                };
-
-                let clipped_world_rect = match world_rect.intersection(&world_culling_rect) {
-                    Some(rect) => rect,
-                    None => {
-                        continue;
-                    }
-                };
-
-                let combined_local_clip_rect = if apply_local_clip_rect {
-                    clip_chain.local_clip_rect
+                prim_instance.vis.combined_local_clip_rect = if apply_local_clip_rect {
+                    prim_instance.vis.clip_chain.local_clip_rect
                 } else {
                     prim_instance.clip_set.local_clip_rect
                 };
 
-                if combined_local_clip_rect.size.is_empty() {
+                if prim_instance.vis.combined_local_clip_rect.size.is_empty() {
                     if prim_instance.is_chased() {
                         println!("\tculled for zero local clip rectangle");
                     }
-                    prim_instance.clear_visibility();
                     continue;
                 }
 
                 // Include the visible area for primitive, including any shadows, in
                 // the area affected by the surface.
-                match combined_local_clip_rect.intersection(&local_rect) {
+                match prim_instance.vis.combined_local_clip_rect.intersection(&local_rect) {
                     Some(visible_rect) => {
                         if let Some(rect) = map_local_to_surface.map(&visible_rect) {
                             surface_rect = surface_rect.union(&rect);
@@ -507,44 +498,55 @@ pub fn update_primitive_visibility(
                         if prim_instance.is_chased() {
                             println!("\tculled for zero visible rectangle");
                         }
-                        prim_instance.clear_visibility();
                         continue;
                     }
                 }
 
-                // Primitive visibility flags default to empty, but may be supplied
-                // by the `update_prim_dependencies` method below when picture caching
-                // is active.
-                let mut vis_flags = PrimitiveVisibilityFlags::empty();
-
-                if let Some(ref mut tile_cache) = frame_state.tile_cache {
-                    // TODO(gw): Refactor how tile_cache is stored in frame_state
-                    //           so that we can pass frame_state directly to
-                    //           update_prim_dependencies, rather than splitting borrows.
-                    match tile_cache.update_prim_dependencies(
-                        prim_instance,
-                        cluster.spatial_node_index,
-                        &clip_chain,
-                        prim_local_rect,
-                        frame_context,
-                        frame_state.data_stores,
-                        frame_state.clip_store,
-                        &store.pictures,
-                        frame_state.resource_cache,
-                        &store.color_bindings,
-                        &frame_state.surface_stack,
-                        &mut frame_state.composite_state,
-                    ) {
-                        Some(flags) => {
-                            vis_flags = flags;
-                        }
-                        None => {
-                            prim_instance.clear_visibility();
-                            // Ensure the primitive clip is popped - perhaps we can use
-                            // some kind of scope to do this automatically in future.
-                            continue;
-                        }
+                match frame_state.tile_cache {
+                    Some(ref mut tile_cache) => {
+                        // TODO(gw): Refactor how tile_cache is stored in frame_state
+                        //           so that we can pass frame_state directly to
+                        //           update_prim_dependencies, rather than splitting borrows.
+                        tile_cache.update_prim_dependencies(
+                            prim_instance,
+                            cluster.spatial_node_index,
+                            prim_local_rect,
+                            frame_context,
+                            frame_state.data_stores,
+                            frame_state.clip_store,
+                            &store.pictures,
+                            frame_state.resource_cache,
+                            &store.color_bindings,
+                            &frame_state.surface_stack,
+                            &mut frame_state.composite_state,
+                        );
                     }
+                    None => {
+                        // When picture cache is not in use, cull against the main world culling rect only.
+                        let clipped_world_rect = calculate_prim_clipped_world_rect(
+                            &prim_instance.vis.clip_chain.pic_clip_rect,
+                            &world_culling_rect,
+                            &map_surface_to_world,
+                        );
+
+                        prim_instance.vis.state = match clipped_world_rect {
+                            Some(_) => {
+                                VisibilityState::Detailed {
+                                    visibility_mask: PrimitiveVisibilityMask::all(),
+                                }
+                            }
+                            None => {
+                                VisibilityState::Culled
+                            }
+                        };
+                    }
+                }
+
+                // Skip post visibility prim update if this primitive was culled above.
+                match prim_instance.vis.state {
+                    VisibilityState::Unset => panic!("bug: invalid state"),
+                    VisibilityState::Culled => continue,
+                    VisibilityState::Coarse { .. } | VisibilityState::Detailed { .. } => {}
                 }
 
                 // When the debug display is enabled, paint a colored rectangle around each
@@ -566,8 +568,14 @@ pub fn update_primitive_visibility(
                         PrimitiveInstanceKind::Backdrop { .. } => debug_colors::MEDIUMAQUAMARINE,
                     };
                     if debug_color.a != 0.0 {
-                        let debug_rect = clipped_world_rect * frame_context.global_device_pixel_scale;
-                        frame_state.scratch.primitive.push_debug_rect(debug_rect, debug_color, debug_color.scale_alpha(0.5));
+                        if let Some(rect) = calculate_prim_clipped_world_rect(
+                            &prim_instance.vis.clip_chain.pic_clip_rect,
+                            &world_culling_rect,
+                            &map_surface_to_world,
+                        ) {
+                            let debug_rect = rect * frame_context.global_device_pixel_scale;
+                            frame_state.scratch.primitive.push_debug_rect(debug_rect, debug_color, debug_color.scale_alpha(0.5));
+                        }
                     }
                 } else if frame_context.debug_flags.contains(::api::DebugFlags::OBSCURE_IMAGES) {
                     let is_image = matches!(
@@ -576,26 +584,23 @@ pub fn update_primitive_visibility(
                     );
                     if is_image {
                         // We allow "small" images, since they're generally UI elements.
-                        let rect = clipped_world_rect * frame_context.global_device_pixel_scale;
-                        if rect.size.width > 70.0 && rect.size.height > 70.0 {
-                            frame_state.scratch.primitive.push_debug_rect(rect, debug_colors::PURPLE, debug_colors::PURPLE);
+                        if let Some(rect) = calculate_prim_clipped_world_rect(
+                            &prim_instance.vis.clip_chain.pic_clip_rect,
+                            &world_culling_rect,
+                            &map_surface_to_world,
+                        ) {
+                            let rect = rect * frame_context.global_device_pixel_scale;
+                            if rect.size.width > 70.0 && rect.size.height > 70.0 {
+                                frame_state.scratch.primitive.push_debug_rect(rect, debug_colors::PURPLE, debug_colors::PURPLE);
+                            }
                         }
                     }
                 }
 
                 if prim_instance.is_chased() {
-                    println!("\tvisible with {:?}", combined_local_clip_rect);
+                    println!("\tvisible with {:?}", prim_instance.vis.combined_local_clip_rect);
                 }
 
-                prim_instance.vis = PrimitiveVisibility {
-                    clipped_world_rect: clipped_world_rect,
-                    clip_chain,
-                    clip_task_index: ClipTaskIndex::INVALID,
-                    combined_local_clip_rect,
-                    visibility_mask: PrimitiveVisibilityMask::all(),
-                    flags: vis_flags,
-                };
-
                 // TODO(gw): This should probably be an instance method on PrimitiveInstance?
                 update_prim_post_visibility(
                     store,
@@ -904,3 +909,15 @@ pub fn compute_conservative_visible_rect(
         None => clip_chain.local_clip_rect,
     }
 }
+
+fn calculate_prim_clipped_world_rect(
+    pic_clip_rect: &PictureRect,
+    world_culling_rect: &WorldRect,
+    map_surface_to_world: &SpaceMapper<PicturePixel, WorldPixel>,
+) -> Option<WorldRect> {
+    map_surface_to_world
+        .map(pic_clip_rect)
+        .and_then(|world_rect| {
+            world_rect.intersection(world_culling_rect)
+        })
+}