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

Changed Msaa to take level rather than int #6954

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ pub fn prepare_core_3d_depth_textures(
height: physical_target_size.y,
},
mip_level_count: 1,
sample_count: msaa.samples,
sample_count: msaa.level as u32,
dimension: TextureDimension::D2,
format: TextureFormat::Depth32Float, /* PERF: vulkan docs recommend using 24
* bit depth for better performance */
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,8 @@ pub fn queue_material_meshes<M: Material>(
let draw_alpha_mask_pbr = alpha_mask_draw_functions.read().id::<DrawMaterial<M>>();
let draw_transparent_pbr = transparent_draw_functions.read().id::<DrawMaterial<M>>();

let mut view_key =
MeshPipelineKey::from_msaa_samples(msaa.samples) | MeshPipelineKey::from_hdr(view.hdr);
let mut view_key = MeshPipelineKey::from_msaa_samples(msaa.level as u32)
| MeshPipelineKey::from_hdr(view.hdr);

if let Some(Tonemapping::Enabled { deband_dither }) = tonemapping {
if !view.hdr {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/wireframe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ fn queue_wireframes(
mut views: Query<(&ExtractedView, &VisibleEntities, &mut RenderPhase<Opaque3d>)>,
) {
let draw_custom = opaque_3d_draw_functions.read().id::<DrawWireframes>();
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.level as u32);
for (view, visible_entities, mut opaque_phase) in &mut views {
let rangefinder = view.rangefinder3d();

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub mod prelude {
render_resource::Shader,
spatial_bundle::SpatialBundle,
texture::{Image, ImagePlugin},
view::{ComputedVisibility, Msaa, Visibility, VisibilityBundle},
view::{ComputedVisibility, Msaa, MultiSampleLevel, Visibility, VisibilityBundle},
};
}

Expand Down
31 changes: 26 additions & 5 deletions crates/bevy_render/src/view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ impl Plugin for ViewPlugin {
/// # use bevy_app::prelude::App;
/// # use bevy_render::prelude::Msaa;
/// App::new()
/// .insert_resource(Msaa { samples: 4 })
/// .insert_resource(Msaa::from(MultiSampleLevel::Sample4))
/// .run();
/// ```
#[derive(Resource, Clone, ExtractResource, Reflect)]
#[reflect(Resource)]
#[non_exhaustive]
Copy link
Contributor

Choose a reason for hiding this comment

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

Wha the reason for this addition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only 1 and 4 are supported for now by wgpu: gfx-rs/wgpu#1832

Copy link
Contributor

Choose a reason for hiding this comment

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

But shouldn't this then be applied to the MultiSampleLevel enum (which would gain the new levels) and not Msaa struct (which shouldn't receive any new fields due to that)?

Copy link
Member

Choose a reason for hiding this comment

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

yup this should be on the new enum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

pub struct Msaa {
/// The number of samples to run for Multi-Sample Anti-Aliasing. Higher numbers result in
/// smoother edges.
Expand All @@ -74,15 +75,35 @@ pub struct Msaa {
/// Note that WGPU currently only supports 1 or 4 samples.
/// Ultimately we plan on supporting whatever is natively supported on a given device.
/// Check out this issue for more info: <https://github.com/gfx-rs/wgpu/issues/1832>
pub samples: u32,
pub level: MultiSampleLevel,
}

impl Msaa {
pub fn samples(&self) -> u32 {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub fn samples(&self) -> u32 {
#[inline]
pub fn samples(&self) -> u32 {

this should probably be inlined

self.level as u32
}
}

impl Default for Msaa {
fn default() -> Self {
Self { samples: 4 }
Self {
level: MultiSampleLevel::Sample4,
}
}
}

impl From<MultiSampleLevel> for Msaa {
fn from(level: MultiSampleLevel) -> Self {
Self { level }
}
}

#[derive(Clone, Copy, Reflect, PartialEq)]
pub enum MultiSampleLevel {
Off = 1,
Sample4 = 4,
}
Comment on lines +104 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we also add Sample2 and Sample8 for native?

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind, the support for this has been merged into Wgpu but not released yet.


#[derive(Component)]
pub struct ExtractedView {
pub projection: Mat4,
Expand Down Expand Up @@ -334,15 +355,15 @@ fn prepare_view_targets(
},
)
.default_view,
sampled: (msaa.samples > 1).then(|| {
sampled: (msaa.level as u32 > 1).then(|| {
texture_cache
.get(
&render_device,
TextureDescriptor {
label: Some("main_texture_sampled"),
size,
mip_level_count: 1,
sample_count: msaa.samples,
sample_count: msaa.level as u32,
dimension: TextureDimension::D2,
format: main_texture_format,
usage: TextureUsages::RENDER_ATTACHMENT,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_sprite/src/mesh2d/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ pub fn queue_material2d_meshes<M: Material2d>(
for (view, visible_entities, tonemapping, mut transparent_phase) in &mut views {
let draw_transparent_pbr = transparent_draw_functions.read().id::<DrawMaterial2d<M>>();

let mut view_key = Mesh2dPipelineKey::from_msaa_samples(msaa.samples)
let mut view_key = Mesh2dPipelineKey::from_msaa_samples(msaa.level as u32)
| Mesh2dPipelineKey::from_hdr(view.hdr);

if let Some(Tonemapping::Enabled { deband_dither }) = tonemapping {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_sprite/src/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ pub fn queue_sprites(
};
}

let msaa_key = SpritePipelineKey::from_msaa_samples(msaa.samples);
let msaa_key = SpritePipelineKey::from_msaa_samples(msaa.level as u32);

if let Some(view_binding) = view_uniforms.uniforms.binding() {
let sprite_meta = &mut sprite_meta;
Expand Down
2 changes: 1 addition & 1 deletion examples/2d/mesh2d_manual.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ pub fn queue_colored_mesh2d(
for (visible_entities, mut transparent_phase, view) in &mut views {
let draw_colored_mesh2d = transparent_draw_functions.read().id::<DrawColoredMesh2d>();

let mesh_key = Mesh2dPipelineKey::from_msaa_samples(msaa.samples)
let mesh_key = Mesh2dPipelineKey::from_msaa_samples(msaa.level as u32)
| Mesh2dPipelineKey::from_hdr(view.hdr);

// Queue all entities visible to that view
Expand Down
16 changes: 9 additions & 7 deletions examples/3d/fxaa.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
//! This examples compares MSAA (Multi-Sample Anti-Aliasing) and FXAA (Fast Approximate Anti-Aliasing).

use std::f32::consts::PI;
use std::{f32::consts::PI, ops::Mul};

use bevy::{
core_pipeline::fxaa::{Fxaa, Sensitivity},
prelude::*,
render::{
render_resource::{Extent3d, SamplerDescriptor, TextureDimension, TextureFormat},
texture::ImageSampler,
view::MultiSampleLevel,
},
};

fn main() {
App::new()
// Disable MSAA be default
.insert_resource(Msaa { samples: 1 })
// Disable MSAA by default
.insert_resource(Msaa::from(MultiSampleLevel::Off))
.add_plugins(DefaultPlugins)
.add_startup_system(setup)
.add_system(toggle_fxaa)
Expand Down Expand Up @@ -118,19 +119,20 @@ fn toggle_fxaa(keys: Res<Input<KeyCode>>, mut query: Query<&mut Fxaa>, mut msaa:
let fxaa_ultra = keys.just_pressed(KeyCode::Key9);
let fxaa_extreme = keys.just_pressed(KeyCode::Key0);
let set_fxaa = set_fxaa | fxaa_low | fxaa_med | fxaa_high | fxaa_ultra | fxaa_extreme;

for mut fxaa in &mut query {
if set_msaa {
fxaa.enabled = false;
msaa.samples = 4;
msaa.level = MultiSampleLevel::Sample4;
info!("MSAA 4x");
}
if set_no_aa {
fxaa.enabled = false;
msaa.samples = 1;
msaa.level = MultiSampleLevel::Off;
info!("NO AA");
}
if set_no_aa | set_fxaa {
msaa.samples = 1;
msaa.level = MultiSampleLevel::Off;
}
if fxaa_low {
fxaa.edge_threshold = Sensitivity::Low;
Expand All @@ -150,7 +152,7 @@ fn toggle_fxaa(keys: Res<Input<KeyCode>>, mut query: Query<&mut Fxaa>, mut msaa:
}
if set_fxaa {
fxaa.enabled = true;
msaa.samples = 1;
msaa.level = MultiSampleLevel::Off;
info!("FXAA {}", fxaa.edge_threshold.get_str());
}
}
Expand Down
10 changes: 5 additions & 5 deletions examples/3d/msaa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
//! Ultimately we plan on supporting whatever is natively supported on a given device.
//! Check out [this issue](https://github.com/gfx-rs/wgpu/issues/1832) for more info.

use bevy::prelude::*;
use bevy::{prelude::*, render::view::MultiSampleLevel};

fn main() {
App::new()
.insert_resource(Msaa { samples: 4 })
.insert_resource(Msaa::from(MultiSampleLevel::Off))
.add_plugins(DefaultPlugins)
.add_startup_system(setup)
.add_system(cycle_msaa)
Expand Down Expand Up @@ -46,12 +46,12 @@ fn setup(

fn cycle_msaa(input: Res<Input<KeyCode>>, mut msaa: ResMut<Msaa>) {
if input.just_pressed(KeyCode::M) {
if msaa.samples == 4 {
if msaa.level == MultiSampleLevel::Sample4 {
info!("Not using MSAA");
msaa.samples = 1;
msaa.level = MultiSampleLevel::Off;
} else {
info!("Using 4x MSAA");
msaa.samples = 4;
msaa.level = MultiSampleLevel::Sample4;
}
}
}
4 changes: 2 additions & 2 deletions examples/3d/transparency_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
//! Shows the effects of different blend modes.
//! The `fade_transparency` system smoothly changes the transparency over time.

use bevy::prelude::*;
use bevy::{prelude::*, render::view::MultiSampleLevel};

fn main() {
App::new()
.insert_resource(Msaa { samples: 4 })
.insert_resource(Msaa::from(MultiSampleLevel::Sample4))
Copy link
Member

Choose a reason for hiding this comment

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

Rather than complicate this with two types, maybe it should just be a flat enum? Msaa::from(MultiSampleLevel::Sample4) is a mouth full. Why not just Msaa::Sample1?

.add_plugins(DefaultPlugins)
.add_startup_system(setup)
.add_system(fade_transparency)
Expand Down
2 changes: 1 addition & 1 deletion examples/shader/shader_instancing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ fn queue_custom(
) {
let draw_custom = transparent_3d_draw_functions.read().id::<DrawCustom>();

let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples());

for (view, mut transparent_phase) in &mut views {
let view_key = msaa_key | MeshPipelineKey::from_hdr(view.hdr);
Expand Down