Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gpui: Shadows look different on Blade and Metal #22711

Open
1 task done
iamnbutler opened this issue Jan 6, 2025 · 3 comments
Open
1 task done

gpui: Shadows look different on Blade and Metal #22711

iamnbutler opened this issue Jan 6, 2025 · 3 comments
Labels
admin read Pending admin review gpui GPUI rendering framework support linux macOS Platform-specific feedback for macOS behaviors, features, design, etc triage Maintainer needs to classify the issue

Comments

@iamnbutler
Copy link
Member

Check for existing issues

  • Completed

Describe the feature

While working on #22441 I noticed that the shadow rendering between blade and metal are pretty different. We should try to bring these a bit closer into alignment.

image

We generally design things from the perspective of the metal renderer on macOS, so we should try to bring the blade rendering closer into alignment to that of the metal.

Reference:
#22441 (comment)

Zed Version and System Specs

N/A

If applicable, add mockups / screenshots to help present your vision of the feature

N/A

@iamnbutler iamnbutler added the gpui GPUI rendering framework support label Jan 6, 2025
@iamnbutler iamnbutler changed the title gpui: Blade and Metal render shadows vastly differently gpui: Shadows look different on Blade and Metal Jan 6, 2025
@jansol
Copy link
Contributor

jansol commented Jan 6, 2025

My first guess would be that blade and metal do blending in different colorspaces (nonlinear sRGB vs linear sRGB). Specifically the big question is whether alpha is treated as linear or nonlinear. This was briefly a very noticeable problem in KDE Plasma when HDR was enabled -- alpha was being treated as linear, but most applications were written with the expectation of it being nonlinear (which is awful mathematically but visually ends up being more in line with what people expect), see https://bugs.kde.org/show_bug.cgi?id=476868

@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Jan 7, 2025
@jansol
Copy link
Contributor

jansol commented Jan 8, 2025

Oh, it occurred to me that if this is an issue with the blending gamma / colorspace, resolving it may also go a long way towards improving the muddy appearance of fonts on Linux, since they would be affected in that case too. cf #7992

@apricotbucket28
Copy link
Contributor

Tried this with and without #11534

Linear (now) sRGB (before)
linear srgb

Diff:

diff --git a/crates/gpui/src/platform/blade/blade_atlas.rs b/crates/gpui/src/platform/blade/blade_atlas.rs
index fb703f2a41..b005cf42b0 100644
--- a/crates/gpui/src/platform/blade/blade_atlas.rs
+++ b/crates/gpui/src/platform/blade/blade_atlas.rs
@@ -193,7 +193,7 @@ impl BladeAtlasState {
                 usage = gpu::TextureUsage::COPY | gpu::TextureUsage::RESOURCE;
             }
             AtlasTextureKind::Polychrome => {
-                format = gpu::TextureFormat::Bgra8UnormSrgb;
+                format = gpu::TextureFormat::Bgra8Unorm;
                 usage = gpu::TextureUsage::COPY | gpu::TextureUsage::RESOURCE;
             }
             AtlasTextureKind::Path => {
diff --git a/crates/gpui/src/platform/blade/blade_renderer.rs b/crates/gpui/src/platform/blade/blade_renderer.rs
index ee8ffdfda7..532ddcaaf4 100644
--- a/crates/gpui/src/platform/blade/blade_renderer.rs
+++ b/crates/gpui/src/platform/blade/blade_renderer.rs
@@ -329,7 +329,7 @@ impl BladeRenderer {
             size: config.size,
             usage: gpu::TextureUsage::TARGET,
             display_sync: gpu::DisplaySync::Recent,
-            color_space: gpu::ColorSpace::Linear,
+            color_space: gpu::ColorSpace::Srgb,
             allow_exclusive_full_screen: false,
             transparent: config.transparent,
         };
diff --git a/crates/gpui/src/platform/blade/shaders.wgsl b/crates/gpui/src/platform/blade/shaders.wgsl
index d497c40d7a..f4c4fd0709 100644
--- a/crates/gpui/src/platform/blade/shaders.wgsl
+++ b/crates/gpui/src/platform/blade/shaders.wgsl
@@ -145,7 +145,7 @@ fn hsla_to_rgba(hsla: Hsla) -> vec4<f32> {
     let c = (1.0 - abs(2.0 * l - 1.0)) * s;
     let x = c * (1.0 - abs(h % 2.0 - 1.0));
     let m = l - c / 2.0;
-    var color = vec3<f32>(m);
+    var color = vec4<f32>(m, m, m, a);
 
     if (h >= 0.0 && h < 1.0) {
         color.r += c;
@@ -167,12 +167,7 @@ fn hsla_to_rgba(hsla: Hsla) -> vec4<f32> {
         color.b += x;
     }
 
-    // Input colors are assumed to be in sRGB space,
-    // but blending and rendering needs to happen in linear space.
-    // The output will be converted to sRGB by either the target
-    // texture format or the swapchain color space.
-    let linear = srgb_to_linear(color);
-    return vec4<f32>(linear, a);
+    return color;
 }
 
 /// Convert a linear sRGB to Oklab space.

@JosephTLyons JosephTLyons removed the admin read Pending admin review label Jan 8, 2025
@github-actions github-actions bot added the admin read Pending admin review label Jan 9, 2025
@JosephTLyons JosephTLyons added macOS Platform-specific feedback for macOS behaviors, features, design, etc linux and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jan 9, 2025
@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin read Pending admin review gpui GPUI rendering framework support linux macOS Platform-specific feedback for macOS behaviors, features, design, etc triage Maintainer needs to classify the issue
Projects
Status: No status
Development

No branches or pull requests

4 participants