swallow + fullscreen bug #85
Comments
|
Fullscreen is a sort of special state and what is the ideal handling of those in relation to window swallowing may depend on the perspective of the user. In the swallow patch there is no special handling of the fullscreen state and I would agree that the current behaviour (of leaving a floating window that spans the entire monitor) is not great and that this should be changed to a behaviour that is more intuitive for daily use. If the fullscreen state is retained on swallow / unswallow then that makes the behaviour very easy to explain and to grasp, but I'd question how practical such a solution would be. Let's say that you open a (floating) terminal and you open an image using nsxiv - and you tell nsxiv to open in fullscreen. $ nsxiv -f <image.png>In the context of window swallowing you could either expect:
The first option retains the floating state and the second doesn't. This is something that came up recently in bakkeby/dwm-flexipatch#430 (in relation to the original swallow patch) and one option is to not allow window swallowing if the new window opens in fullscreen. The other two scenarios that you mentioned:
In this case I am wondering if there is a legitimate case where quitting a fullscreen window and ending up in a fullscreen terminal is practical. I had a situation a couple of months back where I was using a command to generate images and opening them to see the results. Sometimes the generated image would be very large and I would fullscreen the image viewer to see the image in full, then exit after having inspected the results. This would leave a fullscreen terminal (or a floating window the size of the screen) which was quite disruptive to the workflow. Confusing actually. In that scenario it was much more practical for me that the terminal window would be restored in the original position (tiled or floating) rather than adhering to the fullscreen state of the other window.
Problematic. I am wondering if a fullscreen terminal is an edge case or if many people use this workflow. My first impression is that if the terminal window is fullscreen at the time of swallowing then it would be intuitive that the new window is also fullscreen. On the other hand you could have a window that has a fixed size that doesn't support being fullscreened, could it be that it is more practical that the fullscreen terminal disappears and the new window appears as normal (tiled or floating)? If we go with the first option (inheriting fullscreen) and you then immediately exit I am thinking that restoring the terminal in fullscreen is more intuitive. But if you exit fullscreen beforehand then it would make more sense for the terminal to be restored without fullscreen. It is also worth mentioning that if the fullscreen state is simply inherited on swallow / unswallow, and you do not use the toggle fullscreen patch, then you can get into situations where the terminal window is fullscreen and you can't exit fullscreen. Just some of my thoughts on the topic. |
|
Expected behaviour with
Expected behaviour with
Trying to compile an overview of the expected states of the terminal and the swallowing window before and after window swallowing. The correctness of the listed expectations is up for debate. |
since I rarely float windows and only fullscreen with keybinds rather than cli args I didn't consider this case.
if I've specified
it seems to me like this is already how the patch works though. if I run
on second thought I do agree with you and I don't think fullscreen being unset would annoy me in this sort of scenario. that being said, my main point of issue here was the fact that the window would end up floating when you unswallowed despite being tiled when it was swallowed.
it's definitely common for me. often I'll be using lf (a tui file manager) to browse files, opening and closing images and videos which makes the terminal swallow and unswallow as I go... I might be setting and unsetting fullscreen to change focus where necessary and not keeping track of what fullscreen state I last swallowed in and all of a sudden I encounter one of the two scenarios I described.
is this a common thing? my workflow is mostly tui based, but even with the few gui programs I use I've never encountered it. wouldn't it just be best to specify
personally I would find it incredibly annoying if I was kicked out of fullscreen every time the terminal was swallowed. refer to the lf case I gave above.
ideally you'd want both. is there no way for the terminal to inherit the fullscreen / tiled / floating etc state when you unswallow so that both of these could be achieved?
good point. this just makes me wish that fullscreen toggling was native in dwm haha... it's only a few simple lines of code :/
pretty sure I agree with all these. maybe there could be some debate about the third case reverting from fullscreen to tiled. also what's the difference between fullscreen (tiled) and fullscreen (floating)? |
In the table I produced above there is no difference, but I thought it was better to write them out separately in case there would be some ambiguity between a fullscreen window that was previously tiled or floating. I am going to try to experiment with creating a swallow patch that meets all of the criteria. |
|
@2084x if you want to play around, I think these minor changes covers all cases. diff --git a/config.def.h b/config.def.h
index 5dc3f22..c27a7f9 100644
--- a/config.def.h
+++ b/config.def.h
@@ -4,6 +4,7 @@
static const unsigned int borderpx = 1; /* border pixel of windows */
static const unsigned int snap = 32; /* snap pixel */
static const int swallowfloating = 0; /* 1 means swallow floating windows by default */
+static const int swterminheritfs = 0; /* 1 terminal inherits fullscreen on unswallow, 0 otherwise */
static const int showbar = 1; /* 0 means no bar */
static const int topbar = 1; /* 0 means bottom bar */
static const char *fonts[] = { "monospace:size=10" };
diff --git a/dwm.c b/dwm.c
index 7aef964..644d5f8 100644
--- a/dwm.c
+++ b/dwm.c
@@ -1335,7 +1335,6 @@ replaceclient(Client *old, Client *new)
new->mon = mon;
new->tags = old->tags;
- new->isfloating = old->isfloating;
new->next = old->next;
new->snext = old->snext;
@@ -1357,6 +1356,16 @@ replaceclient(Client *old, Client *new)
old->next = NULL;
old->snext = NULL;
+ if (old->isfullscreen == new->isfullscreen) {
+ new->isfloating = old->isfloating;
+ } else if (new->isfullscreen && old->isterminal) {
+ /* allow windows starting in fullscreen to retain fullscreen */
+ } else if (swterminheritfs || !new->isterminal || !old->isfullscreen) {
+ /* inherit fullscreen state from the other client */
+ new->oldstate = old->oldstate;
+ setfullscreen(new, old->isfullscreen);
+ }
+
XMoveWindow(dpy, old->win, WIDTH(old) * -2, old->y);
if (ISVISIBLE(new) && !new->isfullscreen) {
@@ -1763,12 +1772,9 @@ swallow(Client *t, Client *c)
{
if (c->noswallow || c->isterminal)
return 0;
- if (!swallowfloating && c->isfloating)
+ if (!swallowfloating && ((c->isfloating && !c->isfullscreen) || c->oldstate))
return 0;
- if (t->isfullscreen)
- setfullscreen(c, 1);
-
replaceclient(t, c);
c->ignorecfgreqpos = 1;
c->swallowing = t;There is a config option |
|
I tested all the cases in the table and everything works as expected except:
in which case the terminal ends up tiled rather than floating. |
|
That was a very good point. I also discovered that it was a lot more complicated when taking all four states into account:
because you can have edge cases like:
I tried a variety of solutions primarily trying to sort out the state before inheriting fullscreen. This got quite complex considering the 16 possible combinations and the special optional behaviour I wanted to offer where the terminal window does not inherit fullscreen unswallow. In the end I worked out that it was easiest to just trigger fullscreen and then sort out the states afterwards. diff --git a/config.def.h b/config.def.h
index 5dc3f22..776d579 100644
--- a/config.def.h
+++ b/config.def.h
@@ -4,6 +4,7 @@
static const unsigned int borderpx = 1; /* border pixel of windows */
static const unsigned int snap = 32; /* snap pixel */
static const int swallowfloating = 0; /* 1 means swallow floating windows by default */
+static const int swterminheritfs = 1; /* 1 terminal inherits fullscreen on unswallow, 0 otherwise */
static const int showbar = 1; /* 0 means no bar */
static const int topbar = 1; /* 0 means bottom bar */
static const char *fonts[] = { "monospace:size=10" };
diff --git a/dwm.c b/dwm.c
index 7aef964..4ae052f 100644
--- a/dwm.c
+++ b/dwm.c
@@ -1335,21 +1335,20 @@ replaceclient(Client *old, Client *new)
new->mon = mon;
new->tags = old->tags;
- new->isfloating = old->isfloating;
new->next = old->next;
new->snext = old->snext;
- if (old == mon->clients)
+ if (old == mon->clients) {
mon->clients = new;
- else {
+ } else {
for (c = mon->clients; c && c->next != old; c = c->next);
c->next = new;
}
- if (old == mon->stack)
+ if (old == mon->stack) {
mon->stack = new;
- else {
+ } else {
for (c = mon->stack; c && c->snext != old; c = c->snext);
c->snext = new;
}
@@ -1357,13 +1356,34 @@ replaceclient(Client *old, Client *new)
old->next = NULL;
old->snext = NULL;
+ if (!swterminheritfs && new->isterminal && !new->isfullscreen && old->isfullscreen) {
+ /* Do not allow a non-fullscreen terminal to inherit the fullscreen property of
+ * a windoww when unswallowed */
+ new->x = old->oldx;
+ new->y = old->oldy;
+ new->w = old->oldw;
+ new->h = old->oldh;
+ new->isfloating = old->oldstate;
+ } else {
+ setfullscreen(new, old->isfullscreen);
+ new->x = old->x;
+ new->y = old->y;
+ new->w = old->w;
+ new->h = old->h;
+ new->oldx = old->oldx;
+ new->oldy = old->oldy;
+ new->oldw = old->oldw;
+ new->oldh = old->oldh;
+ new->oldbw = old->oldbw;
+ new->bw = old->bw;
+ new->isfloating = old->isfloating;
+ new->oldstate = old->oldstate;
+ }
+
XMoveWindow(dpy, old->win, WIDTH(old) * -2, old->y);The In principle, because the state is not corrected before the I'll push an updated patch for the above. |
actually I noticed this with the first diff but it's gone after the most recent commits. every scenario in the table is working as expected too! the only issue I can find now is when using cli args to start in fullscreen. for example nsxiv does not enter fullscreen at all when using |
|
Had a play around, looks like there are three main scenarios:
At first I thought it was merely having an else if that does nothing for the second case, but in that case we don't properly propagate the floating state and position of the window for when it exits fullscreen. Updated proposal: diff --git a/dwm.c b/dwm.c
index fd78193..65b5d00 100644
--- a/dwm.c
+++ b/dwm.c
@@ -1332,7 +1332,7 @@ replaceclient(Client *old, Client *new)
{
Client *c = NULL;
Monitor *mon = old->mon;
- int x, y, w, h;
+ int x = old->x, y = old->y, w = old->w, h = old->h;
new->mon = mon;
new->tags = old->tags;
@@ -1365,12 +1365,17 @@ replaceclient(Client *old, Client *new)
w = old->oldw;
h = old->oldh;
new->isfloating = old->oldstate;
+ } else if (old->isterminal && new->isfullscreen && !old->isfullscreen) {
+ /* Allow windows starting in fullscreen to retain fullscreen */
+ new->oldx = old->x;
+ new->oldy = old->y;
+ new->oldw = old->w;
+ new->oldh = old->h;
+ new->oldbw = old->bw;
+ new->oldstate = old->isfloating;
} else {
+ /* Inherit fullscreen property from the old client */
setfullscreen(new, old->isfullscreen);
- x = old->x;
- y = old->y;
- w = old->w;
- h = old->h;
new->oldx = old->oldx;
new->oldy = old->oldy;
new->oldw = old->oldw;The behaviour is starting to feel quite good. Thanks for your testing efforts. |
|
as far as I can tell, everything is working perfectly now. thank you for all your hard work fixing this and helping me with the other swallow issue too. I greatly appreciate it!! |
|
While trying to integrate this into my own build I came up with this alternative approach to:
void
replaceclient(Client *old, Client *new)
{
Client *c = NULL;
Monitor *mon = old->mon;
int x, y, w, h, f, bw;
if (old->isfullscreen) {
x = old->oldx;
y = old->oldy;
w = old->oldw;
h = old->oldh;
f = old->oldstate;
bw = old->oldbw;
} else {
x = old->x;
y = old->y;
w = old->w;
h = old->h;
f = old->isfloating;
bw = old->bw;
}
new->mon = mon;
new->tags = old->tags;
new->next = old->next;
new->snext = old->snext;
if (old == mon->clients) {
mon->clients = new;
} else {
for (c = mon->clients; c && c->next != old; c = c->next);
c->next = new;
}
if (old == mon->stack) {
mon->stack = new;
} else {
for (c = mon->stack; c && c->snext != old; c = c->snext);
c->snext = new;
}
old->next = NULL;
old->snext = NULL;
if (!new->isfullscreen && old->isfullscreen && (swterminheritfs || !new->isterminal)) {
/* Gain fullscreen */
setfullscreen(new, 1);
} else if (new->isfullscreen && !old->isfullscreen && !old->isterminal) {
/* Lose fullscreen */
setfullscreen(new, 0);
}
if (new->isfullscreen) {
new->oldbw = bw;
new->oldstate = f;
new->oldx = x;
new->oldy = y;
new->oldw = w;
new->oldh = h;
} else {
new->bw = bw;
new->isfloating = f;
if (ISVISIBLE(new) && new->isfloating) {
resize(new, x, y, w, h, 0);
}
}
XMoveWindow(dpy, old->win, WIDTH(old) * -2, old->y);
}It has a slightly higher line count, but I think that it is easier to reason about. |
|
no issues with that version either. I'll use whichever you think is best. |
|
I think I'll probably go with the latter. Just going to do some more sanity testing first, but logically the approach is a lot more sound and a lot less that could go wrong. |
I'm experiencing this using dwm with only the swallow and togglefullscreen patches applied.
when a window is unswallowed the fullscreen state does not carry over from the swallowed window. I assume this makes dwm think you are in the fullscreen state that the window had when it was initially swallowed.
you can observe this by doing the following:
or
ideally when the window becomes unswallowed it should retain the fullscreen state that the swallowed window had so that windows don't randomly float or have their borders disappear.
The text was updated successfully, but these errors were encountered: