feat(sharp): metadata/toFile objects, Buffer-input factory, .extract()/.sharpen(), SIMD resize#5448
Conversation
…harpen, SIMD resize
Four pure-Rust sharp improvements toward real-world parity (no libvips):
- metadata()/toFile() resolve real objects instead of JSON strings:
metadata -> {format,width,height,channels,space,hasAlpha};
toFile -> sharp info {format,width,height,channels,size}. Built on the main
thread via resolve_with + js_object_alloc_with_shape/js_object_set_field (#1824).
- sharp(input) accepts a Buffer/Uint8Array, not just a path: new js_sharp_from_input
recovers the pointer from raw NaN-box bits and branches on js_buffer_is_buffer
(Buffer -> load_from_memory, string -> open).
- .extract({left,top,width,height}) and .sharpen() are now reachable and chainable
(dispatch rows + FFI decls + manifest entries + fluent-chain allowlist). extract
reads its options object fields by name via js_object_get_field_by_name_f64.
- resize() uses fast_image_resize (SIMD Lanczos3), preserving source pixel layout
(Luma/LumaA/Rgb/Rgba8; 16-bit/float fall back to RGBA8; fast-path failure falls
back to the image crate). Aspect-ratio (height<=0) preserved.
Validated: metadata/toFile fields; sharp(Buffer) decode->resize->re-encode;
extract dims + sharpen chains; resize exact+aspect across RGBA/LumaA; valid JPEG
from toFile; perry-ext-sharp 6/6; api docs regenerated.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughVersion ChangesSharp feature additions
Sequence Diagram(s)sequenceDiagram
participant JS as JS Caller
participant factory as js_sharp_from_input
participant loader as open_image_path / decode_image_bytes
participant resize as fast_resize (SIMD)
participant fallback as image crate resize
participant extract as js_sharp_extract
participant meta as js_sharp_metadata / js_sharp_to_file
JS->>factory: sharp(path | Buffer)
factory->>factory: probe NaN-boxed type
alt string path
factory->>loader: open_image_path(path)
else Buffer/Uint8Array
factory->>loader: decode_image_bytes(bytes)
end
loader-->>factory: ImageHandle
JS->>resize: .resize(w, h)
resize->>resize: fast_image_resize Lanczos3
alt unsupported layout or error
resize->>fallback: image crate resize
end
JS->>extract: .extract({left,top,width,height})
extract->>extract: opts_number_field + crop_imm
JS->>meta: .metadata() / .toFile(path)
meta-->>JS: JS object {format, width, height, channels, ...}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-ext-sharp/src/lib.rs`:
- Around line 427-451: The channels value is incorrectly sourced from
sharp.image.color().channel_count() which represents the pre-encoded pixels
rather than the actual encoded output. When format conversions occur (such as
RGBA to JPEG), the reported channels can diverge from the actual encoded file's
channel count. Instead of using sharp.image.color().channel_count() to determine
the channels value that gets set in js_object_set_field at field index 3, derive
the channel count from the encoded format itself using the sharp.format value
that is already being captured. Map the format to its corresponding channel
count (for example, JPEG typically has 3 channels, PNG with alpha has 4,
grayscale has 1) to ensure the returned metadata accurately reflects the actual
encoded output file.
- Around line 149-164: The js_sharp_from_input function does not validate that
non-buffer pointers are actually valid strings before attempting to read them as
StringHeader. After checking that the pointer is not a buffer using
js_buffer_is_buffer, add a tag validation check (checking for STRING_TAG or
SHORT_STRING_TAG) on the pointer before calling read_string. This ensures that
only pointers with valid string tags are processed as strings, preventing
arbitrary memory from being read as a StringHeader and protecting against
crashes or memory corruption from malformed or attacker-controlled pointers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8e72d2c9-d4ab-4865-8705-42b7b77bd74f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
CHANGELOG.mdCLAUDE.mdCargo.tomlcrates/perry-api-manifest/src/entries.rscrates/perry-codegen/src/lower_call/early_branches.rscrates/perry-codegen/src/lower_call/native_table/media.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi.rscrates/perry-ext-sharp/Cargo.tomlcrates/perry-ext-sharp/src/lib.rsdocs/src/api/reference.md
…ls from output; rustfmt CodeRabbit feedback on #5448: - js_sharp_from_input: a POINTER_TAG value that isn't a registered Buffer is a plain object/array; js_get_string_pointer_unified hands back its heap pointer, which must not be read as a StringHeader (arbitrary-memory read). Reject it (return -1) like sharp's unsupported-input. Strings (long/short) and number-coerced keys aren't POINTER_TAG, so the path-string case is unaffected. - toFile() info.channels now reflects the ENCODED output by re-reading the saved file (RGBA source saved as JPEG reports 3, not 4), falling back to the in-memory count. metadata() keeps in-memory channels (sharp semantics). - rustfmt the fast_resize match arms. Validated: toFile jpeg=3/png=4 channels; string & buffer factory width=8; sharp({})/sharp([]) -> 0 (no crash).
Four sharp improvements toward real-world parity, all pure-Rust (no libvips) — keeping Perry's self-contained static-binary model. Builds on #5444 (toBuffer real Buffer) and #5445 (fluent chaining).
1.
.metadata()/.toFile()resolve real objects, not JSON stringsBefore,
(await sharp(x).metadata()).widthwasundefined— the Promise resolved with a JSON string. Now:metadata()→{ format, width, height, channels, space, hasAlpha }toFile()→ sharp'sinfoobject{ format, width, height, channels, size }Built on the main thread via
resolve_with+js_object_alloc_with_shape/js_object_set_field(object allocation must happen on the main thread — the runtime arena is thread-local, #1824).2.
sharp(input)accepts a Buffer/Uint8ArrayThe factory routes through a new
js_sharp_from_input, which recovers the underlying pointer from the raw NaN-boxed value and branches on the Buffer registry probe (js_buffer_is_buffer): Buffer →image::load_from_memory, string →image::open. Enables in-memory pipelines (sharp(buf).resize(...).toBuffer()).3.
.extract({left,top,width,height})and.sharpen()— reachable + chainableBoth were implemented in
perry-ext-sharpbut had no dispatch rows. Now wired end-to-end: dispatch row (native_table/media.rs), FFI decl (stdlib_ffi.rs), manifest entry (entries.rs), and the fluent-chain allowlist (early_branches.rs)..extract()reads its options object's numeric fields by name (js_object_get_field_by_name_f64) and applies a region crop. This is the substantive follow-through on the earlier CodeRabbit note about allowlist completeness — added with their dispatch rows, not standalone.4.
resize()usesfast_image_resize(SIMD)Swapped the
imagecrate's scalar Lanczos3 forfast_image_resize, preserving the source pixel layout (Luma/LumaA/Rgb/Rgba 8-bit). Uncommon encodings (16-bit, float) convert to RGBA8; any fast-path failure falls back to theimageresizer. Aspect-ratio (height <= 0) behavior preserved.Validation
e2e (compiled, run, output asserted):
metadata()→typeof object,8 8 4 png srgb true;toFile()→jpeg 20 10 4 size>0, file is a validJPEG 20x10 components 3.sharp(Buffer)→ decode →resize(5,5).png().toBuffer()→ PNG magic137 80 78 71..extract({left:0,top:0,width:3,height:6}).metadata()→ dims3 6;.resize(8,8).sharpen().png().toBuffer()ok.12x7, aspect16x16; grayscale→resize exercises the LumaA (U8x2) path (channels 2 space b-w).perry-ext-sharpunit suite 6/6; API docs regenerated (reference.md, gate-clean).Known nuance (follow-up)
toFile/metadatachannelsreflects the in-memory image's channel count (e.g. 4 for an RGBA source saved as JPEG, where the file is 3-channel) rather than the encoded output's.Summary by CodeRabbit
Release Notes v0.5.1192
New Features
sharp()now acceptsBuffer/Uint8Arrayfor in-memory image loading.extract({ left, top, width, height })for cropping.sharpen()for image enhancementImprovements
resize()uses SIMD acceleration (with fallback) and improved dimension handling whenheight <= 0metadata()now returns an object (format,width,height,channels,space,hasAlpha)toFile()now resolves with an info object (format,width,height,channels,size)