Skip to content

fix(osd): read Wayland brightness from Treeland instead of Display1#1667

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix/osd-brightness-treeland
Jul 16, 2026
Merged

fix(osd): read Wayland brightness from Treeland instead of Display1#1667
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:fix/osd-brightness-treeland

Conversation

@yixinshark

Copy link
Copy Markdown
Contributor

Summary

  • Read the primary output brightness from the Treeland output-manager protocol on Wayland.
  • Cache protocol brightness events for the OSD while preserving the Display1 query path on X11.
  • Destroy color-control and manager protocol objects during provider teardown.

Test plan

  • Build osdpanel and osd-brightness.
  • Verify brightness up/down OSD values under Treeland.
  • Verify the X11 brightness OSD still reads Display1.

@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 @yixinshark, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@yixinshark yixinshark force-pushed the fix/osd-brightness-treeland branch 2 times, most recently from bae633a to e3d0639 Compare July 15, 2026 08:00
Comment thread panels/notification/osd/brightness/brightnessapplet.cpp Outdated
Comment thread panels/notification/osd/brightness/treelandbrightness.cpp Outdated
@yixinshark yixinshark force-pushed the fix/osd-brightness-treeland branch from e3d0639 to 596ea1e Compare July 16, 2026 06:48
On Wayland the org.deepin.dde.Display1 service is unavailable, so the brightness OSD always displayed 0%. Bind a read-only treeland_output_color_control_v1 to the primary wl_output through treeland_output_manager_v1, cache the compositor-reported brightness, and update it reactively from protocol events. Keep the Display1 query path on X11. Destroy the color-control and manager protocol objects during provider teardown to avoid stale Wayland listeners.

在 Wayland 下 org.deepin.dde.Display1 不可用,导致亮度 OSD 始终显示 0%。通过 treeland_output_manager_v1 为主屏幕绑定只读的 treeland_output_color_control_v1,缓存合成端上报的亮度,并通过协议事件实时更新;X11 下继续使用 Display1。Provider 析构时依次销毁颜色控制和管理器协议对象,避免残留 Wayland listener。

Log: read Wayland OSD brightness from Treeland
@yixinshark yixinshark force-pushed the fix/osd-brightness-treeland branch from 596ea1e to b577ff9 Compare July 16, 2026 06:50
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码实现了Wayland环境下通过Treeland协议正确获取屏幕亮度,逻辑清晰且资源管理得当。
逻辑基本正确且无安全漏洞,仅因refresh函数的重试机制存在轻微瑕疵扣5分。

■ 【详细分析】

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

TreelandBrightness::refresh() 函数中,当 get_color_control(output) 返回 nullptr 时,函数提前返回,但此时 m_output 已经被赋值为当前的 output。如果后续再次调用 refresh() 且主屏幕未改变,由于 m_control && m_output == output && output 条件不成立(因为 m_control 为空),会继续执行删除和重新获取逻辑。但如果主屏幕未改变,且由于某种原因 compositor 暂时未准备好,虽然不会死锁,但 m_output 的赋值时机不够严谨。
潜在问题:如果 get_color_control 失败,m_output 被赋值可能导致后续状态判断不一致;虽然不会导致崩溃,但可能影响亮度获取的及时性。
建议:在成功获取 raw 指针后再赋值 m_output,或者在 get_color_control 失败时重置 m_outputnullptr

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

代码结构清晰,注释详细解释了 Wayland 下只读亮度的设计意图。使用 QPointer 管理 m_control 有效避免了悬空指针问题。BrightnessApplet::sync() 中对 Wayland 和 X11 环境的区分处理得当。
潜在问题:无明显的代码质量问题。
建议:保持当前的代码风格和注释规范。

  • 3.代码性能(无性能问题)✓

采用事件驱动的响应式更新机制,通过监听 brightnessChangedprimaryScreenChanged 信号来更新状态,避免了轮询,性能表现优秀。
潜在问题:无
建议:无需优化。

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码主要涉及 Wayland 协议交互和本地 DBus 调用,未涉及外部不可信输入的解析或执行,不存在命令注入或缓冲区溢出风险。对 Wayland 对象的空指针检查到位。

  • 建议:无需修复。

■ 【改进建议代码示例】

void TreelandBrightness::refresh()
{
    if (!isActive()) {
        delete m_control;
        m_control = nullptr;
        m_output = nullptr;
        return;
    }

    struct wl_output *output = primaryWlOutput();
    if (m_control && m_output == output && output) {
        return;
    }

    delete m_control;
    m_control = nullptr;
    // 延迟赋值 m_output,直到成功获取 control 对象
    // m_output = output;

    if (!output) {
        m_output = nullptr; // 确保状态一致
        return;
    }

    auto *raw = get_color_control(output);
    if (!raw) {
        m_output = nullptr; // 获取失败时重置状态,以便下次重试
        return;
    }

    m_output = output; // 成功获取后再赋值
    m_control = new TreelandColorControl(raw, this);
    connect(m_control, &TreelandColorControl::brightnessChanged,
            this, &TreelandBrightness::brightnessChanged);
}

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, yixinshark

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

@yixinshark

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 1498b99 into linuxdeepin:master Jul 16, 2026
10 of 13 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