From 2553b6138ca8bbca9df88638a7c91ba13701f9f2 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Mon, 12 Aug 2024 12:14:46 +0100 Subject: [PATCH] check bindgroup samplers --- crates/bevy_pbr/src/material.rs | 3 ++ .../bevy_render/macros/src/as_bind_group.rs | 41 ++++++++++++++++++- crates/bevy_render/src/render_asset.rs | 10 ++++- .../src/render_resource/bind_group.rs | 2 + .../bevy_render/src/texture/image_loader.rs | 4 +- crates/bevy_sprite/src/mesh2d/material.rs | 3 ++ .../src/render/ui_material_pipeline.rs | 3 ++ 7 files changed, 63 insertions(+), 3 deletions(-) diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index a1c07e7413501..755457c87e480 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -949,6 +949,9 @@ impl RenderAsset for PreparedMaterial { Err(AsBindGroupError::RetryNextUpdate) => { Err(PrepareAssetError::RetryNextUpdate(material)) } + Err(AsBindGroupError::InvalidData(msg)) => { + Err(PrepareAssetError::InvalidData(msg)) + } } } } diff --git a/crates/bevy_render/macros/src/as_bind_group.rs b/crates/bevy_render/macros/src/as_bind_group.rs index 25887495f41bf..a03da9f80b891 100644 --- a/crates/bevy_render/macros/src/as_bind_group.rs +++ b/crates/bevy_render/macros/src/as_bind_group.rs @@ -355,6 +355,20 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { let fallback_image = get_fallback_image(&render_path, *dimension); + let expected_samplers = match sampler_binding_type { + SamplerBindingType::Filtering => { + quote!( [#render_path::render_resource::TextureSampleType::Float { filterable: true }] ) + } + SamplerBindingType::NonFiltering => quote!([ + #render_path::render_resource::TextureSampleType::Float { filterable: false }, + #render_path::render_resource::TextureSampleType::Sint, + #render_path::render_resource::TextureSampleType::Uint, + ]), + SamplerBindingType::Comparison => { + quote!( [#render_path::render_resource::TextureSampleType::Depth] ) + } + }; + // insert fallible texture-based entries at 0 so that if we fail here, we exit before allocating any buffers binding_impls.insert(0, quote! { ( @@ -362,7 +376,32 @@ pub fn derive_as_bind_group(ast: syn::DeriveInput) -> Result { #render_path::render_resource::OwnedBindingResource::Sampler({ let handle: Option<&#asset_path::Handle<#render_path::texture::Image>> = (&self.#field_name).into(); if let Some(handle) = handle { - images.get(handle).ok_or_else(|| #render_path::render_resource::AsBindGroupError::RetryNextUpdate)?.sampler.clone() + let image = images.get(handle).ok_or_else(|| #render_path::render_resource::AsBindGroupError::RetryNextUpdate)?; + + let Some(sample_type) = image.texture_format.sample_type(None, None) else { + return Err(#render_path::render_resource::AsBindGroupError::InvalidData( + format!( + "binding index {}: no sampler for format `{:?}`", + #binding_index, + image.texture_format, + ) + )); + }; + + let valid = #expected_samplers.contains(&sample_type); + + if !valid { + return Err(#render_path::render_resource::AsBindGroupError::InvalidData( + format!( + "binding index {}: image sampler type `{:?}` must be one of `{:?}`", + #binding_index, + sample_type, + #expected_samplers + ) + )); + } + + image.sampler.clone() } else { #fallback_image.sampler.clone() } diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index e3a6aab5fb637..460ff32dde1e9 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -9,7 +9,7 @@ use bevy_ecs::{ }; use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize}; use bevy_render_macros::ExtractResource; -use bevy_utils::{tracing::debug, HashMap, HashSet}; +use bevy_utils::{tracing::{debug, warn}, HashMap, HashSet}; use serde::{Deserialize, Serialize}; use std::marker::PhantomData; use thiserror::Error; @@ -18,6 +18,8 @@ use thiserror::Error; pub enum PrepareAssetError { #[error("Failed to prepare asset")] RetryNextUpdate(E), + #[error("The bindgroup data is invalid: {0}")] + InvalidData(String), } /// Describes how an asset gets extracted and prepared for rendering. @@ -349,6 +351,9 @@ pub fn prepare_assets( Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => { prepare_next_frame.assets.push((id, extracted_asset)); } + Err(PrepareAssetError::InvalidData(msg)) => { + warn!("Material2d<{}> Bind group contains invalid data: {msg}", std::any::type_name::()); + } } } @@ -381,6 +386,9 @@ pub fn prepare_assets( Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => { prepare_next_frame.assets.push((id, extracted_asset)); } + Err(PrepareAssetError::InvalidData(msg)) => { + warn!("Material2d<{}> Bind group contains invalid data: {msg}", std::any::type_name::()); + } } } diff --git a/crates/bevy_render/src/render_resource/bind_group.rs b/crates/bevy_render/src/render_resource/bind_group.rs index ff61060ce796d..6bec407b0ae37 100644 --- a/crates/bevy_render/src/render_resource/bind_group.rs +++ b/crates/bevy_render/src/render_resource/bind_group.rs @@ -352,6 +352,8 @@ pub enum AsBindGroupError { /// The bind group could not be generated. Try again next frame. #[error("The bind group could not be generated")] RetryNextUpdate, + #[error("The bindgroup data is invalid: {0}")] + InvalidData(String), } /// A prepared bind group returned as a result of [`AsBindGroup::as_bind_group`]. diff --git a/crates/bevy_render/src/texture/image_loader.rs b/crates/bevy_render/src/texture/image_loader.rs index 3f5f4e3124817..14f776969cf1f 100644 --- a/crates/bevy_render/src/texture/image_loader.rs +++ b/crates/bevy_render/src/texture/image_loader.rs @@ -1,3 +1,5 @@ +use std::ffi::OsStr; + use bevy_asset::{io::Reader, AssetLoader, AsyncReadExt, LoadContext}; use bevy_ecs::prelude::{FromWorld, World}; use thiserror::Error; @@ -97,7 +99,7 @@ impl AssetLoader for ImageLoader { let image_type = match settings.format { ImageFormatSetting::FromExtension => { // use the file extension for the image type - let ext = load_context.path().extension().unwrap().to_str().unwrap(); + let ext = load_context.path().extension().and_then(OsStr::to_str).unwrap_or("image"); ImageType::Extension(ext) } ImageFormatSetting::Format(format) => ImageType::Format(format), diff --git a/crates/bevy_sprite/src/mesh2d/material.rs b/crates/bevy_sprite/src/mesh2d/material.rs index a0257bc01a711..846baf79dc0bb 100644 --- a/crates/bevy_sprite/src/mesh2d/material.rs +++ b/crates/bevy_sprite/src/mesh2d/material.rs @@ -508,6 +508,9 @@ impl RenderAsset for PreparedMaterial2d { Err(AsBindGroupError::RetryNextUpdate) => { Err(PrepareAssetError::RetryNextUpdate(material)) } + Err(AsBindGroupError::InvalidData(msg)) => { + Err(PrepareAssetError::InvalidData(msg)) + } } } } diff --git a/crates/bevy_ui/src/render/ui_material_pipeline.rs b/crates/bevy_ui/src/render/ui_material_pipeline.rs index 60e342f3cb022..286051fe817b9 100644 --- a/crates/bevy_ui/src/render/ui_material_pipeline.rs +++ b/crates/bevy_ui/src/render/ui_material_pipeline.rs @@ -624,6 +624,9 @@ impl RenderAsset for PreparedUiMaterial { Err(AsBindGroupError::RetryNextUpdate) => { Err(PrepareAssetError::RetryNextUpdate(material)) } + Err(AsBindGroupError::InvalidData(msg)) => { + Err(PrepareAssetError::InvalidData(msg)) + } } } }