Skip to content

fix: stop brightness transition on disable#1115

Merged
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:fix-sensor
May 20, 2026
Merged

fix: stop brightness transition on disable#1115
fly602 merged 1 commit into
linuxdeepin:masterfrom
fly602:fix-sensor

Conversation

@fly602
Copy link
Copy Markdown
Contributor

@fly602 fly602 commented May 20, 2026

  1. Cancel any ongoing brightness transition before restoring brightness when auto brightness is stopped.
  2. Cancel the active transition immediately when a manual brightness change is detected, both when auto brightness is being disabled and when auto adjustment is only temporarily paused.
  3. Add AutoBrightnessManager.cancelOngoingTransition to stop the transition for the built-in monitor through TransitionManager.StopMonitor.
  4. Prevent sensor light change handling and compensation ticks from continuing to adjust brightness after auto brightness has been turned off by also checking config.Enabled.
  5. Add per-step transition callbacks in the brightness transition layer so brightness properties are updated progressively during the fade, instead of being set to the target value immediately at transition start.
  6. Remove the premature property update in setBrightnessWithTransition and wire the transition callback in Manager.initTransitionManager to sync built-in monitor brightness and D-Bus properties on each step.

Why:

  1. When users turn off auto brightness or manually change brightness, an unfinished fade could continue running and override the user's latest brightness choice.
  2. The previous implementation also exposed the target brightness property too early, making reported brightness inconsistent with the actual gradual transition state.
  3. These changes make brightness behavior match user intent immediately and keep exported brightness state aligned with real transition progress.

Log: Fixed an issue where brightness fading could continue after auto brightness was turned off, and improved real-time brightness state updates during transitions

Influence:

  1. Enable auto brightness, trigger a brightness transition, then turn off auto brightness and verify the transition stops immediately and brightness is restored without further fading.
  2. During an active auto-brightness fade, manually change brightness and verify the ongoing transition is canceled and does not override the manual setting afterward.
  3. In manual override mode, confirm no further sensor-driven brightness updates occur while auto brightness is disabled or paused.
  4. Observe D-Bus or exposed brightness properties during a transition and verify they change step by step instead of jumping directly to the target value.
  5. Test built-in monitor behavior specifically, including repeated enable/disable operations and rapid manual brightness adjustments, to ensure there are no lingering transitions.
  6. Verify no regression in normal auto-brightness transitions when the feature remains enabled.

fix: 关闭自动亮度时停止亮度渐变

  1. 在停止自动亮度并恢复亮度前,先取消当前正在进行的亮度渐变。
  2. 在检测到用户手动修改亮度时,立即取消当前渐变;无论是要直接关闭自动亮 度,还是仅临时暂停自动调节,都执行该处理。
  3. 新增 AutoBrightnessManager.cancelOngoingTransition,通过 TransitionManager.StopMonitor 停止内置显示器上的渐变任务。
  4. 在环境光变化处理和补偿定时任务中增加 config.Enabled 判断,避免自动亮 度关闭后仍继续调节亮度。
  5. 在亮度渐变层增加逐步回调能力,使亮度属性在渐变过程中按步骤同步更新, 而不是在渐变开始时直接设置为目标值。
  6. 移除 setBrightnessWithTransition 中过早设置亮度属性的逻辑,并在 Manager.initTransitionManager 中接入渐变回调,在每一步同步内置显示器亮度 和 D-Bus 属性。

Why:

  1. 当用户关闭自动亮度或手动修改亮度时,未完成的渐变可能继续执行,并覆盖 用户最新设置的亮度。
  2. 旧实现会过早暴露目标亮度属性,导致对外报告的亮度状态与实际渐变中的亮 度不一致。
  3. 这些修改让亮度行为能够立即响应用户意图,并确保导出的亮度状态与真实渐 变进度保持一致。

Log: 修复关闭自动亮度后亮度渐变仍继续的问题,并改进渐变过程中的亮度状态
实时更新

Influence:

  1. 开启自动亮度并触发一次亮度渐变,然后关闭自动亮度,验证渐变会立即停 止,亮度恢复后不会继续渐变。
  2. 在自动亮度渐变进行中手动修改亮度,验证正在进行的渐变被取消,之后不会 再覆盖手动设置的亮度。
  3. 在手动接管期间,确认自动亮度被关闭或暂停后,不会再收到传感器驱动的亮 度更新。
  4. 在渐变过程中观察 D-Bus 或对外暴露的亮度属性,验证其会随每一步渐变逐步 变化,而不是直接跳到目标值。
  5. 重点测试内置显示器场景,包括反复开关自动亮度和快速连续手动调节亮度, 确保不会残留未停止的渐变任务。
  6. 验证自动亮度保持开启时,正常的亮度渐变流程没有回归问题。

PMS: BUG-358023
Change-Id: I27e29c376de575668e364e9612937c86cec41722

1. Cancel any ongoing brightness transition before restoring brightness
when auto brightness is stopped.
2. Cancel the active transition immediately when a manual brightness
change is detected, both when auto brightness is being disabled and when
auto adjustment is only temporarily paused.
3. Add AutoBrightnessManager.cancelOngoingTransition
to stop the transition for the built-in monitor through
TransitionManager.StopMonitor.
4. Prevent sensor light change handling and compensation ticks from
continuing to adjust brightness after auto brightness has been turned
off by also checking config.Enabled.
5. Add per-step transition callbacks in the brightness transition layer
so brightness properties are updated progressively during the fade,
instead of being set to the target value immediately at transition
start.
6. Remove the premature property update in setBrightnessWithTransition
and wire the transition callback in Manager.initTransitionManager to
sync built-in monitor brightness and D-Bus properties on each step.

Why:
1. When users turn off auto brightness or manually change brightness,
an unfinished fade could continue running and override the user's latest
brightness choice.
2. The previous implementation also exposed the target brightness
property too early, making reported brightness inconsistent with the
actual gradual transition state.
3. These changes make brightness behavior match user intent immediately
and keep exported brightness state aligned with real transition
progress.

Log: Fixed an issue where brightness fading could continue after auto
brightness was turned off, and improved real-time brightness state
updates during transitions

Influence:
1. Enable auto brightness, trigger a brightness transition, then turn
off auto brightness and verify the transition stops immediately and
brightness is restored without further fading.
2. During an active auto-brightness fade, manually change brightness
and verify the ongoing transition is canceled and does not override the
manual setting afterward.
3. In manual override mode, confirm no further sensor-driven brightness
updates occur while auto brightness is disabled or paused.
4. Observe D-Bus or exposed brightness properties during a transition
and verify they change step by step instead of jumping directly to the
target value.
5. Test built-in monitor behavior specifically, including repeated
enable/disable operations and rapid manual brightness adjustments, to
ensure there are no lingering transitions.
6. Verify no regression in normal auto-brightness transitions when the
feature remains enabled.

fix: 关闭自动亮度时停止亮度渐变

1. 在停止自动亮度并恢复亮度前,先取消当前正在进行的亮度渐变。
2. 在检测到用户手动修改亮度时,立即取消当前渐变;无论是要直接关闭自动亮
度,还是仅临时暂停自动调节,都执行该处理。
3. 新增 AutoBrightnessManager.cancelOngoingTransition,通过
TransitionManager.StopMonitor 停止内置显示器上的渐变任务。
4. 在环境光变化处理和补偿定时任务中增加 config.Enabled 判断,避免自动亮
度关闭后仍继续调节亮度。
5. 在亮度渐变层增加逐步回调能力,使亮度属性在渐变过程中按步骤同步更新,
而不是在渐变开始时直接设置为目标值。
6. 移除 setBrightnessWithTransition 中过早设置亮度属性的逻辑,并在
Manager.initTransitionManager 中接入渐变回调,在每一步同步内置显示器亮度
和 D-Bus 属性。

Why:
1. 当用户关闭自动亮度或手动修改亮度时,未完成的渐变可能继续执行,并覆盖
用户最新设置的亮度。
2. 旧实现会过早暴露目标亮度属性,导致对外报告的亮度状态与实际渐变中的亮
度不一致。
3. 这些修改让亮度行为能够立即响应用户意图,并确保导出的亮度状态与真实渐
变进度保持一致。

Log: 修复关闭自动亮度后亮度渐变仍继续的问题,并改进渐变过程中的亮度状态
实时更新

Influence:
1. 开启自动亮度并触发一次亮度渐变,然后关闭自动亮度,验证渐变会立即停
止,亮度恢复后不会继续渐变。
2. 在自动亮度渐变进行中手动修改亮度,验证正在进行的渐变被取消,之后不会
再覆盖手动设置的亮度。
3. 在手动接管期间,确认自动亮度被关闭或暂停后,不会再收到传感器驱动的亮
度更新。
4. 在渐变过程中观察 D-Bus 或对外暴露的亮度属性,验证其会随每一步渐变逐步
变化,而不是直接跳到目标值。
5. 重点测试内置显示器场景,包括反复开关自动亮度和快速连续手动调节亮度,
确保不会残留未停止的渐变任务。
6. 验证自动亮度保持开启时,正常的亮度渐变流程没有回归问题。

PMS: BUG-358023
Change-Id: I27e29c376de575668e364e9612937c86cec41722
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @fly602, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这份代码变更主要解决了自动亮度调节过程中的两个核心问题:

  1. 渐变覆盖手动操作:用户手动调节亮度时,正在进行的渐变动画会覆盖用户的手动设置。
  2. 渐变过程中属性不同步:在使用渐变设置亮度时,D-Bus 属性没有实时更新,而是直接跳到了目标值。

整体来看,代码逻辑清晰,修改合理。但经过仔细审查,在并发安全、逻辑严谨性、性能和安全性方面还有一些可以改进和优化的空间。以下是详细的审查意见:

一、 逻辑与语法改进

1. cancelOngoingTransition 仅处理了内置显示器

auto_brightness.go 中新增的 cancelOngoingTransition 方法硬编码了只获取并停止 builtinMonitor

  • 问题:如果未来系统支持多显示器自动亮度调节,或者当前逻辑本应作用于所有受管理的显示器,这里就会失效。
  • 建议:确认业务逻辑是否仅针对内置显示器。如果是,建议加注释说明原因;如果可能涉及多显示器,建议遍历受影响的显示器列表。

2. StopMonitor 存在竞态条件(TOCTOU)

brightness_transition.go 中新增的 StopMonitor 方法:

func (m *TransitionManager) StopMonitor(monitorName string) {
	m.mu.Lock()
	executor, exists := m.executors[monitorName]
	m.mu.Unlock() // 这里释放了锁

	if exists {
		executor.Stop() // 这里在锁外调用 Stop
	}
}
  • 问题:在 Unlockexecutor.Stop() 之间,其他的协程可能会调用 TransitionManager 的其他方法(如 SetBrightness),将该 executor 从 map 中删除甚至清理,虽然 Go 的 GC 不会立刻回收还在使用的对象,但这破坏了状态的一致性预期。
  • 建议:如果在 executor.Stop() 内部不会产生死锁(不依赖 TransitionManager.mu),建议在持有锁的情况下调用 Stop();或者参考 Stop() 全局方法的逻辑,将需要停止的 executor 收集到 slice 中,再在锁外停止(如果担心死锁)。从目前的代码结构看,executor.Stop() 只依赖自身的锁,直接在临界区内调用是安全的:
func (m *TransitionManager) StopMonitor(monitorName string) {
	m.mu.Lock()
	defer m.mu.Unlock()
    
	if executor, exists := m.executors[monitorName]; exists {
		executor.Stop()
	}
}

3. OnManualBrightnessChange 中重复调用 cancelOngoingTransition

  • 问题:在 OnManualBrightnessChangeif abm.config.Enabledelse 分支中,都调用了 abm.cancelOngoingTransition(manager),这导致了代码冗余。
  • 建议:可以将该调用提取到分支之前(获取 manager 之后),减少重复代码。

二、 代码性能优化

1. 渐变 Step 回调的锁粒度与高频 D-Bus 同步

brightness_transition.gorunTransition 中:

e.mu.Lock()
e.currentPercent = currentPercent
onStep := e.onStepFunc
e.mu.Unlock()

if onStep != nil {
	onStep(e.monitorName, currentPercent)
}

manager.go 中注册的 onStepFunc 做了如下操作:

m.builtinMonitor.setPropBrightnessWithLock(percent)
m.syncPropBrightness()
  • 问题:渐变步进通常每 16ms~30ms 触发一次。在 onStepFunc 中调用 setPropBrightnessWithLocksyncPropBrightness()(通常会触发 D-Bus 属性变更通知 PropertiesChanged),高频的 D-Bus 信号发送会带来显著的 CPU 和 IPC 开销,可能导致系统总线拥堵。
  • 建议
    1. 节流:在 onStepFuncsyncPropBrightness 中增加节流逻辑,例如每 50ms 或 100ms 才真正触发一次 D-Bus 信号通知,而底层 Gamma 值的设置保持原高频不变。
    2. 避免高频加锁:确保 setPropBrightnessWithLocksyncPropBrightness 中的锁不会成为高频竞争的热点。

2. TransitionManager.SetOnStepFunc 遍历加锁

func (m *TransitionManager) SetOnStepFunc(fn func(monitorName string, percent float64)) {
	m.mu.Lock()
	defer m.mu.Unlock()
	m.onStepFunc = fn
	for _, executor := range m.executors {
		executor.SetOnStepFunc(fn) // 内部又加了 executor.mu 锁
	}
}
  • 问题:这里在持有 TransitionManager.mu 的同时,去获取 executor.mu,虽然目前不会死锁,但嵌套加锁增加了锁分析的复杂度。
  • 建议:由于 onStepFunc 通常只在初始化时设置一次,性能影响微乎其微,当前写法可以接受。但更优雅的做法是:在 executorrunTransition 循环中动态读取 m.onStepFunc(通过回调 m.getOnStepFunc()),这样就不需要在 SetOnStepFunc 时去遍历并更新所有 executor 了。

三、 代码安全与健壮性

1. onStepFunc 回调异常崩溃风险

  • 问题onStepFunc 是由外部注入的,如果在 onStepFunc 内部发生 panic,会导致整个渐变协程崩溃退出,且没有恢复机制,导致该显示器的渐变管理器彻底失效。
  • 建议:在 runTransition 中调用 onStepFunc 时增加 recover 保护,或者将回调放在安全的 wrapper 中执行:
if onStep != nil {
    // 防止外部回调 panic 导致渐变协程崩溃
    func() {
        defer func() {
            if r := recover(); r != nil {
                logger.Errorf("Recovered from panic in onStepFunc: %v", r)
            }
        }()
        onStep(e.monitorName, currentPercent)
    }()
}

2. Stop 方法中的状态恢复顺序

auto_brightness.goStop 方法中:

atomic.StoreInt32(&abm.stopping, 1)
abm.stopCompensationTimer()
atomic.StoreInt32(&abm.stopping, 0)

abm.cancelOngoingTransition(abm.manager)
abm.restoreSavedBrightness()
  • 问题cancelOngoingTransition 会停止渐变,而停止渐变的 executor.Stop() 内部通常会等待当前步进完成或安全退出。如果在 cancelOngoingTransition 执行期间,渐变协程刚好调用了 onStepFunc 并更新了亮度属性,紧接着 restoreSavedBrightness 又去设置亮度,可能会产生微小的竞态覆盖(虽然视觉上影响不大)。
  • 建议:当前的逻辑(先取消,再恢复)在业务上是正确的,确保了最终一致性。但需要确保 restoreSavedBrightness 是一个同步阻塞操作,能保证在 Stop 返回前真正将亮度写入了硬件。

四、 代码格式

manager.go 的 diff 中:

-	sessionActive    bool
-	sessionActiveMu  sync.RWMutex
-	newSysCfg        *SysRootConfig
-	cursorShowed  bool
+	sessionActive   bool
+	sessionActiveMu sync.RWMutex
+	newSysCfg       *SysRootConfig
+	cursorShowed    bool
  • 说明:这看起来是对齐格式的调整。建议确认这是 gofmt 的标准输出,避免在代码审查中混入纯格式化修改,以免干扰核心逻辑的审查。

总结

代码的核心修改方向非常正确,解决了痛点。建议重点关注:

  1. 必须修复StopMonitor 中的 TOCTOU 竞态条件,将 executor.Stop() 移入锁内。
  2. 强烈建议:为 onStepFunc 增加 recover 防护,防止外部异常导致渐变协程崩溃。
  3. 性能优化:评估高频 D-Bus 同步带来的开销,必要时对 syncPropBrightness 增加节流机制。
  4. 代码精简:合并 OnManualBrightnessChange 中重复的 cancelOngoingTransition 调用。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, mhduiy

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fly602 fly602 merged commit 8c740b0 into linuxdeepin:master May 20, 2026
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants