Skip to content

Make shader registration more modular #42

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Make shader registration more modular #42

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b1b7921
Move shaders into subfolder.
raldone01 Jun 7, 2025
443de13
* Get rid of shared crate.
raldone01 Jun 7, 2025
8fc3f63
Clippy lints and code quality.
raldone01 Jun 7, 2025
511ccfd
Minor consistency change.
raldone01 Jun 7, 2025
994e8db
Use shader trait.
raldone01 Jun 8, 2025
ebbce16
Semantic improvements.
raldone01 Jun 10, 2025
569d75c
Add another shader.
raldone01 Jun 10, 2025
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
2 changes: 1 addition & 1 deletion shaders/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub fn fs(constants: &ShaderConstants, mut frag_coord: Vec2) -> Vec4 {
let shader_input: ShaderInput;
let shader_output = &mut ShaderResult { color: Vec4::ZERO };

if constants.grid == 0 {
if constants.grid_mode == 0 {
shader_input = ShaderInput {
resolution,
time,
Expand Down
10 changes: 7 additions & 3 deletions shaders/src/shared_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ use bytemuck::{Pod, Zeroable};
#[derive(Copy, Clone, Pod, Zeroable)]
#[allow(unused_attributes)]
pub struct ShaderConstants {
/// Boolean value indicating whether all shaders are rendered in a grid layout.
pub grid: u32,
pub width: u32,
pub height: u32,
pub time: f32,

// UI state
/// Boolean value indicating whether all shaders are rendered in a grid layout.
pub grid_mode: u32,
pub shader_to_show: u32,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer enums with u32 reprs if possible.

Copy link
Author

@raldone01 raldone01 Jun 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

#[repr(C, u32)]
#[derive(Copy, Clone, NoUninit, Zeroable)]
pub enum DisplayMode {
  Grid,
  SingleShader(u32),
}

#[repr(C)]
#[derive(Copy, Clone, NoUninit, Zeroable)]
#[allow(unused_attributes)]
pub struct ShaderConstants {
  pub width: u32,
  pub height: u32,
  pub time: f32,

  // UI state
  pub display_mode: DisplayMode,

  // Mouse state.
  pub cursor_x: f32,
  pub cursor_y: f32,
  pub drag_start_x: f32,
  pub drag_start_y: f32,
  pub drag_end_x: f32,
  pub drag_end_y: f32,
  pub mouse_left_pressed: u32,
  pub mouse_left_clicked: u32,
}

Something like this right?
I am not very familiar with bytemuck and it seems to not support enums at first glance.
At least for the Pod guarantee.
Maybe a lesser one would suffice here?
I assume bytemuck is needed to ensure that the struct layout between spirv and cpu matches right?

Copy link
Author

Choose a reason for hiding this comment

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

There is some activity on Lokathor/bytemuck#292. Maybe if it gets merged in the next week this will become possible by using NoUninit instead of Pod. @LegNeato do you know if the Pod guarantees are really necessary? What is important when sharing data with the gpu? How different is the layout of spirv and rust?

Copy link

Choose a reason for hiding this comment

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

I assume bytemuck is needed to ensure that the struct layout between spirv and cpu matches right?

Unlikely given that just having bytemuck traits would/does not guarantee that. Instead it's probably used to allow casting to a &[u8] when copying to a GPU buffer. That would only require NoUninit, not Pod or Zeroable, so would be possible when that PR lands.

Technically, being able to cast to &[u8] is not a requirement to copy data to the GPU safely, but it may practically be a requirement depending on how wgpu exposes interfaces to copy into buffers which I don't know off the top of my head.

Copy link

@fu5ha fu5ha Jun 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

this is the call that makes bytemuck traits needed. Indeed NoUninit is the only requirement for bytemuck::bytes_of, and wgpu does enforce a fully-initialized byte slice here so we do indeed need to ensure NoUninit

Copy link
Author

@raldone01 raldone01 Jun 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Awesome thanks. This is addressed in ebbce16.


// Mouse state.
pub cursor_x: f32,
pub cursor_y: f32,
pub drag_start_x: f32,
Expand All @@ -17,5 +22,4 @@ pub struct ShaderConstants {
pub drag_end_y: f32,
pub mouse_left_pressed: u32,
pub mouse_left_clicked: u32,
pub shader_to_show: u32,
}
4 changes: 3 additions & 1 deletion src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ struct ShaderToyApp {
shader_module: Option<wgpu::ShaderModule>,
close_requested: bool,
start: Instant,

// UI state
grid_mode: bool,
shader_to_show: u32,

// Mouse state.
cursor_x: f32,
cursor_y: f32,
Expand Down Expand Up @@ -250,7 +252,7 @@ impl ShaderToyApp {
mouse_left_pressed: self.mouse_left_pressed as u32,
mouse_left_clicked: self.mouse_left_clicked as u32,
shader_to_show: self.shader_to_show,
grid: self.grid_mode as u32,
grid_mode: self.grid_mode as u32,
};
self.mouse_left_clicked = false;
rpass.set_pipeline(self.render_pipeline.as_ref().unwrap());
Expand Down