Remove HalSurface and fix layout assumptions in AnySurface and AnyDerive

This commit is contained in:
John-John Tedro 2024-01-17 18:41:02 +01:00 committed by Erich Gubler
parent 84ba4e5461
commit f45d500c1c
8 changed files with 182 additions and 187 deletions

View File

@ -1,92 +1,93 @@
use wgt::Backend;
/// The `AnySurface` type: a `Arc` of a `HalSurface<A>` for any backend `A`.
/// The `AnySurface` type: a `Arc` of a `A::Surface` for any backend `A`.
use crate::hal_api::HalApi;
use crate::instance::HalSurface;
use std::any::Any;
use std::fmt;
use std::sync::Arc;
use std::mem::ManuallyDrop;
use std::ptr::NonNull;
/// A `Arc` of a `HalSurface<A>`, for any backend `A`.
struct AnySurfaceVtable {
// We oppurtunistically store the backend here, since we now it will be used
// with backend selection and it can be stored in static memory.
backend: Backend,
// Drop glue which knows how to drop the stored data.
drop: unsafe fn(*mut ()),
}
/// An `A::Surface`, for any backend `A`.
///
/// Any `AnySurface` is just like an `Arc<HalSurface<A>>`, except that the
/// `A` type parameter is erased. To access the `Surface`, you must
/// downcast to a particular backend with the \[`downcast_ref`\] or
/// \[`take`\] methods.
pub struct AnySurface(Arc<dyn Any + 'static>);
/// Any `AnySurface` is just like an `A::Surface`, except that the `A` type
/// parameter is erased. To access the `Surface`, you must downcast to a
/// particular backend with the \[`downcast_ref`\] or \[`take`\] methods.
pub struct AnySurface {
data: NonNull<()>,
vtable: &'static AnySurfaceVtable,
}
impl AnySurface {
/// Return an `AnySurface` that holds an owning `Arc` to `HalSurface`.
pub fn new<A: HalApi>(surface: HalSurface<A>) -> AnySurface {
AnySurface(Arc::new(surface))
/// Construct an `AnySurface` that owns an `A::Surface`.
pub fn new<A: HalApi>(surface: A::Surface) -> AnySurface {
unsafe fn drop_glue<A: HalApi>(ptr: *mut ()) {
unsafe {
_ = Box::from_raw(ptr.cast::<A::Surface>());
}
}
let data = NonNull::from(Box::leak(Box::new(surface)));
AnySurface {
data: data.cast(),
vtable: &AnySurfaceVtable {
backend: A::VARIANT,
drop: drop_glue::<A>,
},
}
}
/// Get the backend this surface was created through.
pub fn backend(&self) -> Backend {
#[cfg(vulkan)]
if self.downcast_ref::<hal::api::Vulkan>().is_some() {
return Backend::Vulkan;
}
#[cfg(metal)]
if self.downcast_ref::<hal::api::Metal>().is_some() {
return Backend::Metal;
}
#[cfg(dx12)]
if self.downcast_ref::<hal::api::Dx12>().is_some() {
return Backend::Dx12;
}
#[cfg(gles)]
if self.downcast_ref::<hal::api::Gles>().is_some() {
return Backend::Gl;
}
Backend::Empty
self.vtable.backend
}
/// If `self` is an `Arc<HalSurface<A>>`, return a reference to the
/// HalSurface.
pub fn downcast_ref<A: HalApi>(&self) -> Option<&HalSurface<A>> {
self.0.downcast_ref::<HalSurface<A>>()
/// If `self` refers to an `A::Surface`, returns a reference to it.
pub fn downcast_ref<A: HalApi>(&self) -> Option<&A::Surface> {
if A::VARIANT != self.vtable.backend {
return None;
}
// SAFETY: We just checked the instance above implicitly by the backend
// that it was statically constructed through.
Some(unsafe { &*self.data.as_ptr().cast::<A::Surface>() })
}
/// If `self` is an `Arc<HalSurface<A>>`, returns that.
pub fn take<A: HalApi>(self) -> Option<Arc<HalSurface<A>>> {
// `Arc::downcast` returns `Arc<T>`, but requires that `T` be `Sync` and
// `Send`, and this is not the case for `HalSurface` in wasm builds.
//
// But as far as I can see, `Arc::downcast` has no particular reason to
// require that `T` be `Sync` and `Send`; the steps used here are sound.
if (self.0).is::<HalSurface<A>>() {
// Turn the `Arc`, which is a pointer to an `ArcInner` struct, into
// a pointer to the `ArcInner`'s `data` field. Carry along the
// vtable from the original `Arc`.
let raw_erased: *const (dyn Any + 'static) = Arc::into_raw(self.0);
// Remove the vtable, and supply the concrete type of the `data`.
let raw_typed: *const HalSurface<A> = raw_erased.cast::<HalSurface<A>>();
// Convert the pointer to the `data` field back into a pointer to
// the `ArcInner`, and restore reference-counting behavior.
let arc_typed: Arc<HalSurface<A>> = unsafe {
// Safety:
// - We checked that the `dyn Any` was indeed a `HalSurface<A>` above.
// - We're calling `Arc::from_raw` on the same pointer returned
// by `Arc::into_raw`, except that we stripped off the vtable
// pointer.
// - The pointer must still be live, because we've borrowed `self`,
// which holds another reference to it.
// - The format of a `ArcInner<dyn Any>` must be the same as
// that of an `ArcInner<HalSurface<A>>`, or else `AnyHalSurface::new`
// wouldn't be possible.
Arc::from_raw(raw_typed)
};
Some(arc_typed)
} else {
None
/// If `self` is an `Arc<A::Surface>`, returns that.
pub fn take<A: HalApi>(self) -> Option<A::Surface> {
if A::VARIANT != self.vtable.backend {
return None;
}
// Disable drop glue, since we're returning the owned surface. The
// caller will be responsible for dropping it.
let this = ManuallyDrop::new(self);
// SAFETY: We just checked the instance above implicitly by the backend
// that it was statically constructed through.
Some(unsafe { *Box::from_raw(this.data.as_ptr().cast::<A::Surface>()) })
}
}
impl Drop for AnySurface {
fn drop(&mut self) {
unsafe { (self.vtable.drop)(self.data.as_ptr()) }
}
}
impl fmt::Debug for AnySurface {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("AnySurface")
f.debug_tuple("AnySurface")
.field(&self.vtable.backend)
.finish()
}
}

View File

@ -1,72 +1,98 @@
use wgt::Backend;
use super::Device;
/// The `AnyDevice` type: a pointer to a `Device<A>` for any backend `A`.
use crate::hal_api::HalApi;
use std::any::Any;
use std::fmt;
use std::mem::ManuallyDrop;
use std::ptr::NonNull;
use std::sync::Arc;
struct AnyDeviceVtable {
// We oppurtunistically store the backend here, since we now it will be used
// with backend selection and it can be stored in static memory.
backend: Backend,
// Drop glue which knows how to drop the stored data.
drop: unsafe fn(*mut ()),
}
/// A pointer to a `Device<A>`, for any backend `A`.
///
/// Any `AnyDevice` is just like an `Arc<Device<A>>`, except that the
/// `A` type parameter is erased. To access the `Device`, you must
/// downcast to a particular backend with the \[`downcast_ref`\] or
/// \[`downcast_clone`\] methods.
pub struct AnyDevice(Arc<dyn Any + 'static>);
/// Any `AnyDevice` is just like an `Arc<Device<A>>`, except that the `A` type
/// parameter is erased. To access the `Device`, you must downcast to a
/// particular backend with the \[`downcast_ref`\] or \[`downcast_clone`\]
/// methods.
pub struct AnyDevice {
data: NonNull<()>,
vtable: &'static AnyDeviceVtable,
}
impl AnyDevice {
/// Return an `AnyDevice` that holds an owning `Arc` pointer to `device`.
pub fn new<A: HalApi>(device: Arc<Device<A>>) -> AnyDevice {
AnyDevice(device)
unsafe fn drop_glue<A: HalApi>(ptr: *mut ()) {
// Drop the arc this instance is holding.
unsafe {
_ = Arc::from_raw(ptr.cast::<A::Surface>());
}
}
// SAFETY: The pointer returned by Arc::into_raw is gauranteed to be
// non-null.
let data = unsafe { NonNull::new_unchecked(Arc::into_raw(device).cast_mut()) };
AnyDevice {
data: data.cast(),
vtable: &AnyDeviceVtable {
backend: A::VARIANT,
drop: drop_glue::<A>,
},
}
}
/// If `self` is an `Arc<Device<A>>`, return a reference to the
/// device.
pub fn downcast_ref<A: HalApi>(&self) -> Option<&Device<A>> {
self.0.downcast_ref::<Device<A>>()
if self.vtable.backend != A::VARIANT {
return None;
}
// SAFETY: We just checked the instance above implicitly by the backend
// that it was statically constructed through.
Some(unsafe { &*(self.data.as_ptr().cast::<Device<A>>()) })
}
/// If `self` is an `Arc<Device<A>>`, return a clone of that.
pub fn downcast_clone<A: HalApi>(&self) -> Option<Arc<Device<A>>> {
// `Arc::downcast` returns `Arc<T>`, but requires that `T` be `Sync` and
// `Send`, and this is not the case for `Device` in wasm builds.
//
// But as far as I can see, `Arc::downcast` has no particular reason to
// require that `T` be `Sync` and `Send`; the steps used here are sound.
if (self.0).is::<Device<A>>() {
// Get an owned Arc.
let clone = self.0.clone();
// Turn the `Arc`, which is a pointer to an `ArcInner` struct, into
// a pointer to the `ArcInner`'s `data` field. Carry along the
// vtable from the original `Arc`.
let raw_erased: *const (dyn Any + 'static) = Arc::into_raw(clone);
// Remove the vtable, and supply the concrete type of the `data`.
let raw_typed: *const Device<A> = raw_erased.cast::<Device<A>>();
// Convert the pointer to the `data` field back into a pointer to
// the `ArcInner`, and restore reference-counting behavior.
let arc_typed: Arc<Device<A>> = unsafe {
// Safety:
// - We checked that the `dyn Any` was indeed a `Device<A>` above.
// - We're calling `Arc::from_raw` on the same pointer returned
// by `Arc::into_raw`, except that we stripped off the vtable
// pointer.
// - The pointer must still be live, because we've borrowed `self`,
// which holds another reference to it.
// - The format of a `ArcInner<dyn Any>` must be the same as
// that of an `ArcInner<Device<A>>`, or else `AnyDevice::new`
// wouldn't be possible.
Arc::from_raw(raw_typed)
};
Some(arc_typed)
} else {
None
if self.vtable.backend != A::VARIANT {
return None;
}
// We need to prevent the destructor of the arc from running, since it
// refers to the instance held by this object. Dropping it would
// invalidate this object.
//
// SAFETY: We just checked the instance above implicitly by the backend
// that it was statically constructed through.
let this =
ManuallyDrop::new(unsafe { Arc::from_raw(self.data.as_ptr().cast::<Device<A>>()) });
// Cloning it increases the reference count, and we return a new arc
// instance.
Some((*this).clone())
}
}
impl Drop for AnyDevice {
fn drop(&mut self) {
unsafe { (self.vtable.drop)(self.data.as_ptr()) }
}
}
impl fmt::Debug for AnyDevice {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("AnyDevice")
write!(f, "AnyDevice")
}
}

View File

@ -1966,11 +1966,7 @@ impl Global {
let caps = unsafe {
let suf = A::get_surface(surface);
let adapter = &device.adapter;
match adapter
.raw
.adapter
.surface_capabilities(suf.unwrap().raw.as_ref())
{
match adapter.raw.adapter.surface_capabilities(suf.unwrap()) {
Some(caps) => caps,
None => break E::UnsupportedQueueFamily,
}
@ -2055,7 +2051,6 @@ impl Global {
match unsafe {
A::get_surface(surface)
.unwrap()
.raw
.configure(device.raw(), &hal_config)
} {
Ok(()) => (),

View File

@ -3,7 +3,7 @@ use wgt::{Backend, WasmNotSendSync};
use crate::{
global::Global,
hub::Hub,
instance::{HalSurface, Instance, Surface},
instance::{Instance, Surface},
};
pub trait HalApi: hal::Api + 'static + WasmNotSendSync {
@ -11,7 +11,7 @@ pub trait HalApi: hal::Api + 'static + WasmNotSendSync {
fn create_instance_from_hal(name: &str, hal_instance: Self::Instance) -> Instance;
fn instance_as_hal(instance: &Instance) -> Option<&Self::Instance>;
fn hub(global: &Global) -> &Hub<Self>;
fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>>;
fn get_surface(surface: &Surface) -> Option<&Self::Surface>;
}
impl HalApi for hal::api::Empty {
@ -25,7 +25,7 @@ impl HalApi for hal::api::Empty {
fn hub(_: &Global) -> &Hub<Self> {
unimplemented!("called empty api")
}
fn get_surface(_: &Surface) -> Option<&HalSurface<Self>> {
fn get_surface(_: &Surface) -> Option<&Self::Surface> {
unimplemented!("called empty api")
}
}
@ -46,8 +46,8 @@ impl HalApi for hal::api::Vulkan {
fn hub(global: &Global) -> &Hub<Self> {
&global.hubs.vulkan
}
fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
surface.raw.downcast_ref()
fn get_surface(surface: &Surface) -> Option<&Self::Surface> {
surface.raw.downcast_ref::<Self>()
}
}
@ -67,8 +67,8 @@ impl HalApi for hal::api::Metal {
fn hub(global: &Global) -> &Hub<Self> {
&global.hubs.metal
}
fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
surface.raw.downcast_ref()
fn get_surface(surface: &Surface) -> Option<&Self::Surface> {
surface.raw.downcast_ref::<Self>()
}
}
@ -88,8 +88,8 @@ impl HalApi for hal::api::Dx12 {
fn hub(global: &Global) -> &Hub<Self> {
&global.hubs.dx12
}
fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
surface.raw.downcast_ref()
fn get_surface(surface: &Surface) -> Option<&Self::Surface> {
surface.raw.downcast_ref::<Self>()
}
}
@ -110,7 +110,7 @@ impl HalApi for hal::api::Gles {
fn hub(global: &Global) -> &Hub<Self> {
&global.hubs.gl
}
fn get_surface(surface: &Surface) -> Option<&HalSurface<Self>> {
surface.raw.downcast_ref()
fn get_surface(surface: &Surface) -> Option<&Self::Surface> {
surface.raw.downcast_ref::<Self>()
}
}

View File

@ -109,7 +109,7 @@ use crate::{
command::{CommandBuffer, RenderBundle},
device::{queue::Queue, Device},
hal_api::HalApi,
instance::{Adapter, HalSurface, Surface},
instance::{Adapter, Surface},
pipeline::{ComputePipeline, RenderPipeline, ShaderModule},
registry::{Registry, RegistryReport},
resource::{Buffer, QuerySet, Sampler, StagingBuffer, Texture, TextureView},
@ -243,7 +243,7 @@ impl<A: HalApi> Hub<A> {
if let Some(device) = present.device.downcast_ref::<A>() {
let suf = A::get_surface(surface);
unsafe {
suf.unwrap().raw.unconfigure(device.raw());
suf.unwrap().unconfigure(device.raw());
//TODO: we could destroy the surface here
}
}
@ -260,10 +260,10 @@ impl<A: HalApi> Hub<A> {
}
}
pub(crate) fn surface_unconfigure(&self, device: &Device<A>, surface: &HalSurface<A>) {
pub(crate) fn surface_unconfigure(&self, device: &Device<A>, surface: &A::Surface) {
unsafe {
use hal::Surface;
surface.raw.unconfigure(device.raw());
surface.unconfigure(device.raw());
}
}

View File

@ -21,11 +21,6 @@ use thiserror::Error;
pub type RequestAdapterOptions = wgt::RequestAdapterOptions<SurfaceId>;
type HalInstance<A> = <A as hal::Api>::Instance;
//TODO: remove this
#[derive(Clone)]
pub struct HalSurface<A: HalApi> {
pub raw: Arc<A::Surface>,
}
#[derive(Clone, Debug, Error)]
#[error("Limit '{name}' value {requested} is better than allowed {allowed}")]
@ -118,30 +113,22 @@ impl Instance {
}
pub(crate) fn destroy_surface(&self, surface: Surface) {
fn destroy<A: HalApi>(_: A, instance: &Option<A::Instance>, surface: AnySurface) {
fn destroy<A: HalApi>(instance: &Option<A::Instance>, surface: AnySurface) {
unsafe {
if let Some(surface) = surface.take::<A>() {
if let Some(suf) = Arc::into_inner(surface) {
if let Some(raw) = Arc::into_inner(suf.raw) {
instance.as_ref().unwrap().destroy_surface(raw);
} else {
panic!("Surface cannot be destroyed because is still in use");
}
} else {
panic!("Surface cannot be destroyed because is still in use");
}
if let Some(suf) = surface.take::<A>() {
instance.as_ref().unwrap().destroy_surface(suf);
}
}
}
match surface.raw.backend() {
#[cfg(vulkan)]
Backend::Vulkan => destroy(hal::api::Vulkan, &self.vulkan, surface.raw),
Backend::Vulkan => destroy::<hal::api::Vulkan>(&self.vulkan, surface.raw),
#[cfg(metal)]
Backend::Metal => destroy(hal::api::Metal, &self.metal, surface.raw),
Backend::Metal => destroy::<hal::api::Metal>(&self.metal, surface.raw),
#[cfg(dx12)]
Backend::Dx12 => destroy(hal::api::Dx12, &self.dx12, surface.raw),
Backend::Dx12 => destroy::<hal::api::Dx12>(&self.dx12, surface.raw),
#[cfg(gles)]
Backend::Gl => destroy(hal::api::Gles, &self.gl, surface.raw),
Backend::Gl => destroy::<hal::api::Gles>(&self.gl, surface.raw),
_ => unreachable!(),
}
}
@ -182,7 +169,7 @@ impl Surface {
adapter
.raw
.adapter
.surface_capabilities(&suf.raw)
.surface_capabilities(suf)
.ok_or(GetSurfaceSupportError::Unsupported)?
};
@ -223,7 +210,7 @@ impl<A: HalApi> Adapter<A> {
// This could occur if the user is running their app on Wayland but Vulkan does not support
// VK_KHR_wayland_surface.
match suf {
Some(suf) => unsafe { self.raw.adapter.surface_capabilities(&suf.raw) }.is_some(),
Some(suf) => unsafe { self.raw.adapter.surface_capabilities(suf) }.is_some(),
None => false,
}
}
@ -502,7 +489,7 @@ impl Global {
) -> Option<Result<AnySurface, hal::InstanceError>> {
inst.as_ref().map(|inst| unsafe {
match inst.create_surface(display_handle, window_handle) {
Ok(raw) => Ok(AnySurface::new(HalSurface::<A> { raw: Arc::new(raw) })),
Ok(raw) => Ok(AnySurface::new::<A>(raw)),
Err(e) => Err(e),
}
})
@ -557,19 +544,17 @@ impl Global {
presentation: Mutex::new(None),
info: ResourceInfo::new("<Surface>"),
raw: {
let hal_surface: HalSurface<hal::api::Metal> = self
let hal_surface = self
.instance
.metal
.as_ref()
.map(|inst| HalSurface {
raw: Arc::new(
// we don't want to link to metal-rs for this
#[allow(clippy::transmute_ptr_to_ref)]
inst.create_surface_from_layer(unsafe { std::mem::transmute(layer) }),
), //acquired_texture: None,
.map(|inst| {
// we don't want to link to metal-rs for this
#[allow(clippy::transmute_ptr_to_ref)]
inst.create_surface_from_layer(unsafe { std::mem::transmute(layer) })
})
.unwrap();
AnySurface::new(hal_surface)
AnySurface::new::<hal::api::Metal>(hal_surface)
},
};
@ -592,15 +577,13 @@ impl Global {
presentation: Mutex::new(None),
info: ResourceInfo::new("<Surface>"),
raw: {
let hal_surface: HalSurface<hal::api::Dx12> = self
let hal_surface = self
.instance
.dx12
.as_ref()
.map(|inst| HalSurface {
raw: Arc::new(unsafe { inst.create_surface_from_visual(visual as _) }),
})
.map(|inst| unsafe { inst.create_surface_from_visual(visual as _) })
.unwrap();
AnySurface::new(hal_surface)
AnySurface::new::<hal::api::Dx12>(hal_surface)
},
};
@ -623,17 +606,13 @@ impl Global {
presentation: Mutex::new(None),
info: ResourceInfo::new("<Surface>"),
raw: {
let hal_surface: HalSurface<hal::api::Dx12> = self
let hal_surface = self
.instance
.dx12
.as_ref()
.map(|inst| HalSurface {
raw: Arc::new(unsafe {
inst.create_surface_from_surface_handle(surface_handle)
}),
})
.map(|inst| unsafe { inst.create_surface_from_surface_handle(surface_handle) })
.unwrap();
AnySurface::new(hal_surface)
AnySurface::new::<hal::api::Dx12>(hal_surface)
},
};
@ -656,17 +635,15 @@ impl Global {
presentation: Mutex::new(None),
info: ResourceInfo::new("<Surface>"),
raw: {
let hal_surface: HalSurface<hal::api::Dx12> = self
let hal_surface = self
.instance
.dx12
.as_ref()
.map(|inst| HalSurface {
raw: Arc::new(unsafe {
inst.create_surface_from_swap_chain_panel(swap_chain_panel as _)
}),
.map(|inst| unsafe {
inst.create_surface_from_swap_chain_panel(swap_chain_panel as _)
})
.unwrap();
AnySurface::new(hal_surface)
AnySurface::new::<hal::api::Dx12>(hal_surface)
},
};
@ -814,7 +791,7 @@ impl Global {
surface.is_some()
&& exposed
.adapter
.surface_capabilities(&surface.unwrap().raw)
.surface_capabilities(surface.unwrap())
.is_some()
});
}

View File

@ -163,7 +163,6 @@ impl Global {
let suf = A::get_surface(surface.as_ref());
let (texture_id, status) = match unsafe {
suf.unwrap()
.raw
.acquire_texture(Some(std::time::Duration::from_millis(
FRAME_TIMEOUT_MS as u64,
)))
@ -339,7 +338,7 @@ impl Global {
Err(hal::SurfaceError::Lost)
} else if !has_work.load(Ordering::Relaxed) {
log::error!("No work has been submitted for this frame");
unsafe { suf.unwrap().raw.discard_texture(raw.take().unwrap()) };
unsafe { suf.unwrap().discard_texture(raw.take().unwrap()) };
Err(hal::SurfaceError::Outdated)
} else {
unsafe {
@ -347,7 +346,7 @@ impl Global {
.raw
.as_ref()
.unwrap()
.present(&suf.unwrap().raw, raw.take().unwrap())
.present(suf.unwrap(), raw.take().unwrap())
}
}
}
@ -427,7 +426,7 @@ impl Global {
has_work: _,
} => {
if surface_id == parent_id {
unsafe { suf.unwrap().raw.discard_texture(raw.take().unwrap()) };
unsafe { suf.unwrap().discard_texture(raw.take().unwrap()) };
} else {
log::warn!("Surface texture is outdated");
}

View File

@ -1007,10 +1007,7 @@ impl Global {
profiling::scope!("Surface::as_hal");
let surface = self.surfaces.get(id).ok();
let hal_surface = surface
.as_ref()
.and_then(|surface| A::get_surface(surface))
.map(|surface| &*surface.raw);
let hal_surface = surface.as_ref().and_then(|surface| A::get_surface(surface));
hal_surface_callback(hal_surface)
}