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

fullscreen compilation patch - odd behavior relating to fakefullscreen client functionality #9

Closed
vide0hanz opened this issue Sep 9, 2020 · 17 comments

Comments

@vide0hanz
Copy link

Hey again, sorry to keep spamming your github page. Figured this is better than reaching out on reddit. There a couple minor issues that are easily reproduced when using the fake fullscreen client functionality of the fullscreen compilation patch. Tested with a bare dwm (latest commit).

From what I can gather, most of these issues can be prevented by enabling actual fullscreen at any point before going into fakefullscreen mode (with the exception if 2.1 outlined below).

1 : When toggling fakefullscreen on any client on and then off, it removes the border of that client. However, if you go into actual fullscreen mode at any point before going into fakefullscreen mode, the client behaves as expected. Fake/actual fullscreen can now be toggled in any order and the borders remain.

Steps to reproduce:

  • select a tiled client
  • toggle fakefullscreen off/on, watch the border disappear
  • select a new tiled client
  • toggle actualfullscreen
  • exit fullscreen and then toggle fakefullscreen repeatedly; the borders should remain constant now, and the client should behave

2 : On floating clients, entering fakefullscreen works the first time, but exiting fakefullscreen removes the border in addition to placing it back in its tiled position. While the client is floating, going into actual fullscreen mode at any point before fakefullscreen prevents this as well.

Steps to reproduce:

  • select a tiled client and toggle floating on
  • while floating, toggle fakefullscreen off/on, watch the border disappear and the client revert to the tiled position in the stack
  • select a new tiled client make it float, then toggle actual fullscreen
  • with the client now in fullscreen, toggle fakefullscreen repeatedly and everything should behave

2.1 : In an attempt to isolate issue 2, I discovered another odd scenario relating to floating clients:

Steps to reproduce:

  • select a tiled client and make it fullscreen, then toggle fullscreen off again
  • make that same client floatling
  • toggle fakefullscreen on
  • toggle fakefullscreen off; now the client's borders should remain intact, yet it still reverts back to the tiled position in the stack
  • making the same client fullscreen at any point while it is floating fixes this

3 : This last one is probably related to unfocus(). If you focus on a client and toggle actual fullscreen first, and then exit out of it by toggling fake fullscreen, the client loses focus in favor of the client where the mouse cursor is. In this scenario, the borders are fine, and the client goes into fakefullscreen - it just loses focus in favor of the mouse position. If the client is made fullscreen while already in fakefullscreen, toggling fakefullscreen off exits both of the fullscreen modes, which I'd say is the expected behavior.

Steps to reproduce:

  • select a client, make it fullscreen
  • now make it fakefullscreen
  • the client should now be properly in a tiled fake fullscreen mode, but the cursor position will determine which client has focus
  • select the client that has fakefullscreen active
  • make it actual fullscreen
  • toggle fakefullscreen off, now the client should be in a normal tiled state

To summarize:

Fakefullscreen introduces some unexpected behavior when it is the first action called on any selected client. Nearly all of these issues can be prevented by calling actualfullscreen on the client at any point before fakefullscreen. The only instance where this is not the case is outlined in issue 2.1. Issue 3 remains mostly unaffected by this as well, as it has to do with the order the fullscreen modes are initialized.

Just wanted to bring this to your attention. I'm not sure where to look to even attempt to fix this myself, but I'm guessing clientmessage(), configurenotify(), and unfocus(). Any clue why fakefullscreen behaves this way? How is it so interlocked between the order in which fakefullscreen and actual fullscreen is called, in addition to the client state (tiled vs floating)?

Hopefully this post is detailed enough to give you something to work with. I'm happy to test out anything you'd like me to change in my build of dwm and report back, and I'll of course share anything else I discover.

@bakkeby
Copy link
Owner

bakkeby commented Sep 9, 2020

Nice! That's some writeup, much appreciated. I had not noticed the borders disappearing in fake fullscreen mode - I'll look into it and get back to you.

@vide0hanz
Copy link
Author

vide0hanz commented Sep 10, 2020

Been playing with this some more, issues 1 and 2 appear to be completely solved.

Issue 3 is still giving me a hard time, although in and of itself is very minor. It might be easier to visualize what is going on if you apply the noborder patch, because I think its related to the client losing focus in this situation:

In resizeclient() add the following:

    c->oldw = c->w; c->w = wc.width = w;
    c->oldh = c->h; c->h = wc.height = h;
    wc.border_width = c->bw;
+       if (((nexttiled(c->mon->clients) == c && !nexttiled(c->next))
+           || &monocle == c->mon->lt[c->mon->sellt]->arrange)
+           && (c->fakefullscreen == 1 || !c->isfullscreen) && !c->isfloating  
+           && NULL != c->mon->lt[c->mon->sellt]->arrange) {
+               c->w = wc.width += c->bw * 2;
+               c->h = wc.height += c->bw * 2;
+               wc.border_width = 0;
+       }
    XConfigureWindow(dpy, c->win, CWX|CWY|CWWidth|CWHeight|CWBorderWidth, &wc);
    configure(c);
    XSync(dpy, False);

This is the noborder patch with the check for fakefullscreen as per our conversation earlier.
With this applied, follow these steps:

  • Make a client fullscreen (mod + f)
  • Make that same client fakefullscreen (mod + shift + f)

The client should now be reverted back to tiled + fakefullscreen, and there should now be a border drawn around the client, but only on the left and top edges. Strange right?

I only bring this one up because following the same steps causes the same result I outlined in my previous post regarding issue 3:

  • Open several clients, 3 or more for easy visual
  • Move the cursor over one of them
  • Select a different client with mod + j/k to switch focus
  • make the newly selected client fullscreen(actual)
  • now, toggle fakefullscreen

Calling togglefakefullscreen while a client is already in an actual fullcsreen state, the client loses focus from dwm in favor of the client the mouse cursor is on. What I believe should happen is the client remains focused regardless of the fullscreen state it is in.

As always, I welcome any insights you may have.

edit - I think it has to be something to do with the new variables in togglefullscreen() that were added to allow switching back and forth between actual/fake fullscreen. Does something similar need to be done to togglefakefullscreen() as well?

@bakkeby
Copy link
Owner

bakkeby commented Sep 10, 2020

+           && (c->fakefullscreen == 1 || !c->isfullscreen) && !c->isfloating && c->fakefullscreen  

Not related to the issues you are noting here, but you may want to drop that trailing && c->fakefullscreen as otherwise the noborder only applies to clients with fake fullscreen enabled.

The client should now be reverted back to tiled + fakefullscreen, and there should now be a border drawn around the client, but only on the left and top edges. Strange right?

I was not able to replicate this. Could it be that you still have some of the other changes we tried, like the moving of the resizeclient call?

I'll have a look to see if I can replicate the focus issue.

@bakkeby
Copy link
Owner

bakkeby commented Sep 10, 2020

OK, so yes I can replicate the focus issue.

The focus change has to do with an enter notify signal being received for the client underneath the cursor when you go from fullscreen to fake fullscreen. This is something that is handled by the X server.

I was able to work around this by explicitly passing True to the XSync call within the resize function, which, according to https://tronche.com/gui/x/xlib/event-handling/XSync.html causes the X server to disregard all events in the queue. So essentially we are cancelling that enter notify signal before it is sent. I am not entirely sure what other implications this might have.

diff --git a/dwm.c b/dwm.c
index adc05ce..9f6fb5a 100644
--- a/dwm.c
+++ b/dwm.c
@@ -1296,7 +1296,10 @@ resizeclient(Client *c, int x, int y, int w, int h)
        wc.border_width = c->bw;
        XConfigureWindow(dpy, c->win, CWX|CWY|CWWidth|CWHeight|CWBorderWidth, &wc);
        configure(c);
-       XSync(dpy, False);
+       if (c->fakefullscreen == 1)
+               XSync(dpy, True);
+       else
+               XSync(dpy, False);
 }

 void
@@ -1754,6 +1757,7 @@ togglefakefullscreen(const Arg *arg)
                else
                        c->isfullscreen = 0;
        } else {
+               c->fakefullscreen = 1;
                if (c->isfullscreen) {
                        c->isfloating = c->oldstate;
                        c->bw = c->oldbw;
@@ -1763,7 +1767,6 @@ togglefakefullscreen(const Arg *arg)
                        c->h = c->oldh;
                        resizeclient(c, c->x, c->y, c->w, c->h);
                }
-               c->fakefullscreen = 1;
                c->isfullscreen = 0;
        }
        setfullscreen(c, !c->isfullscreen);
(END)

bakkeby added a commit that referenced this issue Sep 10, 2020
@vide0hanz
Copy link
Author

Excellent find with the XSync fix! I didn't have much time to dig deep into the X server side of things last night, but the issue felt very familiar to a conversation we had a while back regarding focusing on hidden clients when you came up with the hide() additions. I figured it was something to do with X and not necessarily dwm specifically.

you may want to drop that trailing && c->fakefullscreen as otherwise the noborder only applies to clients with fake fullscreen enabled.

Whoops, not sure how that snuck in there, that was a mistake. I'll edit my original post in case someone is doing some google-fu and happens upon it.

I was not able to replicate this. Could it be that you still have some of the other changes we tried, like the moving of the resizeclient call?

Everything in setfullscreen() is as it was before the most recent commit, the one which includes the floating window/border fixes.

This most recent commit appears to solve everything from what I can tell, including the border issue you weren't able to replicate. Which seems to confirm my suspicion that the border issue I described earlier was tied to the (apparently) enternotify event regarding fakefullscreen clients. Interestingly, with this fix, rapidly enabling fake fullscreen on single clients no longer appear to flicker on my system either.

I'll have to play around with it some more to see if I can force something to break, or if I can come up with some other weird consistency issues but as far as I can tell I think this is pretty much solved!

@bakkeby
Copy link
Owner

bakkeby commented Sep 10, 2020

I figured there might be some obscure issues if the client window goes into fullscreen (like F11 in your browser), but it worked fine in all my test cases.

@vide0hanz
Copy link
Author

vide0hanz commented Sep 11, 2020

OK, what do you think of this?

@@ -1296,7 +1296,10 @@ resizeclient(Client *c, int x, int y, int w, int h)
    wc.border_width = c->bw;
    XConfigureWindow(dpy, c->win, CWX|CWY|CWWidth|CWHeight|CWBorderWidth, &wc);
    configure(c);
-       XSync(dpy, False);
+       if (c->fakefullscreen == 1 && !c->isfloating) {
+               XLowerWindow(dpy, c->win);
+               XSync(dpy, True);
+       } else
+               XSync(dpy, False);
 }

This fixes a super minor visual discrepancy when calling togglefakefullscreen() while a client is already fullscreen(actual). Note the XRaiseWindow line in setfullscreen() - This ensures that the newly created fullscreen window goes above all other ones. Without it, any clients marked with an override_redirect = True flag (such as the bar) will always be displayed on top of other clients. Calling on XLowerWindow to lower fakefullscreen clients while excluding floating windows seems like a good idea.

The problem with this approach means that since it is excluding floating windows from being lowered, the XSync fix will not apply to them either...

Without this type of change, making a client fakefullscreen while the client was already fullscreen(actual) would cause it retain it's "raised" state. Just can't seem to figure out a good way to properly exclude floating windows from being lowered while still retaining the XSync fix.

Edit I should point out that this can probably only be observed while running a compositor with a shadow offset -- its very minor and doesn't affect anything functional. I'm 99% sure what I described above is the reason it behaves this way though.

@bakkeby
Copy link
Owner

bakkeby commented Sep 11, 2020

Just can't seem to figure out a good way to properly exclude floating windows from being lowered while still retaining the XSync fix.

You are probably going to laugh, but

@@ -1296,7 +1296,10 @@ resizeclient(Client *c, int x, int y, int w, int h)
    wc.border_width = c->bw;
    XConfigureWindow(dpy, c->win, CWX|CWY|CWWidth|CWHeight|CWBorderWidth, &wc);
    configure(c);
-       XSync(dpy, False);
+       if (c->fakefullscreen == 1) {
+               if (!c->isfloating)
+                       XLowerWindow(dpy, c->win);
+               XSync(dpy, True);
+       } else
+               XSync(dpy, False);
 }

Without it, any clients marked with an override_redirect = True flag (such as the bar) will always be displayed on top of other clients.

Aha! Busted, so you are using an external bar :D If I am understanding it correctly, you actually want the bar to be displayed on top of clients? Do you actually have a case where tiled windows are overlapping the bar? I am just trying to understand the exact use-case for lowering the window here.

Note that this is in the resizeclient function. I have not tested this, but I suspect that if you were to change to a floating master layout and the master window happens to be firefox running in fake fullscreen mode, then that would result in firefox snapping into the background beneath all the stack clients every time the layout is arranged.

Before you made the edit you had this as well:

if Firefox is the only client on a tag and is in fakefullscreen mode, spamming F11 causes the window geometry to expand to the right by a few pixels each time and will continue to bleed over the edge of the display

This I can replicate.

I suspect that this is an implication of the noborder patch.

When you press F11 in the browser the browser sends a notification to the window manager and we end up in the clientmessage function.

The code as-is triggers this:

			if (c->fakefullscreen == 1)
				resizeclient(c, c->x, c->y, c->w, c->h);
			else
				setfullscreen(c, (cme->data.l[0] == 1 /* _NET_WM_STATE_ADD    */
					|| (cme->data.l[0] == 2 /* _NET_WM_STATE_TOGGLE */
					&& !c->isfullscreen
				)));

I suspect the issue here is that we are just keeping the same position and height and width as we do currently, but there is no border and the noborder patch adds the border size to the new dimension.

Quite possibly one could just ignore the signal if the client is in fake fullscreen, but I suspect that it is necessary to call setfullscreen in both cases. I notice that if you hit F11 in a fake fullscreen firefox window the client never properly exits fullscreen. I'll have a play around.

@bakkeby
Copy link
Owner

bakkeby commented Sep 11, 2020

I removed the changes in clientmessage, seems they were not needed.

@vide0hanz
Copy link
Author

vide0hanz commented Sep 11, 2020

edit: structure of post

Ah yes, I removed that bit regarding the noborder since I figured it wasn't related to this patch specifically -- turns out it sort of was :D

I can confirm that is solved as well. Removing the changes in clientmessage did the trick.

Do you actually have a case where tiled windows are overlapping the bar? I am just trying to understand the exact use-case for lowering the window here.

Here is the order of events which triggers this as I understand them:

1: client receives fullscreen signal
2: client is raised via XRaiseWindow
3: client receives togglefakefullscreen signal
4: client enters fakefullscreen while in a raised state -- this causes the appearance of the window to seem like it is simultaneously floating + tiled under a very specific set of conditions:

  • compositor running w/ shadows
  • is the only tiled client present
  • no gaps

Since the window is still "raised", it appears as if its floating above the bar even though it is tiled.

As you can see, it is so minor and has no effect on anything besides appearances but I suspect it can be solved with the strategic placement of XLowerWindow somewhere, or by some other means of cancelling out the XRaiseWindow signal on a fullscreen client when it receives a togglefakefullscreen signal.

Perhaps this is best handled by an external script via the IPC patch though, like we discussed earlier? Otherwise, if this works as-is and the only thing I need to sacrifice is the centered floating master layout then so be it, I can live with that.

Note that this is in the resizeclient function. I have not tested this, but I suspect that if you were to change to a floating master layout and the master window happens to be firefox running in fake fullscreen mode, then that would result in firefox snapping into the background beneath all the stack clients every time the layout is arranged.

It seems to behave fine actually, with the exception of calling togglefakefullscreen() while its already in fullscreen - which does cause it to snap to the background. That is to be expected though based on this logic.

Can you think of a better way to check for this? I've tried playing around in restack() a bit but nothing comes to mind. If I can figure out a way to discreetly cancel out of a fullscreen window's raised state when togglefakefullscreen() is called, I think we can mark this as closed.

I'll let you know if I find a solution.

EDIT x2 Turns out I found a situation by accident where this actually does have a real effect. Try this:

  • place a floating window on top of another client
  • make the client below the floating window fullscreen(actual)
  • then, toggle fake fullscreen
  • The floating window will now be behind the fakefullscreen one -- this is because it is retaining its raised state from when it was in fullscreen mode.

Sorry to keep throwing you curve balls here, but I discovered this while trying to isolate even more weirdness I noticed relating to togglefloating -- its the snapping back to position issue all over again, only in reverse. Sending a floating client back to tiled via togglefoating and then making it fakefullscreen has a chance to pop it right back to its floating position. Often times it will pop back, otherwise it will momentarily appear in its last floating position. This is on a bare dwm, mind you.

EDITx3

OK I am almost positive now that there is some cross talk happening between fullscreen, fakefullscreen, and floating states. Turns out I'm not insane after all.

In this order:

  • open a client
  • make it floating
  • toggle fullscreen(actual)
  • now toggle fakefullscreen (the client will be raised, yet tiled)
  • toggle floating (the client will snap back to tiled)
  • toggle fakefullscreen -- the client will now be floating again, ln the same position you left it in.

I can replicate this 100% of the time. This will (hopefully) be my final edit on this post. Sorry for the lengthy write up, but this is almost certainly the issue. I think its probably related to the recorded states for fakefullscreen, which iirc you put in there to allow for swapping between fake/actual fullscreen modes regardless of the order they were triggered in. Hope this helps!

@bakkeby
Copy link
Owner

bakkeby commented Sep 12, 2020

Just to clarify something as you are repeating it many times, XRaiseWindow is not a state that is enabled or disabled. There is no "the client is in a raise state" vs "not in a raised state".

This analogy is pretty good:

If the windows are regarded as overlapping sheets of paper stacked on a desk, then raising a window is analogous to moving the sheet to the top of the stack but leaving its x and y location on the desk constant.

You do not want to ever call XLowerWindow unless you explicitly want the window to appear below all the other windows on screen. If you do then you will encounter other issues as I mentioned before where the fake fullscreen client appears below other tiled clients in a layout that overlaps clients (such as floating master layouts).

The case with the floating window is a valid one. It happens because, as you say, the setfullscreen function calls XRaiseWindow making the fullscreen window on top of the stack of windows (above floating windows). When go from fullscreen to a tiled fake fullscreen window the position in the stack remains.

The only sensible way to address this would be to call XRaiseWindow for every floating window when we exit out of fullscreen.

Is there an easy way to do that? Actually there is - we can call restack(c->mon); which raises floating windows. Adding this at the end of togglefakefullscreen solves this issue.

Now you may ask why you can't replicate this when just toggling fullscreen on and off? This has to do with that the setfullscreen function calls arrange(c->mon); when going out of fullscreen, and the arrange function calls restack down the line. In principle you should only need one restack call I'd think. I'm considering moving this to before the return happens, will have a play around.

@bakkeby
Copy link
Owner

bakkeby commented Sep 12, 2020

@vide0hanz could you try out the latest changes in
https://github.com/bakkeby/patches/blob/master/dwm/dwm-fullscreen-compilation-6.2.diff

Only the setfullscreen function has changed.

I am not particularly happy with the overall solution as coding-wise it feels very hacky, but it will have to do for now.

@bakkeby
Copy link
Owner

bakkeby commented Sep 15, 2020

Just an update in case other people are reading up on this - I did a complete overhaul of all the setfullscreen logic (now available at the existing diff) which appears to have solved all of the issues outlined in this ticket.

Right now I am just doing another round of regression testing before closing this one off.

@vide0hanz
Copy link
Author

Can confirm the overhaul is solid based on my testing. Ive been trying to break it and I can't, all of the issues I mentioned previously are resolved from what I can tell. Will leave open until @bakkeby says otherwise.

@vide0hanz
Copy link
Author

Confirming latest commit successfully resolves the obscure bug we discussed.

For anyone that may be following this:

  • open multiple windows and firefox
  • make firefox fake fullscreen (--> it goes fullscreen within the tiled area)
  • hit F11 to make the client exit fullscreen (--> client is normal, but fake fullscreen)
  • hit MOD+f to make the client actual fullscreen
  • hit F11 to make the client exit fullscreen

Before the most recent commit, the browser in question (firefox, in this example) would be in a simultaneous floating + fakefullscreen state. This has been resolved based on my testing.

@bakkeby
Copy link
Owner

bakkeby commented Oct 12, 2020

There were some not quite ideal situations when moving a fullscreen window between monitors that I found.

Issue 1: Tiled window

  • enable fullscreen
  • move the window to another monitor
  • exit fullscreen

Outcome

  • the window is not tiled, but appears to be floating (despite isfloating being 0)
  • this has to do with that the old x and y positions as well as height and width have been restored

Solution

  • perform a full arrange rather than restoring previous position and size

Issue 2: Floating window

  • make the window large
  • enable fullscreen
  • move the window to another (smaller) monitor
  • exit fullscreen

Outcome:

  • the window is floating, but has dimensions that are larger than the monitor
  • this has to do with that the old x and y positions as well as height and width have been restored

Solution

  • restore the old positions, but restrict them to the window area of the monitor

Updated the patch.

@bakkeby
Copy link
Owner

bakkeby commented Nov 24, 2020

I can't say that I have spotted any fullscreen woes for well over a month, so I think it is time to close this.

@bakkeby bakkeby closed this as completed Nov 24, 2020
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

No branches or pull requests

2 participants