Bug 1773905 - Remove prepared_frame_id and chase id r=gfx-reviewers,lsalzman

As part of the frame building work being done, the PrimitiveInstance
struct will change significantly (most of the state will be moved
to the primitive template, instance will be closer to an index +
some visibility state).

`prepared_frame_id` is no longer relevant (it can't be incorrect
now due to the way prims are reset). The chase is is not used or
maintained, and would need to be quite different after the changes
mentioned above anyway.

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

gfx/wr/webrender/src/batch.rs

@@ -850,9 +850,6 @@ impl BatchBuilder {
             }
         };
 
-        #[cfg(debug_assertions)] //TODO: why is this needed?
-        debug_assert_eq!(prim_instance.prepared_frame_id, render_tasks.frame_id());
-
         let transform_id = transforms
             .get_id(
                 prim_spatial_node_index,

gfx/wr/webrender/src/clip.rs

@@ -1294,16 +1294,12 @@ impl ClipStore {
         world_rect: &WorldRect,
         clip_data_store: &mut ClipDataStore,
         request_resources: bool,
-        is_chased: bool,
     ) -> Option<ClipChainInstance> {
         let local_clip_rect = match self.active_local_clip_rect {
             Some(rect) => rect,
             None => return None,
         };
         profile_scope!("build_clip_chain_instance");
-        if is_chased {
-            info!("\tbuilding clip chain instance with local rect {:?}", local_prim_rect);
-        }
 
         let local_bounding_rect = local_prim_rect.intersection(&local_clip_rect)?;
         let mut pic_coverage_rect = prim_to_pic_mapper.map(&local_bounding_rect)?;
@@ -1341,11 +1337,6 @@ impl ClipStore {
                 }
             };
 
-            if is_chased {
-                info!("\t\tclip {:?}", node.item);
-                info!("\t\tflags {:?}, resulted in {:?}", node_info.conversion.to_flags(), clip_result);
-            }
-
             match clip_result {
                 ClipResult::Accept => {
                     // Doesn't affect the primitive at all, so skip adding to list

gfx/wr/webrender/src/frame_builder.rs

@@ -17,7 +17,7 @@ use crate::picture::{DirtyRegion, SliceId, TileCacheInstance};
 use crate::picture::{SurfaceInfo, SurfaceIndex};
 use crate::picture::{SubpixelMode, RasterConfig, PictureCompositeMode};
 use crate::prepare::prepare_primitives;
-use crate::prim_store::{PictureIndex, PrimitiveDebugId};
+use crate::prim_store::{PictureIndex};
 use crate::prim_store::{DeferredResolve, PrimitiveInstance};
 use crate::profiler::{self, TransactionProfile};
 use crate::render_backend::{DataStores, ScratchBuffer};
@@ -36,22 +36,6 @@ use crate::util::{VecHelper, Preallocator};
 use crate::visibility::{update_prim_visibility, FrameVisibilityState, FrameVisibilityContext};
 use plane_split::Splitter;
 
-
-#[derive(Clone, Copy, Debug, PartialEq)]
-#[cfg_attr(feature = "capture", derive(Serialize))]
-#[cfg_attr(feature = "replay", derive(Deserialize))]
-pub enum ChasePrimitive {
-    Nothing,
-    Id(PrimitiveDebugId),
-    LocalRect(LayoutRect),
-}
-
-impl Default for ChasePrimitive {
-    fn default() -> Self {
-        ChasePrimitive::Nothing
-    }
-}
-
 #[derive(Clone, Copy, Debug)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
@@ -59,7 +43,6 @@ pub struct FrameBuilderConfig {
     pub default_font_render_mode: FontRenderMode,
     pub dual_source_blending_is_supported: bool,
     pub dual_source_blending_is_enabled: bool,
-    pub chase_primitive: ChasePrimitive,
     /// True if we're running tests (i.e. via wrench).
     pub testing: bool,
     pub gpu_supports_fast_clears: bool,

gfx/wr/webrender/src/lib.rs

@@ -207,8 +207,6 @@ pub use crate::composite::{MappableCompositor, MappedTileInfo, SWGLCompositeSurf
 pub use crate::device::{UploadMethod, VertexUsageHint, get_gl_target, get_unoptimized_shader_source};
 pub use crate::device::{ProgramBinary, ProgramCache, ProgramCacheObserver, FormatDesc};
 pub use crate::device::Device;
-pub use crate::frame_builder::ChasePrimitive;
-pub use crate::prim_store::PrimitiveDebugId;
 pub use crate::profiler::{ProfilerHooks, set_profiler_hooks};
 pub use crate::renderer::{
     AsyncPropertySampler, CpuProfile, DebugFlags, GpuProfile, GraphicsApi,

gfx/wr/webrender/src/picture.rs

@@ -2171,7 +2171,6 @@ impl TileCacheInstance {
                 &frame_context.global_screen_world_rect,
                 &mut frame_state.data_stores.clip,
                 true,
-                false,
             );
 
             // Ensure that if the entire picture cache is clipped out, the local

gfx/wr/webrender/src/prepare.rs

@@ -200,20 +200,8 @@ fn prepare_prim_for_render(
             data_stores,
             scratch,
         ) {
-            if prim_instance.is_chased() {
-                info!("\tconsidered invisible");
-            }
             return false;
         }
-
-        if prim_instance.is_chased() {
-            info!("\tconsidered visible and ready with local pos {:?}", prim_rect.min);
-        }
-    }
-
-    #[cfg(debug_assertions)]
-    {
-        prim_instance.prepared_frame_id = frame_state.rg_builder.frame_id();
     }
 
     prepare_interned_prim_for_render(
@@ -246,7 +234,6 @@ fn prepare_interned_prim_for_render(
     scratch: &mut PrimitiveScratchBuffer,
 ) {
     let prim_spatial_node_index = cluster.spatial_node_index;
-    let is_chased = prim_instance.is_chased();
     let device_pixel_scale = frame_state.surfaces[pic_context.surface_index.0].device_pixel_scale;
 
     match &mut prim_instance.kind {
@@ -261,9 +248,6 @@ fn prepare_interned_prim_for_render(
             line_dec_data.update(common_data, frame_state);
 
             // Work out the device pixel size to be used to cache this line decoration.
-            if is_chased {
-                info!("\tline decoration key={:?}", line_dec_data.cache_key);
-            }
 
             // If we have a cache key, it's a wavy / dashed / dotted line. Otherwise, it's
             // a simple solid line.
@@ -1073,7 +1057,6 @@ fn update_clip_task_for_brush(
                     &dirty_world_rect,
                     &mut data_stores.clip,
                     false,
-                    instance.is_chased(),
                 );
 
             let clip_mask_kind = update_brush_segment_clip_task(
@@ -1108,10 +1091,6 @@ pub fn update_clip_task(
 ) -> bool {
     let device_pixel_scale = frame_state.surfaces[pic_context.surface_index.0].device_pixel_scale;
 
-    if instance.is_chased() {
-        info!("\tupdating clip task with pic rect {:?}", instance.vis.clip_chain.pic_coverage_rect);
-    }
-
     build_segments_if_needed(
         instance,
         frame_state,
@@ -1138,9 +1117,6 @@ pub fn update_clip_task(
         &mut scratch.clip_mask_instances,
         device_pixel_scale,
     ) {
-        if instance.is_chased() {
-            info!("\tsegment tasks have been created for clipping: {:?}", clip_task_index);
-        }
         clip_task_index
     } else if instance.vis.clip_chain.needs_mask {
         // Get a minimal device space rect, clipped to the screen that we
@@ -1171,10 +1147,6 @@ pub fn update_clip_task(
             frame_context.fb_config,
             &mut frame_state.surface_builder,
         );
-        if instance.is_chased() {
-            info!("\tcreated task {:?} with device rect {:?}",
-                clip_task_id, device_rect);
-        }
         // Set the global clip mask instance for this primitive.
         let clip_task_index = ClipTaskIndex(scratch.clip_mask_instances.len() as _);
         scratch.clip_mask_instances.push(ClipMaskKind::Mask(clip_task_id));
@@ -1185,9 +1157,6 @@ pub fn update_clip_task(
         );
         clip_task_index
     } else {
-        if instance.is_chased() {
-            info!("\tno mask is needed");
-        }
         ClipTaskIndex::INVALID
     };
 

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

@@ -25,12 +25,8 @@ use crate::render_task_graph::RenderTaskId;
 use crate::resource_cache::ImageProperties;
 use crate::scene::SceneProperties;
 use std::{hash, ops, u32, usize};
-#[cfg(debug_assertions)]
-use std::sync::atomic::{AtomicUsize, Ordering};
 use crate::util::Recycler;
 use crate::internal_types::{FastHashSet, LayoutPrimitiveInfo};
-#[cfg(debug_assertions)]
-use crate::internal_types::FrameId;
 use crate::visibility::PrimitiveVisibility;
 
 pub mod backdrop;
@@ -54,22 +50,6 @@ use text_run::{TextRunDataHandle, TextRunPrimitive};
 
 pub const VECS_PER_SEGMENT: usize = 2;
 
-/// Counter for unique primitive IDs for debug tracing.
-#[cfg(debug_assertions)]
-static NEXT_PRIM_ID: AtomicUsize = AtomicUsize::new(0);
-
-#[cfg(debug_assertions)]
-static PRIM_CHASE_ID: AtomicUsize = AtomicUsize::new(usize::MAX);
-
-#[cfg(debug_assertions)]
-pub fn register_prim_chase_id(id: PrimitiveDebugId) {
-    PRIM_CHASE_ID.store(id.0, Ordering::SeqCst);
-}
-
-#[cfg(not(debug_assertions))]
-pub fn register_prim_chase_id(_: PrimitiveDebugId) {
-}
-
 #[cfg_attr(feature = "capture", derive(Serialize))]
 #[cfg_attr(feature = "replay", derive(Deserialize))]
 #[derive(Debug, Copy, Clone, MallocSizeOf)]
@@ -970,11 +950,6 @@ impl CreateShadow for PrimitiveKeyKind {
     }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq)]
-#[cfg_attr(feature = "capture", derive(Serialize))]
-#[cfg_attr(feature = "replay", derive(Deserialize))]
-pub struct PrimitiveDebugId(pub usize);
-
 #[derive(Debug)]
 #[cfg_attr(feature = "capture", derive(Serialize))]
 pub enum PrimitiveInstanceKind {
@@ -1096,14 +1071,6 @@ pub struct PrimitiveInstance {
     /// can be found.
     pub kind: PrimitiveInstanceKind,
 
-    #[cfg(debug_assertions)]
-    pub id: PrimitiveDebugId,
-
-    /// The last frame ID (of the `RenderTaskGraph`) this primitive
-    /// was prepared for rendering in.
-    #[cfg(debug_assertions)]
-    pub prepared_frame_id: FrameId,
-
     /// All information and state related to clip(s) for this primitive
     pub clip_set: ClipSet,
 
@@ -1122,10 +1089,6 @@ impl PrimitiveInstance {
     ) -> Self {
         PrimitiveInstance {
             kind,
-            #[cfg(debug_assertions)]
-            prepared_frame_id: FrameId::INVALID,
-            #[cfg(debug_assertions)]
-            id: PrimitiveDebugId(NEXT_PRIM_ID.fetch_add(1, Ordering::Relaxed)),
             vis: PrimitiveVisibility::new(),
             clip_set: ClipSet {
                 local_clip_rect,
@@ -1137,28 +1100,12 @@ impl PrimitiveInstance {
     // Reset any pre-frame state for this primitive.
     pub fn reset(&mut self) {
         self.vis.reset();
-
-        if self.is_chased() {
-            #[cfg(debug_assertions)] // needed for ".id" part
-            info!("\tpreparing {:?}", self.id);
-            info!("\t{:?}", self.kind);
-        }
     }
 
     pub fn clear_visibility(&mut self) {
         self.vis.reset();
     }
 
-    #[cfg(debug_assertions)]
-    pub fn is_chased(&self) -> bool {
-        PRIM_CHASE_ID.load(Ordering::SeqCst) == self.id.0
-    }
-
-    #[cfg(not(debug_assertions))]
-    pub fn is_chased(&self) -> bool {
-        false
-    }
-
     pub fn uid(&self) -> intern::ItemUid {
         match &self.kind {
             PrimitiveInstanceKind::Clear { data_handle, .. } |
@@ -1497,7 +1444,7 @@ fn test_struct_sizes() {
     //     test expectations and move on.
     // (b) You made a structure larger. This is not necessarily a problem, but should only
     //     be done with care, and after checking if talos performance regresses badly.
-    assert_eq!(mem::size_of::<PrimitiveInstance>(), 152, "PrimitiveInstance size changed");
+    assert_eq!(mem::size_of::<PrimitiveInstance>(), 136, "PrimitiveInstance size changed");
     assert_eq!(mem::size_of::<PrimitiveInstanceKind>(), 24, "PrimitiveInstanceKind size changed");
     assert_eq!(mem::size_of::<PrimitiveTemplate>(), 56, "PrimitiveTemplate size changed");
     assert_eq!(mem::size_of::<PrimitiveTemplateKind>(), 28, "PrimitiveTemplateKind size changed");

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

@@ -66,7 +66,7 @@ use crate::device::query::{GpuSampler, GpuTimer};
 #[cfg(feature = "capture")]
 use crate::device::FBOId;
 use crate::debug_item::DebugItem;
-use crate::frame_builder::{Frame, ChasePrimitive, FrameBuilderConfig};
+use crate::frame_builder::{Frame, FrameBuilderConfig};
 use crate::glyph_cache::GlyphCache;
 use crate::glyph_rasterizer::{GlyphFormat, GlyphRasterizer, SharedFontResources};
 use crate::gpu_cache::{GpuCacheUpdate, GpuCacheUpdateList};
@@ -1171,7 +1171,6 @@ impl Renderer {
             default_font_render_mode,
             dual_source_blending_is_enabled: true,
             dual_source_blending_is_supported: use_dual_source_blending,
-            chase_primitive: options.chase_primitive,
             testing: options.testing,
             gpu_supports_fast_clears: options.gpu_supports_fast_clears,
             gpu_supports_advanced_blend: ext_blend_equation_advanced,
@@ -5746,7 +5745,6 @@ pub struct RendererOptions {
     pub renderer_id: Option<u64>,
     pub scene_builder_hooks: Option<Box<dyn SceneBuilderHooks + Send>>,
     pub sampler: Option<Box<dyn AsyncPropertySampler + Send>>,
-    pub chase_primitive: ChasePrimitive,
     pub support_low_priority_transactions: bool,
     pub namespace_alloc_by_client: bool,
     /// If namespaces are allocated by the client, then the namespace for fonts
@@ -5844,7 +5842,6 @@ impl Default for RendererOptions {
             cached_programs: None,
             scene_builder_hooks: None,
             sampler: None,
-            chase_primitive: ChasePrimitive::Nothing,
             support_low_priority_transactions: false,
             namespace_alloc_by_client: false,
             shared_font_namespace: None,

gfx/wr/webrender/src/scene.rs

@@ -10,7 +10,7 @@ use crate::render_api::MemoryReport;
 use crate::composite::CompositorKind;
 use crate::clip::{ClipStore, ClipStoreStats};
 use crate::spatial_tree::SpatialTree;
-use crate::frame_builder::{ChasePrimitive, FrameBuilderConfig};
+use crate::frame_builder::{FrameBuilderConfig};
 use crate::hit_test::{HitTester, HitTestingScene, HitTestingSceneStats};
 use crate::internal_types::{FastHashMap, PlaneSplitter};
 use crate::picture::SurfaceInfo;
@@ -314,7 +314,6 @@ impl BuiltScene {
                 default_font_render_mode: FontRenderMode::Mono,
                 dual_source_blending_is_enabled: true,
                 dual_source_blending_is_supported: false,
-                chase_primitive: ChasePrimitive::Nothing,
                 testing: false,
                 gpu_supports_fast_clears: false,
                 gpu_supports_advanced_blend: false,

gfx/wr/webrender/src/scene_building.rs

@@ -52,7 +52,7 @@ use crate::clip::{ClipInternData, ClipNodeKind, ClipInstance, SceneClipInstance}
 use crate::clip::{PolygonDataHandle};
 use crate::segment::EdgeAaSegmentMask;
 use crate::spatial_tree::{SceneSpatialTree, SpatialNodeIndex, get_external_scroll_offset};
-use crate::frame_builder::{ChasePrimitive, FrameBuilderConfig};
+use crate::frame_builder::{FrameBuilderConfig};
 use crate::glyph_rasterizer::{FontInstance, SharedFontResources};
 use crate::hit_test::HitTestingScene;
 use crate::intern::Interner;
@@ -60,7 +60,7 @@ use crate::internal_types::{FastHashMap, LayoutPrimitiveInfo, Filter, PlaneSplit
 use crate::picture::{Picture3DContext, PictureCompositeMode, PicturePrimitive};
 use crate::picture::{BlitReason, OrderedPictureChild, PrimitiveList, SurfaceInfo, PictureFlags};
 use crate::picture_graph::PictureGraph;
-use crate::prim_store::{PrimitiveInstance, register_prim_chase_id};
+use crate::prim_store::{PrimitiveInstance};
 use crate::prim_store::{PrimitiveInstanceKind, NinePatchDescriptor, PrimitiveStore};
 use crate::prim_store::{InternablePrimitive, SegmentInstanceIndex, PictureIndex};
 use crate::prim_store::{PolygonKey};
@@ -1860,9 +1860,6 @@ impl<'a> SceneBuilder<'a> {
         flags: PrimitiveFlags,
     ) {
         // Add primitive to the top-most stacking context on the stack.
-        if prim_instance.is_chased() {
-            info!("\tadded to stacking context at {}", self.sc_stack.len());
-        }
 
         // If we have a valid stacking context, the primitive gets added to that.
         // Otherwise, it gets added to a top-level picture cache slice.
@@ -1975,10 +1972,6 @@ impl<'a> SceneBuilder<'a> {
             clip_chain_id,
             prim,
         );
-        self.register_chase_primitive_by_rect(
-            &info.rect,
-            &prim_instance,
-        );
         self.add_primitive_to_draw_list(
             prim_instance,
             info.rect,
@@ -2615,11 +2608,6 @@ impl<'a> SceneBuilder<'a> {
         viewport_size: &LayoutSize,
         instance: PipelineInstanceId,
     ) {
-        if let ChasePrimitive::Id(id) = self.config.chase_primitive {
-            debug!("Chasing {:?} by index", id);
-            register_prim_chase_id(id);
-        }
-
         let spatial_node_index = self.push_reference_frame(
             SpatialId::root_reference_frame(pipeline_id),
             self.spatial_tree.root_reference_frame_index(),
@@ -3080,26 +3068,6 @@ impl<'a> SceneBuilder<'a> {
         }
     }
 
-    #[cfg(debug_assertions)]
-    fn register_chase_primitive_by_rect(
-        &mut self,
-        rect: &LayoutRect,
-        prim_instance: &PrimitiveInstance,
-    ) {
-        if ChasePrimitive::LocalRect(*rect) == self.config.chase_primitive {
-            debug!("Chasing {:?} by local rect", prim_instance.id);
-            register_prim_chase_id(prim_instance.id);
-        }
-    }
-
-    #[cfg(not(debug_assertions))]
-    fn register_chase_primitive_by_rect(
-        &mut self,
-        _rect: &LayoutRect,
-        _prim_instance: &PrimitiveInstance,
-    ) {
-    }
-
     pub fn add_clear_rectangle(
         &mut self,
         spatial_node_index: SpatialNodeIndex,

gfx/wr/webrender/src/visibility.rs

@@ -312,7 +312,6 @@ pub fn update_prim_visibility(
                     &world_culling_rect,
                     &mut frame_state.data_stores.clip,
                     true,
-                    prim_instance.is_chased(),
                 );
 
             // Ensure the primitive clip is popped
@@ -321,24 +320,10 @@ pub fn update_prim_visibility(
             prim_instance.vis.clip_chain = match clip_chain {
                 Some(clip_chain) => clip_chain,
                 None => {
-                    if prim_instance.is_chased() {
-                        info!("\tunable to build the clip chain, skipping");
-                    }
                     continue;
                 }
             };
 
-            if prim_instance.is_chased() {
-                info!("\teffective clip chain from {:?} {}",
-                         prim_instance.vis.clip_chain.clips_range,
-                         if pic.apply_local_clip_rect { "(applied)" } else { "" },
-                );
-                info!("\tpicture rect {:?} @{:?}",
-                         prim_instance.vis.clip_chain.pic_coverage_rect,
-                         prim_instance.vis.clip_chain.pic_spatial_node_index,
-                );
-            }
-
             prim_instance.vis.combined_local_clip_rect = if pic.apply_local_clip_rect {
                 prim_instance.vis.clip_chain.local_clip_rect
             } else {

gfx/wr/wrench/src/main.rs

@@ -633,25 +633,6 @@ pub fn main() {
             DeviceIntSize::new(w, h)
         })
         .unwrap_or(DeviceIntSize::new(1920, 1080));
-    let chase_primitive = match args.value_of("chase") {
-        Some(s) => {
-            if s.contains(',') {
-                let items = s
-                    .split(',')
-                    .map(|s| s.parse::<f32>().unwrap())
-                    .collect::<Vec<_>>();
-                let rect = LayoutRect::from_origin_and_size(
-                    LayoutPoint::new(items[0], items[1]),
-                    LayoutSize::new(items[2], items[3]),
-                );
-                webrender::ChasePrimitive::LocalRect(rect)
-            } else {
-                let id = s.parse::<usize>().unwrap();
-                webrender::ChasePrimitive::Id(webrender::PrimitiveDebugId(id))
-            }
-        }
-        None => webrender::ChasePrimitive::Nothing,
-    };
 
     let dump_shader_source = args.value_of("dump_shader_source").map(String::from);
 
@@ -729,7 +710,6 @@ pub fn main() {
         args.is_present("no_batch"),
         args.is_present("precache"),
         args.is_present("slow_subpixel"),
-        chase_primitive,
         dump_shader_source,
         notifier,
     );

gfx/wr/wrench/src/wrench.rs

@@ -227,7 +227,6 @@ impl Wrench {
         no_batch: bool,
         precache_shaders: bool,
         disable_dual_source_blending: bool,
-        chase_primitive: webrender::ChasePrimitive,
         dump_shader_source: Option<String>,
         notifier: Option<Box<dyn RenderNotifier>>,
     ) -> Self {
@@ -252,7 +251,6 @@ impl Wrench {
             max_recorded_profiles: 16,
             precache_flags,
             blob_image_handler: Some(Box::new(blob::CheckerboardRenderer::new(callbacks.clone()))),
-            chase_primitive,
             testing: true,
             max_internal_texture_size: Some(8196), // Needed for rawtest::test_resize_image.
             allow_dual_source_blending: !disable_dual_source_blending,