Repository navigation
Conversation
|
The Pull Request introduced native fingerprint changes against the base commit. Generated by PR labeler 🤖 |
|
|
I reviewed this PR and found no actionable defects. Claude review, run |
4e6cd53 to
f0173a9
Compare
|
I found no actionable defects in this PR. Claude review, run |
estrattonbailey
left a comment
There was a problem hiding this comment.
Let's feature flag this and give it a shot
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f0173a9 to
0b7a7a7
Compare
| 'image/jpeg', | ||
| quality / 100, | ||
| ) | ||
| }) |
There was a problem hiding this comment.
non-blocking: SVG images can no longer be posted on web while the gate is on
The web composer accepts image/svg+xml (SUPPORTED_IMAGE_MIME_TYPES in src/view/com/composer/SelectMediaButton.tsx:98, plus the svg extension fallback). If the user doesn't crop, compressImage passes img.source (the SVG blob/data URI) straight to this function. createImageBitmap doesn't decode vector blobs. Chromium rejects an image/svg+xml Blob with "The source image could not be decoded", and Firefox rejects it too. So compressToFit throws and the post fails. The manipulator path this replaces loaded the source through an <img>, which renders SVGs, so this worked before.
One fix: when blob.type === 'image/svg+xml', rasterize it through an HTMLImageElement first. SVGs are small, so the iOS Safari downsampling problem doesn't apply. Another option is to send SVGs to compressWithManipulator.
There was a problem hiding this comment.
Fixed in ff5fe16: SVGs are now drawn with an <img> first, at the size it gives them, then go through the same white-background draw and encode as everything else. Checked in Chrome and Firefox. Both SVGs (with and without an intrinsic size) compress again, and transparent areas now come out white rather than black. GIF and WebP are unchanged.
|
One non-blocking finding, posted inline on The web composer still accepts Claude review, run |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable defects found. Claude review, run |
Stacked on #12363 (compress images natively, decoding each image once).
Summary
On web, images are compressed with the same expo-image-manipulator binary search that native used before #12363. Each image takes up to 7 passes, and each pass:
It also leaves a blob URL behind for every losing pass. And because JPEG has no transparency, transparent areas come out black.
This does what #12363 does on native, using the browser:
createImageBitmap, which applies the EXIF orientation.<img>, because iOS Safari draws large ones from a lower-resolution copy and the output came out blurry.canvas.toBlob, using a TypeScript port ofQualitySearch, and creates a URL only for the winner.QualitySearchnow exists in Swift, Kotlin and TypeScript, and the comments in each point to the others. The TypeScript copy has the unit tests (qualitySearch.test.ts).Feature gate
This is behind
image_uploads:web_compression:enablein GrowthBook, in the App project.compressWithManipulator.ts), including the black transparency.Results
A static test page ran this code and the current code side by side. The current code is the real expo-image-manipulator web implementation, bundled with small shims for
expo. Both used the same 10 images as #12363:They ran two at a time, as the app does. "Frozen" adds up every gap longer than 50 ms in a 16 ms timer, so it measures how long the page couldn't respond.
Post settings (4000px, 2 MB), 10 images
Avatar settings (2000px, 1 MB), 10 images
The desktop runs were headless Chrome 157 and Firefox 159 on an M-series Mac. The current code's desktop ranges cover two runs.
Output quality. I compared each output with an ideal downscale of the source (Lanczos, in PIL). The numbers are the mean difference per channel, out of 255; lower is better.
<img>instead ofcreateImageBitmap, iOS Safari scored 3.36 at post size and looked blurry.Every output is upright, including the EXIF-rotated photo. Transparency comes out white, matching native. The semi-transparent blue bar in the test image becomes (128,127,255), where it used to come out as black and dark navy (1,0,128).
Follow-up: Safari, and Chrome on low-end Android, still do most of this work on the main thread. Safari encodes JPEGs there, and the A10 also draws there. Moving it into a Worker with
OffscreenCanvaswould fix that. I left it out because Metro makes Workers awkward.Test plan
pnpm typecheck,pnpm lint,pnpm testPart of APP-3190.
🤖 Generated with Claude Code