fix(shot): reset selection index in clearSelected and guard deleteCur…

[dengzhongyuan365-dev]

…rentShape bounds

clearSelected() did not reset m_selectedIndex/-hoveredIndex, causing the blue selection border on mosaic/blur shapes to persist and be saved into the final screenshot. Also add lower bound check in deleteCurrentShape() to prevent crash when m_selectedOrder is -1.

bug: https://pms.uniontech.com/bug-view-356629.html

[deepin-ci-robot]

deepin pr auto review

这段代码主要涉及图形选择状态的清理和删除功能的逻辑修正。以下是对这段 diff 的详细审查和改进建议：

1. 语法与逻辑审查

现状分析：

• clearSelected 函数：
  • 新增了对 m_hoveredShape 的清理逻辑，这是一个很好的改进，确保了鼠标悬停状态也被重置。
  • 新增了对 m_selectedIndex、m_hoveredIndex 和 m_selectedOrder 的重置，将其设为 -1。这有助于防止脏数据导致的逻辑错误。
  • m_selectedOrder 的重置非常重要，因为 deleteCurrentShape 依赖它。
• deleteCurrentShape 函数：
  • 修改了边界检查条件，从 m_selectedOrder < m_shapes.length() 改为 m_selectedOrder >= 0 && m_selectedOrder < m_shapes.length()。
  • 逻辑修正正确：原代码仅检查上界，未检查下界。如果 m_selectedOrder 为 -1（在 clearSelected 中被重置），原代码虽然不会越界（因为 removeAt(-1) 在 Qt 中通常不生效或行为未定义），但逻辑上是不严谨的。新代码明确排除了负数索引，逻辑更严密。

2. 代码质量

现状分析：

• 循环重置点坐标：

  for (int j = 0; j < m_selectedShape.mainPoints.length(); j++) {
      m_selectedShape.mainPoints[j] = QPointF(0, 0);
  }

  这段代码在紧接着的 m_selectedShape.points.clear() 之后显得有些多余。如果目的是清空数据，直接 clear() 是最高效的。如果目的是保留容器大小但归零数据（虽然看起来不太像，因为后面有 clear），那么这种写法是可以的。但结合上下文看，这可能是冗余代码。
• 魔法数字：
  • 使用 -1 作为无效索引是 C++ 和 Qt 中的惯例，这是可以接受的。但如果项目中定义了常量（如 INVALID_INDEX），建议使用常量以提高可读性。
• 日志输出：
  • qDebug() << "delete shape"; 过于简单，建议打印出被删除形状的索引或类型，方便调试。

3. 代码性能

改进建议：

• 移除不必要的循环赋值：
  在 clearSelected 中，先遍历数组将点设为 (0,0)，然后又调用了 clear()。
  • 如果 mainPoints 是 QVector 或 QList，直接调用 clear() 会释放内存并析构对象，比循环赋值快得多且更彻底。
  • 建议：直接删除循环赋值的代码块，仅保留 clear()。

4. 代码安全

现状分析：

• 数组越界保护：
  • deleteCurrentShape 中的修改增加了对负数的检查，提高了安全性。
• 状态一致性：
  • clearSelected 中将所有相关状态变量（Index, Order）重置，防止了“对象已删除但索引仍保留”导致的悬空引用风险。

综合改进代码建议

基于以上分析，建议对 clearSelected 函数进行优化，去除冗余操作，并保持 deleteCurrentShape 的逻辑修正。

void ShapesWidget::clearSelected()
{
    qCDebug(dsrApp) << "Clearing selected shape";

    // 性能优化：直接清空容器，无需遍历赋值为0
    m_selectedShape.mainPoints.clear();
    m_selectedShape.points.clear();
    
    m_hoveredShape.mainPoints.clear();
    m_hoveredShape.points.clear();

    // 状态重置
    m_isSelected = false;
    m_selectedIndex = -1;
    m_hoveredIndex = -1;
    m_selectedOrder = -1;

    updateCursorShape();
}

void ShapesWidget::deleteCurrentShape()
{
    // 逻辑修正：保持双重边界检查
    if (m_selectedOrder >= 0 && m_selectedOrder < m_shapes.length()) {
        qDebug() << "Deleting shape at index:" << m_selectedOrder;
        m_shapes.removeAt(m_selectedOrder);
        
        // 安全建议：删除后通常需要重置选择状态，防止后续操作误用旧的 m_selectedOrder
        // 或者如果 UI 逻辑需要自动选中下一个/上一个，在这里处理
        clearSelected(); 
    } else {
        qWarning() << "Invalid index for deletion:" << m_selectedOrder;
    }
}

总结

1. 逻辑修正：deleteCurrentShape 的边界检查修改是正确的，防止了潜在的逻辑错误。
2. 性能优化：clearSelected 中的循环赋值在 clear() 之前是多余的，建议移除以提高性能。
3. 安全建议：在 deleteCurrentShape 成功删除后，显式调用 clearSelected() 或重置相关索引是一个更安全的做法，防止后续代码误用已失效的 m_selectedOrder。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, 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:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[dengzhongyuan365-dev]

/merge

[deepin-bot]

This pr cannot be merged! (status: unstable)

[dengzhongyuan365-dev]

/forcemerge

[deepin-bot]

This pr force merged! (status: unstable)