Skip to content

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

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master
Mar 12, 2026
Merged

fix: In the prompt interface, the grid is hidden to improve user expe…#459
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master

Conversation

@lichaofan2008
Copy link
Copy Markdown
Contributor

@lichaofan2008 lichaofan2008 commented Mar 12, 2026

…rience.

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

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

v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html

Summary by Sourcery

Bug Fixes:

  • Prevent grid lines from appearing on prompt interfaces when no camera is available or the camera is in use.

…rience.

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

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

v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Mar 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts grid overlay visibility logic in the video widget so that grid lines are hidden in prompt/notification UI states while remaining enabled in normal camera view, guarded by existing state flags.

Sequence diagram for prompt UI grid hiding behavior

sequenceDiagram
    actor User
    participant UI as PromptUI
    participant VW as videowidget
    participant GridItem as m_pGridLineItem
    participant GridWidget as m_gridlinewidget

    User->>UI: Trigger no camera / camera used state
    UI->>VW: showNocam() or showCamUsed()
    VW->>VW: check m_GridType != Grid_None
    alt Grid type active
        VW->>GridItem: isVisible()
        alt GridItem is visible
            VW->>GridItem: hide()
        end
        VW->>GridWidget: isVisible()
        alt GridWidget is visible
            VW->>GridWidget: hide()
        end
    end
Loading

Sequence diagram for setGridType with prompt SVG visibility guard

sequenceDiagram
    participant Controller
    participant VW as videowidget
    participant GridItem as m_pGridLineItem
    participant GridWidget as m_gridlinewidget
    participant SvgItem as m_pSvgItem

    Controller->>VW: setGridType(type)
    VW->>VW: m_GridType = type
    alt type == Grid_None
        VW->>GridItem: hide()
        VW->>GridWidget: hide()
    else type != Grid_None
        VW->>VW: decide grid representation
        alt show overlay grid lines
            VW->>GridWidget: hide()
            VW->>SvgItem: isVisible()
            alt SvgItem not visible
                VW->>GridItem: show()
            else SvgItem visible
                VW->>GridItem: keep hidden
            end
        else other grid representation
            VW->>GridItem: hide()
        end
    end
Loading

Class diagram for updated videowidget grid visibility logic

classDiagram
    class videowidget {
        <<QObject>>
        GridType m_GridType
        QGraphicsItem m_pGridLineItem
        QWidget m_gridlinewidget
        QGraphicsItem m_pSvgItem

        void showNocam()
        void showCamUsed()
        void setGridType(GridType type)
    }

    class GridType {
    }

    videowidget --> GridType : uses
Loading

File-Level Changes

Change Details Files
Change grid visibility behavior in prompt states to hide the grid overlay instead of showing it.
  • In showNocam(), invert logic to hide m_pGridLineItem when a grid type is active and the prompt UI is shown, while still hiding m_gridlinewidget.
  • In showCamUsed(), mirror the new behavior by hiding m_pGridLineItem and m_gridlinewidget when the camera-in-use prompt UI is active and a grid type is set.
src/src/videowidget.cpp
Conditionally allow grid to be shown only when the main video SVG item is visible, preventing grid display on the prompt interface.
  • In setGridType(), keep hiding m_pGridLineItem for Grid_None and still hide m_gridlinewidget when a valid grid is selected.
  • Gate m_pGridLineItem->show() behind a visibility check of m_pSvgItem so that the grid appears only when the main video content is visible, not on the prompt UI.
src/src/videowidget.cpp

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

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.

Hey - I've left some high level feedback:

  • The grid-visibility logic for the prompt interface is duplicated in showNocam and showCamUsed; consider extracting a small helper (e.g., updateGridForPromptState()) so the behavior is defined in one place.
  • The condition if (!m_pSvgItem->isVisible()) in setGridType is used as a proxy for whether the prompt interface is active; consider wrapping this in a clearly named helper (e.g., isPromptInterfaceVisible()) to make the intent self-documenting and avoid relying on raw widget visibility checks.
  • The existing debug messages like "Grid detected" and "Grid type is not Grid_None" now precede code that hides the grid; consider updating or adding logs so they clearly indicate that the grid is being hidden in the prompt interface.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The grid-visibility logic for the prompt interface is duplicated in `showNocam` and `showCamUsed`; consider extracting a small helper (e.g., `updateGridForPromptState()`) so the behavior is defined in one place.
- The condition `if (!m_pSvgItem->isVisible())` in `setGridType` is used as a proxy for whether the prompt interface is active; consider wrapping this in a clearly named helper (e.g., `isPromptInterfaceVisible()`) to make the intent self-documenting and avoid relying on raw widget visibility checks.
- The existing debug messages like `"Grid detected"` and `"Grid type is not Grid_None"` now precede code that hides the grid; consider updating or adding logs so they clearly indicate that the grid is being hidden in the prompt interface.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

针对您提供的 git diff 代码差异,我进行了仔细的审查。这段代码主要修改了视频组件(videowidget)中关于井字格(Grid)显示的逻辑。以下是详细的审查意见:

1. 语法逻辑审查

  • 逻辑反转与一致性
    • showNocam()showCamUsed() 函数中,原来的逻辑是“如果网格线不可见,则显示它(!isVisible -> show)”。修改后的逻辑变成了“如果网格线可见,则隐藏它(isVisible -> hide)”。
    • 潜在问题:这种修改改变了函数的原始意图。通常 showNocam(显示无摄像头画面)或 showCamUsed(显示摄像头占用画面)属于“提示界面”。根据注释 // 提示界面不显示井字格,在提示界面强制隐藏网格是合理的。但是,需要确认当用户离开这些提示界面回到正常视频流时,是否有对应的逻辑重新显示网格。如果 setGridType 中没有正确处理状态恢复,可能会导致网格在正常模式下也消失。
  • 条件判断冗余
    • showNocamshowCamUsed 中:
      if (m_pGridLineItem->isVisible()) {
          m_pGridLineItem->hide();
      }
      调用 hide() 方法通常在对象不可见时也是安全的(幂等操作)。直接调用 m_pGridLineItem->hide(); 通常更简洁,除非 hide() 方法本身有副作用或者性能开销极大(通常 Qt 的 hide() 没有)。不过,保留判断可以避免不必要的重绘,属于一种微优化。

2. 代码质量审查

  • 代码重复
    • showNocam()showCamUsed() 中的网格隐藏逻辑完全一致。建议将这段逻辑提取为一个私有成员函数,例如 hideGridOverlay(),以提高可维护性。
  • 注释清晰度
    • // 提示界面不显示井字格 这个注释很好地解释了修改的原因,值得肯定。

3. 代码性能审查

  • 性能影响较小
    • isVisible()hide()/show() 都是 Qt 图形视图框架中非常轻量的操作,主要涉及标志位的设置。这里的修改不会引入明显的性能问题。
  • 状态检查
    • setGridType 中新增了对 m_pSvgItem 的检查:
      if (!m_pSvgItem->isVisible()) {
          m_pGridLineItem->show();
      }
      这增加了一个依赖项。需要确保 m_pSvgItem 的生命周期和状态是稳定的。如果 m_pSvgItem 是提示界面的元素,这个逻辑意味着:只有当提示元素(Svg)不可见时,才显示网格。这是一个合理的逻辑关联。

4. 代码安全审查

  • 空指针风险
    • 代码中使用了 m_pGridLineItemm_gridlinewidgetm_pSvgItem。虽然 diff 中没有显示初始化代码,但必须确保这些指针在使用前已经正确初始化,且未被销毁。
    • 特别是在 setGridType 中引入了对 m_pSvgItem 的访问,如果之前没有对 m_pSvgItem 进行判空保护,建议添加 if (m_pSvgItem && ...) 以防潜在的空指针解引用崩溃。
  • 状态同步
    • 最大的安全风险在于状态一致性。修改后的逻辑使得网格的显示依赖于 m_pSvgItem 的可见性。
    • 场景推演:如果用户在“提示界面”(Svg可见,Grid隐藏)时切换了 GridType,然后退出提示界面(Svg隐藏),此时 setGridType 可能不会被再次调用,导致 Grid 依然保持隐藏状态,与用户预期的 GridType 设置不符。
    • 建议:确保在退出提示界面(即 m_pSvgItem 隐藏)时,或者在恢复正常视频流时,显式调用一次网格状态刷新逻辑,确保 UI 显示与内部状态 m_GridType 同步。

5. 改进建议总结

  1. 提取公共方法:将 showNocamshowCamUsed 中隐藏网格的代码提取为 void hideGridElements()
  2. 增加空指针保护:在访问 m_pSvgItem 前,建议增加判空检查。
  3. 简化条件判断:可以直接调用 hide(),除非有特殊性能需求。
  4. 状态恢复机制:请务必确认当 m_pSvgItem 从可见变为不可见(退出提示界面)时,网格是否能根据当前的 m_GridType 自动恢复显示。如果 setGridType 不会在此时被触发,可能需要在 hideSvg 或类似的逻辑中补充网格显示的刷新代码。

修改后的代码示例建议(针对 setGridType 部分):

void videowidget::setGridType(GridType type)
{
    // ... 其他代码 ...
    
    if (type != Grid_None) {
        if (m_gridlinewidget) {
            m_gridlinewidget->hide();
        }
        
        // 增加空指针保护,并明确逻辑:仅当提示界面(Svg)不可见时,才显示网格
        if (m_pSvgItem && !m_pSvgItem->isVisible()) {
            if (m_pGridLineItem) {
                m_pGridLineItem->show();
            }
        } else {
            // 如果提示界面正在显示,确保网格是隐藏的
            if (m_pGridLineItem) {
                m_pGridLineItem->hide();
            }
        }
    }
    // ... 其他代码 ...
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, lzwind

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
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 6d4f6b0 into linuxdeepin:master Mar 12, 2026
19 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