Skip to content

Comments

fix: 修复插拔触控屏,触摸屏绑定关系会丢失问题#1004

Merged
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:fix-touchscreen
Jan 27, 2026
Merged

fix: 修复插拔触控屏,触摸屏绑定关系会丢失问题#1004
fly602 merged 1 commit intolinuxdeepin:masterfrom
fly602:fix-touchscreen

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Jan 27, 2026

不删除触控屏绑定配置

Log: 修复触控屏绑定问题
PMS: BUG-339549
Influence: 触控屏

Summary by Sourcery

Bug Fixes:

  • Stop deleting touchscreen binding mappings when a touchscreen is disconnected so that associations persist across hot-plug events.

不删除触控屏绑定配置

Log: 修复触控屏绑定问题
PMS: BUG-339549
Influence: 触控屏
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 27, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Preserves touchscreen-to-display binding configuration across device unplug/reattach events by removing cleanup logic that deleted mappings when a touchscreen or its bound output was not currently present.

Sequence diagram for touchscreen change handling after fix

sequenceDiagram
    participant TouchEventSource
    participant Manager
    participant MonitorRepo as MonitorRepository

    TouchEventSource->>Manager: handleTouchscreenChanged()
    Manager->>MonitorRepo: getConnectedMonitors()
    MonitorRepo-->>Manager: monitors

    alt one_touchscreen_and_one_monitor
        Manager->>Manager: associateTouch(monitor, touchUUID, true)
    else multiple_or_zero_devices
        loop for_each_touchscreen_in_Touchscreens
            Manager->>Manager: lookup mapping in touchscreenMap
            alt mapping_exists
                alt bound_output_exists_in_monitors
                    Manager->>Manager: associateTouch(monitor, touchUUID, false)
                else bound_output_missing
                    Manager->>Manager: keep mapping in touchscreenMap
                end
            else mapping_missing
                Manager->>Manager: no association change
            end
        end
    end

    Manager-->>TouchEventSource: return
Loading

Updated class diagram for Manager touchscreen mapping behavior

classDiagram
    class ManagerBeforePR {
        map~string_string~ touchscreenMap
        map~string_TouchInfo~ TouchMap
        Touchscreen[] Touchscreens
        +updateTouchscreenMap(outputName string, touchUUID string, auto bool) error
        +associateTouch(monitor *Monitor, touchUUID string, auto bool) error
        +handleTouchscreenChanged()
        -removeTouchscreenMap(touchUUID string)
    }

    class ManagerAfterPR {
        map~string_string~ touchscreenMap
        map~string_TouchInfo~ TouchMap
        Touchscreen[] Touchscreens
        +updateTouchscreenMap(outputName string, touchUUID string, auto bool) error
        +associateTouch(monitor *Monitor, touchUUID string, auto bool) error
        +handleTouchscreenChanged()
    }

    ManagerBeforePR <|-- ManagerAfterPR
Loading

File-Level Changes

Change Details Files
Stop deleting touchscreen binding entries when touch devices are unplugged or their mapped outputs are temporarily unavailable.
  • Remove the helper function that deleted touchscreen mapping entries from in-memory maps and persisted configuration.
  • Remove the loop in touchscreen change handling that pruned mappings whose touch UUID no longer corresponded to a currently connected touchscreen.
  • Remove the branch in touchscreen change handling that deleted mappings whose configured output no longer existed, so those mappings are now preserved.
display1/manager.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

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.

Hey - I've left some high level feedback:

  • With the removal of removeTouchscreenMap and the logic that pruned entries for unplugged touchscreens, touchscreenMap and TouchMap can now retain stale mappings indefinitely; consider adding a separate, intentional cleanup path (e.g., on explicit reset or after a long period of disuse) so these maps don’t grow unbounded with obsolete UUIDs.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- With the removal of `removeTouchscreenMap` and the logic that pruned entries for unplugged touchscreens, `touchscreenMap` and `TouchMap` can now retain stale mappings indefinitely; consider adding a separate, intentional cleanup path (e.g., on explicit reset or after a long period of disuse) so these maps don’t grow unbounded with obsolete UUIDs.

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

deepin pr auto review

这段代码的 diff 展示了对触摸屏映射管理逻辑的修改,主要涉及删除了 removeTouchscreenMap 函数以及在 handleTouchscreenChanged 中调用它的逻辑。

以下是对代码变更的审查意见,涵盖逻辑、质量、性能和安全四个方面:

1. 逻辑与功能

  • 潜在的数据不一致风险
    • 问题:原代码在 handleTouchscreenChanged 中会遍历 m.touchscreenMap,并检查其中的 UUID 是否存在于当前的 m.Touchscreens 列表中。如果不存在(即触摸屏已拔出),则调用 removeTouchscreenMap 清理该映射。
    • 变更影响:删除这段逻辑后,如果触摸屏被物理拔出,m.touchscreenMapm.TouchMap 中可能仍然保留着该触摸屏的旧配置记录。
    • 后果:如果同一个触摸屏再次插入,系统可能会尝试复用旧的映射配置。虽然这有时是期望的行为(记住配置),但如果旧配置关联的显示器已经不存在或发生了变化,可能会导致触摸映射错误,或者产生无效的脏数据堆积。
    • 建议:确认删除此清理逻辑是否是有意为之。如果是为了保留配置以便热插拔恢复,请确保后续逻辑(如 associateTouch)能够处理这种"僵尸"配置(例如验证关联的显示器是否仍然有效)。如果不需要保留配置,建议恢复清理逻辑,或者在触摸屏移除事件(如果有独立事件处理)中清理。

2. 代码质量

  • 代码整洁度
    • 正面:删除了不再使用的 removeTouchscreenMap 函数,这符合"死代码消除"(Dead Code Elimination)原则,减少了代码库的维护负担,是好的重构。
    • 注意:请确认该函数确实没有被其他地方调用(diff 显示仅被 handleTouchscreenChanged 调用)。

3. 代码性能

  • 性能优化
    • 正面:删除的代码包含一个双重循环(外层遍历 touchscreenMap,内层遍历 Touchscreens)。在触摸屏数量较多时,这可能会产生 $O(N*M)$ 的复杂度。删除这段代码消除了这部分开销。
    • 说明:虽然通常触摸屏数量很少(通常 < 5),性能影响微乎其微,但从算法角度看,移除不必要的遍历是有益的。

4. 代码安全

  • 无明显安全问题:这段代码主要涉及内存中的映射关系维护,不涉及外部输入解析、权限控制或敏感数据处理,因此没有明显的安全漏洞引入。

总结与改进建议

这次改动主要是删除了处理触摸屏移除时的清理逻辑。

主要改进建议:

  1. 验证配置一致性:由于删除了自动清理逻辑,建议在 associateTouch 或相关映射应用函数中增加防御性检查。例如,在应用 m.touchscreenMap 中的配置时,检查 outputName 对应的显示器是否真的存在于 m.getConnectedMonitors() 中。如果显示器不存在,应视为无效配置并清理(即恢复类似 removeTouchscreenMap 的行为,但仅在应用配置时触发)。

    // 伪代码示例
    if touch.outputName != "" {
        monitor := m.findMonitorByName(touch.outputName)
        if monitor == nil {
            // 关联的显示器不存在,清理无效配置
            m.removeTouchscreenMap(touch.UUID) // 或者内联清理逻辑
            continue
        }
        // ... 正常关联逻辑
    }
  2. 补充注释:如果删除清理逻辑是为了支持"记住拔出前的配置",建议在 handleTouchscreenChanged 函数头或相关文档中明确说明这一行为变更,以便后续维护者理解为何不再清理已断开的设备。

结论:代码改动在语法和结构上是正确的,且提升了代码整洁度。但需关注删除清理逻辑后可能带来的"脏数据"残留问题,建议通过防御性编程确保残留的旧配置不会导致运行时错误。

@deepin-ci-robot
Copy link

[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 91830f0 into linuxdeepin:master Jan 27, 2026
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