Fix dirty rectangle coordinate space inside a tile

webrender/src/resource_cache.rs

@@ -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();
@@ -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 {
@@ -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();
@@ -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() {
@@ -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,
+                );
+            }
         }
     }
 
@@ -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(..) => {
@@ -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 => {
@@ -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;
         }
     }
 

webrender/src/texture_cache.rs

@@ -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.");
             }
@@ -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 {

webrender_api/src/image.rs

@@ -186,7 +186,7 @@ pub trait BlobImageRenderer: Send {
 
     fn request(
         &mut self,
-        services: &BlobImageResources,
+        resources: &BlobImageResources,
         key: BlobImageRequest,
         descriptor: &BlobImageDescriptor,
         dirty_rect: Option<DeviceUintRect>,

wrench/src/blob.rs

@@ -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
@@ -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),