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

wayland: fix vertical resize #11420

Conversation

christoph-heinrich
Copy link
Contributor

The first implementation is really ugly, so I tried it a different way. I'm not sure it's actually preferable over the first attempt, so I made it a separate commit to compare.

@christoph-heinrich christoph-heinrich changed the title Fix wayland vertical resize wayland: fix vertical resize Mar 7, 2023
@Dudemanguy
Copy link
Member

Dudemanguy commented Mar 7, 2023

The second commit was more along the lines of what I was thinking but you should choose based on which dimension has the larger absolute value difference since both can change.

Edit: possible third approach just popped in to my head. Maybe just MPSWAP it, do the calculation and then MPSWAP it back?

@christoph-heinrich christoph-heinrich force-pushed the fix_wayland_vertical_resize branch 2 times, most recently from 54a4c01 to 8e8653d Compare March 7, 2023 02:53
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Mar 7, 2023

The MPSWAP variant isn't bad. Not as dense as before, but might be easier to understand.
One can make an argument for inverting the condition, but that's purely up to taste.

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

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

Looks reasonable enough.

Copy link
Member

@Dudemanguy Dudemanguy left a comment

Choose a reason for hiding this comment

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

Now that I actually tested this (on sway). It unexpectedly doesn't work at all and regresses resizing in the horizontal direction even. I presume this worked for you on plasma so that is weird. We'll need a different approach (no good ideas atm).

@christoph-heinrich
Copy link
Contributor Author

When resizing vertically or horizontally I can only make the window bigger, but when resizing both (corner with border) I can also make it smaller 😕. I'm pretty sure that used to work, maybe the change to MPSWAP broke it somehow 🤷. I'll look into it later.

@christoph-heinrich
Copy link
Contributor Author

This is a surprisingly difficult problem to solve... seemingly impossible really.
I'll make it a draft for now and if I ever come up with some genius idea of how to make this work well for all kinds of resizing without having the window jump around or anything, I'll open it up again.

The calculation was only implemented for horizontal resize,
now it works with vertical resize as well.
@christoph-heinrich
Copy link
Contributor Author

christoph-heinrich commented Sep 6, 2023

I think I found a way to make it behave how it should by keeping track of the kind of resize in the wayland state.
Don't worry about how terrible the code is for now, I'll try to clean it up once it works how it should.
Can you please test if this behaves correctly?

Edit: I already found a bug where after a diagonal resize it sometimes doesn't let the window get smaller in a vertical resize, but when trying again it works. It probably doesn't reliably reset the resize state.

@Dudemanguy
Copy link
Member

Succeeded by #11420 instead.

@Dudemanguy Dudemanguy closed this Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants