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.

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

output: store a weak ref in the global to avoid... #1589

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 20 additions & 14 deletions src/wayland/output/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,22 @@ where
data_init: &mut DataInit<'_, D>,
) {
let client_scale = state.client_compositor_state(client).clone_client_scale();
let output = data_init.init(

let Some(output) = global_data.output.upgrade() else {
tracing::warn!("trying to bind a destroyed output, forgot to disable a output global?");
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should and can just return here. Any request to the output would otherwise trigger a panic, given we didn't initialize it's data.

The warning is fine (and a good idea), but I think we should just initialize the OutputUserData with Weak::new.

};

let wl_output = data_init.init(
resource,
OutputUserData {
output: global_data.output.downgrade(),
output: global_data.output.clone(),
last_client_scale: AtomicU32::new(client_scale.load(Ordering::Acquire)),
client_scale,
},
);

let mut inner = global_data.output.inner.0.lock().unwrap();
let mut inner = output.inner.0.lock().unwrap();

let span = warn_span!("output_bind", name = inner.name);
let _enter = span.enter();
Expand All @@ -64,7 +70,7 @@ where
warn!("Output is used with not preferred mode set");
}

inner.send_geometry_to(&output);
inner.send_geometry_to(&wl_output);

for &mode in &inner.modes {
let mut flags = WMode::empty();
Expand All @@ -74,32 +80,32 @@ where
if Some(mode) == inner.preferred_mode {
flags |= WMode::Preferred;
}
output.mode(flags, mode.size.w, mode.size.h, mode.refresh);
wl_output.mode(flags, mode.size.w, mode.size.h, mode.refresh);
}

if output.version() >= 4 {
output.name(inner.name.clone());
output.description(inner.description.clone())
if wl_output.version() >= 4 {
wl_output.name(inner.name.clone());
wl_output.description(inner.description.clone())
}

if output.version() >= 2 {
output.scale(inner.scale.integer_scale());
output.done();
if wl_output.version() >= 2 {
wl_output.scale(inner.scale.integer_scale());
wl_output.done();
}

// Send enter for surfaces already on this output.
for surface in &inner.surfaces {
if let Ok(surface) = surface.upgrade() {
if surface.client().as_ref() == Some(client) {
surface.enter(&output);
surface.enter(&wl_output);
}
}
}

inner.instances.push(output.downgrade());
inner.instances.push(wl_output.downgrade());

drop(inner);
state.output_bound(global_data.output.clone(), output);
state.output_bound(output, wl_output);
}
}

Expand Down
9 changes: 7 additions & 2 deletions src/wayland/output/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ pub struct OutputManagerState {
/// Internal data of a wl_output global
#[derive(Debug)]
pub struct WlOutputData {
output: Output,
output: WeakOutput,
}

/// Events initiated by the clients interacting with outputs
Expand Down Expand Up @@ -204,7 +204,12 @@ impl Output {
{
info!(output = self.name(), "Creating new wl_output");
self.inner.0.lock().unwrap().handle = Some(display.backend_handle().downgrade());
display.create_global::<D, WlOutput, _>(4, WlOutputData { output: self.clone() })
display.create_global::<D, WlOutput, _>(
4,
WlOutputData {
output: self.downgrade(),
},
)
}

/// Attempt to retrieve a [`Output`] from an existing resource
Expand Down
Loading