Skip to content

Conversation

@Hajime-san
Copy link
Contributor

Summary

continues from #25517

The PR(#28470) gave us a chance to reconsider supporting gif and webp, which we had previously abandoned.
I can also confirm that there is demand for webp at least. #25517 (comment)

build size

on main at 1146973

% cargo build --release --target-dir "target/$(git rev-parse --abbrev-ref HEAD)-$(git rev-parse --short HEAD)"
...
% ls -l target/main-1146973cd/release/deno
-rwxr-xr-x  1 user  46682944  132048656 Nov 25 14:53 target/main-1146973cd/release/deno

-> 132.048656 mb

this PR

% cargo build --release --target-dir "target/$(git rev-parse --abbrev-ref HEAD)-$(git rev-parse --short HEAD)"
...
% ls -l target/feat-webp-gif-2485a575d/release/deno
-rwxr-xr-x  1 user  46682944  132311168 Nov 25 17:14 target/feat-webp-gif-2485a575d/release/deno

-> 132.311168 mb

about 263kb larger

@coderabbitai
Copy link

coderabbitai bot commented Nov 25, 2025

Walkthrough

The PR adds GIF and WebP support to createImageBitmap: it removes early MIME-type rejections in the JS canvas layer, enables "gif" and "webp" features in ext/canvas/Cargo.toml, implements GIF and WebP decoding in Rust (returning DynamicImage plus orientation and ICC profile), and updates tests to create imageBitmaps for GIF/WEBP (and other formats) and assert bitmap data instead of expecting rejections.

Sequence Diagram

sequenceDiagram
    actor Client
    participant JS as JS canvas layer
    participant Rust as Rust decoder layer
    participant Tests as Test suite

    rect rgb(250,245,240)
    Note over Client,JS: Request flow (after changes)
    Client->>JS: createImageBitmap(blob with GIF/WEBP)
    JS->>JS: MIME check (no early rejection for GIF/WEBP)
    JS->>Rust: decodeImage(blob)
    rect rgb(230,250,240)
      Rust->>Rust: Instantiate GifDecoder/WebPDecoder
      Rust->>Rust: Decode frames -> DynamicImage
      Rust->>Rust: Extract orientation & ICC profile
      Rust-->>JS: Return bitmap + metadata
    end
    JS->>Client: Resolve ImageBitmap
    Client->>Tests: Test reads bitmap via Deno.internal.getBitmapData
    Tests->>Tests: Assert bitmap contents, dimensions, format
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review ext/canvas/01_image.js changes (removed early rejections) for correct MIME handling and error semantics.
  • Verify ext/canvas/Cargo.toml feature additions ("gif", "webp") match decoder usage and build configuration.
  • Inspect ext/canvas/op_create_image_bitmap.rs decoder code for GIF/WebP: correctness of decoding, orientation and ICC extraction, memory/unsafe usage, and parity with existing decoders.
  • Check tests (tests/unit/image_bitmap_test.ts) for accurate assertions and coverage of animated vs. static cases.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and clearly describes the main change: adding GIF and WebP format support to createImageBitmap in the canvas extension.
Description check ✅ Passed The description explains the context, references prior related work, confirms user demand, and includes relevant build size impact metrics.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d4cbf1 and 14a5998.

📒 Files selected for processing (1)
  • ext/canvas/op_create_image_bitmap.rs (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or use lldb directly
Use eprintln!() or dbg!() macros for debug prints in Rust code

Files:

  • ext/canvas/op_create_image_bitmap.rs
🧬 Code graph analysis (1)
ext/canvas/op_create_image_bitmap.rs (2)
tests/unit/test_util.ts (1)
  • BufReader (245-765)
ext/canvas/lib.rs (1)
  • image_error_to_invalid_image (40-42)
🔇 Additional comments (3)
ext/canvas/op_create_image_bitmap.rs (3)

124-135: GIF Blob decoding path correctly mirrors other formats

The new MimeType::Gif arm follows the same pattern as PNG/JPEG/BMP/ICO: construct GifDecoder over a BufReader<Cursor<_>>, read orientation and ICC profile, then decode via DynamicImage::from_decoder. This should integrate cleanly with the later orientation/color‑space pipeline and meets the “first frame” requirement for animated GIFs.

Given this relies on image::codecs::gif::GifDecoder’s orientation/icc_profile behavior, it’s worth running the updated image_bitmap tests (especially animated GIF cases) to confirm parity with browser behavior.


161-173: WebP Blob decoding path matches GIF/PNG/JPEG behavior

The MimeType::Webp branch sensibly mirrors the other decoders: WebPDecoder over BufReader<Cursor<_>>, then orientation + ICC profile + DynamicImage::from_decoder. This gives WebP the same metadata handling and “first frame” semantics as the other formats, which is what createImageBitmap expects.

Please ensure the new WebP tests (including animated WebP) pass across your supported platforms so we know the WebPDecoder behavior aligns with browser expectations.


12-18: GIF/WebP decoder imports are consistent with new format support

Verified: ext/canvas/Cargo.toml line 20 includes both "webp" and "gif" in the image dependency features. The imports and decode branches are properly wired.


Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant