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.

Sign up for GitHub

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

Maybe sharper egui soon #216

Closed
wants to merge 3 commits into from
Closed

Maybe sharper egui soon #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d669481
per-texture sampler for correct image options
EriKWDev Dec 7, 2024
4cb23bd
Make blade-egui work the same as egui-wgpu in regards to srgb and dit…
EriKWDev Dec 7, 2024
e3e04ac
Mipmap options + update examples using egui to include options in cre…
EriKWDev Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ profiling = "1"
slab = "0.4"
strum = { version = "0.25", features = ["derive"] }
web-sys = "0.3.60"
winit = "0.30"
winit = { version="0.30", default-features = false, features=["rwh_06", "x11"]}

[lib]

Expand Down Expand Up @@ -89,8 +89,7 @@ del-msh-core = "=0.1.33"
del-geo = "=0.1.29"

[target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies]
# see https://github.com/emilk/egui/issues/4270
egui-winit = "0.29"
egui-winit = { version="0.29", deafult-features=false, features=["links"] }

[target.'cfg(target_arch = "wasm32")'.dev-dependencies]
console_error_panic_hook = "0.1.7"
Expand Down
112 changes: 99 additions & 13 deletions blade-egui/shader.wgsl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// NOTE: Borrows heavily from egui-wgpu:s wgsl shaders, used here under the MIT license

struct VertexOutput {
@location(0) tex_coord: vec2<f32>,
@location(1) color: vec4<f32>,
Expand All @@ -6,8 +8,8 @@ struct VertexOutput {

struct Uniforms {
screen_size: vec2<f32>,
convert_to_linear: f32,
padding: f32,
dithering: u32,
padding: u32,
};
var<uniform> r_uniforms: Uniforms;

Expand All @@ -21,22 +23,16 @@ struct Vertex {
}
var<storage, read> r_vertex_data: array<Vertex>;

fn linear_from_srgb(srgb: vec3<f32>) -> vec3<f32> {
let cutoff = srgb < vec3<f32>(10.31475);
let lower = srgb / vec3<f32>(3294.6);
let higher = pow((srgb + vec3<f32>(14.025)) / vec3<f32>(269.025), vec3<f32>(2.4));
return select(higher, lower, cutoff);
}

@vertex
fn vs_main(
@builtin(vertex_index) v_index: u32,
) -> VertexOutput {
let input = r_vertex_data[v_index];
var out: VertexOutput;
out.tex_coord = vec2<f32>(input.tex_coord_x, input.tex_coord_y);
let color = unpack4x8unorm(input.color);
out.color = vec4<f32>(pow(color.xyz, vec3<f32>(2.2)), color.a);
// let color = unpack4x8unorm(input.color);
let color = unpack_color(input.color);
out.color = color;
out.position = vec4<f32>(
2.0 * input.pos_x / r_uniforms.screen_size.x - 1.0,
1.0 - 2.0 * input.pos_y / r_uniforms.screen_size.y,
Expand All @@ -50,6 +46,96 @@ var r_texture: texture_2d<f32>;
var r_sampler: sampler;

@fragment
fn fs_main(in: VertexOutput) -> @location(0) vec4<f32> {
return in.color * textureSample(r_texture, r_sampler, in.tex_coord);
fn fs_main_linear_framebuffer(in: VertexOutput) -> @location(0) vec4<f32> {
// We always have an sRGB aware texture at the moment.
let tex_linear = textureSample(r_texture, r_sampler, in.tex_coord);
let tex_gamma = gamma_from_linear_rgba(tex_linear);
var out_color_gamma = in.color * tex_gamma;
// Dither the float color down to eight bits to reduce banding.
// This step is optional for egui backends.
// Note that dithering is performed on the gamma encoded values,
// because this function is used together with a srgb converting target.
if r_uniforms.dithering == 1 {
let out_color_gamma_rgb = dither_interleaved(out_color_gamma.rgb, 256.0, in.position);
out_color_gamma = vec4<f32>(out_color_gamma_rgb, out_color_gamma.a);
}
let out_color_linear = linear_from_gamma_rgb(out_color_gamma.rgb);
return vec4<f32>(out_color_linear, out_color_gamma.a);
}

@fragment
fn fs_main_gamma_framebuffer(in: VertexOutput) -> @location(0) vec4<f32> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need a separate entry point? it's largely doing the same thing, we might as well just have an if/else at the end based on a runtime flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Runtime flag can do it. Just replicated egui-wgpu for the quick draft.

It would be nice to find the underlying cause so the shader potentially can be simple again..

// We always have an sRGB aware texture at the moment.
let tex_linear = textureSample(r_texture, r_sampler, in.tex_coord);
let tex_gamma = gamma_from_linear_rgba(tex_linear);
var out_color_gamma = in.color * tex_gamma;
// Dither the float color down to eight bits to reduce banding.
// This step is optional for egui backends.
if r_uniforms.dithering == 1 {
let out_color_gamma_rgb = dither_interleaved(out_color_gamma.rgb, 256.0, in.position);
out_color_gamma = vec4<f32>(out_color_gamma_rgb, out_color_gamma.a);
}
return out_color_gamma;
}


// -----------------------------------------------
// Adapted from
// https://www.shadertoy.com/view/llVGzG
// Originally presented in:
// Jimenez 2014, "Next Generation Post-Processing in Call of Duty"
//
// A good overview can be found in
// https://blog.demofox.org/2022/01/01/interleaved-gradient-noise-a-different-kind-of-low-discrepancy-sequence/
// via https://github.com/rerun-io/rerun/
fn interleaved_gradient_noise(n: vec2<f32>) -> f32 {
let f = 0.06711056 * n.x + 0.00583715 * n.y;
return fract(52.9829189 * fract(f));
}

fn dither_interleaved(rgb: vec3<f32>, levels: f32, frag_coord: vec4<f32>) -> vec3<f32> {
var noise = interleaved_gradient_noise(frag_coord.xy);
// scale down the noise slightly to ensure flat colors aren't getting dithered
noise = (noise - 0.5) * 0.95;
return rgb + noise / (levels - 1.0);
}

// 0-1 linear from 0-1 sRGB gamma
fn linear_from_gamma_rgb(srgb: vec3<f32>) -> vec3<f32> {
let cutoff = srgb < vec3<f32>(0.04045);
let lower = srgb / vec3<f32>(12.92);
let higher = pow((srgb + vec3<f32>(0.055)) / vec3<f32>(1.055), vec3<f32>(2.4));
return select(higher, lower, cutoff);
}

// 0-1 sRGB gamma from 0-1 linear
fn gamma_from_linear_rgb(rgb: vec3<f32>) -> vec3<f32> {
let cutoff = rgb < vec3<f32>(0.0031308);
let lower = rgb * vec3<f32>(12.92);
let higher = vec3<f32>(1.055) * pow(rgb, vec3<f32>(1.0 / 2.4)) - vec3<f32>(0.055);
return select(higher, lower, cutoff);
}

// 0-1 sRGBA gamma from 0-1 linear
fn gamma_from_linear_rgba(linear_rgba: vec4<f32>) -> vec4<f32> {
return vec4<f32>(gamma_from_linear_rgb(linear_rgba.rgb), linear_rgba.a);
}

// [u8; 4] SRGB as u32 -> [r, g, b, a] in 0.-1
fn unpack_color(color: u32) -> vec4<f32> {
EriKWDev marked this conversation as resolved.
Show resolved Hide resolved
return vec4<f32>(
f32(color & 255u),
f32((color >> 8u) & 255u),
f32((color >> 16u) & 255u),
f32((color >> 24u) & 255u),
) / 255.0;
}

fn position_from_screen(screen_pos: vec2<f32>) -> vec4<f32> {
return vec4<f32>(
2.0 * screen_pos.x / r_uniforms.screen_size.x - 1.0,
1.0 - 2.0 * screen_pos.y / r_uniforms.screen_size.y,
0.0,
1.0,
);
}
100 changes: 79 additions & 21 deletions blade-egui/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,19 +26,20 @@ use std::{
#[derive(Clone, Copy, bytemuck::Zeroable, bytemuck::Pod)]
struct Uniforms {
screen_size: [f32; 2],
padding: [f32; 2],
dithering: u32,
padding: u32,
}

#[derive(blade_macros::ShaderData)]
struct Globals {
r_uniforms: Uniforms,
r_sampler: blade_graphics::Sampler,
}

#[derive(blade_macros::ShaderData)]
struct Locals {
r_vertex_data: blade_graphics::BufferPiece,
r_texture: blade_graphics::TextureView,
r_sampler: blade_graphics::Sampler,
}

#[derive(Debug, PartialEq)]
Expand All @@ -58,10 +59,16 @@ impl ScreenDescriptor {
struct GuiTexture {
allocation: blade_graphics::Texture,
view: blade_graphics::TextureView,
sampler: blade_graphics::Sampler,
}

impl GuiTexture {
fn create(context: &blade_graphics::Context, name: &str, size: blade_graphics::Extent) -> Self {
fn create(
context: &blade_graphics::Context,
name: &str,
size: blade_graphics::Extent,
options: egui::TextureOptions,
) -> Self {
let format = blade_graphics::TextureFormat::Rgba8UnormSrgb;
let allocation = context.create_texture(blade_graphics::TextureDesc {
name,
Expand All @@ -81,12 +88,62 @@ impl GuiTexture {
subresources: &blade_graphics::TextureSubresources::default(),
},
);
Self { allocation, view }

let sampler = context.create_sampler(blade_graphics::SamplerDesc {
name,
address_modes: {
let mode = match options.wrap_mode {
egui::TextureWrapMode::ClampToEdge => blade_graphics::AddressMode::ClampToEdge,
egui::TextureWrapMode::Repeat => blade_graphics::AddressMode::Repeat,
egui::TextureWrapMode::MirroredRepeat => {
blade_graphics::AddressMode::MirrorRepeat
}
};
[mode; 3]
},
mag_filter: match options.magnification {
egui::TextureFilter::Nearest => blade_graphics::FilterMode::Nearest,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should have helper function to do this mapping, since we are doing it 3 times here

egui::TextureFilter::Linear => blade_graphics::FilterMode::Linear,
},
min_filter: match options.minification {
egui::TextureFilter::Nearest => blade_graphics::FilterMode::Nearest,
egui::TextureFilter::Linear => blade_graphics::FilterMode::Linear,
},
mipmap_filter: match options.mipmap_mode {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would look like options.mipmap_mode.map_or(blade_graphics::FilterMode::Linear, my_mapping_function)

Some(it) => match it {
egui::TextureFilter::Nearest => blade_graphics::FilterMode::Nearest,
egui::TextureFilter::Linear => blade_graphics::FilterMode::Linear,
},
None => blade_graphics::FilterMode::Linear,
},
..Default::default()
});

Self {
allocation,
view,
sampler,
}
}

fn delete(self, context: &blade_graphics::Context) {
context.destroy_texture(self.allocation);
context.destroy_texture_view(self.view);
context.destroy_sampler(self.sampler);
}
}

#[derive(Clone, Copy)]
pub struct GuiPainterOptions {
/// Controls whether to apply dithering to minimize banding artifacts, same as egui-wgpu
///
/// Defaults to true.
pub dithering: bool,
}

impl Default for GuiPainterOptions {
fn default() -> Self {
Self { dithering: true }
}
}

Expand All @@ -103,7 +160,7 @@ pub struct GuiPainter {
//TODO: this could also look better
textures_dropped: Vec<GuiTexture>,
textures_to_delete: Vec<(GuiTexture, blade_graphics::SyncPoint)>,
sampler: blade_graphics::Sampler,
options: GuiPainterOptions,
}

impl GuiPainter {
Expand All @@ -120,15 +177,18 @@ impl GuiPainter {
for (gui_texture, _) in self.textures_to_delete.drain(..) {
gui_texture.delete(context);
}
context.destroy_sampler(self.sampler);
}

/// Create a new painter with a given GPU context.
///
/// It supports renderpasses with only a color attachment,
/// and this attachment format must be The `output_format`.
#[profiling::function]
pub fn new(info: blade_graphics::SurfaceInfo, context: &blade_graphics::Context) -> Self {
pub fn new(
info: blade_graphics::SurfaceInfo,
context: &blade_graphics::Context,
options: GuiPainterOptions,
) -> Self {
let shader = context.create_shader(blade_graphics::ShaderDesc {
source: SHADER_SOURCE,
});
Expand All @@ -144,7 +204,11 @@ impl GuiPainter {
..Default::default()
},
depth_stencil: None, //TODO?
fragment: shader.at("fs_main"),
fragment: if info.format.is_srgb() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks highly suspicious to me.
Generally speaking, I expect the rendering pipeline to assume linear space render targets all the way. I don't think there is value in expanding this assumption any further.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what egui-wgpu did so I just replicated all of it to see if it would help.

Egui examples use a non-srg render target whereas blade always uses one

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimizing changes is a good experiment for sure. Shipping it the way egui_wgpu did seems very concerning to me. I'll try to find some associated discussion there.

shader.at("fs_main_linear_framebuffer")
} else {
shader.at("fs_main_gamma_framebuffer")
},
color_targets: &[blade_graphics::ColorTargetState {
format: info.format,
blend: Some(blade_graphics::BlendState::ALPHA_BLENDING),
Expand All @@ -158,21 +222,13 @@ impl GuiPainter {
alignment: 4,
});

let sampler = context.create_sampler(blade_graphics::SamplerDesc {
name: "gui",
address_modes: [blade_graphics::AddressMode::ClampToEdge; 3],
mag_filter: blade_graphics::FilterMode::Linear,
min_filter: blade_graphics::FilterMode::Linear,
..Default::default()
});

Self {
pipeline,
belt,
textures: Default::default(),
textures_dropped: Vec::new(),
textures_to_delete: Vec::new(),
sampler,
options,
}
}

Expand Down Expand Up @@ -239,15 +295,16 @@ impl GuiPainter {
let texture = match self.textures.entry(texture_id) {
Entry::Occupied(mut o) => {
if image_delta.pos.is_none() {
let texture = GuiTexture::create(context, &label, extent);
let texture =
GuiTexture::create(context, &label, extent, image_delta.options);
command_encoder.init_texture(texture.allocation);
let old = o.insert(texture);
self.textures_dropped.push(old);
}
o.into_mut()
}
Entry::Vacant(v) => {
let texture = GuiTexture::create(context, &label, extent);
let texture = GuiTexture::create(context, &label, extent, image_delta.options);
command_encoder.init_texture(texture.allocation);
v.insert(texture)
}
Expand Down Expand Up @@ -296,9 +353,9 @@ impl GuiPainter {
&Globals {
r_uniforms: Uniforms {
screen_size: [logical_size.0, logical_size.1],
padding: [0.0; 2],
dithering: if self.options.dithering { 1 } else { 0 },
padding: 0,
},
r_sampler: self.sampler,
},
);

Expand Down Expand Up @@ -340,6 +397,7 @@ impl GuiPainter {
&Locals {
r_vertex_data: vertex_buf,
r_texture: texture.view,
r_sampler: texture.sampler,
},
);

Expand Down
Loading