Skip to content

refactor(display): refactor auto brightness logic - #1189

Open
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:auto-brightness
Open

refactor(display): refactor auto brightness logic#1189
fly602 wants to merge 1 commit into
linuxdeepin:masterfrom
fly602:auto-brightness

Conversation

@fly602

@fly602 fly602 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
  1. Use AmbientBrightness1 as the recommendation source and let Display1 arbitrate automatic and manual brightness updates.
  2. Refactor brightness transitions to support target updates and deterministic cancellation.
  3. Move power-saving brightness scaling to Display1 while keeping persisted brightness values unscaled.
  4. Remove the obsolete sensor, filtering, curve, and power-saving dimming logic from Display1 and session/power1.
  5. Add regression tests and documentation for the new brightness architecture.

Log: Refactor auto brightness logic and centralize brightness arbitration in Display1.

Influence:

  1. Verify automatic brightness follows recommendations from AmbientBrightness1.
  2. Verify manual brightness changes stop automatic transitions and disable automatic adjustment.
  3. Verify power-saving scaling affects hardware brightness without changing persisted values.
  4. Verify brightness transitions can update targets and stop cleanly.

refactor(display): 重构自动亮度逻辑

  1. 使用 AmbientBrightness1 提供推荐亮度,由 Display1 统一仲裁自动和手动亮度更新。
  2. 重构亮度渐变逻辑,支持动态更新目标值和确定性停止渐变任务。
  3. 将节能亮度缩放迁移到 Display1,同时保持持久化亮度值不受缩放影响。
  4. 移除 Display1 和 session/power1 中陈旧的传感器、滤波、曲线及节能调光逻辑。
  5. 为新的亮度架构补充回归测试和设计文档。

Log: 重构自动亮度逻辑,由 Display1 统一仲裁亮度更新。

Influence:

  1. 验证自动亮度能够正确应用 AmbientBrightness1 的推荐值。
  2. 验证手动调节亮度时会停止自动渐变并关闭自动调节。
  3. 验证节能缩放影响实际亮度,但不会修改持久化亮度值。
  4. 验证亮度渐变能够动态更新目标并可靠停止。

PMS: BUG-372191

Summary by Sourcery

Centralize automatic brightness control around AmbientBrightness1 recommendations and Display1 arbitration, simplify the brightness transition mechanism, and move power-saving brightness scaling into Display1 while removing legacy sensor and curve-based logic.

New Features:

  • Integrate AmbientBrightness1 as the source of automatic brightness recommendations and expose a compatible auto-brightness interface in Display1.
  • Introduce a dedicated brightness transition executor for the builtin display that supports deterministic cancellation and target updates.
  • Add brightness scaling in Display1 based on Power1 power-saving properties while keeping persisted brightness values unscaled.

Enhancements:

  • Streamline auto-brightness management by replacing sensorproxy-based logic with a recommendation client and session-aware application lifecycle.
  • Refine brightness application paths so manual changes preempt automatic adjustments and share a serialized write path.
  • Unify how initial and refreshed brightness values are derived from configuration and scaled according to current power-saving state.

Documentation:

  • Document the new brightness architecture covering automatic brightness flow, power-saving scaling, and configuration persistence semantics.

Tests:

  • Add unit tests for recommendation state parsing, auto-brightness application conditions, and the new brightness transition behavior, including cancellation and update handling.

Chores:

  • Remove obsolete sensor, Kalman filter, brightness curve, and power-saving dimming code from Display1 and session/power1, along with related DSettings keys and helper wiring.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @fly602, your pull request is larger than the review limit of 150000 diff characters

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602

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

@sourcery-ai

sourcery-ai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors Display1’s auto brightness to consume recommendations from AmbientBrightness1, replaces the old sensor/curve/transition stack with a single deterministic transition engine, and moves power‑saving brightness scaling into Display1 while keeping persisted brightness values unscaled, with tests and docs for the new architecture.

Sequence diagram for automatic brightness recommendation and manual override

sequenceDiagram
    participant AmbientBrightness1
    participant RecommendationClient
    participant AutoBrightnessManager
    participant BrightnessTransition
    participant DisplayManager

    AmbientBrightness1->>RecommendationClient: PropertiesChanged(Enabled,State,RecommendedBrightness)
    RecommendationClient->>AutoBrightnessManager: onAmbientBrightnessStateChanged(state)
    AutoBrightnessManager->>AutoBrightnessManager: applyRecommendedBrightness()
    AutoBrightnessManager->>DisplayManager: getBuiltinMonitor()
    AutoBrightnessManager->>DisplayManager: getBrightnessScale()
    AutoBrightnessManager->>BrightnessTransition: Run(current,target)
    BrightnessTransition->>DisplayManager: setBrightnessAndSync(name,value)
    BrightnessTransition-->>AutoBrightnessManager: onTransitionComplete(target)
    AutoBrightnessManager->>DisplayManager: saveBrightnessInCfg(name: recommendedBrightness)

    actor User
    User->>DisplayManager: SetAndSaveBrightness(name,value)
    DisplayManager->>AutoBrightnessManager: DisableForManualAdjustment()
    AutoBrightnessManager->>BrightnessTransition: Stop()
    AutoBrightnessManager->>AmbientBrightness1: Enable(false)
    AmbientBrightness1->>RecommendationClient: PropertiesChanged(Enabled)
    RecommendationClient->>AutoBrightnessManager: onAmbientBrightnessStateChanged(state)
Loading

File-Level Changes

Change Details Files
Auto brightness now consumes AmbientBrightness1 recommendations and arbitrates them in Display1, replacing the old sensor/Kalman/curve-based implementation.
  • Replaced AutoBrightnessManager’s sensorproxy, Kalman filter, curve, polling, and manual-override logic with a client of AmbientBrightness1 and a simple state machine driven by PropertiesChanged signals.
  • Introduced RecommendationClient to read AmbientBrightness1 properties, track lifecycle state, and invoke callbacks on state changes and name-owner changes.
  • AutoBrightnessManager now tracks service availability, enabled/running/held status, recommended brightness, and uses Manager.isSessionActive as an application gate for automatic updates.
  • Automatic brightness enable/disable in Display1 is proxied to AmbientBrightness1.Enable and synchronized back via state callbacks, rather than using Display1-local config.
  • Manual brightness operations call prepareManualBrightnessChange/DisableForManualAdjustment, which stop any ongoing automatic transition, save current brightness as manual baseline, and disable ambient automatic mode.
display1/auto_brightness.go
display1/recommendation_client.go
display1/manager.go
display1/manager_ifc.go
display1/manager_lid.go
session/power1/manager.go
keybinding1/display_controller.go
session/power1/helper.go
session/power1/lid_switch.go
session/power1/constant.go
session/power1/power_dbusutil.go
misc/dsg-configs/org.deepin.Display.AutoBrightness.json
session/power1/ambient_light_test.go
session/power1/manager_ambient_light.go
Brightness transitions are reimplemented as a single-worker, deterministic engine that supports target updates and synchronous cancellation, and the old per-monitor TransitionManager is removed.
  • Replaced TransitionExecutor/TransitionManager and their configuration (duration, step percent, min step interval, per-output gamma/backlight/DDCCI setters) with BrightnessTransition, which runs a single goroutine that interpolates between from/target using a fixed ramp and easing function.
  • BrightnessTransition exposes Run, Update, Stop, IsRunning, and SetOnComplete, guaranteeing that Stop waits for in-flight writes and that Update restarts timing from the current transactional value without spawning new workers.
  • Manager.setMonitorBrightness now directly uses per-monitor setters without going through TransitionManager, and setBrightness is guarded by brightnessWriteMu to serialize hardware writes.
  • AutoBrightnessManager owns a BrightnessTransition instance, uses Update for new recommendations, and onTransitionComplete persists the unscaled recommended brightness to config.
  • All DSettings-based transition configuration keys and properties, plus TransitionManager initialization and D-Bus step callbacks, are removed from Manager and related code.
display1/brightness/brightness_transition.go
display1/auto_brightness.go
display1/brightness.go
display1/manager.go
display1/display_dbusutil.go
Power-saving brightness scaling is centralized in Display1 using a runtime scale factor derived from Power1 properties, with persisted brightness remaining unscaled.
  • Added brightnessScale field and brightnessScaleMu/brightnessWriteMu to Manager, plus initBrightnessScale/onPowerSavingModeChanged/get/setBrightnessScale/applyBrightnessScale helpers.
  • Display1 now creates and caches a syspower.Power client, listens to PowerSavingModeEnabled and PowerSavingModeBrightnessDropPercent, and computes a scale via calcBrightnessScale; scale changes trigger reapplication of brightness via applyBrightnessScale.
  • Introduced scaleBrightness(base, scale) that enforces [0.1, 1.0] bounds and applies scaling only above the minimum brightness; this is used in initBrightness, applySysMonitorConfigs, RefreshBrightness, color temperature one-shot, and auto brightness application paths.
  • Removed session/power1’s multiBrightnessWithPsm, PowerSavingModeBrightnessData, brightnessSaveWhilePsm, and all logic that previously recalculated and persisted scaled brightness per monitor.
  • New configurations for SysMonitorConfig.Brightness always store 1 (logical full brightness) when creating monitor configs, ensuring persisted values are not contaminated by power-saving scaling.
display1/brightness_scale.go
display1/brightness.go
display1/manager.go
display1/color_temp.go
display1/monitor.go
session/power1/power_save_plan.go
session/power1/manager.go
session/power1/helper.go
session/power1/constant.go
Legacy auto-brightness sensor and curve logic, plus power-side ambient light toggles, are removed to make AmbientBrightness1 the single source of truth.
  • Deleted brightness curve and FLM backlight curve configuration keys and their loading logic from display1 Manager, along with curve-based backlight mapping in brightness._setBacklight.
  • Removed sensorproxy integration, ambient light properties, and claim/release logic from session/power1 Manager, Helper, lid switch handling, and power D-Bus property helpers.
  • Deleted AutoBrightnessConfig, DSettings-based auto brightness config management, Kalman filter usage, sensor compensation timers, and ambient light curve loading from AutoBrightnessManager.
  • Removed ambient light adjust brightness toggling from keybinding1 DisplayController and power DSettings configs for ambient light and light sensor enablement.
  • Eliminated misc dsg-config files related to Display.AutoBrightness and their usage in both display and power modules.
display1/auto_brightness.go
display1/brightness/brightness.go
display1/brightness/curve.go
display1/sensor_proxy.go
display1/manager.go
session/power1/helper.go
session/power1/manager.go
session/power1/lid_switch.go
session/power1/power_dbusutil.go
session/power1/ambient_light_test.go
session/power1/manager_ambient_light.go
misc/dsg-configs/org.deepin.Display.json
misc/dsg-configs/org.deepin.Display.AutoBrightness.json
misc/dsg-configs/org.deepin.dde.daemon.power.json
keybinding1/constants/dsettings.go
keybinding1/display_controller.go
New tests and documentation cover the recommendation client, auto brightness state handling, and brightness transition behavior.
  • Added auto_brightness_test.go to verify RecommendationClient’s parsing of AmbientBrightness1 states, PropertiesChanged handling, invalid recommendation recovery, three-way branch behavior for Active/WaitingForSample/Unavailable/Disabled, and application gating for recommended brightness.
  • Added tests for BrightnessTransition ensuring that Update reaches the latest target, Stop waits for in-flight writes and prevents further writes, and Stop-before-Run is safe.
  • Introduced docs/brightness-architecture.md detailing the new auto brightness and power-saving brightness scaling architecture, interfaces, application conditions, and persistence semantics.
  • Used helper functions like parseRecommendationState and isValidRecommendedBrightness to make tests independent of live D-Bus state.
  • Ensured new docs and tests are wired into the repository alongside the refactored code without altering external D-Bus APIs beyond the ambient brightness delegation changes.
display1/auto_brightness_test.go
docs/brightness-architecture.md
display1/recommendation_client.go
display1/brightness/brightness_transition.go
display1/auto_brightness.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@fly602
fly602 force-pushed the auto-brightness branch 2 times, most recently from a1ef6bd to a69b144 Compare July 30, 2026 12:18
1. Use AmbientBrightness1 as the recommendation source and let Display1 arbitrate automatic and manual brightness updates.
2. Refactor brightness transitions to support target updates and deterministic cancellation.
3. Move power-saving brightness scaling to Display1 while keeping persisted brightness values unscaled.
4. Remove the obsolete sensor, filtering, curve, and power-saving dimming logic from Display1 and session/power1.
5. Remove dde-daemon's iio-sensor-proxy recommendation because sensor access is now owned by AmbientBrightness1.
6. Add regression tests and documentation for the new brightness architecture.

Log: Refactor auto brightness logic and centralize brightness arbitration in Display1.

Influence:
1. Verify automatic brightness follows recommendations from AmbientBrightness1.
2. Verify manual brightness changes stop automatic transitions and disable automatic adjustment.
3. Verify power-saving scaling affects hardware brightness without changing persisted values.
4. Verify brightness transitions can update targets and stop cleanly.

refactor(display): 重构自动亮度逻辑

1. 使用 AmbientBrightness1 提供推荐亮度,由 Display1 统一仲裁自动和手动亮度更新。
2. 重构亮度渐变逻辑,支持动态更新目标值和确定性停止渐变任务。
3. 将节能亮度缩放迁移到 Display1,同时保持持久化亮度值不受缩放影响。
4. 移除 Display1 和 session/power1 中陈旧的传感器、滤波、曲线及节能调光逻辑。
5. 光感访问已由 AmbientBrightness1 负责,因此移除 dde-daemon 对 iio-sensor-proxy 的推荐依赖。
6. 为新的亮度架构补充回归测试和设计文档。

Log: 重构自动亮度逻辑,由 Display1 统一仲裁亮度更新。

Influence:
1. 验证自动亮度能够正确应用 AmbientBrightness1 的推荐值。
2. 验证手动调节亮度时会停止自动渐变并关闭自动调节。
3. 验证节能缩放影响实际亮度,但不会修改持久化亮度值。
4. 验证亮度渐变能够动态更新目标并可靠停止。

PMS: BUG-372191
@fly602
fly602 force-pushed the auto-brightness branch from a69b144 to b55ccbf Compare July 31, 2026 07:01
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码重构精简了自动亮度逻辑并引入了节能缩放机制,架构清晰且性能良好
逻辑基本正确但因结构体比较存在浮点数NaN边界问题扣5分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

display1/recommendation_client.go 中的 handlePropertiesChanged 函数使用了 if state == c.state 来判断状态是否改变。由于 RecommendationState 结构体包含 float64 类型的 RecommendedBrightness 字段,如果该字段被设置为 math.NaN(),根据 IEEE 754 标准,NaN != NaN,这会导致比较结果永远为 false。即使状态实际上没有改变(例如连续收到两次 NaN),也会触发回调,可能导致不必要的亮度重计算和设置。
潜在问题:包含浮点数的结构体直接使用 == 比较存在 NaN 边界问题,可能导致重复回调
建议:避免对包含浮点数的结构体直接使用 ==,或者单独处理 NaN 情况,例如使用逐字段比较或引入一个版本号来判断状态是否真正改变

  • 2.代码质量(良好)✓

本次重构移除了大量冗余的卡尔曼滤波和旧配置管理代码,引入了职责单一的 RecommendationClient 和 BrightnessTransition。代码结构清晰,注释详尽,如 BrightnessTransition 的单 worker 设计说明。资源清理逻辑(如 Destroy 和 Cleanup)完善,错误处理规范。
潜在问题:无
建议:无

  • 3.代码性能(高效)✓

display1/brightness/brightness_transition.go 中的 BrightnessTransition 采用单 worker 循环处理渐变,避免了频繁创建和销毁 goroutine 的开销。RecommendationClient 在本地缓存状态,仅在状态变化时触发回调,减少了 D-Bus 往返和上层计算。亮度缩放计算简单直接,无复杂算法。
潜在问题:无
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码主要涉及 D-Bus 通信和本地亮度计算,无外部输入解析风险,无内存越界或注入风险。并发访问通过互斥锁妥善保护,无竞态条件导致的安全问题。
建议:无

■ 【改进建议代码示例】

// display1/recommendation_client.go
// 修复结构体比较导致的 NaN 问题,改为逐字段比较

// 在 RecommendationClient 中添加状态比较方法
func (s RecommendationState) equals(other RecommendationState) bool {
    if s.Available != other.Available ||
        s.Enabled != other.Enabled ||
        s.Supported != other.Supported ||
        s.State != other.State {
        return false
    }
    // 处理浮点数比较,包括 NaN 情况
    if math.IsNaN(s.RecommendedBrightness) && math.IsNaN(other.RecommendedBrightness) {
        return true
    }
    if s.RecommendedBrightness != other.RecommendedBrightness {
        return false
    }
    return true
}

func (c *RecommendationClient) handlePropertiesChanged(interfaceName string,
	changed map[string]dbus.Variant, invalidated []string) {
	if interfaceName != ambientBrightnessInterface {
		return
	}

	c.mu.Lock()
	state := c.state
	state.Available = true

	for _, name := range invalidated {
		switch name {
		case "Enabled":
			state.Enabled = false
		case "State":
			state.State = ""
		case "Supported":
			state.Supported = false
		case "RecommendedBrightness":
			state.RecommendedBrightness = math.NaN()
		}
	}

	if value, ok := changed["Enabled"]; ok {
		enabled, validType := value.Value().(bool)
		if !validType {
			logger.Warningf("[AutoBrightness] Invalid Enabled property type %T", value.Value())
			state.Enabled = false
		} else {
			state.Enabled = enabled
		}
	}

	if value, ok := changed["State"]; ok {
		stateName, validType := value.Value().(string)
		if !validType {
			logger.Warningf("[AutoBrightness] Invalid State property type %T", value.Value())
			state.State = ""
		} else {
			state.State = stateName
		}
	}

	if value, ok := changed["Supported"]; ok {
		supported, validType := value.Value().(bool)
		if !validType {
			logger.Warningf("[AutoBrightness] Invalid Supported property type %T", value.Value())
			state.Supported = false
		} else {
			state.Supported = supported
		}
	}

	if value, ok := changed["RecommendedBrightness"]; ok {
		recommended, validType := value.Value().(float64)
		if !validType || !isValidRecommendedBrightness(recommended) {
			logger.Warningf("[AutoBrightness] Invalid RecommendedBrightness value %v", value.Value())
			state.RecommendedBrightness = math.NaN()
		} else {
			state.RecommendedBrightness = recommended
		}
	}

    // 使用自定义的 equals 方法替代 ==
	if state.equals(c.state) {
		c.mu.Unlock()
		return
	}
	c.state = state
	callback := c.callback
	c.mu.Unlock()

	if callback != nil {
		callback(state)
	}
}

// 同样修改 setState 方法
func (c *RecommendationClient) setState(state RecommendationState) {
	c.mu.Lock()
	if state.equals(c.state) {
		c.mu.Unlock()
		return
	}
	c.state = state
	callback := c.callback
	c.mu.Unlock()

	if callback != nil {
		callback(state)
	}
}

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.

2 participants