improve: add blur and pixelation mask effects#2012
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
|
||
| return color / weight_sum; | ||
| } | ||
|
|
There was a problem hiding this comment.
rect_mask() extends beyond rect_size by feather, so mode 3u can sample from the horizontal pass in that feather region too. Might be worth expanding the Y-support by uniforms.feather to avoid reintroducing partial-separable blur right at the feathered top/bottom edge.
| return delta.x <= half_size.x && delta.y <= half_size.y + vertical_radius + uniforms.feather; |
|
hey @greptileai, please re-review the PR |
| <RadioGroup | ||
| class="grid grid-cols-2 gap-2" | ||
| value={maskEffect()} | ||
| onChange={(value) => setMaskEffect(value as MaskEffect)} |
There was a problem hiding this comment.
Minor type-safety: it’d be nice to avoid the as MaskEffect cast here since RadioGroup can usually emit arbitrary strings.
| onChange={(value) => setMaskEffect(value as MaskEffect)} | |
| onChange={(value) => { | |
| if (value === "blur" || value === "pixelate") { | |
| setMaskEffect(value); | |
| } | |
| }} |
| @@ -1,6 +1,16 @@ | |||
| import type { XY } from "~/utils/tauri"; | |||
| import maskEffectContract from "../../../../../crates/project/mask-effects.json"; | |||
There was a problem hiding this comment.
Heads up: importing mask-effects.json from crates/ via a deep relative path can be a little fragile in Vite/Tauri setups (dev server fs allow / build sandboxing). If this ever becomes annoying, consider surfacing the contract via a small TS module/package (or generating it) so the desktop app doesn’t need to reach outside apps/desktop.
|
|
||
| for (var index = -12; index <= 12; index++) { | ||
| let distance = f32(index) / 4.0; | ||
| let weight = exp(-0.5 * distance * distance); |
There was a problem hiding this comment.
Perf nit: exp() inside the 25-tap blur loop is going to be pretty expensive per-fragment (and this runs twice for blur). If you see this show up in profiles, you can precompute the 25 Gaussian weights as const values (and a const weight sum) and just index into them in the loop.
Summary
Validation
Greptile Summary
This PR replaces the opacity-based sensitive mask intensity control with dedicated blur and pixelation effect modes. A shared
mask-effects.jsoncontract file is introduced as the single source of truth for encoding constants, consumed by both TypeScript (viaimport) and Rust (viainclude_str!+serde_json), directly addressing the constants-duplication concern raised in previous review threads.pixelation = 1000 + amount; legacy pixelation values stay as raw amounts. Both TS and Rust decode this identically from the JSON contract.REPLACEblend; vertical pass reads from that buffer and composites back withSrcAlpha/OneMinusSrcAlphablend, correctly feathering the result over the original frame.1.0ininterpolate_masks, so old projects with low-opacity sensitive masks are upgraded to fully opaque effects.Confidence Score: 5/5
Safe to merge — the change is well-tested on both the TypeScript and Rust sides, and the JSON contract file cleanly eliminates the encoding drift risk that was the main concern in previous review rounds.
The encoding scheme is sound: the blur offset (1000) is far above any pixelation value the old UI could have produced (max 80), so legacy projects deserialize cleanly. The two-pass blur correctly avoids a texture swap by writing the composite result back to current_texture_view() in pass 2, while render_single_pass still swaps for pixelate/highlight. Switching pixelation to textureLoad sidesteps linear filtering artifacts. All behavior is backed by dedicated Rust and TypeScript tests.
No files require special attention.
Important Files Changed
Reviews (4): Last reviewed commit: "fix: make mask render dispatch explicit" | Re-trigger Greptile