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

Shader hot reloading no longer works for locally imported shaders #10500

Closed
DGriffin91 opened this issue Nov 10, 2023 · 4 comments · Fixed by #10502
Closed

Shader hot reloading no longer works for locally imported shaders #10500

DGriffin91 opened this issue Nov 10, 2023 · 4 comments · Fixed by #10502
Labels
C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled

Comments

@DGriffin91
Copy link
Contributor

Bevy version 0.12

In 0.11 it was possible to hot reload locally imported shaders. This no longer works in 0.12.

#import "shaders/bar.wgsl"::bar
[...]
let a = bar(frag_coord);

If I try to edit shaders/bar.wgsl I get:

2023-11-10T21:55:48.695089Z  INFO bevy_asset::server: Reloading shaders\bar.wgsl because it has changed
2023-11-10T21:55:48.707801Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: required import '"shaders/bar.wgsl"' not found
   ┌─ shaders/foo.wgsl:83:1383let a = bar(frag_coord);
   │             ^= missing import '"shaders/bar.wgsl"'
@DGriffin91 DGriffin91 added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Nov 10, 2023
@cart
Copy link
Member

cart commented Nov 10, 2023

This is what the Embedded Asset Source / multiple asset sources was built for. The old DebugAssetServer approach was pretty clunky and I didn't want to carry that pattern forward in Bevy Asset V2. We haven't ported built in shaders over yet unfortunately. I think an important step is to move to implicit module names (as in, the module syntax directly desugars to the embedded asset paths).

Edit: I misread. This is a different issue.

@cart
Copy link
Member

cart commented Nov 10, 2023

Have you enabled the file_watcher feature? I can hot reload changes to custom_material.wgsl and custom_material_import.wgsl in the shader_material example.

@DGriffin91
Copy link
Contributor Author

I have enabled the file_watcher feature. If I edit custom_material_import.wgsl I get:

2023-11-10T22:44:27.261541Z ERROR bevy_render::render_resource::pipeline_cache: failed to process shader:
error: required import '"shaders/custom_material_import.wgsl"' not found
   ┌─ shaders/custom_material.wgsl:17:9417return material.color * textureSample(base_color_texture, base_color_sampler, mesh.uv) * COLOR_MULTIPLIER;
   │                                                                                              ^
   │
   = missing import '"shaders/custom_material_import.wgsl"'

@cart
Copy link
Member

cart commented Nov 10, 2023

Very strange. This is what I ran:

git checkout v0.12.0
cargo update
cargo run --example shader_material --features file_watcher

I can change either file, add new imports, etc and it all works.

I'm on linux.

github-merge-queue bot pushed a commit that referenced this issue Nov 11, 2023
# Objective

Hot reloading shader imports on windows is currently broken due to
inconsistent `/` and `\` usage ('/` is used in the user facing APIs and
`\` is produced by notify-rs (and likely other OS apis).

Fixes #10500

## Solution

Standardize import paths when loading a `Shader`. The correct long term
fix is to standardize AssetPath on `/`-only, but this is the right scope
of fix for a patch release.

---------

Co-authored-by: François <[email protected]>
cart added a commit that referenced this issue Nov 30, 2023
# Objective

Hot reloading shader imports on windows is currently broken due to
inconsistent `/` and `\` usage ('/` is used in the user facing APIs and
`\` is produced by notify-rs (and likely other OS apis).

Fixes #10500

## Solution

Standardize import paths when loading a `Shader`. The correct long term
fix is to standardize AssetPath on `/`-only, but this is the right scope
of fix for a patch release.

---------

Co-authored-by: François <[email protected]>
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this issue Jan 9, 2024
# Objective

Hot reloading shader imports on windows is currently broken due to
inconsistent `/` and `\` usage ('/` is used in the user facing APIs and
`\` is produced by notify-rs (and likely other OS apis).

Fixes bevyengine#10500

## Solution

Standardize import paths when loading a `Shader`. The correct long term
fix is to standardize AssetPath on `/`-only, but this is the right scope
of fix for a patch release.

---------

Co-authored-by: François <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants