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

Add bevy_shader_hot_reloading feature for easier renderer development #3673

Closed
wants to merge 8 commits into from

Conversation

superdump
Copy link
Contributor

@superdump superdump commented Jan 14, 2022

Description

Having to recompile bevy and re-run test apps in order to iterate on shaders for bevy renderer features is slower than necessary. The goal of this change is to add a non-default feature that allows bevy renderer feature developers to edit core bevy renderer shaders and have them hot reload. This will enormously speed up development due to reducing iteration time. More changes may be needed to not replace shader assets if they in any way invalid, but this should be a good start.

Solution

NOTE: Builds on top of #3643 and watch for changes needs to be enabled using the resource as in that PR for this PR to work.

This was way more complicated than I expected:

  • Hot-reloading only notifies the handle which was created from AssetServer::load which is constructed from the AssetPath. This is convenient as the filesystem watcher for hot-reloading naturally and unsurprisingly produces events based on paths to files that changed.
    • To solve this I chose to add what I have called 'aliases' for handles.
    • Add aliases: HashMap<HandleId, HashSet<HandleId>> to Assets<T> to hold all handles that are aliases for the strong handle for an asset as obtained from AssetServer::load(). An alias handle is a handle made via other means, such as pub const PBR_SHADER_HANDLE: HandleUntyped = HandleUntyped::weak_from_u64(Shader::TYPE_UUID, 4805239651767701046);.
    • Add alias_to_handle: HashMap<HandleId, HandleId> to Assets<T>
    • Add:
      • add_alias<H: Into<HandleId>, A: Into<HandleId>>(&mut self, handle: H, alias: A),
      • remove_alias<H: Into<HandleId>, A: Into<HandleId>>(&mut self, handle: H, alias: A),
      • and get_handle_from_alias<A: Into<HandleId>>(&self, alias: A) -> Option<&HandleId>
      • ...to Assets<T> to be able to add/remove aliases, and for a given alias handle, get the strong handle. Assets<T>::get() uses the latter to attempt to get the asset using an alias handle if it exists. This allows materials to use PBR_SHADER_HANDLE and due to setting that as an alias handle for the strong handle, Assets<Shader>::get() 'just works'.
  • When using Assets<Shader>::set_untracked() with the const HandleUntyped (e.g. PBR_SHADER_HANDLE), the Shader is constructed and immediately chained with Shader::with_import_path() to define the shader import. When using AssetServer::load(), the Shader does not exist until loaded.
    • If a shader import path is necessary, use a system to check for when the shader asset is loaded, mutably get it and set the import path. I think this could be possibly improved somehow but it's good enough as a first shot perhaps.
  • Making use of these changes:
    • Move all the shader files into the assets/shaders/ directory in subdirectories named after their respective crates, e.g. assets/shaders/bevy_pbr/pbr.wgsl.
    • Add the non-default bevy_shader_hot_reloading feature
    • Where shaders are used in bevy rendering crates:
      • Move the current Assets<Shader>::set_untracked() code into a block for when the feature is not set.
      • Add a new block for when the feature is set, and use a pattern of:
pub const SHADER_HANDLE: HandleUntyped =
    HandleUntyped::weak_from_u64(Shader::TYPE_UUID, 1234567890);
pub const IMPORT_HANDLE: HandleUntyped =
    HandleUntyped::weak_from_u64(Shader::TYPE_UUID, 0987654321);
const IMPORT_PATH: &str = "my_crate::import_name";

...

let asset_server = app.world.get_resource::<AssetServer>().unwrap();
let shader_handle = asset_server.load("path/to/shader.wgsl");
let import_handle = asset_server.load("path/to/import.wgsl");

let mut shaders = app.world.get_resource_mut::<Assets<Shader>>().unwrap();
shaders.add_alias(&shader_handle, SHADER_HANDLE);
shaders.add_alias(&import_handle, IMPORT_HANDLE);

// NOTE: We need to store the strong handles created from the asset paths
let mut hot_reload_shaders = app.world.get_resource_mut::<HotReloadShaders>().unwrap();
hot_reload_shaders.keep_shader_alive(shader_handle);
hot_reload_shaders.add_hot_reload_shader(import_handle, IMPORT_PATH.to_string());

@superdump superdump requested a review from cart January 14, 2022 15:01
@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 14, 2022
@superdump superdump added A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Jan 14, 2022
@superdump
Copy link
Contributor Author

bevy_core_shader_hot_reloading.mp4

@superdump superdump self-assigned this Jan 16, 2022
@johanhelsing
Copy link
Contributor

I only get a gray window with when running the sprite example on this branch.

I also checked on #3643 and there sprite shows up as it should.

The goal of this change is to add a non-default feature that allows bevy
renderer feature developers to edit core bevy renderer shaders and have them
hot reload. This will _enormously_ speed up development due to reducing
iteration time. More changes may be needed to not replace shader assets if they
are invalid, but this should be a good start.
@superdump
Copy link
Contributor Author

I only get a gray window with when running the sprite example on this branch.

Fixed.

@@ -53,7 +53,7 @@ impl Default for AssetServerSettings {
fn default() -> Self {
Self {
asset_folder: "assets".to_string(),
watch_for_changes: false,
watch_for_changes: cfg!(feature = "bevy_shader_hot_reloading"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just noting that I did this for convenience. It saves adding this as a resource in example just to set it to true when hacking/testing stuff.

@superdump
Copy link
Contributor Author

superdump commented Feb 9, 2022

I also updated the usage example in the PR description for shaders with entrypoints and for imports.

@mockersf
Copy link
Member

mockersf commented Feb 9, 2022

the new feature should be mentioned in https://github.com/bevyengine/bevy/blob/main/docs/cargo_features.md

bors bot pushed a commit that referenced this pull request Feb 18, 2022
Adds "hot reloading" of internal assets, which is normally not possible because they are loaded using `include_str` / direct Asset collection access.

This is accomplished via the following:
* Add a new `debug_asset_server` feature flag
* When that feature flag is enabled, create a second App with a second AssetServer that points to a configured location (by default the `crates` folder). Plugins that want to add hot reloading support for their assets can call the new `app.add_debug_asset::<T>()` and `app.init_debug_asset_loader::<T>()` functions.
* Load "internal" assets using the new `load_internal_asset` macro. By default this is identical to the current "include_str + register in asset collection" approach. But if the `debug_asset_server` feature flag is enabled, it will also load the asset dynamically in the debug asset server using the file path. It will then set up a correlation between the "debug asset" and the "actual asset" by listening for asset change events.

This is an alternative to #3673. The goal was to keep the boilerplate and features flags to a minimum for bevy plugin authors, and allow them to home their shaders near relevant code. 

This is a draft because I haven't done _any_ quality control on this yet. I'll probably rename things and remove a bunch of unwraps. I just got it working and wanted to use it to start a conversation.

Fixes #3660
@mockersf
Copy link
Member

Should this PR be closed now that #3966 has been merged?

@superdump
Copy link
Contributor Author

Implemented differently by #3966

@superdump superdump closed this Feb 19, 2022
molikto pushed a commit to molikto/bevy that referenced this pull request Feb 20, 2022
Adds "hot reloading" of internal assets, which is normally not possible because they are loaded using `include_str` / direct Asset collection access.

This is accomplished via the following:
* Add a new `debug_asset_server` feature flag
* When that feature flag is enabled, create a second App with a second AssetServer that points to a configured location (by default the `crates` folder). Plugins that want to add hot reloading support for their assets can call the new `app.add_debug_asset::<T>()` and `app.init_debug_asset_loader::<T>()` functions.
* Load "internal" assets using the new `load_internal_asset` macro. By default this is identical to the current "include_str + register in asset collection" approach. But if the `debug_asset_server` feature flag is enabled, it will also load the asset dynamically in the debug asset server using the file path. It will then set up a correlation between the "debug asset" and the "actual asset" by listening for asset change events.

This is an alternative to bevyengine#3673. The goal was to keep the boilerplate and features flags to a minimum for bevy plugin authors, and allow them to home their shaders near relevant code. 

This is a draft because I haven't done _any_ quality control on this yet. I'll probably rename things and remove a bunch of unwraps. I just got it working and wanted to use it to start a conversation.

Fixes bevyengine#3660
kurtkuehnert pushed a commit to kurtkuehnert/bevy that referenced this pull request Mar 6, 2022
Adds "hot reloading" of internal assets, which is normally not possible because they are loaded using `include_str` / direct Asset collection access.

This is accomplished via the following:
* Add a new `debug_asset_server` feature flag
* When that feature flag is enabled, create a second App with a second AssetServer that points to a configured location (by default the `crates` folder). Plugins that want to add hot reloading support for their assets can call the new `app.add_debug_asset::<T>()` and `app.init_debug_asset_loader::<T>()` functions.
* Load "internal" assets using the new `load_internal_asset` macro. By default this is identical to the current "include_str + register in asset collection" approach. But if the `debug_asset_server` feature flag is enabled, it will also load the asset dynamically in the debug asset server using the file path. It will then set up a correlation between the "debug asset" and the "actual asset" by listening for asset change events.

This is an alternative to bevyengine#3673. The goal was to keep the boilerplate and features flags to a minimum for bevy plugin authors, and allow them to home their shaders near relevant code. 

This is a draft because I haven't done _any_ quality control on this yet. I'll probably rename things and remove a bunch of unwraps. I just got it working and wanted to use it to start a conversation.

Fixes bevyengine#3660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants