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

Remove UnsafeFeatures as we decided the top level guard is not useful #764

Merged
merged 1 commit into from
Jul 7, 2020
Merged

Remove UnsafeFeatures as we decided the top level guard is not useful #764

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
1 change: 0 additions & 1 deletion player/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,6 @@ fn main() {
#[cfg(not(feature = "winit"))]
compatible_surface: None,
},
unsafe { wgt::UnsafeFeatures::allow() },
wgc::instance::AdapterInputs::IdSet(
&[wgc::id::TypedId::zip(0, 0, backend)],
|id| id.backend(),
Expand Down
27 changes: 5 additions & 22 deletions wgpu-core/src/instance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,11 @@ pub struct Adapter<B: hal::Backend> {
pub(crate) raw: hal::adapter::Adapter<B>,
features: wgt::Features,
limits: wgt::Limits,
unsafe_features: wgt::UnsafeFeatures,
life_guard: LifeGuard,
}

impl<B: hal::Backend> Adapter<B> {
fn new(raw: hal::adapter::Adapter<B>, unsafe_features: wgt::UnsafeFeatures) -> Self {
fn new(raw: hal::adapter::Adapter<B>) -> Self {
span!(_guard, INFO, "Adapter::new");

let adapter_features = raw.physical_device.features();
Expand Down Expand Up @@ -160,9 +159,6 @@ impl<B: hal::Backend> Adapter<B> {
wgt::Features::MULTI_DRAW_INDIRECT_COUNT,
adapter_features.contains(hal::Features::DRAW_INDIRECT_COUNT),
);
if unsafe_features.allowed() {
// Unsafe features go here
}

let adapter_limits = raw.physical_device.limits();

Expand All @@ -176,7 +172,6 @@ impl<B: hal::Backend> Adapter<B> {
raw,
features,
limits,
unsafe_features,
life_guard: LifeGuard::new(),
}
}
Expand Down Expand Up @@ -325,11 +320,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
self.surfaces.register_identity(id_in, surface, &mut token)
}

pub fn enumerate_adapters(
&self,
unsafe_features: wgt::UnsafeFeatures,
inputs: AdapterInputs<Input<G, AdapterId>>,
) -> Vec<AdapterId> {
pub fn enumerate_adapters(&self, inputs: AdapterInputs<Input<G, AdapterId>>) -> Vec<AdapterId> {
span!(_guard, INFO, "Instance::enumerate_adapters");

let instance = &self.instance;
Expand All @@ -341,7 +332,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
if let Some(inst) = instance_field {
if let Some(id_backend) = inputs.find(backend) {
for raw in inst.enumerate_adapters() {
let adapter = Adapter::new(raw, unsafe_features);
let adapter = Adapter::new(raw);
log::info!("Adapter {} {:?}", backend_info, adapter.raw.info);
adapters.push(backend_hub(self).adapters.register_identity(
id_backend.clone(),
Expand Down Expand Up @@ -369,7 +360,6 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
pub fn pick_adapter(
&self,
desc: &RequestAdapterOptions,
unsafe_features: wgt::UnsafeFeatures,
inputs: AdapterInputs<Input<G, AdapterId>>,
) -> Option<AdapterId> {
span!(_guard, INFO, "Instance::pick_adapter");
Expand Down Expand Up @@ -482,7 +472,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
backends_map! {
let map = |(info_adapter, id_backend, mut adapters_backend, backend_hub)| {
if selected < adapters_backend.len() {
let adapter = Adapter::new(adapters_backend.swap_remove(selected), unsafe_features);
let adapter = Adapter::new(adapters_backend.swap_remove(selected));
log::info!("Adapter {} {:?}", info_adapter, adapter.raw.info);
let id = backend_hub(self).adapters.register_identity(
id_backend.take().unwrap(),
Expand Down Expand Up @@ -585,14 +575,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let adapter = &adapter_guard[adapter_id];
let phd = &adapter.raw.physical_device;

// Verify all features were exposed by the adapter
if !adapter.unsafe_features.allowed() {
assert!(
!desc.features.intersects(wgt::Features::ALL_UNSAFE),
"Cannot enable unsafe features without passing UnsafeFeatures::allow() when getting an adapter. Enabled unsafe extensions: {:?}",
desc.features & wgt::Features::ALL_UNSAFE
)
}
// Verify all features were exposed by the adapter
if !adapter.features.contains(desc.features) {
return Err(RequestDeviceError::UnsupportedFeature(
desc.features - adapter.features,
Expand Down
35 changes: 0 additions & 35 deletions wgpu-types/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,46 +234,11 @@ bitflags::bitflags! {
const MULTI_DRAW_INDIRECT_COUNT = 0x0000_0000_0040_0000;
/// Features which are part of the upstream webgpu standard
const ALL_WEBGPU = 0x0000_0000_0000_FFFF;
/// Features that require activating the unsafe feature flag
const ALL_UNSAFE = 0xFFFF_0000_0000_0000;
/// Features that are only available when targeting native (not web)
const ALL_NATIVE = 0xFFFF_FFFF_FFFF_0000;
}
}

/// Marker type signalling if unsafe features are allowed to be enabled.
///
/// This doesn't enable any unsafe features, but must be set to `allow` if
/// an unsafe features is enabled.
///
/// The safety contract of safe Rust is that it is impossible to cause Undefined Behavior (UB)
/// from safe Rust. If a feature would allow UB to happen, it must preset an unsafe interface.
/// Enabling unsafe features is therefore an inherently unsafe operation.
#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Hash)]
#[cfg_attr(feature = "trace", derive(Serialize))]
#[cfg_attr(feature = "replay", derive(Deserialize))]
pub struct UnsafeFeatures {
allow_unsafe: bool,
}
impl UnsafeFeatures {
/// Allow unsafe features to be enabled. This is an unsafe function and by calling this
/// function, you assert that even with these features on, it is impossible to cause UB
/// from within safe Rust.
pub unsafe fn allow() -> Self {
Self { allow_unsafe: true }
}
/// Disallow unsafe features.
pub fn disallow() -> Self {
Self {
allow_unsafe: false,
}
}
/// Does this marker allow unsafe features.
pub fn allowed(self) -> bool {
self.allow_unsafe
}
}

/// Represents the sets of limits an adapter/device supports.
///
/// Limits "better" than the default must be supported by the adapter and requested when requesting
Expand Down