Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Feb 9, 2026

When a slider has no tick marks, the groove now extends to the full width/height of the slider area without indentation at the ends.

log: adjust slider groove offset when no ticks are displayed
bug: PMS-299727

qreal rectWidth = rectHandle.width() / 2.0;
p->drawLine(QPointF(rectGroove.left() + rectWidth, rectHandle.center().y()), QPointF(rectHandle.center().x(), rectHandle.center().y()));
// 只有当不绘制刻度线的时候,就不要给滑槽两边弄缩进
qreal leftOffset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(slider->tickPosition == QSlider::NoTicks)可以isNoticks去判断,

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts Chameleon style slider groove rendering so that when a slider has no tick marks, the groove is not indented at the ends and instead spans the full available length (per PMS-299727).

Changes:

  • Remove groove end indentation when tickPosition == QSlider::NoTicks for horizontal sliders.
  • Remove groove end indentation when tickPosition == QSlider::NoTicks for vertical sliders.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 3775 to 3782
// 只有当不绘制刻度线的时候,就不要给滑槽两边弄缩进
qreal leftOffset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
p->drawLine(QPointF(rectGroove.left() + leftOffset, rectHandle.center().y()), QPointF(rectHandle.center().x(), rectHandle.center().y()));
// 绘制滑块到最右的位置的滑槽
pen.setColor(color);
p->setPen(pen);
p->drawLine(QPointF(rectGroove.right() - rectWidth, rectHandle.center().y()), QPointF(rectHandle.center().x(), rectHandle.center().y()));
qreal rightOffset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
p->drawLine(QPointF(rectGroove.right() - rightOffset, rectHandle.center().y()), QPointF(rectHandle.center().x(), rectHandle.center().y()));
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new groove-offset logic keys off slider->tickPosition, but this file already treats DSlider as a special case where the effective tick position may come from dslider->tickPosition() (see e.g. the handle drawing/focus logic). This can make groove indentation inconsistent with handle shape/color decisions for DSlider. Consider computing a single effective TickPosition (or noTicks bool) using the same dslider ? dslider->tickPosition() : slider->tickPosition rule and reuse it for all offsets here.

Copilot uses AI. Check for mistakes.
Comment on lines 3787 to 3794
// 只有当不绘制刻度线的时候,就不要给滑槽两边弄缩进
qreal bottomOffset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
p->drawLine(QPointF(rectGroove.center().x(), rectGroove.bottom() - bottomOffset), QPointF(rectGroove.center().x(), rectHandle.center().y()));
// 绘制滑块到最下的位置的滑槽
pen.setColor(color);
p->setPen(pen);
p->drawLine(QPointF(rectGroove.center().x(), rectGroove.top() + rectWidth), QPointF(rectGroove.center().x(), rectHandle.center().y()));
qreal topOffset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
p->drawLine(QPointF(rectGroove.center().x(), rectGroove.top() + topOffset), QPointF(rectGroove.center().x(), rectHandle.center().y()));
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the horizontal case: bottomOffset/topOffset are derived from slider->tickPosition only. For DSlider, other parts of this style use dslider->tickPosition() as the source of truth, so the groove could still be indented even when the handle is rendered in the NoTicks style. Please use a single effective tick-position/noTicks value (including the DSlider override) for these offsets as well.

Copilot uses AI. Check for mistakes.
When a slider has no tick marks, the groove now extends to the full
width/height of the slider area without indentation at the ends.

log: adjust slider groove offset when no ticks are displayed
bug: PMS-299727
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码是针对 Qt 自定义样式中 QSlider(滑块)控件的绘制逻辑进行了修改,主要是为了优化滑槽在不同状态下的显示效果。以下是对这段 diff 的审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 逻辑正确性:修改后的逻辑是合理的。
    • 原代码在绘制滑槽时,左右(或上下)总是会有一个 rectWidth(滑块宽度或高度的一半)的缩进。
    • 新代码引入了 offset 变量,通过判断 slider->tickPosition == QSlider::NoTicks 来决定是否应用这个缩进。
    • 逻辑意图:当滑块没有刻度线时,滑槽应该填满整个可用空间,不需要为刻度预留空间,因此 offset 为 0;当有刻度线时,保持原有的缩进逻辑。这符合 UI 设计的常规直觉。
  • 计算准确性
    • 水平方向:rectGroove.left() + offsetrectGroove.right() - offset 的计算是正确的。
    • 垂直方向:rectGroove.bottom() - offsetrectGroove.top() + offset 的计算也是正确的。

2. 代码质量

  • 可读性:代码质量较高。
    • 引入 offset 变量消除了重复的三元运算符逻辑,使得 drawLine 的参数更加清晰。
    • 注释 // 只有当不绘制刻度线的时候,就不要给滑槽两边弄缩进 虽然口语化,但准确描述了代码意图,有助于理解。
  • 一致性:水平滑块和垂直滑块的修改逻辑保持一致,遵循了对称性原则,易于维护。

3. 代码性能

  • 性能影响:极小。
    • 增加了一次 slider->tickPosition 的读取和一次条件判断。
    • 由于这是在绘图事件中调用的,且计算量非常微小(简单的浮点数加减),不会对性能产生可感知的影响。

4. 代码安全

  • 空指针检查:代码中使用了 slider 指针(slider->tickPosition)。
    • 潜在风险:如果在进入此 if 分支之前没有确保 slider 指针非空,这里存在潜在的空指针解引用风险。
    • 建议:请确认 slider 指针的有效性。通常在 drawComplexControl 中,option 会被 qstyleoption_cast 转换为 const QStyleOptionSlider *,建议在转换后立即检查是否为 nullptr
    • 示例:
      if (const QStyleOptionSlider *slider = qstyleoption_cast<const QStyleOptionSlider *>(option)) {
          // ... 现有代码 ...
      }
  • 类型安全:使用了 qreal 进行坐标计算,符合 Qt 绘图系统的标准做法,类型转换安全。

总结与改进建议

这段代码修改本身是优秀的,它解决了一个具体的 UI 细节问题(无刻度时滑槽的填充),且实现方式简洁。

改进建议:

  1. 注释规范化:建议将注释修改得稍微正式一些,例如:
    // 如果没有刻度线,滑槽不需要预留缩进空间
    qreal offset = (slider->tickPosition == QSlider::NoTicks) ? 0 : rectWidth;
  2. 代码复用(可选):如果这段逻辑在多处使用,可以考虑提取为一个辅助函数,但考虑到上下文特定性,目前的内联写法也是完全可以接受的。

结论: 该代码修改可以合并。逻辑清晰,修正了特定场景下的显示效果,没有引入明显的副作用或性能问题。只需确保调用处对 slider 指针进行了必要的判空处理即可。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, add-uos

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

@18202781743 18202781743 merged commit 77e5b6f into linuxdeepin:master Feb 9, 2026
19 of 20 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