Skip to content

Commit 59261d6

Browse files
committed
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
1 parent 8748db9 commit 59261d6

File tree

7 files changed

+154
-6
lines changed

7 files changed

+154
-6
lines changed

webrender/src/image_source.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,5 +93,5 @@ pub fn resolve_cached_render_task(
9393
let rt_cache_entry = resource_cache
9494
.get_cached_render_task(&handle);
9595

96-
resource_cache.get_texture_cache_item(&rt_cache_entry.handle)
96+
resource_cache.get_texture_cache_item(&rt_cache_entry.handle).unwrap()
9797
}

webrender/src/resource_cache.rs

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,21 @@ pub struct ResourceCache {
501501

502502
/// A pool of render targets for use by the render task graph
503503
render_target_pool: Vec<RenderTarget>,
504+
505+
/// An empty (1x1 transparent) image used when a stacking context snapshot
506+
/// is missing.
507+
///
508+
/// For now it acts as a catch-all solution for cases where WebRender fails
509+
/// to produce a texture cache item for a snapshotted tacking context.
510+
/// These cases include:
511+
/// - Empty stacking contexts.
512+
/// - Stacking contexts that are more aggressively culled out than they
513+
/// should, for example when they are in a perspective transform that
514+
/// cannot be projected to screen space.
515+
/// - Likely other cases we have not found yet.
516+
/// Over time it would be better to handle each of these cases explicitly
517+
/// and make it a hard error to fail to snapshot a stacking context.
518+
fallback_handle: TextureCacheHandle,
504519
}
505520

506521
impl ResourceCache {
@@ -538,6 +553,7 @@ impl ResourceCache {
538553
image_templates_memory: 0,
539554
font_templates_memory: 0,
540555
render_target_pool: Vec::new(),
556+
fallback_handle: TextureCacheHandle::invalid(),
541557
}
542558
}
543559

@@ -661,7 +677,7 @@ impl ResourceCache {
661677
self.texture_cache.shared_color_expected_format(),
662678
flags,
663679
);
664-
680+
665681
// Allocate space in the texture cache, but don't supply
666682
// and CPU-side data to be uploaded.
667683
let user_data = [0.0; 4];
@@ -1342,7 +1358,19 @@ impl ResourceCache {
13421358
pub fn get_cached_image(&self, request: ImageRequest) -> Result<CacheItem, ()> {
13431359
debug_assert_eq!(self.state, State::QueryResources);
13441360
let image_info = self.get_image_info(request)?;
1345-
Ok(self.get_texture_cache_item(&image_info.texture_cache_handle))
1361+
1362+
if let Ok(item) = self.get_texture_cache_item(&image_info.texture_cache_handle) {
1363+
// Common path.
1364+
return Ok(item);
1365+
}
1366+
1367+
if self.resources.image_templates
1368+
.get(request.key)
1369+
.map_or(false, |img| img.data.is_snapshot()) {
1370+
return self.get_texture_cache_item(&self.fallback_handle);
1371+
}
1372+
1373+
panic!("Requested image missing from the texture cache");
13461374
}
13471375

13481376
pub fn get_cached_render_task(
@@ -1364,8 +1392,12 @@ impl ResourceCache {
13641392
}
13651393

13661394
#[inline]
1367-
pub fn get_texture_cache_item(&self, handle: &TextureCacheHandle) -> CacheItem {
1368-
self.texture_cache.get(handle)
1395+
pub fn get_texture_cache_item(&self, handle: &TextureCacheHandle) -> Result<CacheItem, ()> {
1396+
if let Some(item) = self.texture_cache.try_get(handle) {
1397+
return Ok(item);
1398+
}
1399+
1400+
Err(())
13691401
}
13701402

13711403
pub fn get_image_properties(&self, image_key: ImageKey) -> Option<ImageProperties> {
@@ -1480,6 +1512,29 @@ impl ResourceCache {
14801512

14811513
fn update_texture_cache(&mut self, gpu_cache: &mut GpuCache) {
14821514
profile_scope!("update_texture_cache");
1515+
1516+
if self.fallback_handle == TextureCacheHandle::invalid() {
1517+
self.texture_cache.update(
1518+
&mut self.fallback_handle,
1519+
ImageDescriptor {
1520+
size: size2(1, 1),
1521+
stride: None,
1522+
format: ImageFormat::BGRA8,
1523+
flags: ImageDescriptorFlags::empty(),
1524+
offset: 0,
1525+
},
1526+
TextureFilter::Linear,
1527+
Some(CachedImageData::Raw(Arc::new(vec![0, 0, 0, 0]))),
1528+
[0.0; 4],
1529+
DirtyRect::All,
1530+
gpu_cache,
1531+
None,
1532+
UvRectKind::Rect,
1533+
Eviction::Manual,
1534+
TargetShader::Default,
1535+
);
1536+
}
1537+
14831538
for request in self.pending_image_requests.drain() {
14841539
let image_template = self.resources.image_templates.get_mut(request.key).unwrap();
14851540
debug_assert!(image_template.data.uses_texture_cache());

webrender/src/texture_cache.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ impl TextureCache {
774774
// Number of moved pixels after which we stop attempting to move more items for this frame.
775775
// The constant is up for adjustment, the main goal is to avoid causing frame spikes on
776776
// low end GPUs.
777-
let area_threshold = 512*512;
777+
let area_threshold = 512*512;
778778

779779
let mut changes = Vec::new();
780780
allocator_lists[idx].try_compaction(area_threshold, &mut changes);
@@ -1016,6 +1016,36 @@ impl TextureCache {
10161016
}
10171017
}
10181018

1019+
pub fn try_get(&self, handle: &TextureCacheHandle) -> Option<CacheItem> {
1020+
let (texture_id, uv_rect, swizzle, uv_rect_handle, user_data) = self.try_get_cache_location(handle)?;
1021+
Some(CacheItem {
1022+
uv_rect_handle,
1023+
texture_id: TextureSource::TextureCache(
1024+
texture_id,
1025+
swizzle,
1026+
),
1027+
uv_rect,
1028+
user_data,
1029+
})
1030+
}
1031+
1032+
pub fn try_get_cache_location(
1033+
&self,
1034+
handle: &TextureCacheHandle,
1035+
) -> Option<(CacheTextureId, DeviceIntRect, Swizzle, GpuCacheHandle, [f32; 4])> {
1036+
let entry = self.get_entry_opt(handle)?;
1037+
1038+
debug_assert_eq!(entry.last_access, self.now);
1039+
let origin = entry.details.describe();
1040+
Some((
1041+
entry.texture_id,
1042+
DeviceIntRect::from_origin_and_size(origin, entry.size),
1043+
entry.swizzle,
1044+
entry.uv_rect_handle,
1045+
entry.user_data,
1046+
))
1047+
}
1048+
10191049
/// A more detailed version of get(). This allows access to the actual
10201050
/// device rect of the cache allocation.
10211051
///

wrench/reftests/image/empty.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
root:
3+
items:
4+
-
5+
bounds: [200, 200, 1000, 1000]
6+
type: "stacking-context"
7+
items:

wrench/reftests/image/reftest.list

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,9 @@ fuzzy(1,160000) == image-filter-stretch-tile.yaml green-alpha-ref.yaml
2626
== snapshot-filters-02.yaml snapshot-filters-02-ref.yaml
2727
fuzzy(3,3000) == snapshot-shadow.yaml snapshot-shadow-ref.yaml
2828
== snapshot-multiframe.yaml snapshot-multiframe-ref.yaml
29+
== snapshot-empty.yaml empty.yaml
30+
# TODO: At the moment snapshot-perspective-01.yaml renders incorrectly, so the
31+
# reftest acts more as a crash test. When bug 1941577 is fixed the reftest
32+
# reference should be updated to reflect that something needs to be rendered
33+
# instead of leaving the snapshot empty.
34+
== snapshot-perspective-01.yaml empty.yaml
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
root:
3+
items:
4+
-
5+
bounds: [200, 200, 1000, 1000]
6+
type: "stacking-context"
7+
perspective: 256
8+
items:
9+
-
10+
bounds: [0, 0, 100, 100]
11+
type: "stacking-context"
12+
snapshot:
13+
name: "snap0"
14+
area: [0, 0, 100, 100]
15+
items:
16+
- image: snapshot(snap0)
17+
bounds: [10, 10, 200, 200]
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
root:
3+
items:
4+
-
5+
bounds: [200, 200, 1000, 1000]
6+
type: "stacking-context"
7+
perspective: 256
8+
items:
9+
-
10+
bounds: [128, 128, 256, 256]
11+
type: "stacking-context"
12+
transform: rotate-x(-60) rotate-y(-120)
13+
snapshot:
14+
name: "snap0"
15+
area: [0, 0, 100, 200]
16+
items:
17+
- type: clip
18+
id: 101
19+
complex:
20+
- rect: [0, 0, 100, 200]
21+
radius: [32, 32]
22+
- type: clip-chain
23+
id: 201
24+
clips: [101]
25+
-
26+
bounds: [0, 0, 100, 200]
27+
type: rect
28+
color: blue
29+
clip-chain: 201
30+
- image: snapshot(snap0)
31+
bounds: [300, 0, 100, 200]
32+
33+

0 commit comments

Comments
 (0)