Document immediate brightness architecture

This reverts commit 324505bf08.

README.md

@@ -6,12 +6,26 @@ The internal display is the master. Brightness keys change the internal panel, s
 
 This avoids drift from letting two outputs auto-adjust independently.
 
+## Architecture
+
+The real brightness change is intentionally immediate:
+
+```text
+brightness-up/down-all
+  -> brightnessctl immediately changes the internal display
+  -> brightness-osd only displays or updates the visual feedback
+  -> external-brightness-sync.service mirrors the internal percentage later
+```
+
+The actual `brightnessctl` call must not be debounced. Only the OSD may be smoothed by reusing the running OSD process and extending its hide timer.
+
+The external monitor is not changed in the key-binding scripts. `ddcutil` is slower than an internal backlight write, so it runs separately in the sync service.
+
 ## Features
 
 - Internal display controlled with `brightnessctl`
 - External monitor mirrored with `ddcutil` VCP code `10`
 - Compact GTK4/libadwaita OSD for custom brightness shortcuts
-- Debounced brightness changes: fast repeated key presses become one final jump
 - Optional Wayland top-edge positioning with `gtk4-layer-shell`
 - systemd user service for continuous external sync
 - Configurable backlight device, DDC bus, monitor model, step size, and sync interval
@@ -54,7 +68,6 @@ The installer writes:
 
 ```text
 ~/.local/bin/brightness-osd
-~/.local/bin/brightness-step
 ~/.local/bin/brightness-up-all
 ~/.local/bin/brightness-down-all
 ~/.local/bin/brightness-sync-hg342pcb
@@ -84,6 +97,7 @@ Default config:
 ```bash
 BRIGHTNESS_BACKLIGHT_DEVICE="intel_backlight"
 BRIGHTNESS_SYNC_BACKLIGHT="/sys/class/backlight/intel_backlight"
+BRIGHTNESS_OSD_BACKLIGHT="/sys/class/backlight/intel_backlight"
 BRIGHTNESS_STEP="10"
 BRIGHTNESS_DDCUTIL_MODEL="HG342PCB"
 BRIGHTNESS_DDCUTIL_DISPLAY="1"
@@ -91,11 +105,8 @@ BRIGHTNESS_DDCUTIL_BUS="16"
 BRIGHTNESS_SYNC_INTERVAL="1"
 BRIGHTNESS_SYNC_MIN_PERCENT="1"
 BRIGHTNESS_OSD_VISIBLE_MS="1400"
-BRIGHTNESS_APPLY_DEBOUNCE_MS="180"
 ```
 
-`BRIGHTNESS_APPLY_DEBOUNCE_MS` controls how long the scripts wait for another key press before applying the final brightness. The OSD updates immediately to the target value, but `brightnessctl` runs only once at the end of a quick key-press burst.
-
 After changing DDC or sync values:
 
 ```bash
@@ -148,6 +159,14 @@ systemctl --user status external-brightness-sync.service
 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
 
 ```bash

bin/brightness-down-all

@@ -3,8 +3,15 @@ set -euo pipefail
 
 CONFIG="${BRIGHTNESS_AUTOMATION_CONFIG:-$HOME/.config/brightness-automation/env}"
 if [[ -f "$CONFIG" ]]; then
+  set -a
   # shellcheck disable=SC1090
   source "$CONFIG"
+  set +a
 fi
 
-"$HOME/.local/bin/brightness-step" down
+BACKLIGHT_DEVICE="${BRIGHTNESS_BACKLIGHT_DEVICE:-intel_backlight}"
+STEP="${BRIGHTNESS_STEP:-10}"
+export BRIGHTNESS_OSD_BACKLIGHT="${BRIGHTNESS_OSD_BACKLIGHT:-${BRIGHTNESS_SYNC_BACKLIGHT:-/sys/class/backlight/${BACKLIGHT_DEVICE}}}"
+
+brightnessctl -q -d "$BACKLIGHT_DEVICE" set "${STEP}%-"
+"$HOME/.local/bin/brightness-osd" || true

bin/brightness-step

@@ -1,92 +0,0 @@
-#!/usr/bin/env bash
-set -euo pipefail
-
-CONFIG="${BRIGHTNESS_AUTOMATION_CONFIG:-$HOME/.config/brightness-automation/env}"
-if [[ -f "$CONFIG" ]]; then
-  # shellcheck disable=SC1090
-  source "$CONFIG"
-fi
-
-BACKLIGHT_DEVICE="${BRIGHTNESS_BACKLIGHT_DEVICE:-intel_backlight}"
-BACKLIGHT_PATH="${BRIGHTNESS_SYNC_BACKLIGHT:-/sys/class/backlight/${BACKLIGHT_DEVICE}}"
-STEP="${BRIGHTNESS_STEP:-10}"
-DEBOUNCE_MS="${BRIGHTNESS_APPLY_DEBOUNCE_MS:-180}"
-STATE_DIR="${XDG_RUNTIME_DIR:-/tmp}/brightness-automation"
-STATE_FILE="$STATE_DIR/target-percent"
-STAMP_FILE="$STATE_DIR/target-stamp"
-LOCK_FILE="$STATE_DIR/lock"
-
-mkdir -p "$STATE_DIR"
-
-read_current_percent() {
-  local current max
-  current="$(<"${BACKLIGHT_PATH}/brightness")"
-  max="$(<"${BACKLIGHT_PATH}/max_brightness")"
-  printf '%s\n' "$(( (current * 100 + max / 2) / max ))"
-}
-
-clamp_percent() {
-  local value="$1"
-  if (( value < 1 )); then
-    value=1
-  elif (( value > 100 )); then
-    value=100
-  fi
-  printf '%s\n' "$value"
-}
-
-now_ms() {
-  date +%s%3N
-}
-
-apply_after_quiet_period() {
-  while true; do
-    sleep "$(awk "BEGIN { printf \"%.3f\", ${DEBOUNCE_MS} / 1000 }")"
-
-    local now stamp age target
-    now="$(now_ms)"
-    stamp="$(<"$STAMP_FILE")"
-    age="$(( now - stamp ))"
-
-    if (( age >= DEBOUNCE_MS )); then
-      target="$(<"$STATE_FILE")"
-      brightnessctl -q -d "$BACKLIGHT_DEVICE" set "${target}%"
-      exit 0
-    fi
-  done
-}
-
-if [[ "${1:-}" == "--apply-worker" ]]; then
-  apply_after_quiet_period
-fi
-
-if [[ "$#" -ne 1 || ! "$1" =~ ^(up|down)$ ]]; then
-  echo "Usage: $(basename "$0") up|down" >&2
-  exit 2
-fi
-
-(
-  flock 9
-
-  current="$(read_current_percent)"
-  if [[ -f "$STATE_FILE" ]]; then
-    base="$(<"$STATE_FILE")"
-  else
-    base="$current"
-  fi
-
-  case "$1" in
-    up) target="$(( base + STEP ))" ;;
-    down) target="$(( base - STEP ))" ;;
-  esac
-
-  target="$(clamp_percent "$target")"
-  printf '%s\n' "$target" >"$STATE_FILE"
-  now_ms >"$STAMP_FILE"
-
-  "$HOME/.local/bin/brightness-osd" "$target" || true
-) 9>"$LOCK_FILE"
-
-if ! pgrep -f "$HOME/.local/bin/brightness-step --apply-worker" >/dev/null 2>&1; then
-  nohup "$HOME/.local/bin/brightness-step" --apply-worker >/dev/null 2>&1 &
-fi

bin/brightness-up-all

@@ -3,8 +3,15 @@ set -euo pipefail
 
 CONFIG="${BRIGHTNESS_AUTOMATION_CONFIG:-$HOME/.config/brightness-automation/env}"
 if [[ -f "$CONFIG" ]]; then
+  set -a
   # shellcheck disable=SC1090
   source "$CONFIG"
+  set +a
 fi
 
-"$HOME/.local/bin/brightness-step" up
+BACKLIGHT_DEVICE="${BRIGHTNESS_BACKLIGHT_DEVICE:-intel_backlight}"
+STEP="${BRIGHTNESS_STEP:-10}"
+export BRIGHTNESS_OSD_BACKLIGHT="${BRIGHTNESS_OSD_BACKLIGHT:-${BRIGHTNESS_SYNC_BACKLIGHT:-/sys/class/backlight/${BACKLIGHT_DEVICE}}}"
+
+brightnessctl -q -d "$BACKLIGHT_DEVICE" set "+${STEP}%"
+"$HOME/.local/bin/brightness-osd" || true

docs/implementation-log.md

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

install.sh

@@ -17,7 +17,6 @@ install -d "$HOME/.config/systemd/user"
 install -d "$HOME/.config/brightness-automation"
 
 install -m 0755 "$ROOT/bin/brightness-osd" "$HOME/.local/bin/brightness-osd"
-install -m 0755 "$ROOT/bin/brightness-step" "$HOME/.local/bin/brightness-step"
 install -m 0755 "$ROOT/bin/brightness-up-all" "$HOME/.local/bin/brightness-up-all"
 install -m 0755 "$ROOT/bin/brightness-down-all" "$HOME/.local/bin/brightness-down-all"
 install -m 0755 "$ROOT/bin/brightness-sync-hg342pcb" "$HOME/.local/bin/brightness-sync-hg342pcb"
@@ -27,6 +26,7 @@ if [[ ! -f "$HOME/.config/brightness-automation/env" ]]; then
   cat >"$HOME/.config/brightness-automation/env" <<EOF
 BRIGHTNESS_BACKLIGHT_DEVICE="$BACKLIGHT_DEVICE"
 BRIGHTNESS_SYNC_BACKLIGHT="$BACKLIGHT_PATH"
+BRIGHTNESS_OSD_BACKLIGHT="$BACKLIGHT_PATH"
 BRIGHTNESS_STEP="$STEP"
 BRIGHTNESS_DDCUTIL_MODEL="$DDC_MODEL"
 BRIGHTNESS_DDCUTIL_DISPLAY="$DDC_DISPLAY"
@@ -34,7 +34,6 @@ BRIGHTNESS_DDCUTIL_BUS="$DDC_BUS"
 BRIGHTNESS_SYNC_INTERVAL="$INTERVAL"
 BRIGHTNESS_SYNC_MIN_PERCENT="$MIN_PERCENT"
 BRIGHTNESS_OSD_VISIBLE_MS="1400"
-BRIGHTNESS_APPLY_DEBOUNCE_MS="180"
 EOF
 fi
 

uninstall.sh

@@ -4,7 +4,6 @@ set -euo pipefail
 systemctl --user disable --now external-brightness-sync.service 2>/dev/null || true
 rm -f "$HOME/.config/systemd/user/external-brightness-sync.service"
 rm -f "$HOME/.local/bin/brightness-osd"
-rm -f "$HOME/.local/bin/brightness-step"
 rm -f "$HOME/.local/bin/brightness-up-all"
 rm -f "$HOME/.local/bin/brightness-down-all"
 rm -f "$HOME/.local/bin/brightness-sync-hg342pcb"