-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add inhibit_fullscreen command #4255
Conversation
Pending implementation of a command to toggle this. |
64129c9
to
5950d72
Compare
5950d72
to
d7daeb1
Compare
There is an interesting corner case right now if you do as follows:
Other than that, this seems to work quite nicely. |
This is awesome. Thanks again for implementing this, and for doing it so quickly. Arranging a bunch of youtube videos in a grid became really pleasant. I can now have just the video playing and if I want to actually fullscreen one of them I can navigate to it and do |
i3 discussion: i3/i3#1227 I think this is in the same boat as fullscreen transparency, where it may be useful/desirable, but is a xdg-shell protocol violation. I think the relevant portion for this is:
I'll provide my comments on the implementation itself in case the others disagree. I'm not a huge fan of having both
|
As I'm a very happy user of this let me give my view on the two issues. First the question if this should exist at all. It is indeed the exact same discussion as in i3. I think this is very useful and makes sense because there's no reason for the app to know the difference between being in a window and being fullscreen at the same size. I don't see a protocol violation unless it's part of the API that the app can then use the total screen dimensions to make assumptions about it's buffer. The information that this was done leaks that way and for example the current Youtube UI uses it to give a dismissable warning that something strange happened, but still work correctly. Other than that information leakage there's no reason for an app to not react properly to resize events during fullscreen as that also happens on being changed to another output. As for your implementation suggestion, that's how the original version worked, and the current one is much better. If you treat this as another fullscreen state then you don't get one of the major benefits of the current patch which is to be able to make a window fullscreen and back while the app continues with its fullscreen UI. This is very useful when you have a mosaic of video streams and then want to focus on a single one for a bit. I used that to great effect to follow Le Mans this year: Using the previous patch (the method you suggest) meant that every time I went actual fullscreen with one of the streams when I came out of fullscreen the app UI would come out of the fullscreen mode. With this mode everything was seamless. All the streams are fullscreen from the point of view of the browser window and stay that way. Doing fullscreen on/off in sway just creates a resize event while fullscreen which is something all apps already have to support. The only alternative to get the same behavior is to patch every app to be able to not send fullscreen events at all. Firefox has a way to do that globally with an about:config setting for example which is much inferior to deciding that per window. Doing it centrally in sway makes things much more consistent. I have a keybinding that does this and it works for everything (e.g., VMs, video apps, etc). |
Before I comment further, I just want to clarify that I'm in favor of the feature, but I don't want to approve a protocol violation or code that feels more like a hack than a fully fleshed out feature.
I outlined where the violation was in my previous comment. However, there was some discussion on IRC and it looks like ddevault is going to try to get the protocol changed to give the compositor more control/flexibility. The current version of the proposed change is https://paste.sr.ht/%7Esircmpwn/f5e54de82418a202eef9193efe9f1c7fecb9cc8e
I didn't look at the original commit for this PR previously, but it was only a partial implementation of what I proposed above. Fully fleshing that version out would be the better implementation in my opinion.
My proposed implementation would allow the user to toggle the inhibitor, which would toggle the view between |
I saw that but didn't realize the protocol violation was not reacting visually in that way to the same sequence of events. The actual fullscreen/resize events are being handled in a compatible way. But I see now that the protocol definition also includes the actual visual interpretation of the wire protocol.
The current solution is encoding 4 different states which I'm not following how they would map to your proposal. Once you've made a |
The fullscreen commands would change the fullscreen status of the view. The inhibitor changes which fullscreen modes are allowed. |
Got it now, thanks. You're encoding what was previously two different states all in the same state machine. From the point of view of the user the result is the same so it's an implementation decision. |
You should absolutely not approve hacks! We should see the outcome of the new discussion on wayland-devel, and we should shape this how we want it to behave rather than merge and end up with annoying legacy features. ModelThe previous implementation used an additional fullscreen mode, FULLSCREEN_FAKE. This was made to work somewhat, but ended up much more complicated. Some places assume However, I am open to revisiting this behavior. Inhibiting user fullscreen and restoring modeI don't necessarily see this as optimal/practical. It also seems a bit more hacky to me when you consider that other fullscreen modes can be explicitly requested (i.e. global). Having it be a fullscreen mode, but on uninhibit jump back to another fullscreen mode also seems a bit magic to me. This is of course a matter of preference. However, idea: What if we made it a fully exposed fullscreen mode, and instead made a setting for the default fullscreen mode? This would be exposed as two commands: I feel like this approach would be much cleaner. What do you think? |
Just for reference, the thread can be found at https://lists.freedesktop.org/archives/wayland-devel/2019-June/040636.html
This approach sounds fine to me. |
I took a stab at implementing swaywm:d0d0181...pedrocr:87d2484 I left the This mostly works but I don't like it for two reasons:
The reason for both issues is that this shoe-horns two different features into the same state:
Better semantics might be:
|
One of the key aspects of this implementation is the presence of
This is the job of
While I don't really understand your explanation about While I have yet to find a need for this (and at the same time also like to avoid over-engineering for scenarios that will never occur), it is a very valid point that we should consider. Current implementation suggestions
@RedSoxFan 's comments to inhibit_fullscreen makes it work mostly as solution number one, so in that case I would prefer that as a purer solution. Number two is basically the original implementation. |
Those can be also added. But I guess you're also suggesting But if that's done then I don't see the value of
If As for the options I still prefer method 2. Considering the downsides:
I don't think this is a big issue but it would be simple to add convenience functions that do both actions together.
I think the opposite. Decoupling is a well defined concept, there are now two states that are no longer synchronized:
The
In a sense it's actually fairer. It's basically adding a flag to disable support for that bit of the protocol. Instead of lying on what it does with the information sway is just not implementing the fullscreen protocol at all for that client at the request of the user. |
It defines what happens when a client requests fullscreen, or when the user simply issues
It is simple to us, as we know of the fullscreen protocol. I am referring to an average user, who might not be aware of xdg_shell.request_fullscreen and its appropriate configure responses. To the average user (who is not supposed to know much about wayland protocols!), fullscreen means fullscreen. I believe it is easier to explain "There are 3 different fullscreen modes, stay in container, cover entire screen, or cover all screens" than "There are two fullscreen modes, as well as a synchronization toggle for the client fullscreen protocol".
It actually does not: Sway already have workspace and global fullscreen modes. Two options that the client knows nothing of becomes three.
The primary feature here applies to many applications (anything playing video tends to have this "minimal UI only in fullscreen" behavior), so it has strong merit. I'd like to see if there is also strong merit for its inverse (i.e. more than just a potential convenience only in Firefox where an existing button is just inconveniently placed).
I find it a bit less of a lie if we treat it and expose it as a fullscreen mode. We entered fullscreen, it's just not very full. In the other feature, we have a switch that will respond to fullscreen requests with a "Sure, I'll fullscreen you!", without changing any state at all. It's pedantic, sure, but I listed it regardless as pro/con. That aside, I personally do enjoy the UX of the original implementation. I think what I intend to do is to polish that up, and then create the alternative implementation as well. We can then simply do some testing of either. |
While I think this is true for the explanation of what feature is the UX will then be worse, so explaining how the feature works is harder. You pretty much have to bind extra keys to
This has been my impression from trying both as well. Once
Thanks again for taking an interest in this and doing so much work. See the branch I linked for a start on the alternative implementation. I had a look at what's still missing and beyond the simple UI tweaks in the |
d7daeb1
to
e28e9a2
Compare
Note: Firefox has |
Additional corrective note: |
Firefox can be configured to do this globally, and maybe an extension exists or can be written to make that per-window. But this is useful for more situations. I use it for virt-manager for example, and use matching rules to apply this to specific Firefox and other windows automatically. It's really useful and has worked flawlessly for months now. |
I use this effect often with xwayland windows (eg. chromium): get the xwayland window to fullscreen itself (eg. video playing), then un-fullscreen on sway (eg. Mod+F), and manipulate the xwayland window - which remains fullscreen within - as normal. I'd like to see this possible with xdg_shell/wayland windows (eg. firefox) |
inhibit_fullscreen allows a container to service but ignore fullscreen requests from clients. This allows clients to change to fullscreen mode without changing dimensions of its container.
I continue to use this daily. One thing I've noticed recently is that it doesn't work for XWayland windows. Chrome will still go fullscreen on F11 even if inhibit_fullscreen has been done. |
e28e9a2
to
ab58edf
Compare
I started using this patch and it's really helpful! What I'm missing a bit is the |
I understand this PR will allow me to make a window think it's fullscreen without it being fulscreen. Will it also me to actually fullscreen a window without telling it it's fullscreen (e.g.: make it as big as the entire screen, but don't send it the "you are now fullscreen" signal)? If so, I think this might cover #5397, since that's basically the definition of "maximisation". I can't comment much on protocol or code, but I do think this is essentially two states:
The current command basically toggles both attributes at once and keeps them aligned, while this PR would allow toggling one without the other. If there were a command to individually toggle either of these flag, I think that'd likely cover all potential use cases of this feature. |
It does exactly that. If |
Love this i3 "feature," hope it makes it to sway. |
Actually it'd be nice to have this implemented as in Awesome WM, instead of having a separate configuration setting. There they have the following:
|
for those like me who want back their old i3 fullscreen behavior for wayland windows can use my patch i created from the v1.7 branch for now |
And for those who want to install it on arch you can install sway-i3-style-fullscreen-git from AUR. |
Since this is not getting merged, is there an alternative from having to build sway with this patch? |
The AUR package works perfectly! I was almost going back to i3 because sway didnt behave like i3 on fullscreen And for those that were not using sway-git and wlroots-git for AUR, you have to uninstall sway and swaybg before installing sway-i3-style-fullscreen-git, otherwise some dependencies problems with sway and wlroots will arise:
so you have to do: sudo pacman -R sway swaybg
# and then
yay -S sway-i3-style-fullscreen-git |
Can we get the official word on whatever this will be merged or not? I suppose an open PR means it is still up for consideration? This is very useful and Quality Of Life feature for a TWM. on par with Maximising for other WMs. It would be a shame if it is never going to land in the official releases. |
There are no plans to merge this feature at this time. |
swayfx-i3-style-fullscreen is now broken for a while and doesn't seems to get any new update. I've created a new version if anyone ever needs it https://aur.archlinux.org/packages/swayfx-i3-style-fullscreen-2-git |
so why there is no plans to marge this feature? |
Some clients have special UIs in fullscreen mode, commonly optimized for no-fuss content delivery. Examples of such applications would be browsers playing videos. In such cases, the fullscreen UI is often only the video (potentially with overlays), making video consumption much more comfortable. Other applications may remove menu bars or other interfering elements.
However, one may simply wish to consume the fullscreen UI, not actually fullscreen the application. This is especially true for a tiling window manager, where one may have dedicated a fixed amount of screen real estate for the application. For a video, one could for example dedicate the area for the aspect ratio, leaving what would otherwise be black borders open to other work.
This PR implements a fullscreen inhibition feature in sway. When inhibited, sway will answer fullscreen requests correctly, but will not fullscreen the container.