Skip to content

/ webrender

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply dirty rect updates consistently. #2856

Merged
merged 3 commits into from Jul 5, 2018
+155 −69
Merged

Apply dirty rect updates consistently. #2856

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27bc98b
Apply dirty rect updates consistently.
aosmond Jun 27, 2018
58a5c07
Refactor image cache to avoid scanning the whole cache when updating …
aosmond Jun 29, 2018
2964902
Remove epoch from ImageResource as unused.
aosmond Jul 4, 2018
File filter...
Filter file types
Clear filters
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev Next

Refactor image cache to avoid scanning the whole cache when updating … 

…tiled images.

ImageCache is now represented as an enum with three types:
- Err, as before.

- UntiledAuto, the fast path for untiled images using ImageRendering::Auto,
which contains a CachedImageInfo.

- Multi, the slower path used for tiled images or images that use other
ImageRendering types, which stores a hash map containing CachedImageInfo
objects for all of the variants/subsections of the image.
  • Loading branch information
@aosmond
aosmond committed Jul 5, 2018
commit 58a5c07cb26a411eeedfeb16db400815ab23594e
187 webrender/src/resource_cache.rs
View file
Open in desktop
@@ -32,8 +32,8 @@ 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::collections::hash_map::ValuesMut;
use std::{cmp, mem};
use std::fmt::Debug;
use std::hash::Hash;
#[cfg(any(feature = "capture", feature = "replay"))]
@@ -169,21 +169,21 @@ pub fn intersect_for_tile(
}

fn merge_dirty_rect(
prev_dirty_rect: Option<DeviceUintRect>,
dirty_rect: Option<DeviceUintRect>,
descriptor: ImageDescriptor,
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.:
// although we are able to do so elsewhere. We store the descriptor's full rect instead
// There are update sequences which could cause us to forget the correct dirty regions
// 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()),
(&Some(ref rect), &Some(ref prev_rect)) => Some(rect.union(&prev_rect)),
(&Some(ref rect), &None) => Some(*rect),
(&None, _) => Some(descriptor.full_rect()),
}
}

@@ -208,6 +208,10 @@ where
self.resources.insert(key, value);
}

pub fn remove(&mut self, key: &K) {
self.resources.remove(key);
}

pub fn get_mut(&mut self, key: &K) -> &mut V {
self.resources.get_mut(key)
.expect("Didn't find a cached resource with that ID!")
@@ -221,8 +225,8 @@ where
self.resources.entry(key)
}

pub fn iter_mut(&mut self) -> IterMut<K, V> {
self.resources.iter_mut()
pub fn values_mut(&mut self) -> ValuesMut<K, V> {
self.resources.values_mut()
}

pub fn clear(&mut self) {
@@ -251,6 +255,13 @@ where
}
}

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "capture", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
struct CachedImageKey {
pub rendering: ImageRendering,
pub tile: Option<TileOffset>,
}

#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "capture", derive(Serialize))]
@@ -269,6 +280,10 @@ impl ImageRequest {
tile: Some(offset),
}
}

pub fn is_untiled_auto(&self) -> bool {
self.tile.is_none() && self.rendering == ImageRendering::Auto
}
}

impl Into<BlobImageRequest> for ImageRequest {
@@ -280,14 +295,31 @@ impl Into<BlobImageRequest> for ImageRequest {
}
}

impl Into<CachedImageKey> for ImageRequest {
fn into(self) -> CachedImageKey {
CachedImageKey {
rendering: self.rendering,
tile: self.tile,
}
}
}

#[derive(Debug)]
#[cfg_attr(feature = "capture", derive(Clone, Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub enum ImageCacheError {
OverLimitSize,
}

type ImageCache = ResourceClassCache<ImageRequest, Result<CachedImageInfo, ImageCacheError>, ()>;
#[cfg_attr(feature = "capture", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
enum ImageResult {
UntiledAuto(CachedImageInfo),
Multi(ResourceClassCache<CachedImageKey, CachedImageInfo, ()>),
Err(ImageCacheError),
}

type ImageCache = ResourceClassCache<ImageKey, ImageResult, ()>;
pub type FontInstanceMap = Arc<RwLock<FastHashMap<FontInstanceKey, FontInstance>>>;

#[derive(Default)]
@@ -573,30 +605,17 @@ impl ResourceCache {
}

// 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);
}
}
// updated independently.
match self.cached_images.try_get_mut(&image_key) {
Some(&mut ImageResult::UntiledAuto(ref mut 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);
Some(&mut ImageResult::Multi(ref mut entries)) => {
for entry in entries.values_mut() {
entry.dirty_rect = merge_dirty_rect(&entry.dirty_rect, &dirty_rect, &descriptor);
}
}
_ => {}
}

*image = ImageResource {
@@ -610,8 +629,7 @@ impl ResourceCache {
pub fn delete_image_template(&mut self, image_key: ImageKey) {
let value = self.resources.image_templates.remove(image_key);

self.cached_images
.clear_keys(|request| request.key == image_key);
self.cached_images.remove(&image_key);

match value {
Some(image) => if image.data.is_blob() {
@@ -652,39 +670,76 @@ impl ResourceCache {
// The image or tiling size is too big for hardware texture size.
warn!("Dropping image, image:(w:{},h:{}, tile:{}) is too big for hardware!",
template.descriptor.size.width, template.descriptor.size.height, template.tiling.unwrap_or(0));
self.cached_images.insert(request, Err(ImageCacheError::OverLimitSize));
self.cached_images.insert(request.key, ImageResult::Err(ImageCacheError::OverLimitSize));
return;
}

let storage = match self.cached_images.entry(request.key) {
Occupied(e) => {

This comment has been minimized.

Sign in to view
@kvark

kvark Jul 4, 2018

Member

perhaps, it would be cleaner to lift the condition up here:

Occupied(e) if e.is_untiled_auto() => {
...
}
Occupied(e) => {
...
}
Vacant(e) => {..}
// We might have an existing untiled entry, and need to insert
// a second entry. In such cases we need to move the old entry
// out first, replacing it with a dummy entry, and then creating
// the tiled/multi-entry variant.
let entry = e.into_mut();
if !request.is_untiled_auto() {
let untiled_entry = match entry {
&mut ImageResult::UntiledAuto(ref mut entry) => {
Some(mem::replace(entry, CachedImageInfo {
texture_cache_handle: TextureCacheHandle::new(),
dirty_rect: None,
}))
}
_ => None
};

if let Some(untiled_entry) = untiled_entry {
let mut entries = ResourceClassCache::new();
let untiled_key = CachedImageKey {
rendering: ImageRendering::Auto,
tile: None,
};
entries.insert(untiled_key, untiled_entry);
*entry = ImageResult::Multi(entries);
}
}
entry
}
Vacant(entry) => {
entry.insert(if request.is_untiled_auto() {
ImageResult::UntiledAuto(CachedImageInfo {
texture_cache_handle: TextureCacheHandle::new(),
dirty_rect: Some(template.descriptor.full_rect()),
})
} else {
ImageResult::Multi(ResourceClassCache::new())
})
}
};

// 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_ref().unwrap().dirty_rect.is_some();
(info, needs_update)
}
Vacant(entry) => (
entry.insert(Ok(
CachedImageInfo {
let entry = match *storage {
ImageResult::UntiledAuto(ref mut entry) => entry,
ImageResult::Multi(ref mut entries) => {
entries.entry(request.into())
.or_insert(CachedImageInfo {
texture_cache_handle: TextureCacheHandle::new(),
dirty_rect: None,
}
)),
true,
),
dirty_rect: Some(template.descriptor.full_rect()),
})
},
ImageResult::Err(_) => panic!("Errors should already have been handled"),
};

let needs_upload = self.texture_cache
.request(&entry.as_ref().unwrap().texture_cache_handle, gpu_cache);
.request(&entry.texture_cache_handle, gpu_cache);

let dirty_rect = if needs_upload {
// the texture cache entry has been evicted, treat it as all dirty
None
} else if needs_update {
entry.as_ref().unwrap().dirty_rect
} else {
} else if entry.dirty_rect.is_none() {
return
} else {
entry.dirty_rect
};

if !self.pending_image_requests.insert(request) {
@@ -710,7 +765,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();
entry.dirty_rect = None;
self.pending_image_requests.remove(&request);
return
}
@@ -894,11 +949,15 @@ impl ResourceCache {

// TODO(Jerry): add a debug option to visualize the corresponding area for
// the Err() case of CacheItem.
match *self.cached_images.get(&request) {
Ok(ref image_info) => {
match *self.cached_images.get(&request.key) {
ImageResult::UntiledAuto(ref image_info) => {
Ok(self.texture_cache.get(&image_info.texture_cache_handle))
}
Err(_) => {
ImageResult::Multi(ref entries) => {
let image_info = entries.get(&request.into());
Ok(self.texture_cache.get(&image_info.texture_cache_handle))
}
ImageResult::Err(_) => {
Err(())
}
}
@@ -1017,7 +1076,11 @@ impl ResourceCache {
}
};

let entry = self.cached_images.get_mut(&request).as_mut().unwrap();
let entry = match *self.cached_images.get_mut(&request.key) {
ImageResult::UntiledAuto(ref mut entry) => entry,
ImageResult::Multi(ref mut entries) => entries.get_mut(&request.into()),
ImageResult::Err(_) => panic!("Update requested for invalid entry")
};
let mut descriptor = image_template.descriptor.clone();
let local_dirty_rect;

@@ -1124,7 +1187,7 @@ impl ResourceCache {
.images
.retain(|key, _| key.0 != namespace);
self.cached_images
.clear_keys(|request| request.key.0 == namespace);
.clear_keys(|key| key.0 == namespace);

self.resources.font_instances
.write()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.