mirror of
https://github.com/AvengeMedia/DankMaterialShell.git
synced 2026-10-18 17:24:01 -04:00
fix(icc): keep the night light state, prune names without a control, use a 0 sentinel
Fifth review round. - `SaveICCConfig` replaces `SaveConfig` on the three ICC paths (apply, remove, set-temp). Writing the daemon's whole config persisted the night light fields it happens to hold in memory: with the night light on, applying a profile stored `"Enabled": true`, and since the shell only ever sends `wayland.gamma.setEnabled` when the mode is on, turning it off afterwards (an in-memory change on the daemon side, no write) left that `true` on disk — after log out/in the daemon came up driving the schedule while the UI showed night mode off, with no way to resync short of a toggle. The ICC save now reads the file and rewrites only `iccProfiles`/`outputTemps`, so the fields the shell owns stay as they are. - `removeOutputByRegistryName` resolved the output from `m.outputs`, which only has entries once the gamma controls exist. On a default install (night light off, no profile, no override) no control is ever created, so a disconnected monitor kept its `outputNames`/`outputRegNames` entries for the life of the daemon: `dms icc status`/`listOutputs` listed monitors that were gone, and both maps grew with every dock/undock cycle. The object ID is resolved from `m.outputRegNames` (populated for every `wl_output`), the name entries are dropped before the output state is looked up, and a state that is known without a registry name still falls back to matching its own. - The Display Config temperature row used `7000` as its fallback for an output with no override. `publishICCState` only copies entries with `temp != 0`, so the value is absent rather than `0` for an unconfigured output: the card claimed "7000K" while the daemon drove the night light or neutral, and a user who deliberately set 7000K got the "no override" colouring because the highlight tested `!== 7000`. The sentinel is `0` now, which is what the `Default` label branches and the "is overridden" tests were written against. Covered by TestSaveICCConfigKeepsNightLightFields (the previous commit brings the night light back on after a restart), TestManager_RemoveOutputByRegistryNameWithoutControl (the previous commit keeps an unplugged monitor listed) and the existing TestManager_RemoveOutputByRegistryNamePrunesNames.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user