Implement some window properties for x11 shell by maan2003 · Pull Request #1785 · linebender/druid

maan2003 · 2021-05-18T13:00:41Z

Implements:

set_size
get_size
set_min_size
resizable
get_position
set_position
set_level
set_window_state (only for WindowBuilder)

druid-shell/src/platform/x11/window.rs

maan2003 · 2021-05-19T02:59:55Z

Also, dropdown example from nursery is now functional on x11 backend.

druid-shell/src/platform/x11/window.rs

psychon

I just received a notification due to jneem's comments and apparently I have an old, unsubmitted review laying around... huh?

druid-shell/src/platform/x11/window.rs

psychon · 2021-05-23T08:58:34Z

druid-shell/src/platform/x11/window.rs

+        if let WindowLevel::DropDown = level {
+            log_x11!(conn.change_window_attributes(
+                self.id,
+                &ChangeWindowAttributesAux::new().override_redirect(1),


Uhm. Making a window override-redirect after it is already shown sounds like a bad idea.

Can you somehow make this only work before the window is mapped? However, no idea how to do easily do so...

Could we remove the window-level-changing methods from WindowHandle and only allow them on WindowBuilder? Is there a use case on any platform for changing a WindowLevel after the window was created?

Looks like gtk backend is also doing this

Hm, you are right. Gtk really does this: https://github.com/GNOME/gtk/blob/77f32a69c0a35ffd3a1a542a6057c1c6d13c793d/gdk/x11/gdkwindow-x11.c#L3534-L3553

Still seems like a really bad idea to me. If the WM already reparented the window, the resulting state will not work properly. You can move and resize your window insides of the WM's container window, but the container itself is not affected / does not follow...

druid-shell/src/platform/x11/window.rs

jneem · 2021-06-12T16:15:10Z

Thanks! I was actually wondering if we need WindowHandle::set_level on any platform, but I'll open another issue for that.

maan2003 · 2021-06-12T18:22:48Z

either set_position or get_position is not correct. When using dropdown example from nursery on a window manager with a top bar. the drop down is not positioned correctly. Zulip discussion

psychon · 2021-06-12T19:46:20Z

Well, since you are not assigning a window gravity (and IMO you should not), the functions might not really do exactly what you expect. The ConfigureRequest generated by set_position will be interpreted by the WM to be the position for the top left corner of the frame window (Edit: This is NorthWestGravity which is the default window gravity according to ICCCM, I think). The implementation for get_position however calculates the position of the top left corner of your child window.

Both positions are correct, they just do not "do the same thing". Which one is correct depends on the exact definition of these method (and I guess is not really defined).

Edit: Never mind, I am now confused by _get_position

Edit: My new guess is that the positions returns is off by two times the size of the window decoration. One of these comes from my comment in get_geometry and the other time due to window gravity. I guess you also need to implement handling of _NET_FRAME_EXTENTS from EWMH and hope that the WM supports that (check _NET_SUPPORTED on the root window).

psychon · 2021-06-12T19:48:08Z

druid-shell/src/platform/x11/window.rs

+            let geom = conn.get_geometry(window.id)?.reply()?;
+            let cord = conn
+                .translate_coordinates(window.id, geom.root, geom.x, geom.y)?
+                .reply()?;


Shouldn't the above be translate_coordinates(window.id, geom.root, 0, 0)? You want to know which coordinates the point (0,0) in your window has in the root window. As written, this function figures out the position of the window inside of its parent window (which will be the size of the top/left window decorations in a reparenting WM) and then translates this to the root window.

maan2003 · 2021-06-13T04:34:19Z

translate_coordinates(window.id, geom.root, 0, 0) fixes it for gnome and xmonad( with top bar). Not sure if _NET_FRAME_EXTENTS will be required

psychon · 2021-06-13T04:55:33Z

_NET_FRAME_EXTENTS does not appear in https://github.com/xmonad/xmonad, so I guess it is unsupported. And the open issues seem to suggest that xmonad does not implement ICCCM's window gravities anyway, but I guess a tiling WM can get away with that easily...

No idea if it is possible with xmonad for a window to request to be floating, but I would expect that a floating window requesting position (0,0) will end up with its window decorations off-screen. :-(

maan2003 · 2021-06-13T05:12:02Z

working with gnome, kde, lxde, xmonad

gnome

kde

lxde

xmonad

Implement some window properties for x11 shell

6a1f2c3

maan2003 commented May 18, 2021

View reviewed changes

druid-shell/src/platform/x11/window.rs Outdated Show resolved Hide resolved

druid-shell/src/platform/x11/window.rs Outdated Show resolved Hide resolved

maan2003 added the S-waiting-on-author waits for changes from the submitter label May 19, 2021

Refactor

b00ba66

maan2003 added S-needs-review waits for review and removed S-waiting-on-author waits for changes from the submitter labels May 20, 2021

jneem reviewed May 31, 2021

View reviewed changes

druid-shell/src/platform/x11/window.rs Outdated Show resolved Hide resolved

jneem reviewed May 31, 2021

View reviewed changes

druid-shell/src/platform/x11/window.rs Outdated Show resolved Hide resolved

psychon reviewed May 31, 2021

View reviewed changes

psychon approved these changes Jun 12, 2021

View reviewed changes

druid-shell/src/platform/x11/window.rs Show resolved Hide resolved

Suggestions from code review

2a75b0e

maan2003 force-pushed the x11-win-props branch from 5cecf8f to 2a75b0e Compare June 12, 2021 09:23

window width OR height can't be 0

e704572

jneem approved these changes Jun 12, 2021

View reviewed changes

jneem mentioned this pull request Jun 12, 2021

Do we need WindowHandle::set_level? #1824

Closed

psychon reviewed Jun 12, 2021

View reviewed changes

translate coordinates correctly

a6c9469

Merge branch 'master' into x11-win-props

07f6a65

maan2003 merged commit d2e46e6 into linebender:master Jun 14, 2021

xStrom removed the S-needs-review waits for review label May 24, 2022

Conversation

maan2003 commented May 18, 2021

Uh oh!

Uh oh!

Uh oh!

maan2003 commented May 19, 2021

Uh oh!

Uh oh!

Uh oh!

psychon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

psychon May 23, 2021

Choose a reason for hiding this comment

Uh oh!

jneem Jun 1, 2021

Choose a reason for hiding this comment

Uh oh!

maan2003 Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

psychon Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jneem commented Jun 12, 2021

Uh oh!

maan2003 commented Jun 12, 2021

Uh oh!

psychon commented Jun 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psychon Jun 12, 2021

Choose a reason for hiding this comment

Uh oh!

maan2003 Jun 13, 2021

Choose a reason for hiding this comment

Uh oh!

maan2003 commented Jun 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psychon commented Jun 13, 2021

Uh oh!

maan2003 commented Jun 13, 2021

gnome

kde

lxde

xmonad

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

psychon commented Jun 12, 2021 •

edited

Loading

maan2003 commented Jun 13, 2021 •

edited

Loading