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

Simplify egui wgpu fragment shader, remove redundant color conversions #3168

Open
PPakalns opened this issue Jul 20, 2023 · 2 comments
Open

Comments

@PPakalns
Copy link
Contributor

PPakalns commented Jul 20, 2023

By default on desktop gpu devices egui_wgpu / wgpu outputs preferred target format wgpu::TextureFormat::Bgra8Unorm.

Then for rendering it renders to this texture in gamma space (srgb ?)

https://github.com/emilk/egui/blob/master/crates/egui-wgpu/src/egui.wgsl#L84C1-L91C2

@fragment
fn fs_main_gamma_framebuffer(in: VertexOutput) -> @location(0) vec4<f32> {
    // We always have an sRGB aware texture at the moment.
    let tex_linear = textureSample(r_tex_color, r_tex_sampler, in.tex_coord);
    let tex_gamma = gamma_from_linear_rgba(tex_linear);
    let out_color_gamma = in.color * tex_gamma;
    return out_color_gamma;
}

As i understand it happens like this:

  • Texture is in Bgra8UnormSrgb ,
  • texture view for this texture is in Bgra8UnormSrgb,
  • Target texture format is in Bgra8Unorm,
  • This causes in the shader automatically Srgb values to be converted to linear values.
  • Then in the shader rgb value is converted back to srgb value.
  • Then srgb value is outputted to Bgra8Unorm texture instead of Bgra8UnormSrgb. (Because device expects srgb value outputted to it, (intuition would tell that this expects linear rgb value))

Similar thing happens for user registered textures.
I need to provide view to srgb texture with Bgra8UnormSrgb format so that shader receives values in linear format.

It looks like it causes double conversion from srgb -> rgb -> srgb. Couldn't this be avoided by only storing textures in srgb format and using everywhere Bgra8Unorm texture view format view which doesn't cause conversion, therefore no need to convert from linear to gamma space in fragment shader.

As I understand, texture formats and their views are only for automatic color conversion in shader and doesn't represent the real target texture color format. Is this correct understanding?
At least it looks like wgpu preferred target texture is in Bgra8Unorm format but expects textures in gamma (srgb) outputted to it.

Summary:

It looks that we can simplify fragment shader:

Possible problems

One case where this doesn't work is if someone wants to mix textures that are stored in bgr, srgb color spaces.
What are the best practices, are these color conversions in shader costly?

@PPakalns
Copy link
Contributor Author

Good description about gpu color conversion:
https://github.com/gfx-rs/wgpu/wiki/Texture-Color-Formats-and-Srgb-conversions

@Wumpf
Copy link
Collaborator

Wumpf commented Jul 26, 2023

arguably there needs to be 4 combination:

  • fs_main_linear_framebuffer_8UnormSrgb_texture (equal to today's fs_main_linear_framebuffer_8UnormSrgb_texture)
  • fs_main_linear_framebuffer_8Unorm_gamma_texture (has conversion only at return)
  • fs_main_gamma_framebuffer_8UnormSrgb_texture (equal to today's fs_main_gamma_framebuffer)
  • fs_main_gamma_framebuffer_8Unorm_gamma_texture (has no conversion!)

This is only that way egui can deal with incoming textures (which are at least right now to be expected to be 8UnormSrgb) in the correct way. This wouldn't give us removing any of the color conversion code but it would allow egui_wgpu to choose the path with the least conversions for textures it created itself.

Any of these conversions is fairly cheap in the grad scheme of things so I wouldn't worry about it too much. One could easily argue that all this would just unnecessarily bloat and complicate things

Btw. the reason why egui prefers non-gamma-aware framebuffers and does blending in gamma space is detailed here #2071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants