diff --git a/core/internal/server/wayland/manager.go b/core/internal/server/wayland/manager.go index 13bd86846..14d8ecc84 100644 --- a/core/internal/server/wayland/manager.go +++ b/core/internal/server/wayland/manager.go @@ -398,34 +398,49 @@ func (m *Manager) removeAvailableOutput(o *wlclient.Output) { // removeOutputByRegistryName tears down the state of an output that left the // compositor (unplugged or asleep). Only the wayland actor goroutine calls it. func (m *Manager) removeOutputByRegistryName(registryName uint32) { - var foundID uint32 + // Resolve the object ID from the registry name map: both name maps are + // populated for every wl_output, whether or not a gamma control exists for + // it, while m.outputs only has entries once the controls exist. On a default + // install (night light off, no profile, no override) no control is ever + // created, and the name entries still have to go with the output. + foundID, found := m.outputIDForRegistryName(registryName) + var foundOut *outputState - m.outputs.Range(func(id uint32, out *outputState) bool { - if out.registryName == registryName { - foundID = id + if found { + if out, ok := m.outputs.Load(foundID); ok { foundOut = out - return false } - return true - }) - if foundOut == nil { + } else { + // An output whose name never arrived is only known through its state. + m.outputs.Range(func(id uint32, out *outputState) bool { + if out.registryName == registryName { + foundID, foundOut, found = id, out, true + return false + } + return true + }) + } + if !found { return } - if foundOut.gammaControl != nil { - foundOut.gammaControl.(*wlr_gamma_control.ZwlrGammaControlV1).Destroy() - foundOut.gammaControl = nil - } - m.removeAvailableOutput(foundOut.output) - if foundOut.output != nil && !foundOut.output.IsZombie() { - _ = foundOut.output.Release() - } - m.outputs.Delete(foundID) - // The name entries have to go with the output: they are what - // `dms icc status`/`listOutputs` report, and a rebound wl_output reusing a - // released object ID would otherwise attach the previous monitor's profile - // from the stale name. + + // The name entries go first: they are what `dms icc status`/`listOutputs` + // report, and a rebound wl_output reusing a released object ID would + // otherwise attach the previous monitor's profile from the stale name. m.outputNames.Delete(foundID) m.outputRegNames.Delete(foundID) + + if foundOut != nil { + if foundOut.gammaControl != nil { + foundOut.gammaControl.(*wlr_gamma_control.ZwlrGammaControlV1).Destroy() + foundOut.gammaControl = nil + } + m.removeAvailableOutput(foundOut.output) + if foundOut.output != nil && !foundOut.output.IsZombie() { + _ = foundOut.output.Release() + } + m.outputs.Delete(foundID) + } m.publishICCState() hasOutputs := false @@ -438,6 +453,21 @@ func (m *Manager) removeOutputByRegistryName(registryName uint32) { } } +// outputIDForRegistryName maps a wl_output registry name back to the object ID +// it was announced with. +func (m *Manager) outputIDForRegistryName(registryName uint32) (uint32, bool) { + var foundID uint32 + var found bool + m.outputRegNames.Range(func(id, name uint32) bool { + if name == registryName { + foundID, found = id, true + return false + } + return true + }) + return foundID, found +} + func (m *Manager) outputStillValid(out *outputState) bool { switch { case out == nil: @@ -1583,8 +1613,9 @@ func (m *Manager) ApplyICC(outputName, iccPath string) error { savedConfig := cloneConfig(m.config) m.configMutex.Unlock() - // Save to disk - if err := SaveConfig(savedConfig); err != nil { + // Save to disk. Only the ICC fields are written: the night light fields in + // this config belong to the shell, not to the daemon. + if err := SaveICCConfig(savedConfig.ICCProfiles, savedConfig.OutputTemps); err != nil { log.Warnf("icc: failed to save config: %v", err) } @@ -1614,7 +1645,7 @@ func (m *Manager) RemoveICC(outputName string) error { savedConfig := cloneConfig(m.config) m.configMutex.Unlock() - if err := SaveConfig(savedConfig); err != nil { + if err := SaveICCConfig(savedConfig.ICCProfiles, savedConfig.OutputTemps); err != nil { log.Warnf("icc: failed to save config: %v", err) } @@ -1733,7 +1764,7 @@ func (m *Manager) SetOutputTemp(outputName string, temp int) error { savedConfig := cloneConfig(m.config) m.configMutex.Unlock() - if err := SaveConfig(savedConfig); err != nil { + if err := SaveICCConfig(savedConfig.ICCProfiles, savedConfig.OutputTemps); err != nil { log.Warnf("icc: failed to save config: %v", err) } diff --git a/core/internal/server/wayland/manager_test.go b/core/internal/server/wayland/manager_test.go index 92f501f27..e77f60ddb 100644 --- a/core/internal/server/wayland/manager_test.go +++ b/core/internal/server/wayland/manager_test.go @@ -671,6 +671,26 @@ func TestManager_ControlStateReuseKeepsAttachedICC(t *testing.T) { assert.Zero(t, fresh.outputTemp) } +// A wl_output that has no gamma control (the default install: night light off, +// no profile, no override) exists only in the name maps. It still has to +// disappear from listOutputs/status once it is gone, or every dock/undock cycle +// leaves another monitor in maps that live as long as the daemon. +func TestManager_RemoveOutputByRegistryNameWithoutControl(t *testing.T) { + m := &Manager{} + m.outputNames.Store(9, "HDMI-A-1") + m.outputRegNames.Store(9, 77) + + assert.Equal(t, []string{"HDMI-A-1"}, m.ListOutputs()) + + m.removeOutputByRegistryName(77) + + assert.Empty(t, m.ListOutputs(), "a monitor with no control must not stay listed") + _, nameStored := m.outputNames.Load(9) + assert.False(t, nameStored, "the name entry should be gone") + _, regStored := m.outputRegNames.Load(9) + assert.False(t, regStored, "the registry name entry should be gone") +} + // The status payload is what the settings UI shows for a profile, so the // descriptive metadata has to be carried through. func TestManager_GetICCStatusDescribesProfile(t *testing.T) { diff --git a/core/internal/server/wayland/types.go b/core/internal/server/wayland/types.go index d1fa934d0..11c9c5769 100644 --- a/core/internal/server/wayland/types.go +++ b/core/internal/server/wayland/types.go @@ -276,6 +276,22 @@ func SaveConfig(cfg Config) error { return os.WriteFile(path, data, 0o644) } +// SaveICCConfig stores the ICC profiles and the per-output temperatures, leaving +// every other field of the file as it is on disk. +// +// The night light fields are owned by the shell: the daemon only reads them at +// boot and writes them in memory when the shell calls setEnabled/setTemperature, +// so writing its copy back would persist whatever the night light happened to be +// doing when an ICC change was made (for example `"Enabled": true` while the +// user had since turned it off), and the shell only sends setEnabled when the +// mode is on, so the state would silently come back after a session restart. +func SaveICCConfig(iccProfiles map[string]string, outputTemps map[string]int) error { + cfg := LoadConfig() + cfg.ICCProfiles = iccProfiles + cfg.OutputTemps = outputTemps + return SaveConfig(cfg) +} + func (c *Config) Validate() error { if c.LowTemp < 1000 || c.LowTemp > 10000 { return errdefs.ErrInvalidTemperature diff --git a/core/internal/server/wayland/types_test.go b/core/internal/server/wayland/types_test.go index 027059327..79c14dad2 100644 --- a/core/internal/server/wayland/types_test.go +++ b/core/internal/server/wayland/types_test.go @@ -3,6 +3,8 @@ package wayland import ( "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestConfigValidate(t *testing.T) { @@ -410,3 +412,36 @@ func TestStateChanged(t *testing.T) { }) } } + +// The night light fields in wayland.json belong to the shell: the daemon reads +// them at boot and the shell pushes changes through IPC, so an ICC change must +// not write the daemon's copy of them back to disk. Otherwise applying a profile +// while the night light happens to be on persists "Enabled": true, and the mode +// comes back after a session restart with no way to resync from the UI. +func TestSaveICCConfigKeepsNightLightFields(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + + onDisk := Config{ + Enabled: true, + LowTemp: 3000, + HighTemp: 6000, + Gamma: 1.1, + Contrast: 1.0, + Outputs: []string{}, + } + if err := SaveConfig(onDisk); err != nil { + t.Fatalf("SaveConfig: %v", err) + } + + if err := SaveICCConfig(map[string]string{"DP-1": "/tmp/dp1.icc"}, map[string]int{"DP-1": 7000}); err != nil { + t.Fatalf("SaveICCConfig: %v", err) + } + + got := LoadConfig() + assert.True(t, got.Enabled, "the night light state on disk must survive an ICC write") + assert.Equal(t, 3000, got.LowTemp) + assert.Equal(t, 6000, got.HighTemp) + assert.Equal(t, 1.1, got.Gamma) + assert.Equal(t, map[string]string{"DP-1": "/tmp/dp1.icc"}, got.ICCProfiles) + assert.Equal(t, map[string]int{"DP-1": 7000}, got.OutputTemps) +} diff --git a/quickshell/Modules/Settings/DisplayConfig/OutputCard.qml b/quickshell/Modules/Settings/DisplayConfig/OutputCard.qml index 09c648543..0d99dd3b1 100644 --- a/quickshell/Modules/Settings/DisplayConfig/OutputCard.qml +++ b/quickshell/Modules/Settings/DisplayConfig/OutputCard.qml @@ -516,13 +516,16 @@ StyledRect { leftPadding: 0 topPadding: Theme.spacingS - property int currentTemp: ICCService.outputTemps[root.outputName] !== undefined ? ICCService.outputTemps[root.outputName] : 7000 + // 0 is "no override": the daemon only publishes an entry for an + // output that has one, and the night light temperature is not this + // value, so a fallback of 7000K would claim a setting nobody made. + property int currentTemp: ICCService.outputTemps[root.outputName] !== undefined ? ICCService.outputTemps[root.outputName] : 0 property bool editing: false DankIcon { name: "thermostat" size: 18 - color: colorTempRow.currentTemp !== 7000 ? Theme.primary : Theme.surfaceVariantText + color: colorTempRow.currentTemp !== 0 ? Theme.primary : Theme.surfaceVariantText anchors.verticalCenter: parent.verticalCenter } @@ -550,7 +553,7 @@ StyledRect { text: colorTempRow.editing ? (Math.round(tempSlider.value) + "K") : (colorTempRow.currentTemp === 0 ? I18n.tr("Default", "Default") : (colorTempRow.currentTemp + "K")) font.pixelSize: Theme.fontSizeSmall font.weight: Font.Medium - color: colorTempRow.currentTemp !== 7000 && colorTempRow.currentTemp !== 0 ? Theme.primary : Theme.surfaceText + color: colorTempRow.currentTemp !== 0 ? Theme.primary : Theme.surfaceText anchors.verticalCenter: parent.verticalCenter }