fix: putImageData bypassed deferred rendering PageRecorder (#1206)

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: Brooooooklyn

__test__/regression.spec.ts

@@ -9,6 +9,71 @@ import { snapshotImage } from './image-snapshot'
 
 const __dirname = dirname(fileURLToPath(import.meta.url))
 
+// https://github.com/Brooooooklyn/canvas/issues/1204
+test('putImageData should modify the canvas', (t) => {
+  const canvas = createCanvas(100, 100)
+  const ctx = canvas.getContext('2d')
+
+  // Fill canvas with red
+  ctx.fillStyle = 'red'
+  ctx.fillRect(0, 0, 100, 100)
+
+  // Get image data and modify to green
+  const imageData = ctx.getImageData(0, 0, 10, 10)
+  for (let i = 0; i < imageData.data.length; i += 4) {
+    imageData.data[i] = 0       // R
+    imageData.data[i + 1] = 255 // G
+    imageData.data[i + 2] = 0   // B
+    imageData.data[i + 3] = 255 // A
+  }
+
+  // putImageData should replace pixels at (0,0)
+  ctx.putImageData(imageData, 0, 0)
+
+  // Read back and verify the pixels were actually written
+  const result = ctx.getImageData(0, 0, 1, 1)
+  t.is(result.data[0], 0, 'R should be 0 (green)')
+  t.is(result.data[1], 255, 'G should be 255 (green)')
+  t.is(result.data[2], 0, 'B should be 0 (green)')
+  t.is(result.data[3], 255, 'A should be 255')
+})
+
+// https://github.com/Brooooooklyn/canvas/issues/1204
+test('putImageData with dirty rect should modify the canvas', (t) => {
+  const canvas = createCanvas(100, 100)
+  const ctx = canvas.getContext('2d')
+
+  // Fill canvas with blue
+  ctx.fillStyle = 'blue'
+  ctx.fillRect(0, 0, 100, 100)
+
+  // Create green image data
+  const imageData = ctx.getImageData(0, 0, 20, 20)
+  for (let i = 0; i < imageData.data.length; i += 4) {
+    imageData.data[i] = 0       // R
+    imageData.data[i + 1] = 255 // G
+    imageData.data[i + 2] = 0   // B
+    imageData.data[i + 3] = 255 // A
+  }
+
+  // putImageData with dirty rect
+  ctx.putImageData(imageData, 10, 10, 0, 0, 10, 10)
+
+  // Pixel at (15, 15) should be green (inside dirty rect)
+  const inside = ctx.getImageData(15, 15, 1, 1)
+  t.is(inside.data[0], 0, 'R should be 0 (green) inside dirty rect')
+  t.is(inside.data[1], 255, 'G should be 255 (green) inside dirty rect')
+  t.is(inside.data[2], 0, 'B should be 0 (green) inside dirty rect')
+  t.is(inside.data[3], 255, 'A should be 255 inside dirty rect')
+
+  // Pixel at (25, 25) should still be blue (outside dirty rect)
+  const outside = ctx.getImageData(25, 25, 1, 1)
+  t.is(outside.data[0], 0, 'R should be 0 (blue) outside dirty rect')
+  t.is(outside.data[1], 0, 'G should be 0 (blue) outside dirty rect')
+  t.is(outside.data[2], 255, 'B should be 255 (blue) outside dirty rect')
+  t.is(outside.data[3], 255, 'A should be 255 outside dirty rect')
+})
+
 test('transform-with-state', async (t) => {
   const canvas = createCanvas(256, 256)
   const ctx = canvas.getContext('2d')

__test__/snapshots/getImageData.png

@@ -815,19 +815,23 @@ void skiac_canvas_write_pixels(skiac_canvas* c_canvas,
   CANVAS_CAST->writePixels(info, pixels, row_bytes, x, y);
 }
 
-void skiac_canvas_write_pixels_dirty(skiac_canvas* c_canvas,
-                                     int width,
-                                     int height,
-                                     uint8_t* pixels,
-                                     size_t row_bytes,
-                                     size_t length,
-                                     float x,
-                                     float y,
-                                     float dirty_x,
-                                     float dirty_y,
-                                     float dirty_width,
-                                     float dirty_height,
-                                     uint8_t cs) {
+// putImageData: uses drawImageRect with kSrc blend mode for direct pixel
+// replacement. Per HTML spec, putImageData must replace pixels, not composite.
+// Uses drawImageRect (recordable by PictureRecorder) instead of
+// SkCanvas::writePixels (which only works on raster surfaces).
+void skiac_canvas_put_image_data(skiac_canvas* c_canvas,
+                                 int width,
+                                 int height,
+                                 uint8_t* pixels,
+                                 size_t row_bytes,
+                                 size_t length,
+                                 float x,
+                                 float y,
+                                 float dirty_x,
+                                 float dirty_y,
+                                 float dirty_width,
+                                 float dirty_height,
+                                 uint8_t cs) {
   auto color_space = COLOR_SPACE_CAST;
   auto info =
       SkImageInfo::Make(width, height, SkColorType::kRGBA_8888_SkColorType,
@@ -838,7 +842,9 @@ void skiac_canvas_write_pixels_dirty(skiac_canvas* c_canvas,
   auto dst_rect =
       SkRect::MakeXYWH(x + dirty_x, y + dirty_y, dirty_width, dirty_height);
   const auto sampling = SkSamplingOptions(SkCubicResampler::Mitchell());
-  CANVAS_CAST->drawImageRect(image, src_rect, dst_rect, sampling, nullptr,
+  SkPaint paint;
+  paint.setBlendMode(SkBlendMode::kSrc);
+  CANVAS_CAST->drawImageRect(image, src_rect, dst_rect, sampling, &paint,
                              SkCanvas::kFast_SrcRectConstraint);
 }
 

skia-c/skia_c.cpp

@@ -897,19 +897,19 @@ void skiac_canvas_write_pixels(skiac_canvas* c_canvas,
                                size_t row_bytes,
                                int x,
                                int y);
-void skiac_canvas_write_pixels_dirty(skiac_canvas* c_canvas,
-                                     int width,
-                                     int height,
-                                     uint8_t* pixels,
-                                     size_t row_bytes,
-                                     size_t length,
-                                     float x,
-                                     float y,
-                                     float dirty_x,
-                                     float dirty_y,
-                                     float dirty_width,
-                                     float dirty_height,
-                                     uint8_t cs);
+void skiac_canvas_put_image_data(skiac_canvas* c_canvas,
+                                 int width,
+                                 int height,
+                                 uint8_t* pixels,
+                                 size_t row_bytes,
+                                 size_t length,
+                                 float x,
+                                 float y,
+                                 float dirty_x,
+                                 float dirty_y,
+                                 float dirty_width,
+                                 float dirty_height,
+                                 uint8_t cs);
 void skiac_canvas_draw_picture(skiac_canvas* c_canvas,
                                skiac_picture* c_picture,
                                skiac_matrix* c_matrix,

skia-c/skia_c.hpp

@@ -2788,12 +2788,33 @@ impl CanvasRenderingContext2D {
       if dirty_width <= 0f32 || dirty_height <= 0f32 {
         return;
       }
+      // Deferred mode: record via PageRecorder on a fresh layer (no clip/transform)
+      // put_image_data uses drawImageRect with kSrc blend (pixel replacement),
+      // which IS recordable by PictureRecorder unlike SkCanvas::writePixels.
+      if let Some(ref recorder) = self.context.page_recorder {
+        let dx_f = dx as f32;
+        let color_space = image_data.color_space;
+        recorder.borrow_mut().put_pixels(|canvas| {
+          canvas.put_image_data(
+            image_data,
+            dx_f,
+            dy as f32,
+            dirty_x,
+            dirty_y,
+            dirty_width,
+            dirty_height,
+            color_space,
+          );
+        });
+        return;
+      }
+      // Direct mode (SVG/PDF): write to surface canvas with inverted transform
       let inverted = self.context.surface.canvas.get_transform_matrix().invert();
       self.context.surface.canvas.save();
       if let Some(inverted) = inverted {
         self.context.surface.canvas.concat(&inverted);
       };
-      self.context.surface.canvas.write_pixels_dirty(
+      self.context.surface.canvas.put_image_data(
         image_data,
         dx as f32,
         dy as f32,
@@ -2805,6 +2826,20 @@ impl CanvasRenderingContext2D {
       );
       self.context.surface.canvas.restore();
     } else {
+      // Deferred mode: use put_image_data with full image dimensions
+      // because write_pixels (SkCanvas::writePixels) is NOT recordable by PictureRecorder
+      if let Some(ref recorder) = self.context.page_recorder {
+        let dx_f = dx as f32;
+        let dy_f = dy as f32;
+        let w = image_data.width as f32;
+        let h = image_data.height as f32;
+        let color_space = image_data.color_space;
+        recorder.borrow_mut().put_pixels(|canvas| {
+          canvas.put_image_data(image_data, dx_f, dy_f, 0.0, 0.0, w, h, color_space);
+        });
+        return;
+      }
+      // Direct mode (SVG/PDF): write pixels directly
       self.context.surface.canvas.write_pixels(image_data, dx, dy);
     }
   }

src/ctx.rs

@@ -118,6 +118,26 @@ impl PageRecorder {
     }
   }
 
+  /// Begin a new recording and restore the current canvas state (save stack, clip, transform).
+  fn resume_recording(&mut self) {
+    self
+      .current
+      .begin_recording(0.0, 0.0, self.width, self.height);
+    if let Some(canvas) = self.current.get_recording_canvas() {
+      for _ in 0..self.save_count {
+        canvas.save();
+      }
+      if let Some(ref clip_path) = self.current_clip {
+        canvas.reset_transform();
+        canvas.set_clip_path(clip_path);
+      }
+      if let Some(ref transform) = self.current_transform {
+        canvas.set_transform(transform);
+      }
+    }
+    self.changed = false;
+  }
+
   /// Promote current recording to a layer if changed (lazy finalization)
   fn promote_layer(&mut self) {
     if self.changed {
@@ -136,27 +156,7 @@ impl PageRecorder {
           );
         }
       }
-      // Resume recording
-      self
-        .current
-        .begin_recording(0.0, 0.0, self.width, self.height);
-      // Restore canvas state on the new recording canvas
-      if let Some(canvas) = self.current.get_recording_canvas() {
-        // First restore save stack depth so transform/clip are applied at correct level
-        for _ in 0..self.save_count {
-          canvas.save();
-        }
-        // Restore clip state first (clip is stored in device space, apply at identity)
-        if let Some(ref clip_path) = self.current_clip {
-          canvas.reset_transform();
-          canvas.set_clip_path(clip_path);
-        }
-        // Then restore transform state
-        if let Some(ref transform) = self.current_transform {
-          canvas.set_transform(transform);
-        }
-      }
-      self.changed = false;
+      self.resume_recording();
     }
   }
 
@@ -276,6 +276,46 @@ impl PageRecorder {
     surface.read_pixels(x, y, width, height, color_space)
   }
 
+  /// Write pixel data as a separate layer, bypassing clip and transform.
+  /// This is used for putImageData which per HTML spec must ignore
+  /// the current transform, clip, globalAlpha, and compositing state.
+  ///
+  /// The approach:
+  /// 1. Promote current recording to a layer (preserving pending draw operations)
+  /// 2. Create a fresh recording (no clip, identity transform) and execute the draw
+  /// 3. Promote that recording as another layer
+  /// 4. Start a new recording with the original state restored
+  pub fn put_pixels<F>(&mut self, f: F)
+  where
+    F: FnOnce(&mut Canvas),
+  {
+    // Step 1: Promote current recording if it has changes
+    if self.changed {
+      if let Some(picture) = self.current.finish_recording_as_picture() {
+        self.layers.push(picture);
+        self.cached_picture = None;
+      }
+      self.changed = false;
+    }
+
+    // Step 2: Fresh recording for pixel data (clean canvas: no clip, identity transform)
+    self
+      .current
+      .begin_recording(0.0, 0.0, self.width, self.height);
+    if let Some(canvas) = self.current.get_recording_canvas() {
+      f(canvas);
+    }
+
+    // Step 3: Promote the pixel data recording as a layer
+    if let Some(picture) = self.current.finish_recording_as_picture() {
+      self.layers.push(picture);
+      self.cached_picture = None;
+    }
+
+    // Step 4: Start new recording and restore state
+    self.resume_recording();
+  }
+
   /// Get recording canvas for direct access (needed for SVG/PDF direct mode)
   pub fn get_recording_canvas(&mut self) -> Option<&mut Canvas> {
     self.changed = true;

src/page_recorder.rs

@@ -606,7 +606,7 @@ pub mod ffi {
       y: i32,
     );
 
-    pub fn skiac_canvas_write_pixels_dirty(
+    pub fn skiac_canvas_put_image_data(
       canvas: *mut skiac_canvas,
       width: i32,
       height: i32,
@@ -2815,7 +2815,10 @@ impl Canvas {
     }
   }
 
-  pub fn write_pixels_dirty(
+  /// drawImageRect with kSrc blend mode for putImageData.
+  /// Replaces destination pixels per HTML spec.
+  /// Works on recording canvases (PictureRecorder).
+  pub fn put_image_data(
     &mut self,
     image: &ImageData,
     x: f32,
@@ -2827,7 +2830,7 @@ impl Canvas {
     color_space: ColorSpace,
   ) {
     unsafe {
-      ffi::skiac_canvas_write_pixels_dirty(
+      ffi::skiac_canvas_put_image_data(
         self.0,
         image.width as i32,
         image.height as i32,