Repository navigation
Conversation
Masks gain shapes (rectangle, rounded, ellipse), feathered edges, an invert mode that hides everything outside the area, colour tint, a Solid redaction style, opacity, and seven one-click presets. The look is baked into each overlay by a new kernel (mask_style), so the preview and the export composite the same pixels. Options live in an optional maskOptions field that older builds ignore; Solid keeps a hiding maskType alongside so a redaction never degrades to nothing.
|
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[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| fn mask_anchor(&self, panel: MaskPanel) -> BoundsCell { | ||
| self.annotation_state | ||
| .mask_anchors | ||
| .get(&panel) | ||
| .cloned() | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
Both mask popovers always open at the fallback position. Nothing inserts a cell into mask_anchors, so mask_anchor() returns a fresh cell each time. The button writes its bounds into a cell that the popover never reads. Initialize and reuse one cell per panel so each popover opens under its button.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_annotations/mask.rs
Line: 602-607
Comment:
**Popovers lose their button positions**
Both mask popovers always open at the fallback position. Nothing inserts a cell into `mask_anchors`, so `mask_anchor()` returns a fresh cell each time. The button writes its bounds into a cell that the popover never reads. Initialize and reuse one cell per panel so each popover opens under its button.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Good catch, valid. Fixed in 4f20ec7: both panel anchors are now seeded once in init_annotation_inputs, so the button writes into the same cell the popover reads.
| ColorTarget::Mask => { | ||
| let options = annotation.mask_options.get_or_insert_with(Default::default); | ||
| Self::ensure_mask_tint_visible(options); | ||
| &mut options.color | ||
| } | ||
| }; | ||
| if *field == value { | ||
| return false; |
There was a problem hiding this comment.
Picking the current color on an untinted blur or pixelate mask changes tint to 0.4, then returns false because the color matches. Picking black on a default mask takes this path. edit_project() then skips the preview refresh, save, and history entry. Count the tint change when deciding whether the edit changed anything.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_annotations.rs
Line: 2773-2780
Comment:
**Same-color picks skip tint updates**
Picking the current color on an untinted blur or pixelate mask changes `tint` to `0.4`, then returns `false` because the color matches. Picking black on a default mask takes this path. `edit_project()` then skips the preview refresh, save, and history entry. Count the tint change when deciding whether the edit changed anything.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
Valid. Fixed in 4f20ec7: ensure_mask_tint_visible now returns whether it changed the tint, and the colour edit reports a change when either the colour or the tint moved. Added a unit test.
| /// A new mask copies the look of the selected mask, or else the last one | ||
| /// drawn, so a run of redactions does not need restyling one by one. |
There was a problem hiding this comment.
The new comment above mask_style_source() only describes the feature's behavior. AGENTS.md requires comments to capture non-obvious context and explicitly bans comments that merely describe the change being made. Remove it or replace it with a necessary explanation of a non-obvious constraint. This repository requirement must be satisfied before merging.
| /// A new mask copies the look of the selected mask, or else the last one | |
| /// drawn, so a run of redactions does not need restyling one by one. |
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/screenshot_annotations/mask.rs
Line: 541-542
Comment:
**Comment breaks repository rules**
The new comment above `mask_style_source()` only describes the feature's behavior. `AGENTS.md` requires comments to capture non-obvious context and explicitly bans comments that merely describe the change being made. Remove it or replace it with a necessary explanation of a non-obvious constraint. 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!
There was a problem hiding this comment.
Agreed. Removed in 4f20ec7, along with a few other comments in the new mask code that only narrated behaviour.
|
@greptileai please re-review — all three findings are addressed in 4f20ec7. |
…ask model Masks are Sensitive (blur, pixelate or solid) or Highlight (dim and optionally blur everything outside), the same split the video editor's mask panel uses. Shape sits in the bar; feather, corners and tint move into one settings popover that reuses the header popovers' chrome. The presets list, invert pill and Adjust panel are gone.
AatmanAJ
changed the title
…images The GNOME export run on this branch lost Cap to a SIGSEGV while the shared rendering device was being created: the loader had just logged "Removing driver libvulkan_radeon.so due to not having any physical devices", which is where it destroys that driver's instance and dlcloses its library, while the main thread was still drawing with lavapipe. The hardware drivers never find a device on vkms/vgem, so VK_DRIVER_FILES now names the lavapipe manifest alone, next to the existing device_select workaround. A real GPU passed in through DOCKER_EXTRA keeps the full driver list.
The Hyprland OpenGL step hides every Vulkan driver with VK_ICD_FILENAMES=/nonexistent.json to put Cap on wgpu's GL backend. The loader prefers VK_DRIVER_FILES over that older variable, so the lavapipe pin brought the driver back and the step's "(Gl)" adapter check failed. The pin now stays off whenever the caller passes VK_DRIVER_FILES or VK_ICD_FILENAMES itself.
Arch now ships Hyprland 0.57 (2026-10-28), which removed its hyprlang config support and reads hyprland.lua alone. The harness's hyprland.conf was ignored: Hyprland logged "No config file found; attempting to generate", left the vkms output at its 1024x768 default and stopped logging to stdout, so every "Hyprland screenshot" step since 2026-10-29 failed on screenshot_size. The same monitor, misc, debug and autostart settings now go into hyprland.lua, with stdout logging turned on so compositor.log keeps collecting them.
Summary
Turns the screenshot editor's Mask tool into a proper mask feature, laid out the same way as the video editor's mask panel.
The mask bar (one row, same pills and sliders as the rest of the annotation bar):
[Sensitive | Highlight]│ mode controls │[▢ ◯]⚙How it renders
cap_gpui_kernels::mask_style::styled_mask, applies the effect (blur, pixelate or solid), then the tint, then the shape. The shape is a signed-distance coverage for a rounded rect or ellipse, with a feather that only fades inward. An inverted mask flips the coverage, and opacity is applied last, into alpha. The overlay is straight alpha, so the preview (GPUI sprite blend) and the export (blit_over) composite the same pixels.RawFrame::content(computed with the sameimage_rectthe preview uses), andscale_annotationsscales feather and corner radius along withmaskLevel.MaskRequestcarries a hashableMaskStyleplus the shape rect, so the overlay cache still reuses, cancels and retires images correctly when any style field changes.Data model / compatibility
Annotation.maskOptions(shape,cornerRadius,feather,invert,solid,color,tint). Every field has a default, and it isskip_serializing_if = None, so existing projects are unchanged on disk.maskOptionsand still see a validmaskType. A Solid redaction falls back to a blur rather than exposing what's under it, and a corrupt (NaN) opacity falls back to opaque.MaskTypevariants.i18n: eight new strings, translated in all seven catalogs, and added to the literal surface test.
screenshot_editor.annotation.mask_stylewas removed because nothing reads it any more. The Sensitive/Highlight labels reuse the video editor's existing strings.Test plan
cargo test --bin cap-gpui: 1572 passedcargo test -p cap-gpui-kernels: 82 lib tests passed, including tests for the mask kernel (byte-identical legacy output, solid fill, tint, ellipse, rounded corners, feather falloff, invert hole, opacity, non-finite fallbacks)cargo test -p cap-i18n-build,cargo test -p cap-project(maskOptionsserde round-trip)Linux harness
The GNOME "editor export to MP4 and GIF" step failed once here with Cap gone (
gif_cap_alive=no). Its log ends in aSIGSEGVwhile the shared rendering device was being created: the Vulkan loader had just loggedRemoving driver libvulkan_radeon.so due to not having any physical devices, the point where it destroys that driver's instance anddlcloses its library, while the main thread was drawing with lavapipe. The editor had opened a video recording, so no mask code ran.run-desktop.shnow points the loader at the lavapipe manifest alone on the vkms images (VK_DRIVER_FILES), next to the existing device_select workaround, so the six hardware drivers that never find a device there are not loaded and unloaded under Cap. A real GPU handed in throughDOCKER_EXTRA --gpuskeeps the full list, and a caller that setsVK_DRIVER_FILESorVK_ICD_FILENAMESitself (the Hyprland OpenGL step hides every driver that way) is left alone, since the loader prefersVK_DRIVER_FILES. The README's harness-only workarounds note covers it.The "Hyprland screenshot" step has failed on every PR since 2026-10-29 (
screenshot_size=1024x768): Arch now ships Hyprland 0.57, which removed its hyprlang config support and readshyprland.luaalone, so the harness'shyprland.conf(1920x1080 monitor, logo off, stdout logs, waybar autostart) was ignored and a default config generated.arch-up.shnow writes the same settings ashyprland.lua.