Skip to content

fix: In the prompt interface, the grid is hidden to improve user expe…#454

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle
Mar 3, 2026
Merged

fix: In the prompt interface, the grid is hidden to improve user expe…#454
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle

Conversation

@lichaofan2008
Copy link
Contributor

…rience.

In the prompt interface, the grid is hidden to improve user experience.
提示界面下,不显示井字格,提升用户体验。

Bug: https://pms.uniontech.com/bug-view-351495.html

…rience.

In the prompt interface, the grid is hidden to improve user experience.
提示界面下,不显示井字格,提升用户体验。

Bug: https://pms.uniontech.com/bug-view-351495.html
Copy link

@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 @lichaofan2008, 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

deepin pr auto review

代码审查意见

经过对 git diff 的审查,这段代码主要修改了 videowidget 类中关于网格线(m_pGridLineItem)显示/隐藏的逻辑。以下是从语法逻辑、代码质量、代码性能和代码安全四个方面的详细分析和改进建议:

1. 语法逻辑

  • 现状:代码逻辑从“显示网格”变更为“在特定条件下隐藏网格”。具体来说,在 showNocamshowCamUsed 方法中,原本的逻辑是“如果网格不可见则显示它”,现在改为“如果网格可见则隐藏它”。在 setGridType 中,增加了对 m_SvgItem(推测为提示界面元素)的判断,只有当提示界面不可见时才显示网格。
  • 分析:逻辑上看起来是为了实现“当显示无摄像头提示或摄像头占用提示时,强制隐藏背景的井字格”。这个逻辑本身是自洽的,解决了UI层级遮挡或视觉干扰的问题。
  • 潜在问题
    • 状态不一致风险:在 showNocam/showCamUsed 中隐藏了网格,但没有重置 m_GridType 变量。如果用户随后切换回正常视频流,代码可能依赖 m_GridType 来决定是否恢复网格。如果 showNocam 只是临时隐藏,恢复逻辑需要确保能正确读取 m_GridType 的值。
    • setGridType 中的逻辑冲突:在 setGridType 中,逻辑是 if (!m_SvgItem->isVisible()) 才显示。这意味着如果 m_SvgItem 正在显示,调用 setGridType 设置为开启网格时,网格实际上不会显示。这符合“提示界面不显示井字格”的需求,但需要确保当 m_SvgItem 隐藏时,有机制重新触发网格的显示。

2. 代码质量

  • 重复代码
    • showNocamshowCamUsed 中的修改代码完全一致。这违反了 DRY(Don't Repeat Yourself)原则。建议将这段逻辑提取为一个私有成员函数,例如 hideGridOverlay()
  • 注释
    • // 提示界面不显示井字格 这个注释很有帮助,解释了修改的原因,值得保留。
  • 变量命名
    • m_pGridLineItemm_gridlinewidget 的命名风格略有不同(一个是下划线开头大写,一个是全小写),建议统一风格(如 Qt 风格通常推荐 m_gridLineItem)。

3. 代码性能

  • 现状:代码仅涉及简单的布尔判断和 UI 控件的 show()/hide() 调用。
  • 分析:性能开销极小,对运行效率几乎没有影响。Qt 的 show()/hide() 通常只会触发重绘事件,在非高频调用的场景下(如切换摄像头状态)完全可接受。

4. 代码安全

  • 空指针检查
    • 代码中使用了 m_pGridLineItem->isVisible()m_SvgItem->isVisible()。虽然 Qt 指针通常在类构造函数中初始化,但在修改逻辑前,最好确认这些指针是否可能为 nullptr。如果存在为空的风险,应添加判空保护:
      if (m_pGridLineItem && m_pGridLineItem->isVisible()) { ... }
  • 线程安全
    • 如果这些方法可能从不同线程调用(虽然 UI 操作通常在主线程),直接操作 UI 对象是不安全的。假设代码遵循 Qt 规范仅在主线程调用,则无问题。

改进建议代码示例

针对代码质量和逻辑的改进建议如下:

// 在 videowidget.h 中声明私有辅助函数
private:
    void hideGridOverlay();

// 在 videowidget.cpp 中实现
void videowidget::hideGridOverlay()
{
    // 提示界面不显示井字格
    if (m_pGridLineItem && m_pGridLineItem->isVisible()) {
        m_pGridLineItem->hide();
    }
    if (m_gridlinewidget && m_gridlinewidget->isVisible()) {
        m_gridlinewidget->hide();
    }
}

void videowidget::showNocam()
{
    // ... 其他逻辑 ...
    
    if (m_GridType != Grid_None) {
        hideGridOverlay(); // 调用统一函数
    }
}

void videowidget::showCamUsed()
{
    // ... 其他逻辑 ...

    if (m_GridType != Grid_None) {
        hideGridOverlay(); // 调用统一函数
    }
}

void videowidget::setGridType(GridType type)
{
    // ... 其他逻辑 ...
    
    if (type == Grid_None) {
        // ... 隐藏逻辑 ...
    } else {
        if (m_gridlinewidget) m_gridlinewidget->hide();
        
        // 提示界面不显示井字格
        // 增加空指针检查,确保逻辑安全
        if (!m_SvgItem || !m_SvgItem->isVisible()) {
            if (m_pGridLineItem) m_pGridLineItem->show();
        } else {
            // 如果提示界面正在显示,确保网格是隐藏的
            if (m_pGridLineItem) m_pGridLineItem->hide();
        }
    }
}

总结

这段代码的修改在功能上是合理的,主要解决了UI显示冲突的问题。主要的改进点在于消除重复代码、增加必要的空指针检查以及确保状态管理的一致性。通过提取公共函数和增加防御性编程,可以提高代码的健壮性和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, max-lvs

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

@lichaofan2008
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Mar 3, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 0d20c75 into linuxdeepin:release/eagle Mar 3, 2026
15 of 17 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