Apply dirty rect updates consistently. #2856
+155
−69
Apply dirty rect updates consistently.
This patch moves the dirty rect storage from ImageResource to CachedImageInfo. This is because cache entries may be updated independently across frame updates, and thus cannot share the dirty rect state. We no longer clear the dirty rect when we get an update without a dirty rect. This is because we can get a subsequent update (before applying the previous) which provides a new dirty rect, and we still want to reupload the whole image. Instead we now store a dirty rect for the whole descriptor. In addition to special handling for tiled requests, we also properly handle requests which change the ImageRendering type. Before we would take the dirty rect for the first request and ignore it for the rest.
- Loading branch information
commit 27bc98b7b60a10613e6b8ab72791aafc6714e4f9
| @@ -32,6 +32,7 @@ use render_backend::FrameId; | ||
| use render_task::{RenderTaskCache, RenderTaskCacheKey, RenderTaskId}; | ||
| use render_task::{RenderTaskCacheEntry, RenderTaskCacheEntryHandle, RenderTaskTree}; | ||
| use std::collections::hash_map::Entry::{self, Occupied, Vacant}; | ||
| use std::collections::hash_map::IterMut; | ||
| use std::cmp; | ||
| use std::fmt::Debug; | ||
| use std::hash::Hash; | ||
| @@ -101,7 +102,6 @@ struct ImageResource { | ||
| descriptor: ImageDescriptor, | ||
| epoch: Epoch, | ||
| tiling: Option<TileSize>, | ||
| dirty_rect: Option<DeviceUintRect>, | ||
| } | ||
|
|
||
| #[derive(Clone, Debug)] | ||
| @@ -137,7 +137,7 @@ impl ImageTemplates { | ||
| #[cfg_attr(feature = "replay", derive(Deserialize))] | ||
| struct CachedImageInfo { | ||
| texture_cache_handle: TextureCacheHandle, | ||
| epoch: Epoch, | ||
| dirty_rect: Option<DeviceUintRect>, | ||
| } | ||
|
|
||
| #[cfg_attr(feature = "capture", derive(Serialize))] | ||
| @@ -168,6 +168,24 @@ pub fn intersect_for_tile( | ||
| }) | ||
| } | ||
|
|
||
| fn merge_dirty_rect( | ||
| prev_dirty_rect: Option<DeviceUintRect>, | ||
| dirty_rect: Option<DeviceUintRect>, | ||
| descriptor: ImageDescriptor, | ||
|
||
| ) -> Option<DeviceUintRect> { | ||
| // It is important to never assume an empty dirty rect implies a full reupload here, | ||
| // although we are able to do so elsewhere. We use store the descriptor's full rect | ||
|
||
| // instead. There are update sequences which could cause us to forget the correct | ||
| // dirty regions if we cleared the dirty rect when we received None, e.g.: | ||
| // 1) Update with no dirty rect. We want to reupload everything. | ||
| // 2) Update with dirty rect B. We still want to reupload everything, not just B. | ||
| // 3) Perform the upload some time later. | ||
| match (dirty_rect, prev_dirty_rect) { | ||
| (Some(rect), Some(prev_rect)) => Some(rect.union(&prev_rect)), | ||
| (Some(rect), None) => Some(rect), | ||
| (None, _) => Some(descriptor.full_rect()), | ||
| } | ||
| } | ||
|
|
||
| impl<K, V, U> ResourceClassCache<K, V, U> | ||
| where | ||
| @@ -195,10 +213,18 @@ where | ||
| .expect("Didn't find a cached resource with that ID!") | ||
| } | ||
|
|
||
| pub fn try_get_mut(&mut self, key: &K) -> Option<&mut V> { | ||
| self.resources.get_mut(key) | ||
| } | ||
|
|
||
| pub fn entry(&mut self, key: K) -> Entry<K, V> { | ||
| self.resources.entry(key) | ||
| } | ||
|
|
||
| pub fn iter_mut(&mut self) -> IterMut<K, V> { | ||
| self.resources.iter_mut() | ||
| } | ||
|
|
||
| pub fn clear(&mut self) { | ||
| self.resources.clear(); | ||
| } | ||
| @@ -510,14 +536,12 @@ impl ResourceCache { | ||
| tiling, | ||
| ); | ||
| } | ||
| let dirty_rect = Some(descriptor.full_rect()); | ||
|
|
||
| let resource = ImageResource { | ||
| descriptor, | ||
| data, | ||
| epoch: Epoch(0), | ||
| tiling, | ||
| dirty_rect, | ||
| }; | ||
|
|
||
| self.resources.image_templates.insert(image_key, resource); | ||
| @@ -548,16 +572,38 @@ impl ResourceCache { | ||
| .update(image_key, Arc::clone(blob), dirty_rect); | ||
| } | ||
|
|
||
| // Each cache entry stores its own copy of the image's dirty rect. This allows them to be | ||
| // updated independently. If we are tiling we need to scan the whole cache otherwise there | ||
| // is a finite number of keys. | ||
| // TODO(aosmond): We should consider changing how we store tiled entries to avoid O(N * M) | ||
| // complexity, where N is the number of updates, and M is the number of cached entries. | ||
| if tiling.is_some() { | ||
| for (request, entry) in self.cached_images.iter_mut() { | ||
|
||
| if request.tile.is_some() && request.key == image_key { | ||
| if let Ok(ref mut entry) = *entry { | ||
| entry.dirty_rect = merge_dirty_rect(entry.dirty_rect, dirty_rect, descriptor); | ||
| } | ||
| } | ||
| } | ||
| } else { | ||
| for &render in &[ImageRendering::Auto, ImageRendering::CrispEdges, ImageRendering::Pixelated] { | ||
| let request = ImageRequest { | ||
| key: image_key, | ||
| rendering: render, | ||
| tile: None | ||
| }; | ||
|
|
||
| if let Some(&mut Ok(ref mut entry)) = self.cached_images.try_get_mut(&request) { | ||
| entry.dirty_rect = merge_dirty_rect(entry.dirty_rect, dirty_rect, descriptor); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| *image = ImageResource { | ||
| descriptor, | ||
| data, | ||
| epoch: Epoch(image.epoch.0 + 1), | ||
| tiling, | ||
| dirty_rect: match (dirty_rect, image.dirty_rect) { | ||
| (Some(rect), Some(prev_rect)) => Some(rect.union(&prev_rect)), | ||
| (Some(rect), None) => Some(rect), | ||
| (None, _) => None, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| @@ -610,21 +656,19 @@ impl ResourceCache { | ||
| return; | ||
| } | ||
|
|
||
| // If this image exists in the texture cache, *and* the epoch | ||
| // in the cache matches that of the template, then it is | ||
| // valid to use as-is. | ||
| // If this image exists in the texture cache, *and* the dirty rect | ||
| // in the cache is empty, then it is valid to use as-is. | ||
| let (entry, needs_update) = match self.cached_images.entry(request) { | ||
| Occupied(entry) => { | ||
| let info = entry.into_mut(); | ||
| let needs_update = info.as_mut().unwrap().epoch != template.epoch; | ||
| info.as_mut().unwrap().epoch = template.epoch; | ||
| let needs_update = info.as_ref().unwrap().dirty_rect.is_some(); | ||
| (info, needs_update) | ||
| } | ||
| Vacant(entry) => ( | ||
| entry.insert(Ok( | ||
| CachedImageInfo { | ||
| epoch: template.epoch, | ||
| texture_cache_handle: TextureCacheHandle::new(), | ||
| dirty_rect: None, | ||
| } | ||
| )), | ||
| true, | ||
| @@ -636,9 +680,9 @@ impl ResourceCache { | ||
|
|
||
| let dirty_rect = if needs_upload { | ||
| // the texture cache entry has been evicted, treat it as all dirty | ||
| Some(template.descriptor.full_rect()) | ||
| None | ||
| } else if needs_update { | ||
| template.dirty_rect | ||
| entry.as_ref().unwrap().dirty_rect | ||
| } else { | ||
| return | ||
| }; | ||
| @@ -666,6 +710,7 @@ impl ResourceCache { | ||
| if let Some(dirty) = dirty_rect { | ||
| if intersect_for_tile(dirty, actual_size, tile_size, tile_offset).is_none() { | ||
| // don't bother requesting unchanged tiles | ||
| entry.as_mut().unwrap().dirty_rect.take(); | ||
| self.pending_image_requests.remove(&request); | ||
| return | ||
| } | ||
| @@ -934,8 +979,6 @@ impl ResourceCache { | ||
| } | ||
|
|
||
| fn update_texture_cache(&mut self, gpu_cache: &mut GpuCache) { | ||
| let mut keys_to_clear_dirty_rect = FastHashSet::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()); | ||
| @@ -982,12 +1025,10 @@ impl ResourceCache { | ||
| let tile_size = image_template.tiling.unwrap(); | ||
| let clipped_tile_size = compute_tile_size(&descriptor, tile_size, tile); | ||
|
|
||
| local_dirty_rect = if let Some(ref rect) = image_template.dirty_rect { | ||
| keys_to_clear_dirty_rect.insert(request.key.clone()); | ||
|
|
||
| local_dirty_rect = if let Some(rect) = entry.dirty_rect.take() { | ||
| // We should either have a dirty rect, or we are re-uploading where the dirty | ||
| // rect is ignored anyway. | ||
| let intersection = intersect_for_tile(*rect, clipped_tile_size, tile_size, tile); | ||
| let intersection = intersect_for_tile(rect, clipped_tile_size, tile_size, tile); | ||
| debug_assert!(intersection.is_some() || | ||
| self.texture_cache.needs_upload(&entry.texture_cache_handle)); | ||
| intersection | ||
| @@ -1010,7 +1051,7 @@ impl ResourceCache { | ||
|
|
||
| descriptor.size = clipped_tile_size; | ||
| } else { | ||
| local_dirty_rect = image_template.dirty_rect.take(); | ||
| local_dirty_rect = entry.dirty_rect.take(); | ||
| } | ||
|
|
||
| let filter = match request.rendering { | ||
| @@ -1052,11 +1093,6 @@ impl ResourceCache { | ||
| UvRectKind::Rect, | ||
| ); | ||
| } | ||
|
|
||
| for key in keys_to_clear_dirty_rect.drain() { | ||
| let image_template = self.resources.image_templates.get_mut(key).unwrap(); | ||
| image_template.dirty_rect.take(); | ||
| } | ||
| } | ||
|
|
||
| pub fn end_frame(&mut self) { | ||
| @@ -1476,7 +1512,6 @@ impl ResourceCache { | ||
| descriptor: template.descriptor, | ||
| tiling: template.tiling, | ||
| epoch: template.epoch, | ||
| dirty_rect: None, | ||
| }); | ||
| } | ||
|
|
||
ProTip!
Use n and p to navigate between commits in a pull request.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
let's pass it by reference here