Repository navigation
perf(desktop-gpui): blur the glass backdrop in render passes on macOS - #3512
Conversation
… on macOS instead of a blit and MPS
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| + for (int offset = -gaussian.radius; offset <= gaussian.radius; offset++) { | ||
| + int2 at = clamp(center + gaussian.direction * offset, int2(0), last); | ||
| + float weight = exp(float(offset * offset) * falloff); | ||
| + sum += weight * source.read(uint2(at)); | ||
| + total += weight; | ||
| + } | ||
| + return sum / total; |
There was a problem hiding this comment.
backdrop_gaussian_fragment recalculates the same weights and their total for every pixel in both passes. At sigma = 20, that means 121 exp calls per pixel per pass, even though the weights depend only on sigma. This adds GPU work as panes and blur radii grow.
Precompute the normalized weights once per blur radius and pass them to the shader. That preserves the exact blur without repeating these calculations for every pixel.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/patches/zed-glass.patch
Line: 848-854
Comment:
**Blur weights repeat per pixel**
`backdrop_gaussian_fragment` recalculates the same weights and their total for every pixel in both passes. At `sigma = 20`, that means 121 `exp` calls per pixel per pass, even though the weights depend only on `sigma`. This adds GPU work as panes and blur radii grow.
Precompute the normalized weights once per blur radius and pass them to the shader. That preserves the exact blur without repeating these calculations for every pixel.
---
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!
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
|
@greptileai addressed in c461899: the normalised weights are computed once per blur on the CPU and passed as fragment bytes; the shader reads them and sums symmetric taps. The Metal backdrop blur tests (including the CPU-model match) pass. Please re-review. |
| + backdrop_copy_pipeline_state: metal::RenderPipelineState, | ||
| + /// The gaussian's horizontal pass, into the half-float `backdrop_temp`. | ||
| + backdrop_gaussian_wide_pipeline_state: metal::RenderPipelineState, | ||
| + /// The vertical pass, into `backdrop_blurred`. |
There was a problem hiding this comment.
The comment on backdrop_gaussian_pipeline_state only restates which pass it holds. The comment above backdrop_pass_vertex similarly describes the triangle the code draws. AGENTS.md explicitly bans comments that narrate the code rather than explain non-obvious context. Remove these comments to satisfy the repository requirement before merging.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/patches/zed-glass.patch
Line: 210
Comment:
**Comments repeat the code**
The comment on `backdrop_gaussian_pipeline_state` only restates which pass it holds. The comment above `backdrop_pass_vertex` similarly describes the triangle the code draws. `AGENTS.md` explicitly bans comments that narrate the code rather than explain non-obvious context. Remove these comments to satisfy the repository requirement before merging.
**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!
|
@greptileai removed both comments in 155bf68; please re-review. |
Summary
On macOS, each glass pane copied the area beneath it out of the drawable with a blit, then blurred it with
MPSImageGaussianBlur, a compute pass. Measured on the Mac mini, any blit or compute pass between a frame's render passes made the window hold a block of graphics memory: in the area picker, glass cost 48 MB of graphics memory more than without it, and almost all of that disappeared when the copy and blur ran as render passes instead.The Metal renderer (
zed-glass.patch) now does both in render passes, as the Direct3D and wgpu renderers already do:framebufferOnly = false) so a shader can read them; that costs no memory and makes the old blit from the drawable valid too.ceil(3 sigma), normalised weights). The pass between them is stored in half floats so the vertical pass reads it unrounded.MPSImageGaussianBlurand the MetalPerformanceShaders link are gone; nothing else used them.Measurements
Mac mini (M-series, 1080p display), release build, glass on against glass off in the same binary, two runs per state, CPU as a share of one core, memory as the process footprint:
Quality
gpui_macosbackdrop blur tests pass, includingglass_matches_the_cpu_modelandblur_flush_against_window_edge_has_no_vignette.Test plan
cargo test -p gpui_macos --features runtime_shaders --lib backdrop_blur_tests
The PR appears safe to merge; both previous findings are fixed.Summary
The PR moves macOS glass copying and blur work into render passes and computes blur weights once per pane.
Reviews (3) · Last reviewed commit: "chore(desktop-gpui): drop two comments t..." · Reviewed by Greptile