Skip to content

Conversation

@mrsobakin
Copy link
Collaborator

Implements toggling feature. More detailed description can be seen in #121.

To keep the implementation simple, I have decided to implement per-display toggling features as plain config modifications, meaning that regexes and fuzzy names will toggle only the exact matches in config.

In other words, if display was previously disabled via !Disp.*, it will not be re-enabled by toggling !Display.* or !Di.*. The user will have to toggle the exact !Disp.* setting.

Also, a small fix has been applied to prevent AUTO_SCALE_MIN and AUTO_SCALE_MAX from resetting on toggles: now clone_cfg clones these values regardless of AUTO_SCALE state.

Resolves #121.

@alex-courtis
Copy link
Owner

Looks great! I'll get to review/test in the next few days.

@alex-courtis
Copy link
Owner

I've approved CI workflows and it's run with a minor failure: https://github.com/alex-courtis/way-displays/actions/runs/14132979074/job/39626268018?pr=193

include-what-you-use is usually problematic. Would you like me to push a fix to your fork?

@mrsobakin
Copy link
Collaborator Author

Hi! I've ran iwyu and uploaded a fix myself. CI workflow passes now.

@alex-courtis
Copy link
Owner

Hi! I've ran iwyu and uploaded a fix myself. CI workflow passes now.

Legend, thank you.

@alex-courtis
Copy link
Owner

In other words, if display was previously disabled via !Disp.*, it will not be re-enabled by toggling !Display.* or !Di.*. The user will have to toggle the exact !Disp.* setting.

That's completely fine - there's no way to determine which head the other settings would apply to, nor avoiding clobbering other matches.

Copy link
Owner

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

Looking great, a couple of edge case failures:

Testing with empty cfg and one with a value set.

Ran all testing of client and server with valgrind, no leaks.

--toggle DISABLED

Works nicely

--toggle VRR_OFF

All good

--toggle AUTO_SCALE

Not working in the case of AUTO_SCALE: FALSE in cfg.yaml - value is null
Works in other cases.

==17610== Memcheck, a memory error detector
==17610== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==17610== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==17610== Command: way-displays --toggle AUTO_SCALE
==17610==

Client sending request: toggle
Auto scale: ON (min: 0.000)

Server received request: toggle
Auto scale: ON (min: 0.000)

New configuration:
Arrange in a ROW aligned at the BOTTOM
Order:
M28U
LG Display 0x05EF
Sharp Corporation 0x148D
Samsung Display Corp. 0x4165
Ancor Communications Inc ROG PG27AQ
Samsung Electric Company C34J79x
Scaling: ON
Auto scale: (null) (min: 1.000)
Mode:
MAG274QRF-QD: 2560x1440@120Hz
M28U: MAX
Change success command:
notify-send "way-displays ${CALLBACK_LEVEL}" "${CALLBACK_MSG}"
==17610==
==17610== HEAP SUMMARY:
==17610==     in use at exit: 37,240 bytes in 256 blocks
==17610==   total heap usage: 15,653 allocs, 15,397 frees, 1,302,125 bytes allocated
==17610==
==17610== LEAK SUMMARY:
==17610==    definitely lost: 0 bytes in 0 blocks
==17610==    indirectly lost: 0 bytes in 0 blocks
==17610==      possibly lost: 0 bytes in 0 blocks
==17610==    still reachable: 0 bytes in 0 blocks
==17610==         suppressed: 35,392 bytes in 235 blocks
==17610==
==17610== For lists of detected and suppressed errors, rerun with: -s
==17610== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

--toggle SCALING

Not applying for empty cfg - looks like it's always setting ON
Working when SCALING: FALSE set in cfg

==17826== Memcheck, a memory error detector
==17826== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==17826== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==17826== Command: way-displays --toggle SCALING
==17826==

Client sending request: toggle
Scaling: ON

Server received request: toggle
Scaling: ON

New configuration:
Arrange in a ROW aligned at the TOP
Auto scale: ON (min: 1.000)
Change success command:
notify-send "way-displays ${CALLBACK_LEVEL}" "${CALLBACK_MSG}"
==17826==
==17826== HEAP SUMMARY:
==17826==     in use at exit: 37,240 bytes in 256 blocks
==17826==   total heap usage: 14,606 allocs, 14,350 frees, 1,225,836 bytes allocated
==17826==
==17826== LEAK SUMMARY:
==17826==    definitely lost: 0 bytes in 0 blocks
==17826==    indirectly lost: 0 bytes in 0 blocks
==17826==      possibly lost: 0 bytes in 0 blocks
==17826==    still reachable: 0 bytes in 0 blocks
==17826==         suppressed: 35,392 bytes in 235 blocks
==17826==
==17826== For lists of detected and suppressed errors, rerun with: -s
==17826== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)




==17843== Memcheck, a memory error detector
==17843== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==17843== Using Valgrind-3.24.0 and LibVEX; rerun with -h for copyright info
==17843== Command: way-displays --toggle SCALING
==17843==

Client sending request: toggle
Scaling: ON

Server received request: toggle
Scaling: ON

New configuration:
Arrange in a ROW aligned at the TOP
Scaling: ON
Auto scale: ON (min: 1.000)
Change success command:
notify-send "way-displays ${CALLBACK_LEVEL}" "${CALLBACK_MSG}"
==17843==
==17843== HEAP SUMMARY:
==17843==     in use at exit: 37,240 bytes in 256 blocks
==17843==   total heap usage: 14,656 allocs, 14,400 frees, 1,229,344 bytes allocated
==17843==
==17843== LEAK SUMMARY:
==17843==    definitely lost: 0 bytes in 0 blocks
==17843==    indirectly lost: 0 bytes in 0 blocks
==17843==      possibly lost: 0 bytes in 0 blocks
==17843==    still reachable: 0 bytes in 0 blocks
==17843==         suppressed: 35,392 bytes in 235 blocks
==17843==
==17843== For lists of detected and suppressed errors, rerun with: -s
==17843== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

@alex-courtis
Copy link
Owner

I'll update contributing to remove the outdated ubuntu image notes, replacing with arch, as per CI.
Apologies if that was confusing.

@alex-courtis
Copy link
Owner

Apologies for closing/reopening; annoying github shortcut ctrl-shift-enter closes the issue.

@alex-courtis
Copy link
Owner

Looks like CI is failing following release of cmake 4.0, several hours ago.

I'll fix it on master.

@alex-courtis
Copy link
Owner

Please rebase from master, ci fix #194 merged.

@mrsobakin
Copy link
Collaborator Author

mrsobakin commented Mar 30, 2025

I've fixed the issue with AUTO_SCALE and SCALING toggling.

A nasty blunder -- I mistook cfg->scaling and cfg->auto_scale for bools, when in fact they were OnOff enums.

Both code and tests are fixed.

@mrsobakin mrsobakin requested a review from alex-courtis March 30, 2025 00:15
Copy link
Owner

@alex-courtis alex-courtis left a comment

Choose a reason for hiding this comment

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

All scaling working nicely, many thanks for your contribution!

Ready to merge when you are.

I'll release 1.13.0 afterwards.

@mrsobakin
Copy link
Collaborator Author

Nice, everything looks good to me too! You can merge.

Shortly, I'll open a draft PR regarding another feature, which is a bit less straightforward than toggles and will require some discussion.

@alex-courtis
Copy link
Owner

Nice, everything looks good to me too! You can merge.

Shortly, I'll open a draft PR regarding another feature, which is a bit less straightforward than toggles and will require some discussion.

That'd be fantastic... you seem to know your way around this codebase ;)

@alex-courtis alex-courtis reopened this Mar 30, 2025
@alex-courtis alex-courtis changed the title #121 Implement toggles #121 add --toggle for SCALING, AUTO_SCALE, VRR_OFF, DISABLED Mar 30, 2025
@alex-courtis alex-courtis merged commit 2bf862c into alex-courtis:master Mar 30, 2025
2 checks passed
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.

Feature: toggles

2 participants