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

core: transfer views to same workspace as on destroyed output #2294

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

lcolitti
Copy link
Contributor

Currently, when an output is destroyed, views are transferred to the new output in roughly the same position as they were on the old output, but they are all transferred to the first workspace.

This is annoying: if they were not on the first workspace, then the user must have moved them, and after they're transferred, the user must will have to move them agein.

Instead, in the (common?) case that the old and the new output have the same number of workspaces, move each view to the same workspace it was on before.

@soreau
Copy link
Member

soreau commented Mar 29, 2024

Is this not able to be fixed by only modifying the preserve-output plugin? Seems like that might be a better place for this to live.

@lcolitti
Copy link
Contributor Author

Is this not able to be fixed by only modifying the preserve-output plugin? Seems like that might be a better place for this to live.

Perhaps, but why require the user to run the preserve output plugin to get the correct behaviour? Core obviously needs to do the right thing here in case the user is not running the plugin. And in fact the code does try to do the right thing, by putting the views in roughly the same place as they were on the previous output, scaled by the ratio of output sizes. This commit just makes it put the views on the right desktop as well. 😄

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't ideal because move_view_to_output is used in other places as well, for example the oswitch plugin, and this will break the plugin.

I think the idea in general makes sense when destroying an output. It might make sense to add more flags to the function (instead of a bool). Maybe even add two overloads (to avoid breaking changes), one bool, one with flags, have one call the other :)

@lcolitti lcolitti force-pushed the move-same-workspace branch 2 times, most recently from 71d21cb to 536d767 Compare April 4, 2024 12:45
@lcolitti
Copy link
Contributor Author

lcolitti commented Apr 4, 2024

I think the idea in general makes sense when destroying an output. It might make sense to add more flags to the function (instead of a bool). Maybe even add two overloads (to avoid breaking changes), one bool, one with flags, have one call the other :)

Done

Currently, when an output is destroyed, views are transferred to
the new output in roughly the same position as they were on the
old output, but they are all transferred to the first workspace.

This is annoying: if they were not on the first workspace, then
the user must have moved them, and after they're transferred, the
user must will have to move them agein.

Instead, in the (common?) case that the old and the new output
have the same number of workspaces, move each view to the same
workspace it was on before.
When outputs are unplugged, views are temporarily moved to the
NOOP-1 output. Because this output is currently only 1280x720,
windoes moved to it get moved and cropped to match its small
screen size.

Make the output 4k so windows stay where they are unless the user
is running a display larger than 4k.

This does not affect the WL-x outputs used when running in
headless mode, which continue to be created as 1280x720 by
wlroots code (attempt_headless_backend).
Currently, when destroying the current output, core focuses the
next output (if any) but does not switch to the same workspace
that the user was previously using. Switch to the same workspace
so the user can pick up where they left off.
@@ -536,6 +540,11 @@ void wf::move_view_to_output(wayfire_toplevel_view v, wf::output_t *new_output,
{
auto new_g = wf::clamp(view_g, new_output->workarea->get_workarea());
v->set_geometry(new_g);
if (same_workspace &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it also make sense to have similar treatment for fullscreen and tiled windows? You can pass the requested workspace to fullscreen/tile request too.

transfer_views(wo, shutdown ? nullptr : get_core().seat->get_active_output());
wf::output_t *new_output = shutdown ? nullptr : get_core().seat->get_active_output();
transfer_views(wo, new_output);
if (new_output &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehh I'm not quite sure about this particular change. Is this really what users expect?

auto handle = wlr_headless_add_output(noop_backend, 1280, 720);
// NOOP output should be at least as large as actual screen sizes. Otherwise, when
// when windows are temporarily mapped to it, they will be moved/cropped to match it.
auto handle = wlr_headless_add_output(noop_backend, 3840, 2160);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The noop output can be configured from the config file (IIRC), so this is only the default size if nothing else is set. I would rather not make it that big by default because if the user has some underpowered GPU mayybe this buffer would be too big?

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

Successfully merging this pull request may close these issues.

None yet

3 participants