Skip to content

Silenced layershell warning#457

Open
stefonarch wants to merge 3 commits into
masterfrom
silence_warning
Open

Silenced layershell warning#457
stefonarch wants to merge 3 commits into
masterfrom
silence_warning

Conversation

@stefonarch
Copy link
Copy Markdown
Member

No description provided.

@stefonarch
Copy link
Copy Markdown
Member Author

@marcusbritanicus there are identical PRs for lxqt-runner, lxqt-session (leave), qterminal dropdown and notificationd.

@stefonarch stefonarch marked this pull request as draft March 27, 2026 08:21
@stefonarch stefonarch marked this pull request as ready for review April 20, 2026 12:34
@stefonarch stefonarch requested review from tsujan and removed request for marcusbritanicus April 20, 2026 12:34
Comment thread src/core/regionselect.cpp Outdated
layershell->setExclusiveZone(-1); // not moved to accommodate for other surfaces
win->setScreen(_selectedScreen == nullptr ? qApp->primaryScreen() : _selectedScreen);
layershell->setScreenConfiguration(LayerShellQt::Window::ScreenConfiguration::ScreenFromQWindow);
layershell->setWantsToBeOnActiveScreen(true);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So this is wrong too, but actually it makes screengrab with default=region more usable when as it will automatically select the region on the active screen and ignore the setting for the monitor.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So this is wrong too

I'm afraid, yes. It should be

                layershell->setWantsToBeOnActiveScreen(false);
                layershell->setScreen(nullptr);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With this set to true when default is "region" it behaves exactly as niri's screenshot, opening the select area overlay on the current screen.
Wouldn't it be possible to make this a setting in the combo also for fullscreen? Just

  • active screen
  • screen 1
  • screen 2
  • ...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The goal of this PR is silencing a warning, not changing usability. Doing the latter here would be very wrong.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't it be possible to make this…

It should be, but it'll need more changes and should be done backward compatibly. Please make a feature request instead.

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.

2 participants