Skip to content

Commit

Permalink
bevy_render: Do not automatically enable MAPPABLE_PRIMARY_BUFFERS (#3698
Browse files Browse the repository at this point in the history
)

# Objective

- When using `WgpuOptionsPriority::Functionality`, which is the default, wgpu::Features::MAPPABLE_PRIMARY_BUFFERS would be automatically enabled. This feature can and does have a significant negative impact on performance for discrete GPUs where resizable bar is not supported, which is a common case. As such, this feature should not be automatically enabled.
- Fixes the performance regression part of #3686 and at least some, if not all cases of #3687

## Solution

- When using `WgpuOptionsPriority::Functionality`, use the adapter-supported features, enable `TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES` and disable `MAPPABLE_PRIMARY_BUFFERS`
  • Loading branch information
superdump committed Jan 17, 2022
1 parent a3e43b6 commit ef823d3
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions crates/bevy_render/src/renderer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ pub type RenderQueue = Arc<Queue>;
/// aswell as to create [`WindowSurfaces`](crate::view::window::WindowSurfaces).
pub type RenderInstance = Instance;

/// `wgpu::Features` that are not automatically enabled due to having possibly-negative side effects.
/// `MAPPABLE_PRIMARY_BUFFERS` can have a significant, negative performance impact so should not be
/// automatically enabled.
pub const DEFAULT_DISABLED_WGPU_FEATURES: wgpu::Features = wgpu::Features::MAPPABLE_PRIMARY_BUFFERS;

/// Initializes the renderer by retrieving and preparing the GPU instance, device and queue
/// for the specified backend.
pub async fn initialize_renderer(
Expand All @@ -86,8 +91,9 @@ pub async fn initialize_renderer(
let trace_path = None;

if matches!(options.priority, WgpuOptionsPriority::Functionality) {
options.features =
adapter.features() | wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES;
options.features = (adapter.features()
| wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES)
- DEFAULT_DISABLED_WGPU_FEATURES;
options.limits = adapter.limits();
}

Expand Down

3 comments on commit ef823d3

@CatCode79
Copy link

Choose a reason for hiding this comment

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

I noticed that there is also another occurrence of TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES here:

features: wgpu::Features::TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES,

Does this also need to be changed?

@bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented on ef823d3 Jan 28, 2022

Choose a reason for hiding this comment

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

I don't think so. TEXTURE_ADAPTER_SPECIFIC_FORMAT_FEATURES doesn't include MAPPABLE_PRIMARY_BUFFERS, so disabling MAPPABLE_PRIMARY_BUFFERS doesn't have any effect.

@CatCode79
Copy link

Choose a reason for hiding this comment

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

You are right. I got confused testing a version without this commit and mistakenly assumed the line was meaningful.

Please sign in to comment.