Skip to content

Commit

Permalink
Merge #696
Browse files Browse the repository at this point in the history
696: Rustification of Extensions and SamplerDescriptor r=kvark a=cwfitzgerald

**Connections**

This begins the rustificaiton of `wgpu-types` as discussed in #689, starting with `Extensions` and `SamplerDescriptor`.

#691 and the resulting discussion was used to advise the shape of the extension struct.

**Description**

The PR should be fairly straight forward. As discussed, I have left extensions as a bitflag until the webgpu-native issue can be opened and discussed regarding allocation of extensions.

The most controversial part of this will be the `AnisotropyLevel` enum. I created it for two reasons, one that matters, one that doesn't:
 - It means that, with the exception of enabling the aniso extension (and lod_bias), the sampler is correct by construction, which is helpful to both beginners and experts. It also better exposes the range of valid values and means less panics in user code.
 - It saves a byte in the `Option<u8>` 😄 

I definitely think that, if at all possible, we should have explicitly typed enums for "numbers" which have a limited amount of valid values (so _not_ lod bias, though that may be better expressed, idk) to make it very explicit which values can be accepted. This also feels more "rusty" and reduces the amount of "magicness" in the interface.

Ofc, I'm not married to the idea.

**Testing**

Follow up PR into wgpu-rs will include conversion of the examples.

**TODO**

- [x] wgpu-native pr rolling up this PR and #690 (gfx-rs/wgpu-native#34)
- [x] wgpu-rs pr updating examples with the new structures (gfx-rs/wgpu-rs#345)

Co-authored-by: Connor Fitzgerald <[email protected]>
  • Loading branch information
bors[bot] and cwfitzgerald authored Jun 3, 2020
2 parents 78ab563 + ade7ce1 commit ad550be
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 32 deletions.
17 changes: 6 additions & 11 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1003,16 +1003,15 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let (device_guard, mut token) = hub.devices.read(&mut token);
let device = &device_guard[device_id];

if desc.anisotropy_clamp > 1 {
if let Some(clamp) = desc.anisotropy_clamp {
assert!(
device.extensions.anisotropic_filtering,
device.extensions.contains(wgt::Extensions::ANISOTROPIC_FILTERING),
"Anisotropic clamp may only be used when the anisotropic filtering extension is enabled"
);
let valid_clamp = desc.anisotropy_clamp <= MAX_ANISOTROPY
&& conv::is_power_of_two(desc.anisotropy_clamp as u32);
let valid_clamp = clamp <= MAX_ANISOTROPY && conv::is_power_of_two(clamp as u32);
assert!(
valid_clamp,
"Anisotropic clamp must be one of the values: 0, 1, 2, 4, 8, or 16"
"Anisotropic clamp must be one of the values: 1, 2, 4, 8, or 16"
);
}

Expand All @@ -1027,14 +1026,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
),
lod_bias: hal::image::Lod(0.0),
lod_range: hal::image::Lod(desc.lod_min_clamp)..hal::image::Lod(desc.lod_max_clamp),
comparison: conv::map_compare_function(desc.compare),
comparison: desc.compare.and_then(conv::map_compare_function),
border: hal::image::PackedColor(0),
normalized: true,
anisotropy_clamp: if desc.anisotropy_clamp > 1 {
Some(desc.anisotropy_clamp)
} else {
None
},
anisotropy_clamp: desc.anisotropy_clamp,
};

let sampler = resource::Sampler {
Expand Down
14 changes: 10 additions & 4 deletions wgpu-core/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let features = adapter.raw.physical_device.features();

wgt::Extensions {
anisotropic_filtering: features.contains(hal::Features::SAMPLER_ANISOTROPY),
}
let mut extensions = wgt::Extensions::default();
extensions.set(
wgt::Extensions::ANISOTROPIC_FILTERING,
features.contains(hal::Features::SAMPLER_ANISOTROPY),
);
extensions
}

pub fn adapter_limits<B: GfxBackend>(&self, adapter_id: AdapterId) -> wgt::Limits {
Expand Down Expand Up @@ -601,7 +604,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

// Check features needed by extensions
if desc.extensions.anisotropic_filtering {
if desc
.extensions
.contains(wgt::Extensions::ANISOTROPIC_FILTERING)
{
assert!(
available_features.contains(hal::Features::SAMPLER_ANISOTROPY),
"Missing feature SAMPLER_ANISOTROPY for anisotropic filtering extension"
Expand Down
35 changes: 18 additions & 17 deletions wgpu-types/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,18 @@ impl From<Backend> for BackendBit {
}
}

#[repr(C)]
#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct Extensions {
/// This is a native only extension. Support is planned to be added to webgpu,
/// but it is not yet implemented.
///
/// https://github.com/gpuweb/gpuweb/issues/696
pub anisotropic_filtering: bool,
bitflags::bitflags! {
#[repr(transparent)]
#[derive(Default)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct Extensions: u64 {
/// This is a native only extension. Support is planned to be added to webgpu,
/// but it is not yet implemented.
///
/// https://github.com/gpuweb/gpuweb/issues/696
const ANISOTROPIC_FILTERING = 0x01;
}
}

#[repr(C)]
Expand Down Expand Up @@ -178,7 +180,7 @@ pub fn read_spirv<R: io::Read + io::Seek>(mut x: R) -> io::Result<Vec<u32>> {
bitflags::bitflags! {
#[repr(transparent)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct ShaderStage: u32 {
const NONE = 0;
const VERTEX = 1;
Expand Down Expand Up @@ -401,7 +403,7 @@ pub enum TextureFormat {
bitflags::bitflags! {
#[repr(transparent)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct ColorWrite: u32 {
const RED = 1;
const GREEN = 2;
Expand Down Expand Up @@ -939,7 +941,6 @@ impl Default for FilterMode {
}
}

#[repr(C)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
Expand All @@ -953,12 +954,12 @@ pub struct SamplerDescriptor<L> {
pub mipmap_filter: FilterMode,
pub lod_min_clamp: f32,
pub lod_max_clamp: f32,
pub compare: CompareFunction,
pub compare: Option<CompareFunction>,
/// Anisotropic filtering extension must be enabled if this value is
/// anything other than 0 and 1.
/// anything other than 0 or 1.
///
/// Valid values are 0, 1, 2, 4, 8, and 16.
pub anisotropy_clamp: u8,
/// Valid values: 1, 2, 4, 8, and 16.
pub anisotropy_clamp: Option<u8>,
}

impl<L> SamplerDescriptor<L> {
Expand Down

0 comments on commit ad550be

Please sign in to comment.