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

Respect DownLevel flag for Cube Array Textures for point_light_shadow_map_array_texture_view. #12849

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 56 additions & 34 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ use bevy_render::{
view::{ExtractedView, RenderLayers, ViewVisibility},
Extract,
};
#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
use bevy_render::{renderer::RenderAdapter, DownlevelFlags};

use bevy_transform::{components::GlobalTransform, prelude::Transform};
#[cfg(feature = "trace")]
use bevy_utils::tracing::info_span;
Expand Down Expand Up @@ -507,12 +514,24 @@ pub(crate) fn spot_light_clip_from_view(angle: f32) -> Mat4 {
Mat4::perspective_infinite_reverse_rh(angle * 2.0, 1.0, POINT_LIGHT_NEAR_Z)
}

#[derive(Default)]
pub struct PrepareLightsWarningEmitted {
max_directional_lights: bool,
max_cascades_per_light: bool,
}

#[allow(clippy::too_many_arguments)]
pub fn prepare_lights(
mut commands: Commands,
mut texture_cache: ResMut<TextureCache>,
render_device: Res<RenderDevice>,
render_queue: Res<RenderQueue>,
#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
render_adapter: Res<RenderAdapter>,
mut global_light_meta: ResMut<GlobalClusterableObjectMeta>,
mut light_meta: ResMut<LightMeta>,
views: Query<
Expand All @@ -528,8 +547,7 @@ pub fn prepare_lights(
point_light_shadow_map: Res<PointLightShadowMap>,
directional_light_shadow_map: Res<DirectionalLightShadowMap>,
mut shadow_render_phases: ResMut<ViewBinnedRenderPhases<Shadow>>,
mut max_directional_lights_warning_emitted: Local<bool>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Started to fail compilation after merging in main, I think it was due to many arguments tripping the compiler out:

error[E0599]: the method `in_set` exists for fn item `fn(Commands<'a, 'b>, ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ..., ...) {prepare_lights}`, but its trait bounds were not satisfied
   --> crates/bevy_pbr/src/lib.rs:401:26

Easiest way around it I could find was to merge these two locals into a container struct.

mut max_cascades_per_light_warning_emitted: Local<bool>,
mut warning_emitted: Local<PrepareLightsWarningEmitted>,
point_lights: Query<(
Entity,
&ExtractedPointLight,
Expand Down Expand Up @@ -578,17 +596,17 @@ pub fn prepare_lights(
#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
let max_texture_cubes = 1;

if !*max_directional_lights_warning_emitted && directional_lights.len() > MAX_DIRECTIONAL_LIGHTS
if !warning_emitted.max_directional_lights && directional_lights.len() > MAX_DIRECTIONAL_LIGHTS
{
warn!(
"The amount of directional lights of {} is exceeding the supported limit of {}.",
directional_lights.len(),
MAX_DIRECTIONAL_LIGHTS
);
*max_directional_lights_warning_emitted = true;
warning_emitted.max_directional_lights = true;
}

if !*max_cascades_per_light_warning_emitted
if !warning_emitted.max_cascades_per_light
&& directional_lights
.iter()
.any(|(_, light)| light.cascade_shadow_config.bounds.len() > MAX_CASCADES_PER_LIGHT)
Expand All @@ -597,7 +615,7 @@ pub fn prepare_lights(
"The number of cascades configured for a directional light exceeds the supported limit of {}.",
MAX_CASCADES_PER_LIGHT
);
*max_cascades_per_light_warning_emitted = true;
warning_emitted.max_cascades_per_light = true;
}

let point_light_count = point_lights
Expand Down Expand Up @@ -1091,34 +1109,38 @@ pub fn prepare_lights(
}
}

let point_light_depth_texture_view =
point_light_depth_texture
.texture
.create_view(&TextureViewDescriptor {
label: Some("point_light_shadow_map_array_texture_view"),
format: None,
// NOTE: iOS Simulator is missing CubeArray support so we use Cube instead.
// See https://github.com/bevyengine/bevy/pull/12052 - remove if support is added.
#[cfg(all(
not(feature = "ios_simulator"),
any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
)
))]
dimension: Some(TextureViewDimension::CubeArray),
#[cfg(any(
feature = "ios_simulator",
all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu"))
))]
dimension: Some(TextureViewDimension::Cube),
aspect: TextureAspect::DepthOnly,
base_mip_level: 0,
mip_level_count: None,
base_array_layer: 0,
array_layer_count: None,
});
#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
let supports_cube_array_textures = false;

#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
let supports_cube_array_textures = render_adapter
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason that this would not work in all cases? I feel like it should and then the cfg stuff isn’t needed which simplifies this quite a bit.

Copy link
Member

Choose a reason for hiding this comment

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

that's how the PR was before #12849 (comment), but that crashed on WebGL2

.get_downlevel_capabilities()
.flags
.contains(DownlevelFlags::CUBE_ARRAY_TEXTURES);

let point_light_texture_descriptor = &TextureViewDescriptor {
label: Some("point_light_shadow_map_array_texture_view"),
format: None,
dimension: if supports_cube_array_textures {
Some(TextureViewDimension::CubeArray)
} else {
Some(TextureViewDimension::Cube)
},
aspect: TextureAspect::DepthOnly,
base_mip_level: 0,
mip_level_count: None,
base_array_layer: 0,
array_layer_count: None,
};

let point_light_depth_texture_view = point_light_depth_texture
.texture
.create_view(point_light_texture_descriptor);

let directional_light_depth_texture_view = directional_light_depth_texture
.texture
.create_view(&TextureViewDescriptor {
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,7 @@ impl FromWorld for MeshPipeline {
Res<RenderQueue>,
Res<MeshPipelineViewLayouts>,
)> = SystemState::new(world);

let (render_device, default_sampler, render_queue, view_layouts) =
system_state.get_mut(world);

Expand Down
53 changes: 38 additions & 15 deletions crates/bevy_pbr/src/render/mesh_view_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,21 @@ use bevy_render::{
globals::{GlobalsBuffer, GlobalsUniform},
render_asset::RenderAssets,
render_resource::{binding_types::*, *},
renderer::RenderDevice,
renderer::{RenderAdapter, RenderDevice},
texture::{BevyDefault, FallbackImage, FallbackImageMsaa, FallbackImageZero, GpuImage},
view::{
Msaa, RenderVisibilityRanges, ViewUniform, ViewUniforms,
VISIBILITY_RANGES_STORAGE_BUFFER_COUNT,
},
};

#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
use bevy_render::DownlevelFlags;

#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
use bevy_render::render_resource::binding_types::texture_cube;
#[cfg(debug_assertions)]
Expand Down Expand Up @@ -184,7 +191,28 @@ fn layout_entries(
visibility_ranges_buffer_binding_type: BufferBindingType,
layout_key: MeshPipelineViewLayoutKey,
render_device: &RenderDevice,
#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
render_adapter: &RenderAdapter,
#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
_render_adapter: &RenderAdapter,
) -> Vec<BindGroupLayoutEntry> {
#[cfg(all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu")))]
let supports_cube_array_textures = false;

#[cfg(any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
))]
let supports_cube_array_textures = render_adapter
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Why can’t this code be used in all cases?

.get_downlevel_capabilities()
.flags
.contains(DownlevelFlags::CUBE_ARRAY_TEXTURES);

let mut entries = DynamicBindGroupLayoutEntries::new_with_indices(
ShaderStages::FRAGMENT,
(
Expand All @@ -198,20 +226,11 @@ fn layout_entries(
// Point Shadow Texture Cube Array
(
2,
#[cfg(all(
not(feature = "ios_simulator"),
any(
not(feature = "webgl"),
not(target_arch = "wasm32"),
feature = "webgpu"
)
))]
texture_cube_array(TextureSampleType::Depth),
#[cfg(any(
feature = "ios_simulator",
all(feature = "webgl", target_arch = "wasm32", not(feature = "webgpu"))
))]
texture_cube(TextureSampleType::Depth),
if supports_cube_array_textures {
texture_cube_array(TextureSampleType::Depth)
} else {
texture_cube(TextureSampleType::Depth)
},
),
// Point Shadow Texture Array Sampler
(3, sampler(SamplerBindingType::Comparison)),
Expand Down Expand Up @@ -361,6 +380,7 @@ impl FromWorld for MeshPipelineViewLayouts {
// [`MeshPipelineViewLayoutKey`] flags.

let render_device = world.resource::<RenderDevice>();
let render_adapter = world.resource::<RenderAdapter>();

let clustered_forward_buffer_binding_type = render_device
.get_supported_read_only_binding_type(CLUSTERED_FORWARD_STORAGE_BUFFER_COUNT);
Expand All @@ -374,6 +394,7 @@ impl FromWorld for MeshPipelineViewLayouts {
visibility_ranges_buffer_binding_type,
key,
render_device,
render_adapter,
);
#[cfg(debug_assertions)]
let texture_count: usize = entries
Expand Down Expand Up @@ -410,6 +431,7 @@ impl MeshPipelineViewLayouts {
/// [`MeshPipelineViewLayoutKey`] flags.
pub fn generate_view_layouts(
render_device: &RenderDevice,
render_adapter: &RenderAdapter,
clustered_forward_buffer_binding_type: BufferBindingType,
visibility_ranges_buffer_binding_type: BufferBindingType,
) -> [MeshPipelineViewLayout; MeshPipelineViewLayoutKey::COUNT] {
Expand All @@ -420,6 +442,7 @@ pub fn generate_view_layouts(
visibility_ranges_buffer_binding_type,
key,
render_device,
render_adapter,
);

#[cfg(debug_assertions)]
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use batching::gpu_preprocessing::BatchingPlugin;
use bevy_ecs::schedule::ScheduleBuildSettings;
use bevy_utils::prelude::default;
pub use extract_param::Extract;
pub use wgpu::DownlevelFlags;

use bevy_hierarchy::ValidParentCheckPlugin;
use bevy_window::{PrimaryWindow, RawHandleWrapperHolder};
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_render/src/render_resource/pipeline_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,11 @@ impl ShaderCache {
shader_defs.push("SIXTEEN_BYTE_ALIGNMENT".into());
}

if cfg!(feature = "ios_simulator") {
if !self
.composer
.capabilities
.contains(Capabilities::CUBE_ARRAY_TEXTURES)
{
shader_defs.push("NO_CUBE_ARRAY_TEXTURES_SUPPORT".into());
}

Expand Down