Repository navigation
Conversation
Scale preview GPU renders to the letterboxed viewport times display scale and max zoom, capped at full project base size, and re-publish when the preview area resizes.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| &decoded, | ||
| &update.config, | ||
| None, | ||
| Some(update.preview_render_size), |
There was a problem hiding this comment.
On a large screenshot, the smaller preview frame makes frame_arrived rewrite self.project.annotations into smaller coordinates. But export_image passes them unchanged to composite, which still scales them using the full-sized RawFrame.base_width and RawFrame.base_height. Exported arrows, shapes, and masks shrink and move toward the top-left instead of matching the preview. Saved annotations also keep the smaller coordinates without recording their frame size.
Keep annotation coordinates independent of preview resolution, or convert them at every export and save boundary.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_editor.rs
Line: 250
Comment:
**Exported marks move**
On a large screenshot, the smaller preview frame makes `frame_arrived` rewrite `self.project.annotations` into smaller coordinates. But `export_image` passes them unchanged to `composite`, which still scales them using the full-sized `RawFrame.base_width` and `RawFrame.base_height`. Exported arrows, shapes, and masks shrink and move toward the top-left instead of matching the preview. Saved annotations also keep the smaller coordinates without recording their frame size.
Keep annotation coordinates independent of preview resolution, or convert them at every export and save boundary.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| entity.update(cx, |this, cx| { | ||
| this.notify_preview_resolution_change( | ||
| scale_factor, | ||
| cx, | ||
| ); |
There was a problem hiding this comment.
This replaces an unconditional cx.notify() with notify_preview_resolution_change, which returns early when the render size is unchanged. But render_preview computes its layout before this canvas updates preview_area. Resizing while render resolution stays capped at the full base therefore sends neither a new frame nor a notification to apply the new bounds. The preview can keep its previous size until another action redraws it.
Notify on every bounds change; only skip the renderer update.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_editor.rs
Line: 4205-4209
Comment:
**Preview keeps old bounds**
This replaces an unconditional `cx.notify()` with `notify_preview_resolution_change`, which returns early when the render size is unchanged. But `render_preview` computes its layout before this canvas updates `preview_area`. Resizing while render resolution stays capped at the full base therefore sends neither a new frame nor a notification to apply the new bounds. The preview can keep its previous size until another action redraws it.
Notify on every bounds change; only skip the renderer update.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if area_cell.get() != Some(bounds) { | ||
| area_cell.set(Some(bounds)); | ||
| let scale_factor = window.scale_factor(); |
There was a problem hiding this comment.
If moving the editor between displays changes window.scale_factor() without changing the logical bounds, this branch never sends a new preview size. A large image moved to a sharper display keeps the old, lower-resolution preview, which can look blurry when zoomed until a resize or render-affecting edit.
Track display scale alongside the bounds, or observe display-scale changes separately.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_editor.rs
Line: 4200-4202
Comment:
**Preview misses sharper display**
If moving the editor between displays changes `window.scale_factor()` without changing the logical bounds, this branch never sends a new preview size. A large image moved to a sharper display keeps the old, lower-resolution preview, which can look blurry when zoomed until a resize or render-affecting edit.
Track display scale alongside the bounds, or observe display-scale changes separately.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Stop rescaling annotations when preview GPU frames arrive at reduced resolution, and still notify when preview bounds change without a render-size update.
Track display scale for preview rerenders, skip rescaling annotations on load, and keep marks in logical frame coordinates.
dda1252 to
16f0d98
Compare
|
@greptile-apps please re-review. Addressed the open P1/P2 items in the latest push on this branch. |
| let next = (size.0.max(1) as f32, size.1.max(1) as f32); | ||
| self.rescale_annotations_for(next); | ||
| self.frame_size = next; | ||
| self.frame_size = self.logical_frame_size(); |
There was a problem hiding this comment.
When the preview is downscaled, frame_arrived sets frame_size to the full output size, but mask overlays still read the smaller preview image. mask_requests passes those full dimensions to mask_filter, which rejects the shorter pixel buffer and returns None. Blur and pixelate masks therefore disappear from the preview.
Keep annotation coordinates full-sized, but convert mask regions and filter sizes to the preview image's actual dimensions.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_editor.rs
Line: 1186
Comment:
**Preview masks disappear**
When the preview is downscaled, `frame_arrived` sets `frame_size` to the full output size, but mask overlays still read the smaller preview image. `mask_requests` passes those full dimensions to `mask_filter`, which rejects the shorter pixel buffer and returns `None`. Blur and pixelate masks therefore disappear from the preview.
Keep annotation coordinates full-sized, but convert mask regions and filter sizes to the preview image's actual dimensions.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| self.previous_transform = None; | ||
| self.frame_size = self.logical_frame_size(); |
There was a problem hiding this comment.
After loading a project, previous_transform stays None because frame_arrived no longer calls rescale_annotations_for. If the first project edit changes padding, crop, or aspect ratio, edit_project calls that helper only after changing the configuration. The helper records the new transform without moving existing annotations, so marks no longer point at the same part of the screenshot.
Record the old transform when loading, or before applying the first edit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_editor.rs
Line: 1174-1175
Comment:
**First layout edit misplaces marks**
After loading a project, `previous_transform` stays `None` because `frame_arrived` no longer calls `rescale_annotations_for`. If the first project edit changes padding, crop, or aspect ratio, `edit_project` calls that helper only after changing the configuration. The helper records the new transform without moving existing annotations, so marks no longer point at the same part of the screenshot.
Record the old transform when loading, or before applying the first edit.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
16f0d98 to
cf5f02c
Compare
Annotations, masks and layout stay in one frame space that the export composite shares, independent of the resolution the preview renders at. Mask requests carry the preview texture's own pixel size and its scale against frame space, so blur and pixelate overlays resample the smaller preview instead of being rejected. The annotation baseline is recorded at load so the first layout edit carries marks across, and the preview resolution is re-requested whenever its bounds or the display scale change.
|
@greptile-apps review |
…rness Hyprland 0.57 dropped hyprland.conf and only reads hyprland.lua, so the harness's monitor mode, session export and waybar were ignored and the output came up at vkms's preferred 1024x768. The screenshot scenario then reported screenshot_size=1024x768 instead of yes. Write the same settings as Lua and use the Lua form of the focus dispatcher.
| mask.x * scale[0], | ||
| mask.y * scale[1], | ||
| mask.width * scale[0], | ||
| mask.height * scale[1], |
There was a problem hiding this comment.
mask_request scales mask bounds into preview pixels but leaves mask_level unchanged. With a quarter-sized preview, a pixelation level of 8 produces blocks covering 32 frame-space pixels, while a full-sized export uses 8-pixel blocks. Blur strength also changes with preview resolution. Users therefore see stronger masking than the exported image provides.
Scale the filter level alongside the bounds so resizing the window does not change the apparent mask strength.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_annotations.rs
Line: 4036-4039
Comment:
**Preview masks look stronger**
`mask_request` scales mask bounds into preview pixels but leaves `mask_level` unchanged. With a quarter-sized preview, a pixelation level of 8 produces blocks covering 32 frame-space pixels, while a full-sized export uses 8-pixel blocks. Blur strength also changes with preview resolution. Users therefore see stronger masking than the exported image provides.
Scale the filter level alongside the bounds so resizing the window does not change the apparent mask strength.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
@greptile-apps review |
|
Someone is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
|
@greptile-apps review |
|
@greptile-apps review |
Summary
preview_render_sizethroughConfigUpdateso the still renderer uses it for preview frames instead of always rendering at full project base size.MAX_ZOOM, capped byProjectUniforms::get_base_size.Test plan
The PR appears safe to merge; all six previous findings are addressed and no new blocking issue was found.Summary
The PR renders screenshot previews at a size suited to the display while keeping exports at full resolution.
All six previous findings are addressed. No new actionable issues were found.
Reviews (6) · Last reviewed commit: "fix(ci): walk GIF blocks when measuring ..." · Reviewed by Greptile