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

Jump to bottom

vello should import vello_shaders #467

Closed
PoignardAzur opened this issue Feb 26, 2024 · 1 comment · Fixed by #563
Closed

vello should import vello_shaders #467

PoignardAzur opened this issue Feb 26, 2024 · 1 comment · Fixed by #563
Labels
architecture Changes to the way it's put together

Comments

@PoignardAzur
Copy link
Collaborator

PoignardAzur commented Feb 26, 2024 •
edited
Loading

The same preprocessor code is written twice:

  • In crates/shaders/src/compile/preprocess.rs
  • In src/shaders/preprocess.rs

According to Raph, this is because the vello_shaders crate was meant for Skia integration, and currently isn't used by Vello.

We should fix that and remove the duplication.
@raphlinus raphlinus mentioned this issue Feb 26, 2024
Merged
@armansito
Copy link
Collaborator

We always intended for the main Vello crate to use vello_shaders but never got around to it. The crate supports generating native shaders (such as MSL) from the WGSL which allows external engines to target Metal, Vulkan, etc directly. This code path isn't exercised in the Vello repository but we have wanted to have native runners (i.e. alternative engines alongside wgpu) eventually but decided to put that in the backburner due to other priorities.

I think this issue can be repurposed as "Move to shader management via crates/shaders".

@PoignardAzur PoignardAzur changed the title The preprocess code is duplicated Vello should import vello_shaders Feb 29, 2024
@PoignardAzur PoignardAzur changed the title Vello should import vello_shaders vello should import vello_shaders Feb 29, 2024
@DJMcNab DJMcNab mentioned this issue Mar 1, 2024
Merged
@simbleau simbleau added the architecture Changes to the way it's put together label Mar 4, 2024
@raphlinus raphlinus mentioned this issue Mar 9, 2024
Merged
@DJMcNab DJMcNab mentioned this issue Apr 19, 2024
Merged
@armansito armansito mentioned this issue May 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue May 3, 2024
@armansito
Use vello_shaders crate to load and preprocess WGSL (#563)
e4ca553 
This replaces the shader preprocessing code in the vello crate with
invocations to the vello_shaders crate, which supports both reading from
a directory at runtime (for hot_reload) and bundlng pre-compiled shader
code into the binary (which is suitable for wasm).

This also moves the CPU shader code over to the vello_shaders crate
behind the "cpu" feature.

Resolves #467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
architecture Changes to the way it's put together
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use vello_shaders crate to load and preprocess WGSL linebender/vello
3 participants
@armansito @PoignardAzur @simbleau