From 6763931c151f266cd1cdb0a20f286b5889d674c9 Mon Sep 17 00:00:00 2001 From: KIDult2226 <22439293@life.hkbu.edu.hk> Date: Sun, 13 Sep 2026 01:06:05 +0800 Subject: [PATCH] fix(icc): load the configured profiles even when controls already exist Restoring the upstream "output returned, re-establishing controls" branch made the registry handler establish the gamma controls before the startup post runs, and that post returned early on `m.controlsInitialized`, so the configured ICC profiles and per-output temperatures were never attached: every output reported "(none)" and the night light ran with defaults until the user re-applied a profile by hand. - `initializeControlsAndICC` (the startup post) loads the configuration first and only creates the controls when they are missing. - The loading is split into `loadConfiguredICC` / `applyConfiguredICCForOutput` so it can also run per output. - A hotplugged output now gets its configured profile and temperature as soon as its name is known (name handler) or when its control is created, which also makes the "re-applies on hotplug" claim in the description true. - After attaching, the output's dedup state is cleared so the ramp is written. Covered by TestManager_LoadConfiguredICCWhenControlsAlreadyExist and TestManager_AttachConfiguredICCForNamedOutput. --- core/internal/server/wayland/manager.go | 159 +++++++++++++------ core/internal/server/wayland/manager_test.go | 47 ++++++ 2 files changed, 157 insertions(+), 49 deletions(-) diff --git a/core/internal/server/wayland/manager.go b/core/internal/server/wayland/manager.go index a5b0c2989..8eab9da08 100644 --- a/core/internal/server/wayland/manager.go +++ b/core/internal/server/wayland/manager.go @@ -85,59 +85,26 @@ func NewManager(display wlclient.WaylandDisplay, config Config) (*Manager, error // temperatures, so controls are set up whenever any of them is configured. if m.needsControls() { m.post(func() { - if m.controlsInitialized { - return - } - log.Info("Gamma control enabled at startup") - m.createControls() + // Profiles and temperatures are configured state, not control state: + // the registry handler may already have established the controls, and + // the configuration still has to be attached. + loaded := m.loadConfiguredICC() - // Load ICC profiles from config (always, regardless of gamma enabled) - m.configMutex.RLock() - iccProfiles := m.config.ICCProfiles - outputTemps := m.config.OutputTemps - m.configMutex.RUnlock() - - if len(iccProfiles) > 0 { - for outputName, iccPath := range iccProfiles { - profile, err := icc.ParseFile(iccPath) - if err != nil { - log.Warnf("icc: failed to load profile for output %q: %v", outputName, err) - continue - } - m.outputs.Range(func(_ uint32, out *outputState) bool { - if name, ok := m.outputNames.Load(out.id); ok && name == outputName { - out.iccPath = iccPath - out.iccProfile = profile - return false - } - return true - }) - } - log.Infof("icc: loaded %d ICC profiles from config", len(iccProfiles)) + if !m.controlsInitialized { + log.Info("Gamma control enabled at startup") + m.createControls() } - // Load per-output temperatures from config - if len(outputTemps) > 0 { - for outputName, temp := range outputTemps { - m.outputs.Range(func(_ uint32, out *outputState) bool { - if name, ok := m.outputNames.Load(out.id); ok && name == outputName { - out.outputTemp = temp - return false - } - return true - }) - } - log.Infof("icc: loaded %d output temperatures from config", len(outputTemps)) + if loaded { + // The attached configuration is invisible to the per-output + // dedup, which only compares the schedule-derived + // temp/gamma/contrast, so clear that state before applying. + m.outputs.Range(func(_ uint32, out *outputState) bool { + out.lastTemp = 0 + return true + }) + m.applyCurrentTemp("startup") } - - // The profiles and temperatures loaded above are invisible to the - // per-output dedup, which only compares the schedule-derived - // temp/gamma/contrast, so clear that state before applying. - m.outputs.Range(func(_ uint32, out *outputState) bool { - out.lastTemp = 0 - return true - }) - m.applyCurrentTemp("startup") }) } @@ -222,6 +189,9 @@ func (m *Manager) setupRegistry() error { output.SetNameHandler(func(ev wlclient.OutputNameEvent) { outputNames[outputID] = ev.Name m.outputNames.Store(outputID, ev.Name) + m.post(func() { + m.attachConfiguredICC(outputID, ev.Name) + }) }) if gammaMgr != nil { outputs = append(outputs, output) @@ -468,6 +438,12 @@ func (m *Manager) addOutputControl(output *wlclient.Output) error { } m.setupControlHandlers(outState, control) m.outputs.Store(outputID, outState) + + // The name may already be known (hotplug after the output was announced), + // in which case the configured profile/temperature attaches right away. + if name, ok := m.outputNames.Load(outputID); ok { + m.attachConfiguredICC(outputID, name) + } return nil } @@ -1364,6 +1340,91 @@ func (m *Manager) destroyControls() { m.controlsInitialized = false } +// loadConfiguredICC attaches the configured ICC profiles and per-output +// temperatures to the outputs that are already known, and reports whether +// anything was attached. +func (m *Manager) loadConfiguredICC() bool { + m.configMutex.RLock() + iccProfiles := m.config.ICCProfiles + outputTemps := m.config.OutputTemps + m.configMutex.RUnlock() + + if len(iccProfiles) == 0 && len(outputTemps) == 0 { + return false + } + + attached := 0 + m.outputs.Range(func(_ uint32, out *outputState) bool { + name, ok := m.outputNames.Load(out.id) + if !ok { + return true + } + if m.applyConfiguredICCForOutput(out, name, iccProfiles, outputTemps) { + attached++ + } + return true + }) + + log.Infof("icc: config has %d profile(s) and %d temperature override(s), attached to %d output(s)", + len(iccProfiles), len(outputTemps), attached) + return attached > 0 +} + +// attachConfiguredICC attaches the configured profile and temperature to an +// output whose name just became known, so a hotplugged output comes up +// corrected without an explicit re-apply. +func (m *Manager) attachConfiguredICC(outputID uint32, outputName string) { + m.configMutex.RLock() + iccProfiles := m.config.ICCProfiles + outputTemps := m.config.OutputTemps + m.configMutex.RUnlock() + + if len(iccProfiles) == 0 && len(outputTemps) == 0 { + return + } + + changed := false + m.outputs.Range(func(_ uint32, out *outputState) bool { + if out.id != outputID { + return true + } + changed = m.applyConfiguredICCForOutput(out, outputName, iccProfiles, outputTemps) + if changed { + log.Infof("icc: attached configured profile/temperature to output %s", outputName) + out.lastTemp = 0 + } + return false + }) + + if changed { + m.applyCurrentTemp("output-name") + } +} + +// applyConfiguredICCForOutput applies the configured profile and temperature of +// one output and reports whether anything changed. +func (m *Manager) applyConfiguredICCForOutput(out *outputState, outputName string, iccProfiles map[string]string, outputTemps map[string]int) bool { + changed := false + + if temp, ok := outputTemps[outputName]; ok { + out.outputTemp = temp + changed = true + } + + if iccPath, ok := iccProfiles[outputName]; ok { + profile, err := icc.ParseFile(iccPath) + if err != nil { + log.Warnf("icc: failed to load profile for output %q: %v", outputName, err) + } else { + out.iccPath = iccPath + out.iccProfile = profile + changed = true + } + } + + return changed +} + // ApplyICC loads and applies an ICC profile to a specific output. // outputName is the wl_output name (e.g., "DP-1"). // iccPath is the path to the .icm/.icc profile file. diff --git a/core/internal/server/wayland/manager_test.go b/core/internal/server/wayland/manager_test.go index 5dfe2c533..1760070ab 100644 --- a/core/internal/server/wayland/manager_test.go +++ b/core/internal/server/wayland/manager_test.go @@ -2,6 +2,7 @@ package wayland import ( "errors" + "path/filepath" "sync" "testing" "time" @@ -558,3 +559,49 @@ func TestManager_NeedsControlsCoversICCAndOutputTemps(t *testing.T) { }) } } + +// The registry handler can establish the gamma controls before the startup post +// runs, so loading the configured ICC profiles and temperatures must not depend +// on the controls still being uninitialized. +func TestManager_LoadConfiguredICCWhenControlsAlreadyExist(t *testing.T) { + dir := t.TempDir() + m := &Manager{config: Config{ + ICCProfiles: map[string]string{"DP-2": filepath.Join(dir, "missing.icm")}, + OutputTemps: map[string]int{"DP-1": 7000}, + }} + m.controlsInitialized = true + + dp1 := &outputState{id: 1} + m.outputs.Store(1, dp1) + m.outputNames.Store(1, "DP-1") + + dp2 := &outputState{id: 2} + m.outputs.Store(2, dp2) + m.outputNames.Store(2, "DP-2") + + if !m.loadConfiguredICC() { + t.Fatal("loadConfiguredICC() = false, want true") + } + assert.Equal(t, 7000, dp1.outputTemp, "configured temperature should attach") + + // An unreadable profile must be skipped without aborting the rest. + assert.Empty(t, dp2.iccPath, "unparsable profile should not attach") +} + +// A hotplugged output gets its configured profile and temperature attached as +// soon as its name is known. +func TestManager_AttachConfiguredICCForNamedOutput(t *testing.T) { + m := &Manager{config: Config{OutputTemps: map[string]int{"DP-3": 6500}}} + + configured := &outputState{id: 3} + m.outputs.Store(3, configured) + m.outputNames.Store(3, "DP-3") + + m.attachConfiguredICC(3, "DP-3") + assert.Equal(t, 6500, configured.outputTemp) + + plain := &outputState{id: 4} + m.outputs.Store(4, plain) + m.attachConfiguredICC(4, "HDMI-A-1") + assert.Zero(t, plain.outputTemp, "outputs without configuration are left alone") +}