Skip to content

Commit

Permalink
Fix merge conflicts and improve id mngmnt
Browse files Browse the repository at this point in the history
  • Loading branch information
@gents83
gents83 committed Sep 16, 2023
1 parent 90c118d commit 45a618f
Show file tree
Hide file tree
Showing 10 changed files with 335 additions and 384 deletions.
11 changes: 6 additions & 5 deletions player/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,22 @@
#![cfg(not(target_arch = "wasm32"))]
#![warn(unsafe_op_in_unsafe_fn)]

use wgc::{device::trace, identity::IdentityManager};
use wgc::device::trace;

use std::{borrow::Cow, fs, path::Path, sync::Arc};
use std::{borrow::Cow, fs, path::Path};

pub struct IdentityPassThroughFactory;

impl<I: wgc::id::TypedId> wgc::identity::IdentityHandlerFactory<I> for IdentityPassThroughFactory {
type Input = I;
fn spawn(&self) -> Option<Arc<IdentityManager<I>>> {
None
}

fn input_to_id(id_in: Self::Input) -> I {
id_in
}

fn autogenerate_ids() -> bool {
false
}
}
impl wgc::identity::GlobalIdentityHandlerFactory for IdentityPassThroughFactory {}

Expand Down
2 changes: 1 addition & 1 deletion tests/tests/device.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ fn request_device_error_on_native() {
async fn request_device_error_message() {
// Not using initialize_test() because that doesn't let us catch the error
// nor .await anything
let (adapter, _surface_guard) = wgpu_test::initialize_adapter();
let (_instance, adapter, _surface_guard) = wgpu_test::initialize_adapter();

let device_error = adapter
.request_device(
Expand Down
13 changes: 8 additions & 5 deletions tests/tests/mem_leaks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,15 +156,16 @@ fn draw_test_with_reports(
let global_report = ctx.instance.generate_report();
let report = global_report.hub_report();
assert_eq!(report.buffers.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 1);
//1 is clear_view and 1 is user's texture_view
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.textures.num_allocated, 1);

drop(texture);

let global_report = ctx.instance.generate_report();
let report = global_report.hub_report();
assert_eq!(report.buffers.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.texture_views.num_kept_from_user, 1);
assert_eq!(report.textures.num_allocated, 1);
assert_eq!(report.textures.num_kept_from_user, 0);
Expand Down Expand Up @@ -203,7 +204,7 @@ fn draw_test_with_reports(
assert_eq!(report.compute_pipelines.num_allocated, 0);
assert_eq!(report.command_buffers.num_allocated, 1);
assert_eq!(report.render_bundles.num_allocated, 0);
assert_eq!(report.texture_views.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.textures.num_allocated, 1);

function(&mut rpass);
Expand All @@ -228,7 +229,7 @@ fn draw_test_with_reports(
assert_eq!(report.bind_group_layouts.num_kept_from_user, 0);
assert_eq!(report.bind_groups.num_allocated, 1);
assert_eq!(report.bind_groups.num_kept_from_user, 0);
assert_eq!(report.texture_views.num_allocated, 1);
assert_eq!(report.texture_views.num_allocated, 2);
assert_eq!(report.texture_views.num_kept_from_user, 0);
assert_eq!(report.buffers.num_allocated, 1);
assert_eq!(report.buffers.num_kept_from_user, 0);
Expand All @@ -250,8 +251,10 @@ fn draw_test_with_reports(
assert_eq!(report.bind_groups.num_allocated, 0);
assert_eq!(report.bind_group_layouts.num_allocated, 0);
assert_eq!(report.pipeline_layouts.num_allocated, 0);
assert_eq!(report.texture_views.num_allocated, 0);
assert_eq!(report.buffers.num_allocated, 0);
//surface is still there
assert_eq!(report.texture_views.num_allocated, 1);
assert_eq!(report.textures.num_allocated, 1);

drop(ctx.queue);
drop(ctx.device);
Expand Down
220 changes: 117 additions & 103 deletions wgpu-core/src/device/global.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use smallvec::SmallVec;

use wgt::{BufferAddress, TextureFormat};

use std::{borrow::Cow, iter, ops::Range, ptr, sync::Arc};
use std::{borrow::Cow, iter, ops::Range, ptr};

use super::{ImplicitPipelineIds, InvalidDevice, UserClosures};

Expand Down Expand Up @@ -132,122 +132,133 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let hub = A::hub(self);
let fid = hub.buffers.prepare::<G>(id_in);

let error = loop {
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
let device = match hub.devices.get(device_id) {
Ok(device) => device,
Err(_) => {
let id = fid.assign_error(desc.label.borrow_or_default());
return (id, Some(DeviceError::Invalid.into()));
}
};

if desc.usage.is_empty() {
// Per spec, `usage` must not be zero.
break resource::CreateBufferError::InvalidUsage(desc.usage);
if desc.usage.is_empty() {
// Per spec, `usage` must not be zero.
let id = fid.assign_error(desc.label.borrow_or_default());
return (
id,
Some(resource::CreateBufferError::InvalidUsage(desc.usage)),
);
}

#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
let mut desc = desc.clone();
let mapped_at_creation = std::mem::replace(&mut desc.mapped_at_creation, false);
if mapped_at_creation && !desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
desc.usage |= wgt::BufferUsages::COPY_DST;
}
trace.add(trace::Action::CreateBuffer(fid.id(), desc));
}

#[cfg(feature = "trace")]
if let Some(ref mut trace) = *device.trace.lock() {
let mut desc = desc.clone();
let mapped_at_creation = std::mem::replace(&mut desc.mapped_at_creation, false);
if mapped_at_creation && !desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
desc.usage |= wgt::BufferUsages::COPY_DST;
}
trace.add(trace::Action::CreateBuffer(fid.id(), desc));
let buffer = match device.create_buffer(device_id, desc, false) {
Ok(buffer) => buffer,
Err(e) => {
let id = fid.assign_error(desc.label.borrow_or_default());
return (id, Some(e));
}
};

let buffer = match device.create_buffer(device_id, desc, false) {
Ok(buffer) => buffer,
Err(e) => break e,
};
let (id, resource) = fid.assign(buffer);
log::info!("Created Buffer {:?} with {:?}", id, desc);

let buffer_use = if !desc.mapped_at_creation {
hal::BufferUses::empty()
} else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
// buffer is mappable, so we are just doing that at start
let map_size = buffer.size;
let ptr = if map_size == 0 {
std::ptr::NonNull::dangling()
} else {
match map_buffer(device.raw(), &buffer, 0, map_size, HostMap::Write) {
Ok(ptr) => ptr,
Err(e) => {
device.lock_life().schedule_resource_destruction(
queue::TempResource::Buffer(Arc::new(buffer)),
!0,
);
break e.into();
}
}
};
*buffer.map_state.lock() = resource::BufferMapState::Active {
ptr,
range: 0..map_size,
host: HostMap::Write,
};
hal::BufferUses::MAP_WRITE
let buffer_use = if !desc.mapped_at_creation {
hal::BufferUses::empty()
} else if desc.usage.contains(wgt::BufferUsages::MAP_WRITE) {
// buffer is mappable, so we are just doing that at start
let map_size = resource.size;
let ptr = if map_size == 0 {
std::ptr::NonNull::dangling()
} else {
// buffer needs staging area for initialization only
let stage_desc = wgt::BufferDescriptor {
label: Some(Cow::Borrowed(
"(wgpu internal) initializing unmappable buffer",
)),
size: desc.size,
usage: wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::COPY_SRC,
mapped_at_creation: false,
};
let stage = match device.create_buffer(device_id, &stage_desc, true) {
Ok(stage) => stage,
match map_buffer(device.raw(), &resource, 0, map_size, HostMap::Write) {
Ok(ptr) => ptr,
Err(e) => {
device.lock_life().schedule_resource_destruction(
queue::TempResource::Buffer(Arc::new(buffer)),
queue::TempResource::Buffer(resource),
!0,
);
break e;
hub.buffers
.force_replace_with_error(id, desc.label.borrow_or_default());
return (id, Some(e.into()));
}
};
let mapping = match unsafe { device.raw().map_buffer(stage.raw(), 0..stage.size) } {
Ok(mapping) => mapping,
Err(e) => {
let mut life_lock = device.lock_life();
life_lock.schedule_resource_destruction(
queue::TempResource::Buffer(Arc::new(buffer)),
!0,
);
life_lock.schedule_resource_destruction(
queue::TempResource::Buffer(Arc::new(stage)),
!0,
);
break DeviceError::from(e).into();
}
};

assert_eq!(buffer.size % wgt::COPY_BUFFER_ALIGNMENT, 0);
// Zero initialize memory and then mark both staging and buffer as initialized
// (it's guaranteed that this is the case by the time the buffer is usable)
unsafe { ptr::write_bytes(mapping.ptr.as_ptr(), 0, buffer.size as usize) };
buffer.initialization_status.write().drain(0..buffer.size);
stage.initialization_status.write().drain(0..buffer.size);

*buffer.map_state.lock() = resource::BufferMapState::Init {
ptr: mapping.ptr,
needs_flush: !mapping.is_coherent,
stage_buffer: Arc::new(stage),
};
hal::BufferUses::COPY_DST
}
};
*resource.map_state.lock() = resource::BufferMapState::Active {
ptr,
range: 0..map_size,
host: HostMap::Write,
};
hal::BufferUses::MAP_WRITE
} else {
// buffer needs staging area for initialization only
let stage_desc = wgt::BufferDescriptor {
label: Some(Cow::Borrowed(
"(wgpu internal) initializing unmappable buffer",
)),
size: desc.size,
usage: wgt::BufferUsages::MAP_WRITE | wgt::BufferUsages::COPY_SRC,
mapped_at_creation: false,
};
let stage = match device.create_buffer(device_id, &stage_desc, true) {
Ok(stage) => stage,
Err(e) => {
device
.lock_life()
.schedule_resource_destruction(queue::TempResource::Buffer(resource), !0);
hub.buffers
.force_replace_with_error(id, desc.label.borrow_or_default());
return (id, Some(e));
}
};
let stage_fid = hub.buffers.request::<G>();
let stage = stage_fid.init(stage);

let (id, resource) = fid.assign(buffer);
log::info!("Created Buffer {:?} with {:?}", id, desc);

device
.trackers
.lock()
.buffers
.insert_single(id, resource, buffer_use);
let mapping = match unsafe { device.raw().map_buffer(stage.raw(), 0..stage.size) } {
Ok(mapping) => mapping,
Err(e) => {
let mut life_lock = device.lock_life();
life_lock
.schedule_resource_destruction(queue::TempResource::Buffer(resource), !0);
life_lock.schedule_resource_destruction(queue::TempResource::Buffer(stage), !0);
hub.buffers
.force_replace_with_error(id, desc.label.borrow_or_default());
return (id, Some(DeviceError::from(e).into()));
}
};

return (id, None);
assert_eq!(resource.size % wgt::COPY_BUFFER_ALIGNMENT, 0);
// Zero initialize memory and then mark both staging and buffer as initialized
// (it's guaranteed that this is the case by the time the buffer is usable)
unsafe { ptr::write_bytes(mapping.ptr.as_ptr(), 0, resource.size as usize) };
resource
.initialization_status
.write()
.drain(0..resource.size);
stage.initialization_status.write().drain(0..resource.size);

*resource.map_state.lock() = resource::BufferMapState::Init {
ptr: mapping.ptr,
needs_flush: !mapping.is_coherent,
stage_buffer: stage,
};
hal::BufferUses::COPY_DST
};

let id = fid.assign_error(desc.label.borrow_or_default());
(id, Some(error))
device
.trackers
.lock()
.buffers
.insert_single(id, resource, buffer_use);

(id, None)
}

/// Assign `id_in` an error with the given `label`.
Expand Down Expand Up @@ -455,7 +466,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let hub = A::hub(self);

if let Some(buffer) = hub.buffers.unregister(buffer_id) {
if buffer.is_unique() {
if buffer.ref_count() == 1 {
buffer.destroy().ok();
}

Expand Down Expand Up @@ -548,7 +559,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
},
};
let view = device.create_texture_view(&resource, &desc).unwrap();
clear_views.push(Arc::new(view));
let view_fid = hub.texture_views.request::<G>();
let view = view_fid.init(view);
clear_views.push(view);
}
}
let mut clear_mode = resource.clear_mode.write();
Expand Down Expand Up @@ -798,6 +811,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
}

#[allow(unused_unsafe)]
pub fn texture_create_view<A: HalApi>(
&self,
texture_id: id::TextureId,
Expand Down Expand Up @@ -825,7 +839,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
});
}

let view = match device.create_texture_view(&texture, desc) {
let view = match unsafe { device.create_texture_view(&texture, desc) } {
Ok(view) => view,
Err(e) => break e,
};
Expand Down
Loading

0 comments on commit 45a618f

Please sign in to comment.