From e12c1ae551d68eb4da7fa607f7aaa7d447d7718c Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 10 Feb 2025 07:52:56 +0000 Subject: [PATCH] Bug 1946361 - Fallback to a transparent image for missing snapshots. r=gw A followup patch will add debug prefs to panic or use pink instead of transparent for the fallback image. Differential Revision: https://phabricator.services.mozilla.com/D237007 --- gfx/wr/webrender/src/image_source.rs | 2 +- gfx/wr/webrender/src/resource_cache.rs | 63 +++++++++++++++++-- gfx/wr/webrender/src/texture_cache.rs | 32 +++++++++- gfx/wr/wrench/reftests/image/empty.yaml | 7 +++ gfx/wr/wrench/reftests/image/reftest.list | 6 ++ .../wrench/reftests/image/snapshot-empty.yaml | 17 +++++ .../image/snapshot-perspective-01.yaml | 33 ++++++++++ 7 files changed, 154 insertions(+), 6 deletions(-) create mode 100644 gfx/wr/wrench/reftests/image/empty.yaml create mode 100644 gfx/wr/wrench/reftests/image/snapshot-empty.yaml create mode 100644 gfx/wr/wrench/reftests/image/snapshot-perspective-01.yaml diff --git a/gfx/wr/webrender/src/image_source.rs b/gfx/wr/webrender/src/image_source.rs index 6c730134038e..7942ca496be5 100644 --- a/gfx/wr/webrender/src/image_source.rs +++ b/gfx/wr/webrender/src/image_source.rs @@ -93,5 +93,5 @@ pub fn resolve_cached_render_task( let rt_cache_entry = resource_cache .get_cached_render_task(&handle); - resource_cache.get_texture_cache_item(&rt_cache_entry.handle) + resource_cache.get_texture_cache_item(&rt_cache_entry.handle).unwrap() } diff --git a/gfx/wr/webrender/src/resource_cache.rs b/gfx/wr/webrender/src/resource_cache.rs index 2c9712229a69..01a410117bdf 100644 --- a/gfx/wr/webrender/src/resource_cache.rs +++ b/gfx/wr/webrender/src/resource_cache.rs @@ -501,6 +501,21 @@ pub struct ResourceCache { /// A pool of render targets for use by the render task graph render_target_pool: Vec, + + /// An empty (1x1 transparent) image used when a stacking context snapshot + /// is missing. + /// + /// For now it acts as a catch-all solution for cases where WebRender fails + /// to produce a texture cache item for a snapshotted tacking context. + /// These cases include: + /// - Empty stacking contexts. + /// - Stacking contexts that are more aggressively culled out than they + /// should, for example when they are in a perspective transform that + /// cannot be projected to screen space. + /// - Likely other cases we have not found yet. + /// Over time it would be better to handle each of these cases explicitly + /// and make it a hard error to fail to snapshot a stacking context. + fallback_handle: TextureCacheHandle, } impl ResourceCache { @@ -538,6 +553,7 @@ impl ResourceCache { image_templates_memory: 0, font_templates_memory: 0, render_target_pool: Vec::new(), + fallback_handle: TextureCacheHandle::invalid(), } } @@ -661,7 +677,7 @@ impl ResourceCache { self.texture_cache.shared_color_expected_format(), flags, ); - + // Allocate space in the texture cache, but don't supply // and CPU-side data to be uploaded. let user_data = [0.0; 4]; @@ -1342,7 +1358,19 @@ impl ResourceCache { pub fn get_cached_image(&self, request: ImageRequest) -> Result { debug_assert_eq!(self.state, State::QueryResources); let image_info = self.get_image_info(request)?; - Ok(self.get_texture_cache_item(&image_info.texture_cache_handle)) + + if let Ok(item) = self.get_texture_cache_item(&image_info.texture_cache_handle) { + // Common path. + return Ok(item); + } + + if self.resources.image_templates + .get(request.key) + .map_or(false, |img| img.data.is_snapshot()) { + return self.get_texture_cache_item(&self.fallback_handle); + } + + panic!("Requested image missing from the texture cache"); } pub fn get_cached_render_task( @@ -1364,8 +1392,12 @@ impl ResourceCache { } #[inline] - pub fn get_texture_cache_item(&self, handle: &TextureCacheHandle) -> CacheItem { - self.texture_cache.get(handle) + pub fn get_texture_cache_item(&self, handle: &TextureCacheHandle) -> Result { + if let Some(item) = self.texture_cache.try_get(handle) { + return Ok(item); + } + + Err(()) } pub fn get_image_properties(&self, image_key: ImageKey) -> Option { @@ -1480,6 +1512,29 @@ impl ResourceCache { fn update_texture_cache(&mut self, gpu_cache: &mut GpuCache) { profile_scope!("update_texture_cache"); + + if self.fallback_handle == TextureCacheHandle::invalid() { + self.texture_cache.update( + &mut self.fallback_handle, + ImageDescriptor { + size: size2(1, 1), + stride: None, + format: ImageFormat::BGRA8, + flags: ImageDescriptorFlags::empty(), + offset: 0, + }, + TextureFilter::Linear, + Some(CachedImageData::Raw(Arc::new(vec![0, 0, 0, 0]))), + [0.0; 4], + DirtyRect::All, + gpu_cache, + None, + UvRectKind::Rect, + Eviction::Manual, + TargetShader::Default, + ); + } + for request in self.pending_image_requests.drain() { let image_template = self.resources.image_templates.get_mut(request.key).unwrap(); debug_assert!(image_template.data.uses_texture_cache()); diff --git a/gfx/wr/webrender/src/texture_cache.rs b/gfx/wr/webrender/src/texture_cache.rs index 29d92e55a1c6..3154f4433ab4 100644 --- a/gfx/wr/webrender/src/texture_cache.rs +++ b/gfx/wr/webrender/src/texture_cache.rs @@ -774,7 +774,7 @@ impl TextureCache { // Number of moved pixels after which we stop attempting to move more items for this frame. // The constant is up for adjustment, the main goal is to avoid causing frame spikes on // low end GPUs. - let area_threshold = 512*512; + let area_threshold = 512*512; let mut changes = Vec::new(); allocator_lists[idx].try_compaction(area_threshold, &mut changes); @@ -1016,6 +1016,36 @@ impl TextureCache { } } + pub fn try_get(&self, handle: &TextureCacheHandle) -> Option { + let (texture_id, uv_rect, swizzle, uv_rect_handle, user_data) = self.try_get_cache_location(handle)?; + Some(CacheItem { + uv_rect_handle, + texture_id: TextureSource::TextureCache( + texture_id, + swizzle, + ), + uv_rect, + user_data, + }) + } + + pub fn try_get_cache_location( + &self, + handle: &TextureCacheHandle, + ) -> Option<(CacheTextureId, DeviceIntRect, Swizzle, GpuCacheHandle, [f32; 4])> { + let entry = self.get_entry_opt(handle)?; + + debug_assert_eq!(entry.last_access, self.now); + let origin = entry.details.describe(); + Some(( + entry.texture_id, + DeviceIntRect::from_origin_and_size(origin, entry.size), + entry.swizzle, + entry.uv_rect_handle, + entry.user_data, + )) + } + /// A more detailed version of get(). This allows access to the actual /// device rect of the cache allocation. /// diff --git a/gfx/wr/wrench/reftests/image/empty.yaml b/gfx/wr/wrench/reftests/image/empty.yaml new file mode 100644 index 000000000000..4449d4df2b7e --- /dev/null +++ b/gfx/wr/wrench/reftests/image/empty.yaml @@ -0,0 +1,7 @@ +--- +root: + items: + - + bounds: [200, 200, 1000, 1000] + type: "stacking-context" + items: diff --git a/gfx/wr/wrench/reftests/image/reftest.list b/gfx/wr/wrench/reftests/image/reftest.list index 3f31d815d259..bf41c1bd3a84 100644 --- a/gfx/wr/wrench/reftests/image/reftest.list +++ b/gfx/wr/wrench/reftests/image/reftest.list @@ -26,3 +26,9 @@ fuzzy(1,160000) == image-filter-stretch-tile.yaml green-alpha-ref.yaml == snapshot-filters-02.yaml snapshot-filters-02-ref.yaml fuzzy(3,3000) == snapshot-shadow.yaml snapshot-shadow-ref.yaml == snapshot-multiframe.yaml snapshot-multiframe-ref.yaml +== snapshot-empty.yaml empty.yaml +# TODO: At the moment snapshot-perspective-01.yaml renders incorrectly, so the +# reftest acts more as a crash test. When bug 1941577 is fixed the reftest +# reference should be updated to reflect that something needs to be rendered +# instead of leaving the snapshot empty. +== snapshot-perspective-01.yaml empty.yaml diff --git a/gfx/wr/wrench/reftests/image/snapshot-empty.yaml b/gfx/wr/wrench/reftests/image/snapshot-empty.yaml new file mode 100644 index 000000000000..fca8e5c7fc93 --- /dev/null +++ b/gfx/wr/wrench/reftests/image/snapshot-empty.yaml @@ -0,0 +1,17 @@ +--- +root: + items: + - + bounds: [200, 200, 1000, 1000] + type: "stacking-context" + perspective: 256 + items: + - + bounds: [0, 0, 100, 100] + type: "stacking-context" + snapshot: + name: "snap0" + area: [0, 0, 100, 100] + items: + - image: snapshot(snap0) + bounds: [10, 10, 200, 200] diff --git a/gfx/wr/wrench/reftests/image/snapshot-perspective-01.yaml b/gfx/wr/wrench/reftests/image/snapshot-perspective-01.yaml new file mode 100644 index 000000000000..0d8a4a823666 --- /dev/null +++ b/gfx/wr/wrench/reftests/image/snapshot-perspective-01.yaml @@ -0,0 +1,33 @@ +--- +root: + items: + - + bounds: [200, 200, 1000, 1000] + type: "stacking-context" + perspective: 256 + items: + - + bounds: [128, 128, 256, 256] + type: "stacking-context" + transform: rotate-x(-60) rotate-y(-120) + snapshot: + name: "snap0" + area: [0, 0, 100, 200] + items: + - type: clip + id: 101 + complex: + - rect: [0, 0, 100, 200] + radius: [32, 32] + - type: clip-chain + id: 201 + clips: [101] + - + bounds: [0, 0, 100, 200] + type: rect + color: blue + clip-chain: 201 + - image: snapshot(snap0) + bounds: [300, 0, 100, 200] + +