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

Fix initial position of X11 override-redirects #283

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

Conversation

djpohly
Copy link

@djpohly djpohly commented Oct 16, 2023

Presumably this fixes #228. The position of the scene node for XWayland override-redirect windows was never being set.

Presumably this fixes cage-kiosk#228.  The position of the scene node for XWayland
override-redirect windows was never being set.
@emersion
Copy link
Contributor

  • When an Xwayland view is created, shouldn't we read back xwayland_view->xwayland_surface->{x,y} if it's unmanaged?
  • When set_geometry is triggered, we update view->{x,y} but we don't update the scene node position?

@djpohly
Copy link
Author

djpohly commented Oct 16, 2023

Good points, both. Updated.

Edit: Further updated to fix a segfault when a window is mapped, unmapped, moved, and mapped again. There's a little bit of null-checking clunkiness here, because set_geometry can happen while a window is unmapped, but the scene node in cage only exists while the window is mapped. An alternative would be for each view to have an empty "container" node that always exists, then add/remove the surface tree as a child of that container on map/unmap.

A null check had to be added because, since a view's scene isn't created
until .map, there's a possibility that .set_geometry is called before
the scene node exists.
@djpohly djpohly force-pushed the position-override-redirect branch from 802b747 to 14ba8d3 Compare October 17, 2023 15:37
@Supreeeme
Copy link
Contributor

Is there anything blocking this from getting merged? It fixes positioning issues with Steam's dropdown menus.

Comment on lines +177 to +178
xwayland_view->view.lx = xwayland_surface->x;
xwayland_view->view.ly = xwayland_surface->y;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, should we add a if (!xwayland_view_should_manage(view)) here?

Copy link
Author

Choose a reason for hiding this comment

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

Then it wouldn't cover the case where override_redirect is set between creating and mapping the window, e.g.:

Window w = XCreateSimpleWindow(dpy, root, 100, 100, 50, 50, 3, 0xffff00, 0xff00ff);

XSetWindowAttributes attrs = { .override_redirect = True };
XChangeWindowAttributes(dpy, w, CWOverrideRedirect, &attrs);

// Should appear at position (100, 100)
XMapWindow(dpy, w);
XFlush(dpy);

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm… What about override redirect changing after the window is unmapped? Sounds like we should probably set up a set_override_redirect listener to handle all cases correctly?

I'd be more comfortable only copying the X11 position when the window is O-R.

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.

xwayland views may reposition themselves
3 participants