Tabbed and stacked layout #566

Merged
merged 14 commits into from Apr 25, 2016

Conversation

Projects
None yet
3 participants
@mikkeloscar
Collaborator

mikkeloscar commented Mar 31, 2016

This is now ready for review/merge.

There's one known bug in wlc, that causes views to become invisible when moved away from a tabbed/stacked layout in some cases. (Will be fixed separately, obviously).


This is just to let other people follow the progress and for me to have some sort of checklist for bugs I find.

Will update when ready for review.

Known issues

  • Nested containers does not work well with these layouts.
    • VERT layout inside a tabbed/stacked layout behaves weird.
  • Does not work well with gaps_inner right now. (inner gaps are disabled in tabbed/stacked mode).
  • Default layout not correctly handled.
  • Resize after rendering resulting in ugly flashes (relates to #220).
  • move command does not work correctly with these layouts.
    • sometimes views get invisible when moving away from tabbed/stacked layout.
  • Add text title to container titlebars e.g. sway: T[term term]
  • Making a child of a tabbed/stacked container floating, does not work.
    • Floating is still a bit messed up in general.

SirCmpwn added a commit that referenced this pull request Apr 1, 2016

@mikkeloscar mikkeloscar changed the title from [WIP] Tabbed and stacked layout to Tabbed and stacked layout Apr 24, 2016

sway/layout.c
@@ -300,7 +304,7 @@ void move_container(swayc_t *container, enum movement_direction dir) {
parent = child->parent;
}
// Dirty hack to fix a certain case
- arrange_windows(parent, -1, -1);
+ /* arrange_windows(parent, -1, -1); */

This comment has been minimized.

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn Apr 24, 2016

Member

I have some thoughts on the implementation. I'm concerned about the performance of walking up the tree to find a stacked/tabbed parents somewhere. I'm also not a fan of the approach to not drawing children's borders, and how stacked/tabbed is an explicit special case in many places. I would prefer an implementation that, perhaps, would instead allow containers to have borders and code it to stop rendering borders when recursively laying out going into a stacked/tabbed container.

Member

SirCmpwn commented Apr 24, 2016

I have some thoughts on the implementation. I'm concerned about the performance of walking up the tree to find a stacked/tabbed parents somewhere. I'm also not a fan of the approach to not drawing children's borders, and how stacked/tabbed is an explicit special case in many places. I would prefer an implementation that, perhaps, would instead allow containers to have borders and code it to stop rendering borders when recursively laying out going into a stacked/tabbed container.

@mikkeloscar

This comment has been minimized.

Show comment
Hide comment
@mikkeloscar

mikkeloscar Apr 25, 2016

Collaborator

I have some thoughts on the implementation. I'm concerned about the performance of walking up the tree to find a stacked/tabbed parents somewhere.
[...]
and how stacked/tabbed is an explicit special case in many places.

We need some way to detect this, since tabbed/stacked is a special case, but it could possible be handled better by making a separate arrange_windows path for tabbed/stacked layout (would mean a tiny bit of code duplication), and have a flag set on the container/view which determines if it's in a tabbed/stacked layout or not. That flag could be easily updated in windows_arrange_r.

I'm also not a fan of the approach to not drawing children's borders,

I'm not sure I follow. Since we draw the borders in pre_render for a view, everything has to be rendered by the visible view. So there's no need to draw borders of invisible siblings/children.

[...]
I would prefer an implementation that, perhaps, would instead allow containers to have borders and code it to stop rendering borders when recursively laying out going into a stacked/tabbed container.

I'm not sure what you mean. If you mean that the container could draw the tabbed/stacked titlebars then I don't see the benefit since the visible view has to render the border anyway.

Collaborator

mikkeloscar commented Apr 25, 2016

I have some thoughts on the implementation. I'm concerned about the performance of walking up the tree to find a stacked/tabbed parents somewhere.
[...]
and how stacked/tabbed is an explicit special case in many places.

We need some way to detect this, since tabbed/stacked is a special case, but it could possible be handled better by making a separate arrange_windows path for tabbed/stacked layout (would mean a tiny bit of code duplication), and have a flag set on the container/view which determines if it's in a tabbed/stacked layout or not. That flag could be easily updated in windows_arrange_r.

I'm also not a fan of the approach to not drawing children's borders,

I'm not sure I follow. Since we draw the borders in pre_render for a view, everything has to be rendered by the visible view. So there's no need to draw borders of invisible siblings/children.

[...]
I would prefer an implementation that, perhaps, would instead allow containers to have borders and code it to stop rendering borders when recursively laying out going into a stacked/tabbed container.

I'm not sure what you mean. If you mean that the container could draw the tabbed/stacked titlebars then I don't see the benefit since the visible view has to render the border anyway.

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn Apr 25, 2016

Member

Hmm, those are fair points. I agree. Will test this over the course of the day today and merge this afternoon.

Member

SirCmpwn commented Apr 25, 2016

Hmm, those are fair points. I agree. Will test this over the course of the day today and merge this afternoon.

@mikkeloscar

This comment has been minimized.

Show comment
Hide comment
@mikkeloscar

mikkeloscar Apr 25, 2016

Collaborator

Ok, cool.

I have just pushed a commit removing the commented code you noticed above.

Collaborator

mikkeloscar commented Apr 25, 2016

Ok, cool.

I have just pushed a commit removing the commented code you noticed above.

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn Apr 25, 2016

Member

Here we go 🎉

Member

SirCmpwn commented Apr 25, 2016

Here we go 🎉

@SirCmpwn SirCmpwn merged commit dba1195 into swaywm:master Apr 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
+ // for tabbed/stacked layouts the focused view has to draw all the
+ // titlebars of the hidden views.
+ swayc_t *p = swayc_tabbed_stacked_parent(view);
+ if (p && view->parent->focused == view) {

This comment has been minimized.

@StringEpsilon

StringEpsilon Apr 25, 2016

You could make a bit of a performance improvement by checking view->parent->focused first and only get p only if you actually need it.

Downside: Makes the ifs a bit uglier.

@StringEpsilon

StringEpsilon Apr 25, 2016

You could make a bit of a performance improvement by checking view->parent->focused first and only get p only if you actually need it.

Downside: Makes the ifs a bit uglier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment