Repository navigation
Conversation
Co-authored-by: Kyzer <kyzer1023@users.noreply.github.com>
|
@kyzer1023 is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
| let bytes = [ | ||
| 0xFF, 0xD8, 0xFF, 0xC0, 0x00, 0x14, 0x08, 0x00, 0x08, 0x00, 0x08, 0x04, 0x01, 0x11, | ||
| 0x00, 0x02, 0x11, 0x00, 0x03, 0x11, 0x00, 0x04, 0x11, 0x00, | ||
| ]; | ||
| assert!(decode_jpeg_thumbnail(&bytes, |w, h| (w, h)).is_none()); |
There was a problem hiding this comment.
cmyk_frame_returns_none uses a truncated JPEG header with no scan data. The test passes when decoding fails, even if CMYK rejection is removed. Use a complete CMYK JPEG so the test guards the unsupported-format branch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/jpeg_thumbnail.rs
Line: 101-105
Comment:
**CMYK test misses its branch**
`cmyk_frame_returns_none` uses a truncated JPEG header with no scan data. The test passes when decoding fails, even if CMYK rejection is removed. Use a complete CMYK JPEG so the test guards the unsupported-format branch.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let pixels = decoder.decode().ok()?; | ||
| let decoded = match info.pixel_format { |
There was a problem hiding this comment.
Unsupported JPEGs decode twice
The shared helper decodes pixels before rejecting unsupported formats. Library thumbnails now pay for that discarded decode and then decode again through the fallback. Check info.pixel_format before calling decoder.decode() so CMYK and 16-bit gray files go straight to the fallback.
| let pixels = decoder.decode().ok()?; | |
| let decoded = match info.pixel_format { | |
| if !matches!( | |
| info.pixel_format, | |
| jpeg_decoder::PixelFormat::RGB24 | jpeg_decoder::PixelFormat::L8 | |
| ) { | |
| return None; | |
| } | |
| let pixels = decoder.decode().ok()?; | |
| let decoded = match info.pixel_format { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/jpeg_thumbnail.rs
Line: 17-18
Comment:
**Unsupported JPEGs decode twice**
The shared helper decodes pixels before rejecting unsupported formats. Library thumbnails now pay for that discarded decode and then decode again through the fallback. Check `info.pixel_format` before calling `decoder.decode()` so CMYK and 16-bit gray files go straight to the fallback.
```suggestion
if !matches!(
info.pixel_format,
jpeg_decoder::PixelFormat::RGB24 | jpeg_decoder::PixelFormat::L8
) {
return None;
}
let pixels = decoder.decode().ok()?;
let decoded = match info.pixel_format {
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| Some((decoded, u32::from(info.width), u32::from(info.height))) | ||
| } | ||
|
|
||
| /// Cover size for a card. Never upscales. |
There was a problem hiding this comment.
The new cover_target comment only describes what the function does. The repository guide bans comments that narrate code and requires non-obvious context instead. Remove this comment or express the guarantee through a test. This repository requirement must be satisfied before merging.
| /// Cover size for a card. Never upscales. |
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/jpeg_thumbnail.rs
Line: 34
Comment:
**Comment breaks the guide**
The new `cover_target` comment only describes what the function does. The repository guide bans comments that narrate code and requires non-obvious context instead. Remove this comment or express the guarantee through a test. This repository requirement must be satisfied before merging.
```suggestion
```
**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Summary
Library card thumbnails (
thumbnail_rgbainapps/desktop-gpui/src/library.rs) decoded JPEGs at full resolution and then resampled to the 392×224 cover size. JPEGs now usejpeg_decoder::Decoder::scaleto IDCT-scale toward the cover size first, then resize to the exact size. A 1920×1080 or 3840×2160 frame decodes at 480×270.The sidebar's existing IDCT decode moved into a shared
library/jpeg_thumbnail.rs, sodecode_jpeg_thumbnailis the only IDCT decoder. The sidebar still asks for a square max; library cards pass the cover size. PNG is unchanged, and CMYK / 16-bit gray JPEGs still fall back to a full decode.The pixels differ slightly from the old full-resolution resample, so the cache prefix now ends in
-idct. Old cache files are left on disk.One commit, 4 files, +153/-30. Based on
mainat85219015.Correctness
jpeg_thumbnailtests pass (square downsample, historical cover sizes, 1080p to 480×270, CMYK returnsNone), run through a standalone crate that path-includes the module. The full desktop-gpui app and the cache-prefix tests inlibrary.rswere not built here because gpui is patched to a localzed-capcheckout.Benchmarks
Release, Linux VM, median of 7 samples after one warmup. Before is the old
imagefull decode plus cover resample.sequoia-dark.jpg4000×2500tahoe-dark.jpg3840×2160A second run put synthetic 1080p at 1.67× and 4K at 1.87×.
Overlap
Does not touch clip thumbnails (#2707) or recording recovery in
library.rs(#2630).No blocking behavior defect was found, but the explicit comments policy must be satisfied before merging.
Findings
Fix with agent prompt
Summary
Library JPEG thumbnails now decode at a smaller size before the final resize.
Reviews (1) · Last reviewed commit: "perf(desktop-gpui): IDCT-scale JPEG libr..." · Reviewed by Greptile