FedeDP / Clightd

A linux bus interface that lets you change screen brightness, compute captured webcam frames brightness and change screen temperature.
GNU General Public License v3.0
83 stars 10 forks source link

Gamma resets on wayland after some time #77

Closed frozen-sea closed 2 years ago

frozen-sea commented 3 years ago

I'm on Void Linux and have Clightd 5.4 installed from the official repos, it's running as root. Compositor is Sway. Everything else in Clight works, except the gamma feature. Based on trying to probe Clightd manually, it seems that's where the issue originates. Checked gamma.c to see what the error message might indicate, but since it's a catch-all I wasn't able to deduce much. As far as I can tell, elogind is setting up the environment correctly. Would be thankful for any help, since I really like Clight. :)

~$ echo $WAYLAND_DISPLAY
wayland-1
~$ echo $XDG_RUNTIME_DIR
/run/user/1000
~$ dbus-send --system --print-reply --dest=org.clightd.clightd /org/clightd/clightd/Gamma org.clightd.clightd.Gamma.Get string:$WAYLAND_DISPLAY string:$XDG_RUNTIME_DIR
Error org.freedesktop.DBus.Error.Failed: Failed to get screen temperature.
FedeDP commented 3 years ago

Hi! Thanks for opening this issue!

About "Get" method: sway gamma protocol does not support any Get method, thus it is unsupported on it. Can you share output of "Set" method call?

frozen-sea commented 3 years ago

About "Get" method: sway gamma protocol does not support any Get method, thus it is unsupported on it.

Ah, that makes sense.

Can you share output of "Set" method call?

~$ gdbus call --system --dest org.clightd.clightd  --object-path /org/clightd/clightd/Gamma --method org.clightd.clightd.Gamma.Set $WAYLAND_DISPLAY $XDG_RUNTIME_DIR 4000 "(false, 0, 0)"
(true,)

Gamma gets set correctly by that call, so I guess we move on to looking at Clight then.

On startup clight --verbose complains that:

init(): Failed to get screen temperature.
Failed to init. Killing module.

The manually set gamma gets reset to default whenever clight outputs:

Set timeout of 30s 0ns on fd 33.

Thank you for your time.

Edit: I should also add that this is with the default clight.conf.

FedeDP commented 3 years ago

Sway gamma protocol has a weird thing that it reverts screen temperature whenever it detects a client is "misbehaving"; in Clightd, i already had to use a workaround for that: https://github.com/FedeDP/Clightd/blob/master/src/modules/gamma_plugins/wl.c#L134. Ie: Clightd is caching the wayland display connection because as soon as it disconnects, the gamma temperature is resetted by sway/wlroots. May be there has been some updates on wlroots side that added some more "logic" to that "client misbehaving" detector? I will take a look for sure testing on sway ;)

FedeDP commented 3 years ago

Uh note also that latest Clight (4.7) has a bug that disables gamma module on wlroots; can you share a log from Clight? The fix is quite small (in Clightd) but it needs a release. Btw i tried on sway and it works fine.

frozen-sea commented 3 years ago

I have now tested 4.6 also and there is a difference, but neither are working as expected. Starting 4.7 does not change the gamma at all, 4.6 changes the gamma but it then gets reset after a while when Set timeout of 30s 0ns on fd 33. is logged. Also tried master, which gives same result as 4.7.

Log output of master:

Clightd found, version: 5.4.
AC screen backlight curve: y = -0.024825 + 0.191641x + -0.008928x^2
BL
^
|        **
|      **  
|     *    
|    *     
|   *      
|  *       
|          
| *        
|*         
|          
*---------->BR
BATT screen backlight curve: y = -0.010629 + 0.153844x + -0.007284x^2
BL
^
|          
|          
|       ***
|     **   
|    *     
|   *      
|  *       
| *        
|          
|*         
*---------->BR
init_kbd_backlight(): No such file or directory
Keyboard backlight calibration unsupported.
init(): Failed to get screen temperature.
Failed to init. Killing module.
org.freedesktop.ScreenSaver dbus interface exposed.
Disarmed timerfd on fd 32.
AC cable connected.
Laptop lid opened.
Emitting 'AcState' property
Set timeout of 0s 1ns on fd 33.
Emitting 'LidState' property
Set timeout of 30s 0ns on fd 33.
Set timeout of 0s 1ns on fd 34.
Currently inside an event.
Next alarm due to: Sun Sep  5 20:54:00 2021
Set timeout of 1601s 0ns on fd 34.
Emitting 'Sunrise' property
Emitting 'Sunset' property
Emitting 'DayTime' property
Emitting 'NextEvent' property
Emitting 'InEvent' property
Set timeout of 0s 1ns on fd 35.
Sensor '/dev/video0' is now available.
Resumed as a sensor is now available.
Emitting 'SensorAvail' property
Captured [5/5] from '/dev/video0'. Ambient brightness: 0.911.
Ambient brightness: 0.911 -> Backlight pct: 0.980.
Set timeout of 300s 0ns on fd 35.
Emitting 'AmbientBr' property
Emitting 'BlPct' property
Backlight 'intel_backlight' level updated: 0.98.
Emitting 'BlPct' property
Set timeout of 30s 0ns on fd 33.
Set timeout of 30s 0ns on fd 33.
Received 2. Leaving.
FedeDP commented 3 years ago

Also tried master, which gives same result as 4.7.

You should try Clight and Clightd masters; it should work (at least like 4.6). In my case, sway worked quite reliably...

4.6 changes the gamma but it then gets reset after a while when Set timeout of 30s 0ns on fd 33

Are you able to better specify how much time is "a while"? I mean, like 10minutes, 45seconds, 2 seconds ;) It will greatly help me test this issue! Btw that Set timeout of 30s 0ns on fd 33 comes from Screen module; you can try disabling it and see if it makes any difference!

FedeDP commented 3 years ago

Btw that Set timeout of 30s 0ns on fd 33 comes from Screen module; you can try disabling it and see if it makes any difference!

This would also explain why i am not seeing the issue: i tested on battery, and by default Screen module is stopped while on battery!

frozen-sea commented 3 years ago

Right, both are on master now. Gamma gets set correctly on startup of Clight and then gets reset after 30 seconds, since that is the default timeout of the screen module. Disabling screen resolves the issue, at least I've been using it like that for slightly over an hour now without incident.

frozen-sea commented 3 years ago

Even with the screen module disabled, there is still something else that eventually triggers the reset. But that takes something on the order of multiple hours to happen.

FedeDP commented 2 years ago

Hi! I am writing only to let you know that i am still willing to fix this before next release ;) Let's see if i am able to understand what happens!

FedeDP commented 2 years ago

Hi! I should've finally managed to fix this :) care to try @frozen-sea?

frozen-sea commented 2 years ago

@FedeDP Sorry for the late reply, only just got around to testing it out tonight. On current masters everything now works perfectly without needing to mess around with disabling any modules in the config.

FedeDP commented 2 years ago

Great to hear that!