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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - bevy_render: Support overriding wgpu features and limits #3912

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - bevy_render: Support overriding wgpu features and limits #3912

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions crates/bevy_render/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@ pub struct WgpuOptions {
pub backends: Option<Backends>,
pub power_preference: PowerPreference,
pub priority: WgpuOptionsPriority,
/// The enabled features. Setting features will require them to be enabled when initializing
/// the renderer.
pub features: WgpuFeatures,
/// The features to ensure are disabled regardless of what the adapter/backend supports
pub disabled_features: Option<WgpuFeatures>,
/// The imposed limits. Updated based on adapter/backend limits when initializing the renderer
/// if using WgpuOptionsPriority::Functionality
pub limits: WgpuLimits,
/// The constraints on limits allowed regardless of what the adapter/backend supports
pub constrained_limits: Option<WgpuLimits>,
}

impl Default for WgpuOptions {
Expand Down Expand Up @@ -50,7 +58,9 @@ impl Default for WgpuOptions {
power_preference: PowerPreference::HighPerformance,
priority,
features: wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES,
disabled_features: None,
limits,
constrained_limits: None,
}
}
}
Expand Down
110 changes: 106 additions & 4 deletions crates/bevy_render/src/renderer/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,120 @@ pub async fn initialize_renderer(
#[cfg(not(feature = "wgpu_trace"))]
let trace_path = None;

// Maybe get features and limits based on what is supported by the adapter/backend
let mut features = wgpu::Features::empty();
let mut limits = options.limits.clone();
if matches!(options.priority, WgpuOptionsPriority::Functionality) {
let mut features =
adapter.features() | wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
features = adapter.features() | wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
if adapter_info.device_type == wgpu::DeviceType::DiscreteGpu {
// `MAPPABLE_PRIMARY_BUFFERS` can have a significant, negative performance impact for
// discrete GPUs due to having to transfer data across the PCI-E bus and so it
// should not be automatically enabled in this case. It is however beneficial for
// integrated GPUs.
features -= wgpu::Features::MAPPABLE_PRIMARY_BUFFERS;
}
options.features = features;
options.limits = adapter.limits();
limits = adapter.limits();
}

// Enforce the disabled features
if let Some(disabled_features) = options.disabled_features {
features -= disabled_features;
}
// NOTE: |= is used here to ensure that any explicitly-enabled features are respected.
options.features |= features;
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking that we should make WgpuOptions a "read only" configuration item. Users shouldn't be reading these settings to get "actual" values, they should be querying the device. And writing on top like this makes it hard to talk about what these values mean. Buuuut given that these changes are going into a patch release, we should probably hold off on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking from the beginning that it’s a bit sketchy to reuse WgpuOptions to present the configured state after using it but it also isn’t used after init otherwise.

As regards this specific thing… we could have a private field with the final result or a separate resource so that WgpuOptions is used for configuration and WgpuCapabilities or some good name is what you can actually use. Both practically read only. But how do you even mark something as read-only? I didn’t know you could?

Copy link
Member

Choose a reason for hiding this comment

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

In terms of "read only config", that ties into the conversation here: #2988
At the very least, I think it makes sense to "generally" treat plugin settings as "immutable after insertion", even though we don't have that functionality yet.

Copy link
Contributor

@molikto molikto Feb 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

plz also consider making plugin settings freeze when read https://discord.com/channels/691052431525675048/692572690833473578/941182628499947520

This means you can:

  • reconfigure settings before it is read by plugin
  • disallow changing settings after it is read, so avoiding user confusion
  • plugin can change to "reactive" settings when it want to support it, without changing it's API

Also compare to the proposed changes, this is a very minimal change (requires add world.freeze_resource)


// Enforce the limit constraints
if let Some(constrained_limits) = options.constrained_limits.as_ref() {
// NOTE: Respect the configured limits as an 'upper bound'. This means for 'max' limits, we
// take the minimum of the calculated limits according to the adapter/backend and the
// specified max_limits. For 'min' limits, take the maximum instead. This is intended to
// err on the side of being conservative. We can't claim 'higher' limits that are supported
// but we can constrain to 'lower' limits.
options.limits = wgpu::Limits {
max_texture_dimension_1d: limits
.max_texture_dimension_1d
.min(constrained_limits.max_texture_dimension_1d),
max_texture_dimension_2d: limits
.max_texture_dimension_2d
.min(constrained_limits.max_texture_dimension_2d),
max_texture_dimension_3d: limits
.max_texture_dimension_3d
.min(constrained_limits.max_texture_dimension_3d),
max_texture_array_layers: limits
.max_texture_array_layers
.min(constrained_limits.max_texture_array_layers),
max_bind_groups: limits
.max_bind_groups
.min(constrained_limits.max_bind_groups),
max_dynamic_uniform_buffers_per_pipeline_layout: limits
.max_dynamic_uniform_buffers_per_pipeline_layout
.min(constrained_limits.max_dynamic_uniform_buffers_per_pipeline_layout),
max_dynamic_storage_buffers_per_pipeline_layout: limits
.max_dynamic_storage_buffers_per_pipeline_layout
.min(constrained_limits.max_dynamic_storage_buffers_per_pipeline_layout),
max_sampled_textures_per_shader_stage: limits
.max_sampled_textures_per_shader_stage
.min(constrained_limits.max_sampled_textures_per_shader_stage),
max_samplers_per_shader_stage: limits
.max_samplers_per_shader_stage
.min(constrained_limits.max_samplers_per_shader_stage),
max_storage_buffers_per_shader_stage: limits
.max_storage_buffers_per_shader_stage
.min(constrained_limits.max_storage_buffers_per_shader_stage),
max_storage_textures_per_shader_stage: limits
.max_storage_textures_per_shader_stage
.min(constrained_limits.max_storage_textures_per_shader_stage),
max_uniform_buffers_per_shader_stage: limits
.max_uniform_buffers_per_shader_stage
.min(constrained_limits.max_uniform_buffers_per_shader_stage),
max_uniform_buffer_binding_size: limits
.max_uniform_buffer_binding_size
.min(constrained_limits.max_uniform_buffer_binding_size),
max_storage_buffer_binding_size: limits
.max_storage_buffer_binding_size
.min(constrained_limits.max_storage_buffer_binding_size),
max_vertex_buffers: limits
.max_vertex_buffers
.min(constrained_limits.max_vertex_buffers),
max_vertex_attributes: limits
.max_vertex_attributes
.min(constrained_limits.max_vertex_attributes),
max_vertex_buffer_array_stride: limits
.max_vertex_buffer_array_stride
.min(constrained_limits.max_vertex_buffer_array_stride),
max_push_constant_size: limits
.max_push_constant_size
.min(constrained_limits.max_push_constant_size),
min_uniform_buffer_offset_alignment: limits
.min_uniform_buffer_offset_alignment
.max(constrained_limits.min_uniform_buffer_offset_alignment),
min_storage_buffer_offset_alignment: limits
.min_storage_buffer_offset_alignment
.max(constrained_limits.min_storage_buffer_offset_alignment),
max_inter_stage_shader_components: limits
.max_inter_stage_shader_components
.min(constrained_limits.max_inter_stage_shader_components),
max_compute_workgroup_storage_size: limits
.max_compute_workgroup_storage_size
.min(constrained_limits.max_compute_workgroup_storage_size),
max_compute_invocations_per_workgroup: limits
.max_compute_invocations_per_workgroup
.min(constrained_limits.max_compute_invocations_per_workgroup),
max_compute_workgroup_size_x: limits
.max_compute_workgroup_size_x
.min(constrained_limits.max_compute_workgroup_size_x),
max_compute_workgroup_size_y: limits
.max_compute_workgroup_size_y
.min(constrained_limits.max_compute_workgroup_size_y),
max_compute_workgroup_size_z: limits
.max_compute_workgroup_size_z
.min(constrained_limits.max_compute_workgroup_size_z),
max_compute_workgroups_per_dimension: limits
.max_compute_workgroups_per_dimension
.min(constrained_limits.max_compute_workgroups_per_dimension),
};
} else {
options.limits = limits;
}

let (device, queue) = adapter
Expand Down