Skip to content

Commit

Permalink
Properly handle user callbacks in surface_configure (#4227)
Browse files Browse the repository at this point in the history 
* Properly handle user callbacks in surface_configure

* Add a changelog entry
  • Loading branch information
@nical
nical authored Oct 11, 2023
1 parent 35ebd4a commit b3135b9
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 115 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ By @teoxoy in [#4185](https://github.com/gfx-rs/wgpu/pull/4185)
- `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036).
- Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057).
- Add `Feature::SHADER_UNUSED_VERTEX_OUTPUT` to allow unused vertex shader outputs. By @Aaron1011 in [#4116](https://github.com/gfx-rs/wgpu/pull/4116).
- Fix a panic in `surface_configure`. By @nical in [#4220](https://github.com/gfx-rs/wgpu/pull/4220) and [#4227](https://github.com/gfx-rs/wgpu/pull/4227)

#### Vulkan
- Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772).
Expand Down
231 changes: 116 additions & 115 deletions wgpu-core/src/device/global.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2262,152 +2262,153 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Ok(())
}

// User callbacks must not be called while we are holding locks.
let mut user_callbacks = None;

log::info!("configuring surface with {:?}", config);

let error = 'outer: loop {
let hub = A::hub(self);
let mut token = Token::root();

let (mut surface_guard, mut token) = self.surfaces.write(&mut token);
let (adapter_guard, mut token) = hub.adapters.read(&mut token);
let (device_guard, mut token) = hub.devices.read(&mut token);
// User callbacks must not be called while we are holding locks.
let user_callbacks;
{
let (mut surface_guard, mut token) = self.surfaces.write(&mut token);
let (adapter_guard, mut token) = hub.adapters.read(&mut token);
let (device_guard, mut token) = hub.devices.read(&mut token);

let device = match device_guard.get(device_id) {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}
let device = match device_guard.get(device_id) {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
if !device.valid {
break DeviceError::Invalid.into();
}

#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
.lock()
.add(trace::Action::ConfigureSurface(surface_id, config.clone()));
}
#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
.lock()
.add(trace::Action::ConfigureSurface(surface_id, config.clone()));
}

let surface = match surface_guard.get_mut(surface_id) {
Ok(surface) => surface,
Err(_) => break E::InvalidSurface,
};
let surface = match surface_guard.get_mut(surface_id) {
Ok(surface) => surface,
Err(_) => break E::InvalidSurface,
};

let caps = unsafe {
let suf = A::get_surface(surface);
let adapter = &adapter_guard[device.adapter_id.value];
match adapter.raw.adapter.surface_capabilities(&suf.unwrap().raw) {
Some(caps) => caps,
None => break E::UnsupportedQueueFamily,
}
};
let caps = unsafe {
let suf = A::get_surface(surface);
let adapter = &adapter_guard[device.adapter_id.value];
match adapter.raw.adapter.surface_capabilities(&suf.unwrap().raw) {
Some(caps) => caps,
None => break E::UnsupportedQueueFamily,
}
};

let mut hal_view_formats = vec![];
for format in config.view_formats.iter() {
if *format == config.format {
continue;
}
if !caps.formats.contains(&config.format) {
break 'outer E::UnsupportedFormat {
requested: config.format,
available: caps.formats,
};
}
if config.format.remove_srgb_suffix() != format.remove_srgb_suffix() {
break 'outer E::InvalidViewFormat(*format, config.format);
let mut hal_view_formats = vec![];
for format in config.view_formats.iter() {
if *format == config.format {
continue;
}
if !caps.formats.contains(&config.format) {
break 'outer E::UnsupportedFormat {
requested: config.format,
available: caps.formats,
};
}
if config.format.remove_srgb_suffix() != format.remove_srgb_suffix() {
break 'outer E::InvalidViewFormat(*format, config.format);
}
hal_view_formats.push(*format);
}
hal_view_formats.push(*format);
}

if !hal_view_formats.is_empty() {
if let Err(missing_flag) =
device.require_downlevel_flags(wgt::DownlevelFlags::SURFACE_VIEW_FORMATS)
{
break 'outer E::MissingDownlevelFlags(missing_flag);
if !hal_view_formats.is_empty() {
if let Err(missing_flag) =
device.require_downlevel_flags(wgt::DownlevelFlags::SURFACE_VIEW_FORMATS)
{
break 'outer E::MissingDownlevelFlags(missing_flag);
}
}
}

let num_frames = present::DESIRED_NUM_FRAMES
.clamp(*caps.swap_chain_sizes.start(), *caps.swap_chain_sizes.end());
let mut hal_config = hal::SurfaceConfiguration {
swap_chain_size: num_frames,
present_mode: config.present_mode,
composite_alpha_mode: config.alpha_mode,
format: config.format,
extent: wgt::Extent3d {
width: config.width,
height: config.height,
depth_or_array_layers: 1,
},
usage: conv::map_texture_usage(config.usage, hal::FormatAspects::COLOR),
view_formats: hal_view_formats,
};

if let Err(error) = validate_surface_configuration(&mut hal_config, &caps) {
break error;
}
let num_frames = present::DESIRED_NUM_FRAMES
.clamp(*caps.swap_chain_sizes.start(), *caps.swap_chain_sizes.end());
let mut hal_config = hal::SurfaceConfiguration {
swap_chain_size: num_frames,
present_mode: config.present_mode,
composite_alpha_mode: config.alpha_mode,
format: config.format,
extent: wgt::Extent3d {
width: config.width,
height: config.height,
depth_or_array_layers: 1,
},
usage: conv::map_texture_usage(config.usage, hal::FormatAspects::COLOR),
view_formats: hal_view_formats,
};

// Wait for all work to finish before configuring the surface.
match device.maintain(hub, wgt::Maintain::Wait, &mut token) {
Ok((closures, _)) => {
user_callbacks = Some(closures);
if let Err(error) = validate_surface_configuration(&mut hal_config, &caps) {
break error;
}
Err(e) => {
break e.into();

// Wait for all work to finish before configuring the surface.
match device.maintain(hub, wgt::Maintain::Wait, &mut token) {
Ok((closures, _)) => {
user_callbacks = closures;
}
Err(e) => {
break e.into();
}
}
}

// All textures must be destroyed before the surface can be re-configured.
if let Some(present) = surface.presentation.take() {
if present.acquired_texture.is_some() {
break E::PreviousOutputExists;
// All textures must be destroyed before the surface can be re-configured.
if let Some(present) = surface.presentation.take() {
if present.acquired_texture.is_some() {
break E::PreviousOutputExists;
}
}
}

// TODO: Texture views may still be alive that point to the texture.
// this will allow the user to render to the surface texture, long after
// it has been removed.
//
// https://github.com/gfx-rs/wgpu/issues/4105
// TODO: Texture views may still be alive that point to the texture.
// this will allow the user to render to the surface texture, long after
// it has been removed.
//
// https://github.com/gfx-rs/wgpu/issues/4105

match unsafe {
A::get_surface_mut(surface)
.unwrap()
.raw
.configure(&device.raw, &hal_config)
} {
Ok(()) => (),
Err(error) => {
break match error {
hal::SurfaceError::Outdated | hal::SurfaceError::Lost => E::InvalidSurface,
hal::SurfaceError::Device(error) => E::Device(error.into()),
hal::SurfaceError::Other(message) => {
log::error!("surface configuration failed: {}", message);
E::InvalidSurface
match unsafe {
A::get_surface_mut(surface)
.unwrap()
.raw
.configure(&device.raw, &hal_config)
} {
Ok(()) => (),
Err(error) => {
break match error {
hal::SurfaceError::Outdated | hal::SurfaceError::Lost => {
E::InvalidSurface
}
hal::SurfaceError::Device(error) => E::Device(error.into()),
hal::SurfaceError::Other(message) => {
log::error!("surface configuration failed: {}", message);
E::InvalidSurface
}
}
}
}

surface.presentation = Some(present::Presentation {
device_id: Stored {
value: id::Valid(device_id),
ref_count: device.life_guard.add_ref(),
},
config: config.clone(),
num_frames,
acquired_texture: None,
});
}

surface.presentation = Some(present::Presentation {
device_id: Stored {
value: id::Valid(device_id),
ref_count: device.life_guard.add_ref(),
},
config: config.clone(),
num_frames,
acquired_texture: None,
});
user_callbacks.fire();

return None;
};

if let Some(callbacks) = user_callbacks {
callbacks.fire();
}

Some(error)
}

Expand Down

0 comments on commit b3135b9

Please sign in to comment.