Skip to content

Commit

Permalink
Make AsBindGroup unsized (bevyengine#6937)
Browse files Browse the repository at this point in the history
# Objective

`AsBindGroup` can't be used as a trait object because of the constraint `Sized` and because of the associated function.

This is a problem for [`bevy_atmosphere`](https://github.com/JonahPlusPlus/bevy_atmosphere) because it needs to use a trait that depends on `AsBindGroup` as a trait object, for switching out different shaders at runtime. The current solution it employs is reimplementing the trait and derive macro into that trait, instead of constraining to `AsBindGroup`.

## Solution

Remove the `Sized` constraint from `AsBindGroup` and add the constraint `where Self: Sized` to the associated function `bind_group_layout`. Also change `PreparedBindGroup<T: AsBindGroup>` to `PreparedBindGroup<T>` and use it as `PreparedBindGroup<Self::Data>` instead of `PreparedBindGroup<Self>`.

This weakens the constraints, but increases the flexibility of `AsBindGroup`.
I'm not entirely sure why the `Sized` constraint was there, because it worked fine without it (maybe @cart wasn't aware of use cases for `AsBindGroup` as a trait object or this was just leftover from legacy code?).

---

## Changelog

- `AsBindGroup` can be used as a trait object.
  • Loading branch information
JonahPlusPlus authored and ItsDoot committed Feb 1, 2023
1 parent a381662 commit 9f09de5
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_render/macros/src/as_bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result<TokenStream> {
render_device: &#render_path::renderer::RenderDevice,
images: &#render_path::render_asset::RenderAssets<#render_path::texture::Image>,
fallback_image: &#render_path::texture::FallbackImage,
) -> Result<#render_path::render_resource::PreparedBindGroup<Self>, #render_path::render_resource::AsBindGroupError> {
) -> Result<#render_path::render_resource::PreparedBindGroup<Self::Data>, #render_path::render_resource::AsBindGroupError> {
let bindings = vec![#(#binding_impls,)*];

let bind_group = {
Expand Down
12 changes: 7 additions & 5 deletions crates/bevy_render/src/render_resource/bind_group.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ impl Deref for BindGroup {
/// }
/// }
/// ```
pub trait AsBindGroup: Sized {
pub trait AsBindGroup {
/// Data that will be stored alongside the "prepared" bind group.
type Data: Send + Sync;

Expand All @@ -264,10 +264,12 @@ pub trait AsBindGroup: Sized {
render_device: &RenderDevice,
images: &RenderAssets<Image>,
fallback_image: &FallbackImage,
) -> Result<PreparedBindGroup<Self>, AsBindGroupError>;
) -> Result<PreparedBindGroup<Self::Data>, AsBindGroupError>;

/// Creates the bind group layout matching all bind groups returned by [`AsBindGroup::as_bind_group`]
fn bind_group_layout(render_device: &RenderDevice) -> BindGroupLayout;
fn bind_group_layout(render_device: &RenderDevice) -> BindGroupLayout
where
Self: Sized;
}

/// An error that occurs during [`AsBindGroup::as_bind_group`] calls.
Expand All @@ -277,10 +279,10 @@ pub enum AsBindGroupError {
}

/// A prepared bind group returned as a result of [`AsBindGroup::as_bind_group`].
pub struct PreparedBindGroup<T: AsBindGroup> {
pub struct PreparedBindGroup<T> {
pub bindings: Vec<OwnedBindingResource>,
pub bind_group: BindGroup,
pub data: T::Data,
pub data: T,
}

/// An owned binding resource of any type (ex: a [`Buffer`], [`TextureView`], etc).
Expand Down
2 changes: 1 addition & 1 deletion examples/3d/skybox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl AsBindGroup for CubemapMaterial {
render_device: &RenderDevice,
images: &RenderAssets<Image>,
_fallback_image: &FallbackImage,
) -> Result<PreparedBindGroup<Self>, AsBindGroupError> {
) -> Result<PreparedBindGroup<Self::Data>, AsBindGroupError> {
let base_color_texture = self
.base_color_texture
.as_ref()
Expand Down

0 comments on commit 9f09de5

Please sign in to comment.