Skip to content

Commit

Permalink
Fix dirty rectangle coordinate space inside a tile
Browse files Browse the repository at this point in the history
  • Loading branch information
kvark committed Jun 21, 2018
1 parent eeffc83 commit acb7a43
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 73 deletions.
104 changes: 47 additions & 57 deletions webrender/src/resource_cache.rs
Expand Up @@ -525,7 +525,7 @@ impl ResourceCache {
&mut self,
image_key: ImageKey,
descriptor: ImageDescriptor,
mut data: ImageData,
data: ImageData,
dirty_rect: Option<DeviceUintRect>,
) {
let max_texture_size = self.max_texture_size();
Expand All @@ -539,11 +539,11 @@ impl ResourceCache {
tiling = Some(DEFAULT_TILE_SIZE);
}

if let ImageData::Blob(ref mut blob) = data {
if let ImageData::Blob(ref blob) = data {
self.blob_image_renderer
.as_mut()
.unwrap()
.update(image_key, Arc::clone(&blob), dirty_rect);
.update(image_key, Arc::clone(blob), dirty_rect);
}

*image = ImageResource {
Expand Down Expand Up @@ -645,11 +645,6 @@ impl ResourceCache {
// - The image is a blob.
// - The blob hasn't already been requested this frame.
if self.pending_image_requests.insert(request) && template.data.is_blob() {
let renderer = match self.blob_image_renderer {
Some(ref mut renderer) => renderer,
None => return,
};

let (offset, size) = match template.tiling {
Some(tile_size) => {
let tile_offset = request.tile.unwrap();
Expand All @@ -658,10 +653,6 @@ impl ResourceCache {
tile_size,
tile_offset,
);
let offset = DevicePoint::new(
tile_offset.x as f32 * tile_size as f32,
tile_offset.y as f32 * tile_size as f32,
);

if let Some(dirty) = dirty_rect {
if intersect_for_tile(dirty, actual_size, tile_size, tile_offset).is_none() {
Expand All @@ -671,21 +662,27 @@ impl ResourceCache {
}
}

let offset = DevicePoint::new(
tile_offset.x as f32 * tile_size as f32,
tile_offset.y as f32 * tile_size as f32,
);
(offset, actual_size)
}
None => (DevicePoint::zero(), template.descriptor.size),
};

renderer.request(
&self.resources,
request.into(),
&BlobImageDescriptor {
size,
offset,
format: template.descriptor.format,
},
dirty_rect,
);
if let Some(ref mut renderer) = self.blob_image_renderer {
renderer.request(
&self.resources,
request.into(),
&BlobImageDescriptor {
size,
offset,
format: template.descriptor.format,
},
dirty_rect,
);
}
}
}

Expand Down Expand Up @@ -929,7 +926,6 @@ impl ResourceCache {
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());
let mut dirty_rect = image_template.dirty_rect;

let image_data = match image_template.data {
ImageData::Raw(..) | ImageData::External(..) => {
Expand Down Expand Up @@ -965,37 +961,41 @@ impl ResourceCache {
}
};

let descriptor = if let Some(tile) = request.tile {
let tile_size = image_template.tiling.unwrap();
let image_descriptor = &image_template.descriptor;
let entry = self.cached_images.get_mut(&request).as_mut().unwrap();
let mut descriptor = image_template.descriptor.clone();
//TODO: erasing the dirty rectangle here is incorrect for tiled images,
// since other tile requests may follow that depend on it
let mut local_dirty_rect = image_template.dirty_rect.take();

let clipped_tile_size = compute_tile_size(image_descriptor, tile_size, tile);
if let Some(tile) = request.tile {
let tile_size = image_template.tiling.unwrap();
let clipped_tile_size = compute_tile_size(&descriptor, tile_size, tile);

if let Some(ref mut rect) = local_dirty_rect {
match intersect_for_tile(*rect, clipped_tile_size, tile_size, tile) {
Some(intersection) => *rect = intersection,
None => {
// if re-uploaded, the dirty rect is ignored anyway
debug_assert!(self.texture_cache.needs_upload(&entry.texture_cache_handle))
}
}
}

// The tiled image could be stored on the CPU as one large image or be
// already broken up into tiles. This affects the way we compute the stride
// and offset.
let tiled_on_cpu = image_template.data.is_blob();

let (stride, offset) = if tiled_on_cpu {
(image_descriptor.stride, 0)
} else {
let bpp = image_descriptor.format.bytes_per_pixel();
let stride = image_descriptor.compute_stride();
let offset = image_descriptor.offset +
if !tiled_on_cpu {
let bpp = descriptor.format.bytes_per_pixel();
let stride = descriptor.compute_stride();
descriptor.stride = Some(stride);
descriptor.offset +=
tile.y as u32 * tile_size as u32 * stride +
tile.x as u32 * tile_size as u32 * bpp;
(Some(stride), offset)
};

ImageDescriptor {
size: clipped_tile_size,
stride,
offset,
..*image_descriptor
}
} else {
image_template.descriptor.clone()
};

descriptor.size = clipped_tile_size;
}

let filter = match request.rendering {
ImageRendering::Pixelated => {
Expand Down Expand Up @@ -1023,28 +1023,18 @@ impl ResourceCache {
}
};

let entry = self.cached_images.get_mut(&request).as_mut().unwrap();
if cfg!(debug_assertions) {
// confirm that we agree with the logic in `request_image` here, in debug builds only
if let (Some(tile), Some(dirty)) = (request.tile, dirty_rect) {
let needs_upload = self.texture_cache.needs_upload(&entry.texture_cache_handle);
let intersection = intersect_for_tile(dirty, descriptor.size, image_template.tiling.unwrap(), tile);
debug_assert!(needs_upload || intersection.is_some());
}
}

//Note: at this point, the dirty rectangle is local to the descriptor space
self.texture_cache.update(
&mut entry.texture_cache_handle,
descriptor,
filter,
Some(image_data),
[0.0; 3],
dirty_rect,
local_dirty_rect,
gpu_cache,
None,
UvRectKind::Rect,
);
image_template.dirty_rect = None;
}
}

Expand Down
32 changes: 20 additions & 12 deletions webrender/src/texture_cache.rs
Expand Up @@ -1221,7 +1221,7 @@ impl TextureUpdate {
layer_index: i32,
dirty_rect: Option<DeviceUintRect>,
) -> TextureUpdate {
let data_src = match data {
let source = match data {
ImageData::Blob(..) => {
panic!("The vector image should have been rasterized.");
}
Expand All @@ -1246,25 +1246,33 @@ impl TextureUpdate {

let update_op = match dirty_rect {
Some(dirty) => {
// the dirty rectangle doesn't have to be within the area but has to intersect it, at least
let stride = descriptor.compute_stride();
let offset = descriptor.offset + dirty.origin.y * stride + dirty.origin.x * descriptor.format.bytes_per_pixel();
let origin =
DeviceUintPoint::new(origin.x + dirty.origin.x, origin.y + dirty.origin.y);

TextureUpdateOp::Update {
rect: DeviceUintRect::new(origin, dirty.size),
source: data_src,
rect: DeviceUintRect::new(
DeviceUintPoint::new(origin.x + dirty.origin.x, origin.y + dirty.origin.y),
DeviceUintSize::new(
dirty.size.width.min(size.width - dirty.origin.x),
dirty.size.height.min(size.height - dirty.origin.y),
),
),
source,
stride: Some(stride),
offset,
layer_index,
}
}
None => TextureUpdateOp::Update {
rect: DeviceUintRect::new(origin, size),
source: data_src,
stride: descriptor.stride,
offset: descriptor.offset,
layer_index,
},
None => {
TextureUpdateOp::Update {
rect: DeviceUintRect::new(origin, size),
source,
stride: descriptor.stride,
offset: descriptor.offset,
layer_index,
}
}
};

TextureUpdate {
Expand Down
2 changes: 1 addition & 1 deletion webrender_api/src/image.rs
Expand Up @@ -186,7 +186,7 @@ pub trait BlobImageRenderer: Send {

fn request(
&mut self,
services: &BlobImageResources,
resources: &BlobImageResources,
key: BlobImageRequest,
descriptor: &BlobImageDescriptor,
dirty_rect: Option<DeviceUintRect>,
Expand Down
7 changes: 4 additions & 3 deletions wrench/src/blob.rs
Expand Up @@ -26,7 +26,7 @@ fn deserialize_blob(blob: &[u8]) -> Result<ColorU, ()> {
};
}

// perform floor((x * a) / 255. + 0.5) see "Three wrongs make a right" for deriviation
// perform floor((x * a) / 255. + 0.5) see "Three wrongs make a right" for derivation
fn premul(x: u8, a: u8) -> u8 {
let t = (x as u32) * (a as u32) + 128;
((t + (t >> 8)) >> 8) as u8
Expand Down Expand Up @@ -55,8 +55,9 @@ fn render_blob(
};

let mut dirty_rect = dirty_rect.unwrap_or(DeviceUintRect::new(
DeviceUintPoint::new(0, 0),
DeviceUintSize::new(descriptor.size.width, descriptor.size.height)));
DeviceUintPoint::origin(),
descriptor.size,
));

if let Some((tile_size, tile)) = tile {
dirty_rect = intersect_for_tile(dirty_rect, size2(tile_size as u32, tile_size as u32),
Expand Down

0 comments on commit acb7a43

Please sign in to comment.