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

window: add client side decorations interface #362

Merged
merged 2 commits into from
Mar 24, 2023

Conversation

kchibisov
Copy link
Member

While the new decorations look the same as the ones before, the
approach in drawing them was changed, and now decorations are more
instant mode, meaning that you create and drop them when needed.

Fixes #273.

--

This series also adds subcompositor and subsurface handlers. The parent surface stuff is
intended to be used from inside winit, yet, I haven't added it into it.

The example which is using CSD decorations is themed_window.

All the drawing was ported from before.

cc @PolyMeilex

@kchibisov kchibisov force-pushed the csd-decorations branch 6 times, most recently from b287561 to d85e846 Compare March 2, 2023 16:30
@@ -208,8 +208,8 @@ impl Window {
decoration.data::<WindowData>().and_then(|data| data.0.upgrade()).map(Window)
}

pub fn show_window_menu(&self, seat: &wl_seat::WlSeat, serial: u32, position: (u32, u32)) {
self.xdg_toplevel().show_window_menu(seat, serial, position.0 as i32, position.1 as i32);
pub fn show_window_menu(&self, seat: &wl_seat::WlSeat, serial: u32, position: (i32, i32)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

That was wrong, it's i32, because relative to main surface, so could be negative.

@kchibisov
Copy link
Member Author

You might notice, that set_hidden is missing, and the reason for it is that you want to hide CSD, you can simply drop the entire frame all together.

src/shell/xdg/frame/fallback_frame.rs Outdated Show resolved Hide resolved
src/shell/xdg/frame/mod.rs Outdated Show resolved Hide resolved
@i509VCB
Copy link
Member

i509VCB commented Mar 2, 2023

You might notice, that set_hidden is missing, and the reason for it is that you want to hide CSD, you can simply drop the entire frame all together.

For more heavyweight decorations it seems wasteful to throw away then start from scratch with internal state (the decorations might use EGL). Maybe a compromise for that scenario would be a way to detach decorations from a window and then reattach the decorations?

@kchibisov
Copy link
Member Author

For more heavyweight decorations it seems wasteful to throw away then start from scratch with internal state (the decorations might use EGL). Maybe a compromise for that scenario would be a way to detach decorations from a window and then reattach the decorations?

I mean why you should even consume resources when you were asked to use ServerSide again? Right now the API enforces shm, so it's not like you want to use EGL for them, if we want to allow using something other than shm for decorations the init should be different, and likely not attached to the frame, then in this case you might want hide actually.

@kchibisov kchibisov force-pushed the csd-decorations branch 5 times, most recently from 8592055 to 220b917 Compare March 2, 2023 21:41
@kchibisov
Copy link
Member Author

I've moved constructor, added back is_hidden and set_hidden.

@kchibisov
Copy link
Member Author

I wonder whether we should maybe put it in a separate crate somehow, the current interface has very few connections with Wayland what so ever. Though, Like the trait by itself doesn't force anything specific, other than asking for WlSurface in one place and for WindowStates in other, though the window states could be refactored all together.

What do you think? Though, I'm not sure how compatible it could be though.

@kchibisov
Copy link
Member Author

The sctk-adwaita update using this branch PolyMeilex/sctk-adwaita#22 (well, it's my winit branch, but it's a rebase of this one).

The winit patch which implements both sctk-adwaita(might be not yet by the time you click it) and default decorations rust-windowing/winit#2676

@kchibisov
Copy link
Member Author

Will need #366 to refactor this further, given right now we pass arrays around. It'll also have the ability to hide certain buttons.

/// The interface for the client side decorations.
pub trait DecorationsFrame: Sized {
/// Configurations for this frame.
type Config;
Copy link
Member

Choose a reason for hiding this comment

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

I'd argue the config doesn't really need to be part of the frame trait. The config is going to be dependent on the frame type anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the goal is to transparently pass it, we might remove it, sure?

/// effect for the end user.
fn click_point_moved(&mut self, surface: &WlSurface, x: f64, y: f64) -> Option<&str>;

/// All clicks left the decorations.
Copy link
Member

Choose a reason for hiding this comment

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

Clarify what was meant here?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's simply to move pointer away from the decoration frame.

Copy link
Member

Choose a reason for hiding this comment

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

Isn't moving the pointer off the frame possible even without clicking? I do understand that clicking and the client not doing an interactive resize or move is possible.

I could see more advanced decorations possibly highlighting parts of the frame if the pointer went over some point if not clicking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't moving the pointer off the frame possible even without clicking?

Ah, guess comment is what actually confuses you? because it should probably be all click points left the decorations.

Be aware that I use click terminology to show that we're not talking about wl_pointer events in the first place, you could use all other sorts of inputs with it, like touch.

The use case for such API is to simply leave so Hovered state will go away, for example when clicking on minimize button and then restoring back.

@kchibisov kchibisov force-pushed the csd-decorations branch 3 times, most recently from eb028ac to 4d5dcfe Compare March 17, 2023 04:06
@kchibisov
Copy link
Member Author

I don't think I have anything else left for this.

src/shell/xdg/frame/mod.rs Outdated Show resolved Hide resolved
@kchibisov kchibisov requested a review from i509VCB March 20, 2023 01:33
@kchibisov
Copy link
Member Author

ping.

Copy link
Member

@i509VCB i509VCB left a comment

Choose a reason for hiding this comment

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

This seems ready minus some minor commenfs

pub mod fallback_frame;

/// The interface for the client side decorations.
pub trait DecorationsFrame: Sized {
Copy link
Member

Choose a reason for hiding this comment

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

Decorations feels redundant in the name imo

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel like you'd need to rename the trait most of the time, so I've added the prefix, if anything, it's in a frame, not in decorations module.

src/subcompositor.rs Show resolved Hide resolved
While the new decorations look the same as the ones before, the
approach in drawing them was changed, and now decorations are more
instant mode, meaning that you create and drop them when needed.

Fixes Smithay#273.
@wash2 wash2 merged commit be8168c into Smithay:master Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.30 client side decorations for xdg shell
3 participants