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

Raise/lower brightness by specific amount #68

Closed
emilyyyylime opened this issue Dec 15, 2023 · 10 comments · Fixed by #67
Closed

Raise/lower brightness by specific amount #68

emilyyyylime opened this issue Dec 15, 2023 · 10 comments · Fixed by #67

Comments

@emilyyyylime
Copy link

Since #60 we have the option to use swayosd-client --brightness <number>|raise|lower, and supposedly --brightness +n|-n, but the latter two are parsed as if the prefixes weren't there. This is inconsistent with both the behaviour of the other control elements and the documentation (swayosd --help).

@MintyGitOut
Copy link

Having the same issue, hope dev fixes this because at night 5% is too bright, and 0% is obviously off. Haven't found any other app that does the same thing as this one either (giving the user an OSD)

@trollLemon
Copy link
Contributor

I can look into this and see if I can find whats causing this bug, unless someone else has started already.

@trollLemon
Copy link
Contributor

trollLemon commented Jan 9, 2024

Since #60 we have the option to use swayosd-client --brightness <number>|raise|lower, and supposedly --brightness +n|-n, but the latter two are parsed as if the prefixes weren't there. This is inconsistent with both the behaviour of the other control elements and the documentation (swayosd --help).

Yep I'm having the same issue. Not sure if this is also happening with y'all but when I run for instance swayosd-client --brightness +10 my brightness goes very low, almost completely off.

Do you also get:

Trying BrightnessCtl Backend...
...Command failed! Falling back to Blight

When ever you run the commands as well?

@trollLemon
Copy link
Contributor

OK so I've noticed that at the start of the program when it parsed the user input, I believe that is where the +/- is ignored.

I printed out the command line argument data and the output is Some("70"), when i do both

./target/debug/swayosd-client  --brightness  -70
./target/debug/swayosd-client  --brightness  +70

Here is what is confusing me:
From my understanding the code to get the brightness values from the user input is in src/client/main.rs:

        app.add_main_option(
                "brightness",
                glib::Char::from(0),
                OptionFlags::NONE,
                OptionArg::String,
                "Shows brightness osd and raises or loweres all available sources of brightness device",
                Some("raise|lower|(±)number"),
        );

If we have OptionArg::String, why wouldn't it grab the whole string of "-70"? I tried looking at the gtk-rs documentation but couldn't find anything that suggests this as a problem.

@trollLemon
Copy link
Contributor

Do you also get:

Trying BrightnessCtl Backend...
...Command failed! Falling back to Blight

When ever you run the commands as well?

OK so this error isn't related to the problem its because I didn't have brightnessctl installed.

@MintyGitOut
Copy link

Since #60 we have the option to use swayosd-client --brightness <number>|raise|lower, and supposedly --brightness +n|-n, but the latter two are parsed as if the prefixes weren't there. This is inconsistent with both the behaviour of the other control elements and the documentation (swayosd --help).

Yep I'm having the same issue. Not sure if this is also happening with y'all but when I run for instance swayosd-client --brightness +10 my brightness goes very low, almost completely off.

Do you also get:


Trying BrightnessCtl Backend...

...Command failed! Falling back to Blight



When ever you run the commands as well?

What you're experiencing with the low brightness is SwayOSD actually setting your brightness to the number it's supposed to be adding by. No idea why it does this, but it's what I've observed.

@trollLemon
Copy link
Contributor

Since #60 we have the option to use swayosd-client --brightness <number>|raise|lower, and supposedly --brightness +n|-n, but the latter two are parsed as if the prefixes weren't there. This is inconsistent with both the behaviour of the other control elements and the documentation (swayosd --help).

Yep I'm having the same issue. Not sure if this is also happening with y'all but when I run for instance swayosd-client --brightness +10 my brightness goes very low, almost completely off.
Do you also get:


Trying BrightnessCtl Backend...

...Command failed! Falling back to Blight

When ever you run the commands as well?

What you're experiencing with the low brightness is SwayOSD actually setting your brightness to the number it's supposed to be adding by. No idea why it does this, but it's what I've observed.

Hmm, that's odd.

I did figure our why the prefix was getting cut off though. Turns out it wasn't the problem in main.rs. In the global_utils.rs file, for the brightness change function the step value was being converted to the absolute value, then converted to a string. Though removing the .abs() causes issues since the server-side operates with u8, so we can't work with negative numbers unless we change the types for the brightness backend. I also noticed that when using +n/-n for the brightness command the program flow goes to backend.set instead of backend.raise or backend.lower. I'll look into this more later.

@trollLemon
Copy link
Contributor

trollLemon commented Feb 6, 2024

Ok, Looked at this issue again and I think I figured out the issue.

The brightness command --brightness +n|-n will set the brightness as a percentage instead of raising or lowering.
So, --brightness +10 will be interpreted as 10% of the screen brightness.

In brightnessctl.rs the set function calls a set_percent function:

     	fn set_percent(&mut self, mut val: u32) -> anyhow::Result<()> {
		val = val.clamp(0, 100);
		self.current = self.max.map(|max| val * max / 100);
		let _: String = self.run(("set", &*format!("{val}%")))?;
		Ok(())
	}
   
       ...
      
	fn set(&mut self, val: u32) -> anyhow::Result<()> {
		self.device.set_percent(val)
	}

I'm not sure if this was intended because the documentation doesn't mention anything about setting as a percentage of the screen brightness with --brightness +n|-n.

I think we could either have set_percent take into account the current percentage and calculate the new percentage after the brightness increase (or decrease), or change the backend to run the raise and lower functions instead of set for --brightness +n|-n. I think the former would be better as the changes would just be in the set_percent function rather than re-writing the command logic.

Also, it doesn't look like the blight.rs file has this same issue.

EDIT:

Looks like set_percent is also used by raise and lower, so it might be better to have a set_by_value function instead of changing set_percent.

EDIT 2:

Ok, so turns out it wasn't a lot of code to get the backend to call the functions that raise or lower the brightness.
I put dbg! in the brightnessctl and this is my output:

02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness +5
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 81
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 206
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness +5
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 86
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 219
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness +5
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 90
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 229
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness -10
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 80
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 204
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness -10
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 70
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 178
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness -10
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 60
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 153
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness +10
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 70
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 178
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:46 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness +10
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 80
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 204
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255

This is assuming the -n/+n is a percentage.

Now the other problem I found is that swayosd-client --brightness <number>|raise|lower doesn't keep the raise/lower string in the input. However, it does work when there is no number supplied, as it defaults to 5%:

Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 73
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 186
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:49 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness lower
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 67
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 170
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:49 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness lower
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 62
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 158
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255
02:49 SwayOSD (main) ✗ ./target/debug/swayosd-client  --brightness raise
Trying BrightnessCtl Backend...
[src/server/../brightness_backend/brightnessctl.rs:113] &val = 66
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_current() = 168
[src/server/../brightness_backend/brightnessctl.rs:113] self.get_max() = 255

I'm not sure if we should try and get the raise and lower with a custom amount working, or replace it with the -n/+n since it does the same thing. I'll ping @ErikReider and see what they think. In the meantime I'll get stuff ready for a pr and see if we can resolve this issue.

@emilyyyylime
Copy link
Author

What's a raise or lower with a custom amount? afai'm aware, SwayOSD only accepts [|+|-]<num>|raise|lower

@trollLemon
Copy link
Contributor

What's a raise or lower with a custom amount? afai'm aware, SwayOSD only accepts [|+|-]<num>|raise|lower

Ah, I misunderstood that its +/- num or raise or lower. In that case, I can set up a PR fixing the prefix not being parsed, so --brightness +n|-n works correctly.

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