Raise/lower brightness by specific amount

Question

Raise/lower brightness by specific amount

emilyyyylime opened this issue 9 months ago · 10 comments

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).

Answer 1 · 2024-01-09T03:48:07.000Z

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)

Answer 2 · 2024-01-09T22:07:33.000Z

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

Answer 3 · 2024-01-09T22:17:06.000Z

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?

Answer 4 · 2024-01-09T23:37:23.000Z

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.

Answer 5 · 2024-01-10T01:26:10.000Z

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.

Answer 6 · 2024-01-10T14:08:42.000Z

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.

Answer 7 · 2024-01-10T17:38:48.000Z

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.

Answer 8 · 2024-02-06T21:27:33.000Z

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.

Answer 9 · 2024-02-08T10:29:09.000Z

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

Answer 10 · 2024-02-08T17:24:57.000Z

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.