-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Opt-in raw vulkan initialization with hooks #20565
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
Conversation
You added a new feature but didn't update the readme. Please run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow accidentally approved this while reviewing last night, reviewing for real now. :)
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think to make this pattern more robust, beyond upstream wgpu
support for not forcing the instance creation, we need to more closely integrate render init with the ECS.
&mut additional_vulkan_features, | ||
) | ||
.await | ||
.unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Realize this is explicitly is stated to force the Vulkan instance, but we could fallback to the regular init here with a warning instead of unwrapping.
&device_descriptor.memory_hints, | ||
Some(Box::new(|mut args| { | ||
for callback in &settings.create_device_callbacks { | ||
(callback)(&mut args, &raw_adapter, additional_features); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long term, we also need access to &mut World
here in order to initialize additional resources (additional queues, etc).
#20565 broke compiling with `--all-features` on macos. Add the `vulkan-portability` feature to `bevy_render` required to make vulkan compile with wgpu on mac. Co-authored-by: François Mockers <francois.mockers@vleue.com>
Objective
Currently registering additional required Vulkan features requires either hard-coding them into
bevy_render
(see the current DLSS proposal), or forcing the plugin to take full manual control over wgpu initialization. Neither is an acceptable or scalable option for a modular engine like Bevy.Solution
raw_vulkan_init
Cargo feature, that when enabled switches to wgpu's raw Vulkan init path, which accepts callbacks that allow checking and requiring additional Vulkan features.WgpuRawVulkanInitSettings
resource, which provides wgpu Vulkan Instance and Device init callbacks, which can be used to detect and register vulkan features.AdditionalVulkanFeatures
resource, which is inserted into the RenderApp at the same time that the RenderDevice is.This enables plugins to register initialization callbacks, which must happen before RenderPlugin. They can then feed off of
AdditionalVulkanFeatures
, after the renderer is initialized, to detect if a given feature is supported.Due to the current lifecycles, this is best accomplished with either:
Plugin::build()
access to render device state, which needs to be registered after RenderPlugin to have access. The proposed DLSS feature needs this pattern.With deferred plugin init, we could remove the need for this split.