Skip to content

Monitor v2 config #9761

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Monitor v2 config #9761

wants to merge 5 commits into from

Conversation

UjinT34
Copy link
Contributor

@UjinT34 UjinT34 commented Mar 27, 2025

Describe your PR, what does it fix/add?

Adds a monitorv2 config special category similar to device.

CM needs more monitor settings. Placing them into the current monitor config makes it too complicated.
Minimum monitorv2 config should look like this:

monitorv2 {
  output = DP-1
  mode = 1920x1080@144
  position = 0x0
  scale = 1
}

Other settings have the same names as in monitor config.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

handleMonitorv2 is called for every monitorv2 at once. addSpecialCategory or SSpecialCategoryOptions need an additional handler parameter similar to registerHandler to handle each monitorv2 individually.
monitorv2 rules will override anything set by monitor rules whenever handleMonitorv2 is called. During a config reload it'll happen after every other monitor rule regardless of their order inside the config.

Is it ready for merging, or does it need work?

Ready

@vaxerski
Copy link
Member

what about dynamic calls?

@UjinT34
Copy link
Contributor Author

UjinT34 commented Mar 29, 2025

Do they work with device? Do we need those with monitorv2? Some config includes can be used to achieve same results. Existing monitor dynamic calls should work the same.

@vaxerski
Copy link
Member

vaxerski commented Mar 30, 2025

yes they do work, I'm just curious how you'd handle them with this without a billion commits

@github-actions github-actions bot added the debug label Apr 5, 2025
@UjinT34
Copy link
Contributor Author

UjinT34 commented Apr 5, 2025

Looks like hyprctl keyword monitorv2[desc:something]... isn't supported by parser. hyprctl keyword monitorv2[output]... should work fine.
I don't think there is a point in adding anything else with this PR. Need some hyprlang and special categories changes first.

@vaxerski
Copy link
Member

vaxerski commented Apr 5, 2025

hyprctl keyword monitorv2[DP-2]:blahblah 2 works though, right? desc: might break due to the colon, that might be something to-fix in hyprlang.

@@ -1087,6 +1087,19 @@ static std::string dispatchKeyword(eHyprCtlOutputFormat format, std::string in)
if (COMMAND == "monitor" || COMMAND == "source")
g_pConfigManager->m_bWantsMonitorReload = true; // for monitor keywords

if (COMMAND.contains("monitorv2")) {
if (COMMAND.contains('[') && COMMAND.contains(']')) {
Copy link
Member

Choose a reason for hiding this comment

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

why are we handling this? This is handled by hyprlang, no?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Special categories are parsed without any handlers attached. Need a way to trigger a handler after the parsing for a given output.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's just because this MR has a weird approach. Why not use the approach like for device configs? After a dyncall, or after parsing, recheck monitorv2 stuff. If anything changed, update and reload monitor mode

@UjinT34
Copy link
Contributor Author

UjinT34 commented Apr 6, 2025

hyprctl keyword monitorv2[DP-2]:blahblah 2 works though, right? desc: might break due to the colon, that might be something to-fix in hyprlang.

DP-2 works. Probably should leave ``hyprctl keyword monitorv2` as unsupported/experimental for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants