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

Layout correctly with several new windows #4283

Closed
wants to merge 3 commits into from

Conversation

pedrocr
Copy link
Contributor

@pedrocr pedrocr commented Jun 23, 2019

If there is more than one new window layout correctly by calculating the default size of the new windows using the information of how many of them there are in total.

This helps with issue #3547 but doesn't fix it in all situations. Things now work correctly if the first layout of new windows happens after leaving fullscreen. But if for some reason an arrange_container() gets called while we are fullscreen the windows will still be incorrectly sized after saved_width/saved_height get used to restore the first window's size before going fullscreen.

If there is more than one new window layout correctly by calculating the
default size of the new windows using the information of how many of
them there are in total.

This helps with issue swaywm#3547 but doesn't fix it in all situations. Things
now work correctly if the first layout of new windows happens after
leaving fullscreen. But if for some reason an arrange_container() gets
called while we are fullscreen the windows will still be incorrectly
sized after saved_width/saved_height get used to restore the first
window's size before going fullscreen.
@pedrocr pedrocr force-pushed the layout_multiple_new_windows branch from 71d219d to e1a6c5e Compare June 25, 2019 12:41
@ddevault ddevault requested a review from RyanDwyer June 27, 2019 16:11
@pedrocr
Copy link
Contributor Author

pedrocr commented Jun 27, 2019

I've pushed as a second commit what may be a full fix for #3547. The solution seems fairly sane so if it hasn't regressed anything #4284 can probably be abandoned.

When a container that includes a child that's fullscreen gets
re-arranged also adjust the saved width/height of the fullscreen window
so that on fullscreen exit the layout is correct.

Fixes swaywm#3547
@pedrocr pedrocr force-pushed the layout_multiple_new_windows branch from 2e60b6a to 89ce6a8 Compare June 27, 2019 18:18
When doing the layout of a container that includes a fullscreen child
also layout new windows correctly if gaps are being used. This is done
by also adjusting the saved versions x/y/width/height by the gaps so
that the layout adjustments take them into account.
@RyanDwyer
Copy link
Member

I wonder if it would be worth changing the approach to one which stores width/height weighting values in each container rather than doing it by pixels. So each new container would default to weighting values of 1. If you had 3 siblings with weights 2, 1, 1 then the sizes would be 50%, 25%, 25%.

This would mean the fullscreen container would retain the weighting of 1, the siblings would also have weighting 1, and everything would size equally when exiting fullscreen. You wouldn't need to track the saved width or saved height.

The resize command would need to be changed to set the weighting instead of the width and height. I don't think it would be too difficult.

@pedrocr
Copy link
Contributor Author

pedrocr commented Jun 28, 2019

I don't think that changes the problem really. You'd just be replacing saved_width with saved_width_weight, no? Since the layout code already does a scale calculation on each run the end result is the same as doing it in pixels.

@pedrocr
Copy link
Contributor Author

pedrocr commented Jun 28, 2019

Nevermind, I think I see what you mean. I'll try a version with that.

@pedrocr
Copy link
Contributor Author

pedrocr commented Jun 28, 2019

New pull request sent as #4293. The code does look a bit better and should be less brittle. @RyanDwyer could you please review that one?

@RyanDwyer
Copy link
Member

This PR adds a significant amount of complexity to fix the problem. I'd be happy to approve the first commit only, but I understand it doesn't fix all the cases. If the first commit were merged, what would the steps be to reproduce the remaining issue? Just trying to understand what the latter commits are solving.

@pedrocr
Copy link
Contributor Author

pedrocr commented Jun 29, 2019

To reproduce the issue after the first commit:

  1. Launch three containers horizontally
  2. In one of them switch to vertical splits
  3. Make that container fullscreen
  4. Launch two new containers vertically
  5. Unfullscreen

I'm pretty sure this PR can be abandoned in favor of #4293 though. The weights calculation can probably be made simpler but the solution is just much more robust.

@pedrocr
Copy link
Contributor Author

pedrocr commented Jul 6, 2019

Closed in favor of #4293

@pedrocr pedrocr closed this Jul 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants