Document OSD implementation decisions
This commit is contained in:
@@ -143,6 +143,14 @@ systemctl --user status external-brightness-sync.service
|
|||||||
journalctl --user -u external-brightness-sync.service -n 50 --no-pager
|
journalctl --user -u external-brightness-sync.service -n 50 --no-pager
|
||||||
```
|
```
|
||||||
|
|
||||||
|
## Design Notes
|
||||||
|
|
||||||
|
Implementation decisions and failed attempts are documented in:
|
||||||
|
|
||||||
|
```text
|
||||||
|
docs/implementation-log.md
|
||||||
|
```
|
||||||
|
|
||||||
## Uninstall
|
## Uninstall
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
|
|||||||
155
docs/implementation-log.md
Normal file
155
docs/implementation-log.md
Normal file
@@ -0,0 +1,155 @@
|
|||||||
|
# Implementation Log
|
||||||
|
|
||||||
|
This file documents the brightness/OSD design decisions, failed attempts, and current state.
|
||||||
|
|
||||||
|
## Current Working Design
|
||||||
|
|
||||||
|
The current setup intentionally keeps the brightness path simple:
|
||||||
|
|
||||||
|
1. A brightness shortcut immediately changes the internal panel with `brightnessctl`.
|
||||||
|
2. The OSD is shown or updated after the internal change.
|
||||||
|
3. The external monitor follows through `external-brightness-sync.service`.
|
||||||
|
|
||||||
|
The internal panel is the master. The external monitor is never adjusted independently by `wluma`.
|
||||||
|
|
||||||
|
## Why Internal Is The Master
|
||||||
|
|
||||||
|
Letting `wluma` control both the internal backlight and the external DDC/CI monitor independently caused occasional drift between displays.
|
||||||
|
|
||||||
|
The cleaner design is:
|
||||||
|
|
||||||
|
- `wluma` controls only `/sys/class/backlight/intel_backlight`
|
||||||
|
- `external-brightness-sync.service` reads the internal percentage
|
||||||
|
- `ddcutil` sets the external monitor to the same percentage
|
||||||
|
|
||||||
|
This keeps one source of truth for automatic brightness.
|
||||||
|
|
||||||
|
## Native GNOME OSD Attempt
|
||||||
|
|
||||||
|
GNOME Shell exposes a DBus method:
|
||||||
|
|
||||||
|
```text
|
||||||
|
org.gnome.Shell.ShowOSD
|
||||||
|
```
|
||||||
|
|
||||||
|
On GNOME Shell 49.6, external calls to this method failed with:
|
||||||
|
|
||||||
|
```text
|
||||||
|
GDBus.Error:org.freedesktop.DBus.Error.AccessDenied: ShowOSD is not allowed
|
||||||
|
```
|
||||||
|
|
||||||
|
Because of that, the project uses a small GTK4/libadwaita OSD instead of trying to call GNOME Shell's private OSD API.
|
||||||
|
|
||||||
|
## GTK OSD Positioning On Wayland
|
||||||
|
|
||||||
|
The first GTK OSD version used a normal GTK window. It worked, but GNOME Wayland placed it like a normal window, so it appeared near the center of the screen.
|
||||||
|
|
||||||
|
Under Wayland, normal clients cannot reliably choose arbitrary screen coordinates. A top-edge OSD should use the layer-shell protocol.
|
||||||
|
|
||||||
|
The project therefore supports `gtk4-layer-shell`:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
sudo dnf install -y gtk4-layer-shell
|
||||||
|
```
|
||||||
|
|
||||||
|
When available, the OSD uses it for proper top-edge positioning.
|
||||||
|
|
||||||
|
Without `gtk4-layer-shell`, the OSD still works, but GNOME may place it in the normal window position.
|
||||||
|
|
||||||
|
## OSD Flicker Fix
|
||||||
|
|
||||||
|
An early OSD version killed the existing OSD process on every new brightness key press.
|
||||||
|
|
||||||
|
That caused a chopped feeling:
|
||||||
|
|
||||||
|
```text
|
||||||
|
key press -> old OSD exits -> new OSD starts
|
||||||
|
```
|
||||||
|
|
||||||
|
The current OSD instead keeps a running process alive and updates it:
|
||||||
|
|
||||||
|
```text
|
||||||
|
key press -> write new value -> signal running OSD -> reset hide timer
|
||||||
|
```
|
||||||
|
|
||||||
|
Files used for that:
|
||||||
|
|
||||||
|
```text
|
||||||
|
/tmp/brightness-osd.pid
|
||||||
|
/tmp/brightness-osd.value
|
||||||
|
```
|
||||||
|
|
||||||
|
This smooths repeated key presses without delaying the actual brightness change.
|
||||||
|
|
||||||
|
## Failed Debounce Attempt
|
||||||
|
|
||||||
|
There was an attempt to coalesce multiple fast brightness key presses into one final brightness operation.
|
||||||
|
|
||||||
|
The attempted flow was:
|
||||||
|
|
||||||
|
```text
|
||||||
|
key press -> store target value -> show target in OSD -> wait for quiet period -> run brightnessctl once
|
||||||
|
```
|
||||||
|
|
||||||
|
The debounce delay was controlled by:
|
||||||
|
|
||||||
|
```text
|
||||||
|
BRIGHTNESS_APPLY_DEBOUNCE_MS="180"
|
||||||
|
```
|
||||||
|
|
||||||
|
This was reverted because it made brightness changes feel laggy.
|
||||||
|
|
||||||
|
### Why It Felt Bad
|
||||||
|
|
||||||
|
Brightness keys are expected to affect the panel immediately.
|
||||||
|
|
||||||
|
Debouncing the actual `brightnessctl` call introduced a visible delay:
|
||||||
|
|
||||||
|
```text
|
||||||
|
press keys -> screen does not change immediately -> final jump happens later
|
||||||
|
```
|
||||||
|
|
||||||
|
That is unpleasant for display brightness because visual feedback matters more than reducing the number of brightness writes.
|
||||||
|
|
||||||
|
The external monitor already has some delay because `ddcutil` is slower than an internal backlight write. Delaying the internal display as well made the whole interaction feel worse.
|
||||||
|
|
||||||
|
### Lesson
|
||||||
|
|
||||||
|
Do not debounce the real brightness change.
|
||||||
|
|
||||||
|
Only smooth the OSD:
|
||||||
|
|
||||||
|
```text
|
||||||
|
brightnessctl immediately
|
||||||
|
OSD update/reuse
|
||||||
|
external sync follows
|
||||||
|
```
|
||||||
|
|
||||||
|
## Reverted Commit
|
||||||
|
|
||||||
|
The debounce attempt was committed and then reverted:
|
||||||
|
|
||||||
|
```text
|
||||||
|
324505b Debounce brightness key bursts
|
||||||
|
a84963e Revert "Debounce brightness key bursts"
|
||||||
|
```
|
||||||
|
|
||||||
|
The reverted implementation added:
|
||||||
|
|
||||||
|
```text
|
||||||
|
bin/brightness-step
|
||||||
|
BRIGHTNESS_APPLY_DEBOUNCE_MS
|
||||||
|
```
|
||||||
|
|
||||||
|
Those are intentionally not part of the current working design.
|
||||||
|
|
||||||
|
## Current Recommendation
|
||||||
|
|
||||||
|
For brightness controls:
|
||||||
|
|
||||||
|
- keep internal brightness immediate
|
||||||
|
- keep OSD process reusable instead of restarting it
|
||||||
|
- keep external sync asynchronous
|
||||||
|
- avoid batching or delaying real brightness changes
|
||||||
|
|
||||||
|
This gives the most responsive feel while preserving internal/external sync.
|
||||||
Reference in New Issue
Block a user