gpui: Remove blade, reimplement linux renderer with wgpu#46758
gpui: Remove blade, reimplement linux renderer with wgpu#46758reflectronic merged 27 commits intozed-industries:mainfrom
Conversation
|
|
Thank you for the pull request. Yes, we are interested in this approach. From a brief look at the patch, things look good at a high level. I will make time for a deeper review soon. |
| use metal_renderer as renderer; | ||
|
|
||
| #[cfg(feature = "macos-blade")] | ||
| use crate::platform::blade as renderer; |
There was a problem hiding this comment.
Since WGPU is a cross-platform API, I think it would be nice to be able to use it on Mac and Windows builds via a feature flag, similar to how macos-blade previously enabled the Blade renderer on Mac.
There was a problem hiding this comment.
Agreed, but I can't test that, don't have the necessary hardware
There was a problem hiding this comment.
Since WGPU is a cross-platform API, I think it would be nice to be able to use it on Mac and Windows builds via a feature flag, similar to how
macos-bladepreviously enabled the Blade renderer on Mac.
With Zed moving to wgpu (WebGPU), does this mean a web-based client is now technically possible, similar to Flutter Web?
There was a problem hiding this comment.
@bamboo512 There is significant work beyond the renderer that would need to happen to run Zed in a browser - notably background tasks and filesystem/input APIs would need web/wasm-compatible implementations.
There was a problem hiding this comment.
True, I do think that WASI as a minimum feature-set would ease that work
| // Safety: The caller guarantees that the window handle is valid for the | ||
| // lifetime of this renderer. In practice, the RawWindow struct is created | ||
| // from the native window handles and the surface is dropped before the window. | ||
| let surface = unsafe { |
There was a problem hiding this comment.
Is it worth using SurfaceTargetUnsafe::from_window?
let surface = unsafe {
context
.instance
.create_surface_unsafe(
wgpu::SurfaceTargetUnsafe::from_window(window)
.map_err(|e| anyhow::anyhow!("Failed to create surface target: {e}"))?,
)
.map_err(|e| anyhow::anyhow!("Failed to create surface: {e}"))?
};There was a problem hiding this comment.
To be honest I think this is "unnecessary complexity", since, as far as I understand, in the current implementation we would still wrap the underlying error. The only difference with this is the separate wrapper message, but the underlying error should make it very clear which error path was hit anyway, but please correct me if I'm wrong.
There was a problem hiding this comment.
We wouldn't gain much by using their function over manually doing it ourselves, and it's a fair argument that the error messages in the more manual case are less layered. It's probably not important in practice to see directly in the error that we failed to get a window or display handle while creating a surface target, because you are going to see that context by virtue of where the error occurs (in the creation of the renderer).
|
Will there be plans to use wgpu for the Windows and macOS platforms in the future? |
|
I don't think we plan to replace the Windows or macOS renderers at this time. Our native renderers on those platforms probably have better performance and wider compatibility than WGPU. |
Not super familiar with CF, metal and the macos ecosystem, but why do you think using metal directly results in wider compatibility? Does it not just use metal on macos anyway? Just curious, not trying to push that change. I know there are also people relying on CF stuff for e.g. hardware accelerated video decoding etc. |
|
Well, I'm not sure if it's true on macOS, sorry. On Windows, I believe we support some GPUs that don't have DirectX 12. |
|
Again, not trying to push anything, but wouldn't wgpu not increase the range of supported devices, specifically on windows, given the different backends it supports? On windows it supports vulkan, DirectX12 directly, and also has a ANGLE based backend, that, if I understand correctly, would support OpenGL ES and DirectX9 and up. Please correct me if I'm wrong, again, not super familiar with any platforms other than linux, but I'd assume that using wgpu would pretty much maximize device compatibility with minimum effort, with even WebGPU or WebGL becoming a possibility... |
|
WGPU consumes more memory than gpui's custom renderer. On my Windows 11 machine, launching an empty window with WGPU uses around 100MB of memory (I used the official WGPU example), while an identical empty window with gpui only takes up about 10MB. |
Probably a fair argument, but would be interesting to see how big the difference is once you actually render anything significant. There are also some ways to reduce initial memory usage with wgpu (see e.g. MemoryHints), I believe by default wgpu pre-allocates relatively big buffers. Probably will still use more than a manual DX12 implementation.. |
|
Hm, right, I suppose OpenGL ES will have broad compatibility (likely even broader than what we target for D3D11). I'm not sure if WGPU is using ANGLE or native OpenGL drivers for that. In general, the native drivers for Khronos APIs are somewhat buggier and have higher latency (DirectX has special presentation paths through the composition engine) than D3D. ANGLE might be OK, but our workload is simple enough that, in my opinion, it's worth having a DirectX 11 renderer that avoids the need for multiple translation layers. |
|
This PR causes a significant regression in VRAM usage for me. Here's 
Here's 
|
|
@zeux Wouldn't call 30MiB "significant regression", but anyway, I can't reproduce this. On my system, zed built from commit this branch is based on sits at 542M, latest commit on this PR settles at 537M, slightly better (4k resolution, same window size, project, open files etc.). This is also similar for my zlaunch launcher using gpui, which also actually seems to sit at slightly less memory usage with the wgpu implementation. We could set Memory hints to prefer smaller buffers over performance, but I don't think that will make much difference in practice. The wgpu renderer is essentially a port of the blade implementation, so except some differences in wgpu's buffer allocation I don't expect any significant difference. |
|
@zortax Re: 30 MB difference - you are looking at the wrong column in my screenshots; the rightmost column is "host memory" which I'm not interested in; the 6th column accounts for GPU memory allocation and that is where the difference is significant for me. |
|
How to disable this? after update editor constantly freezes on Radeon 780M Graphic (Framework Laptop) |
Can you provide some debug logs? I've tested here in a rx 6600 and it's running fine. Also check if you are using the latest mesa version |
|
@jlucaso1 Could you please advice me how to collect them? There's nothing interesting in `~/.local/share/zed/logs/Zed.log`, and regarding profiling, the documentation only mentions macOS. https://zed.dev/docs/troubleshooting#performance-issues-profiling Mesa version on my host $ rpm -q mesa-libGL
mesa-libGL-25.1.9-1.fc42.x86\_64
mesa-libGL-25.1.9-1.fc42.i686 |
The blade graphics library is a mess and causes several issues for both Zed users as well as other 3rd party apps using GPUI. This PR removes blade and implements the linux platform using
wgpuwhich is the de-facto standard in the rust UI and graphics ecosystem. This will not just fix issues that Zed users have today, but also profit from wgpu improvements in the futures, from other projects contributing (such as the bevy game engine, Iced, or pretty much every other relevant project).This will close several related issues on the zed repo as well. See https://github.com/zed-industries/zed/issues?q=frozen%20nvidia%20linux (probably not all of them, have only tested the freeze on nvidia and Smithay-based wayland compositors).
Some related issues:
#44814
#40481
niri-wm/niri#2335
zortax/zlaunch#15
Would appreciate feedback if this is something the zed maintainers would be interested in.
Release Notes: