Skip to content

Update smithay with layer shell updates #1726

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update smithay with layer shell updates #1726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
931fb22
Define a `with_toplevel_state`, generic over `pending` bool
ids1024 Oct 9, 2025
c30e3d5
Update `smithay`
ids1024 Oct 10, 2025
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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,4 @@ cosmic-protocols = { git = "https://github.com/pop-os//cosmic-protocols", branch
cosmic-client-toolkit = { git = "https://github.com/pop-os//cosmic-protocols", branch = "main" }

[patch.crates-io]
smithay = { git = "https://github.com/smithay/smithay.git", rev = "eb45814" }
smithay = { git = "https://github.com/smithay/smithay.git", rev = "d743e1a" }
138 changes: 52 additions & 86 deletions src/shell/element/surface.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ use smithay::{
wayland::{
compositor::{SurfaceData, TraversalAction, with_states, with_surface_tree_downward},
seat::WaylandFocus,
shell::xdg::{SurfaceCachedState, ToplevelSurface, XdgToplevelSurfaceData},
shell::xdg::{
SurfaceCachedState, ToplevelCachedState, ToplevelSurface, XdgToplevelSurfaceData,
},
},
xwayland::{X11Surface, xwm::X11Relatable},
};
Expand Down Expand Up @@ -214,18 +216,9 @@ impl CosmicSurface {

pub fn is_activated(&self, pending: bool) -> bool {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if pending {
toplevel.with_pending_state(|pending| {
pending.states.contains(ToplevelState::Activated)
})
} else {
toplevel
.current_state()
.states
.contains(ToplevelState::Activated)
}
}
WindowSurface::Wayland(toplevel) => with_toplevel_state(toplevel, pending, |state| {
state.is_some_and(|state| state.states.contains(ToplevelState::Activated))
}),
WindowSurface::X11(surface) => surface.is_activated(),
}
}
Expand Down Expand Up @@ -255,18 +248,13 @@ impl CosmicSurface {
.and_then(|data| data.lock().unwrap().mode.map(|m| m != KdeMode::Server))
});

let xdg_state = if pending {
toplevel.with_pending_state(|pending| {
pending
let xdg_state = with_toplevel_state(toplevel, pending, |state| {
state.and_then(|state| {
state
.decoration_mode
.map(|mode| mode == DecorationMode::ClientSide)
})
} else {
toplevel
.current_state()
.decoration_mode
.map(|mode| mode == DecorationMode::ClientSide)
};
});

kde_state.or(xdg_state).unwrap_or(true)
}
Expand All @@ -278,7 +266,9 @@ impl CosmicSurface {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if enable {
let previous_decoration_state = toplevel.current_state().decoration_mode;
let previous_decoration_state = toplevel.with_committed_state(|state| {
state.map_or_else(Default::default, |state| state.decoration_mode)
});
if PreferredDecorationMode::is_unset(&self.0) {
PreferredDecorationMode::update(&self.0, previous_decoration_state);
}
Expand Down Expand Up @@ -313,18 +303,9 @@ impl CosmicSurface {
pub fn is_resizing(&self, pending: bool) -> Option<bool> {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if pending {
Some(toplevel.with_pending_state(|pending| {
pending.states.contains(ToplevelState::Resizing)
}))
} else {
Some(
toplevel
.current_state()
.states
.contains(ToplevelState::Resizing),
)
}
Some(with_toplevel_state(toplevel, pending, |state| {
state.is_some_and(|state| state.states.contains(ToplevelState::Resizing))
}))
}
WindowSurface::X11(_surface) => None,
}
Expand All @@ -346,18 +327,9 @@ impl CosmicSurface {
pub fn is_tiled(&self, pending: bool) -> Option<bool> {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if pending {
Some(toplevel.with_pending_state(|pending| {
pending.states.contains(ToplevelState::TiledLeft)
}))
} else {
Some(
toplevel
.current_state()
.states
.contains(ToplevelState::TiledLeft),
)
}
Some(with_toplevel_state(toplevel, pending, |state| {
state.is_some_and(|state| state.states.contains(ToplevelState::TiledLeft))
}))
}
WindowSurface::X11(_surface) => None,
}
Expand All @@ -384,18 +356,9 @@ impl CosmicSurface {

pub fn is_fullscreen(&self, pending: bool) -> bool {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if pending {
toplevel.with_pending_state(|pending| {
pending.states.contains(ToplevelState::Fullscreen)
})
} else {
toplevel
.current_state()
.states
.contains(ToplevelState::Fullscreen)
}
}
WindowSurface::Wayland(toplevel) => with_toplevel_state(toplevel, pending, |state| {
state.is_some_and(|state| state.states.contains(ToplevelState::Fullscreen))
}),
WindowSurface::X11(surface) => surface.is_fullscreen(),
}
}
Expand All @@ -417,18 +380,9 @@ impl CosmicSurface {

pub fn is_maximized(&self, pending: bool) -> bool {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => {
if pending {
toplevel.with_pending_state(|pending| {
pending.states.contains(ToplevelState::Maximized)
})
} else {
toplevel
.current_state()
.states
.contains(ToplevelState::Maximized)
}
}
WindowSurface::Wayland(toplevel) => with_toplevel_state(toplevel, pending, |state| {
state.is_some_and(|state| state.states.contains(ToplevelState::Maximized))
}),
WindowSurface::X11(surface) => surface.is_maximized(),
}
}
Expand Down Expand Up @@ -543,10 +497,9 @@ impl CosmicSurface {
.lock()
.unwrap();
attrs
.configure_serial
.last_acked
.as_ref()
.map(|s| s >= serial)
.unwrap_or(false)
.is_some_and(|configure| configure.serial >= *serial)
}),
WindowSurface::X11(_surface) => true,
}
Expand All @@ -555,17 +508,12 @@ impl CosmicSurface {
pub fn serial_past(&self, serial: &Serial) -> bool {
match self.0.underlying_surface() {
WindowSurface::Wayland(toplevel) => with_states(toplevel.wl_surface(), |states| {
let attrs = states
.data_map
.get::<XdgToplevelSurfaceData>()
.unwrap()
.lock()
.unwrap();
attrs
.current_serial
let mut guard = states.cached_state.get::<ToplevelCachedState>();
guard
.current()
.last_acked
.as_ref()
.map(|s| s >= serial)
.unwrap_or(false)
.is_some_and(|configure| configure.serial >= *serial)
}),
WindowSurface::X11(_surface) => true,
}
Expand All @@ -583,12 +531,18 @@ impl CosmicSurface {
.unwrap();

let current_server = attributes.current_server_state();
if attributes.current.size == current_server.size {
let mut guard = states.cached_state.get::<ToplevelCachedState>();
if guard
.current()
.last_acked
.as_ref()
.is_some_and(|configure| configure.state.size == current_server.size)
{
Copy link
Member

@Drakulix Drakulix Oct 31, 2025

Choose a reason for hiding this comment

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

Can't we simplify this whole thing with the new with_committed_state helper or am I misunderstanding how that works internally? Seems to me, that we don't need to manually check the ack'd serial.

Copy link
Member Author

@ids1024 ids1024 Nov 3, 2025

Choose a reason for hiding this comment

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

Yeah, I guess with_committed_state() would work here; though we also use states.data_map, in addition to the states.cached_state, to get the current_server_state().

I guess it wouldn't really be a problem to take the with_states lock twice (but not at the same time) here? Though it might not simplify things much.

// The window had committed for our previous size change, so we can
// change the size again.
trace!(
"current size matches server size: {:?}",
attributes.current.size
guard.current().last_acked.as_ref().unwrap().state.size
);
true
} else {
Expand Down Expand Up @@ -978,3 +932,15 @@ where
self.0.render_elements(renderer, location, scale, alpha)
}
}

fn with_toplevel_state<T, F: FnOnce(Option<&smithay::wayland::shell::xdg::ToplevelState>) -> T>(
toplevel: &ToplevelSurface,
pending: bool,
cb: F,
) -> T {
if pending {
toplevel.with_pending_state(|pending| cb(Some(pending)))
} else {
toplevel.with_committed_state(|committed| cb(committed))
}
}
18 changes: 9 additions & 9 deletions src/wayland/handlers/xdg_shell/popup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use smithay::{
wayland::{
compositor::{get_role, with_states},
seat::WaylandFocus,
shell::xdg::{PopupSurface, ToplevelSurface, XDG_POPUP_ROLE, XdgPopupSurfaceData},
shell::xdg::{
PopupCachedState, PopupSurface, ToplevelSurface, XDG_POPUP_ROLE, XdgPopupSurfaceData,
},
},
};
use tracing::warn;
Expand Down Expand Up @@ -87,14 +89,12 @@ pub fn update_reactive_popups<'a>(
for (popup, _) in PopupManager::popups_for_surface(toplevel.wl_surface()) {
match popup {
PopupKind::Xdg(surface) => {
let positioner = with_states(surface.wl_surface(), |states| {
let attributes = states
.data_map
.get::<XdgPopupSurfaceData>()
.unwrap()
.lock()
.unwrap();
attributes.current.positioner
let positioner = with_states(&surface.wl_surface(), |states| {
let mut guard = states.cached_state.get::<PopupCachedState>();
guard
.current()
.last_acked
.map_or_else(Default::default, |configure| configure.state.positioner)
});
if positioner.reactive {
let anchor_point = loc + positioner.get_anchor_point().as_global();
Expand Down