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

Jump to bottom

Cherry pick bug fixes #5323

Closed
wants to merge 4 commits into from
Closed

Cherry pick bug fixes #5323

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8430692
[PATCH] Handle user callbacks in surface_configure
nical Oct 9, 2023
c53b3a0
Properly handle user callbacks in surface_configure (#4227)
nical Oct 11, 2023
3565d64
HACK: roll back the device.valid check
bonsairobo Mar 1, 2024
3ee424f
Fix Panic in Surface Configure (#4635)
cwfitzgerald Nov 6, 2023
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
224 changes: 121 additions & 103 deletions wgpu-core/src/device/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2105,130 +2105,148 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}

log::info!("configuring surface with {:?}", config);
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);

let error = 'outer: loop {
let device = match device_guard.get(device_id) {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
.lock()
.add(trace::Action::ConfigureSurface(surface_id, config.clone()));
}
// User callbacks must not be called while we are holding locks.
let user_callbacks;
{
let hub = A::hub(self);
let mut token = Token::root();

let surface = match surface_guard.get_mut(surface_id) {
Ok(surface) => surface,
Err(_) => break E::InvalidSurface,
};
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 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 device = match device_guard.get(device_id) {
Ok(device) => device,
Err(_) => break DeviceError::Invalid.into(),
};
// if !device.valid {
// break DeviceError::Invalid.into();
// }

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,
};
#[cfg(feature = "trace")]
if let Some(ref trace) = device.trace {
trace
.lock()
.add(trace::Action::ConfigureSurface(surface_id, config.clone()));
}
if config.format.remove_srgb_suffix() != format.remove_srgb_suffix() {
break 'outer E::InvalidViewFormat(*format, config.format);

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 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,
};
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;
}
if let Err(error) = validate_surface_configuration(&mut hal_config, &caps) {
break error;
}

// Wait for all work to finish before configuring the surface.
if let Err(e) = device.maintain(hub, wgt::Maintain::Wait, &mut token) {
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;
};
Expand Down
Loading