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

xwayland: Use configure request mask to position surfaces #308

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Conversation

soreau
Copy link
Member

@soreau soreau commented Oct 5, 2019

The mask contains information about whether or not to use the
position and size fields. Since size is determined by commit,
we only concern ourselves with the position here. This requires
wlroots #1843. This fixes bugs with positioning xwayland surfaces
with and without place plugin enabled. Fixes #307.

@soreau soreau force-pushed the fix-307 branch 6 times, most recently from 461e461 to 95c2ce2 Compare October 5, 2019 23:16
if ((ev->mask & XCB_CONFIG_WINDOW_X) && (ev->mask & XCB_CONFIG_WINDOW_Y))
{
self_positioned = true;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need the else here? Wlroots will already set x and y to xsurface->x/y

Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed or else xterm is placed at 0,0 instead of 100,100 without place enabled.

Copy link
Member

Choose a reason for hiding this comment

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

Okay but this is still wrong because ev->x/y is supposed to be global coordinates, and geometry.x/y is geometry relative to the view output.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be fixed now.


/* Freely-floating X windows might have set a geometry hint, try to
* take this into account */
if (!tiled_edges && !fullscreen && !parent && !self_positioned)
Copy link
Member

Choose a reason for hiding this comment

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

I just realize this, but we may not need this at all. Can you try removing the if and its contents?

My logic is the following: if we had a configure request before mapping, we have already applied it so the view is at the correct place.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed or else xterm is placed as if it doesn't have decorations, without place enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed on IRC, it doesn't really matter if place is disabled.

@soreau soreau force-pushed the fix-307 branch 3 times, most recently from 9add43f to 6bad967 Compare October 9, 2019 16:57
The mask contains information about whether or not to use the
position and size fields. Since size is determined by commit,
we only concern ourselves with the position here. This requires
wlroots #1843. This fixes bugs with positioning xwayland surfaces
with and without place plugin enabled. Fixes #307. Thanks to
ammen99 for the initial work on this.
Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ammen99 ammen99 merged commit 7557994 into master Oct 9, 2019
@ammen99 ammen99 deleted the fix-307 branch October 14, 2019 12:09
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.

Some xwayland windows get wrong position
2 participants