Closed flaviozavan closed 2 years ago
Thanks yet again - the code is good but I'd like to be a little careful with this one, as for xdg-shell views (which as the main shell should be our generic behaviour I think) the fullscreen behaviour required is quite specific:
The output passed by the request indicates the client's preference as to which display it should be set fullscreen on. If this value is NULL, it's up to the compositor to choose which display will be used to map this surface.
If the surface doesn't cover the whole output, the compositor will position the surface in the center of the output and compensate with with border fill covering the rest of the output. The content of the border fill is undefined, but should be assumed to be in some way that attempts to blend into the surrounding area (e.g. solid black).
If the fullscreened surface is not opaque, the compositor must make sure that other screen content not part of the same surface tree (made up of subsurfaces, popups or similarly coupled surfaces) are not visible below the fullscreened surface.
I'm not a huge fan of how prescriptive this is, but it's what the protocol says so if we tell an xdg-shell view it's fullscreen we should obey it. I think that means some tweaks from what you currently have:
// Render fullscreen views last
we should render any fullscreen view first and if one was rendered on an output then skip the rest for that outputI'd also like to have a config option named something like "global_allow_fullscreen" which, if false, retains the current behaviour - this is my preference (and why it's currently that way), because I don't want apps to have control over how they're displayed within the output. This is important for e.g. firefox, which asks to go to xdg-shell fullscreen mode when you fullscreen a video, but you can reasonably want to do that within the firefox window without that window having to fill the output. I'm expecting to give more control over that later (via a system of configurable window hooks), but for now a global toggle should be good.
(Firefox actually has a bug right now (link) if fullscreen requests are denied, but that's a separate issue!)
How does that sound to you?
This all sounds very fair. I got some of it done already, I am slowly working on it and will update when done.
OK. So I changed the implementation to match the specification. I slowly tested all possible cases, but it is rather difficult to find a piece of software that wants to set a specific fullscreen resolution and doesn't want to hog all the real estate as soon as it's available. The only examples I could find were old windows games under wine (e.g. age of mythology lets you set any custom resolution).
There is a missing puzzle piece here. I implemented handling of these lower resolution cases (and you can test this with an early return in the new grow implementation function and a native application that can go fullscreen, like mpv). But, for these old games, if you request a lower resolution than the displays, they will crash, as they can't find a compatible mode. They don't crash under sway, but they don't go fullscreen with borders either. I could not determine for certain what is expected to handle this, but I believe it's not necessarily related to proper fullscreen support, but rather some other part, possibly output managing, as the same games work just fine with these resolutions in wine's emulated desktop mode.
Sorry for leaving you hanging on this, I missed the notification originally but I'm catching up now and I'll get to it shortly.
They don't crash under sway, but they don't go fullscreen with borders either. I could not determine for certain what is expected to handle this, but I believe it's not necessarily related to proper fullscreen support, but rather some other part, possibly output managing
I wonder if this is related to output passthrough that I think sway does during fullscreen, maybe this exposes the output more directly. It's certainly not a blocker here.
Looks good, I was about to merge with a minor tidy up that I've pushed at https://github.com/inclement/vivarium/pull/106 but I noticed there seems to be something wrong with damage tracking on this branch: when I drag a floating view around its border is not correctly overdrawn. It's probably a simple issue and I'll try to resolve it myself, or @flaviozavan if you see the issue and push a fix I'm happy to merge.
I think the rendering issue is affecting views resized via the cursor, and only when full damage tracking is enabled (which isn't the default). Looks like the border is drawn slightly too large. I'm looking to fix it but haven't tracked down quite what's changed.
Merged now as 1231805504c11e00e4a60ec4c841844a67a14b7f (#106), thanks again.
Introduces support for fullscreen requests and initializations in both xdg and xwayland
Not implemented: handling full screen requests to specific outputs. We always go with the one the view is already in.
Testing:
mpv
is very versatile for this. Try all of the combinations:-fs -vo wlshm
);-vo wlshm
);-fs -vo x11
);-vo x11
).Toggle to and from fullscreen a few times. Move to other workspaces, try floating fullscreen view, try resizing, dragging...
Also try making a floating window fullscreen and back. It should remember the old geometry.