fix(editor): enhance inertial scrolling with improved speed calculation#458
Conversation
Reviewer's GuideRefines the editor’s inertial scrolling by switching from per-move speed estimation to accumulated distance/time averaging, introducing configurable limits on speed and duration, adding idle-time checks to suppress unwanted inertia, and resetting new tracking state on mouse release. Sequence diagram for the updated inertial scrolling on mouse releasesequenceDiagram
actor User
participant TextEdit
participant TweenX
participant TweenY
User->>TextEdit: mousePressEvent
activate TextEdit
TextEdit-->>User: start slide gesture
loop while dragging
User->>TextEdit: mouseMoveEvent
TextEdit->>TextEdit: update m_endX m_endY
TextEdit->>TextEdit: compute diffXpos diffYpos diffTimeX diffTimeY
TextEdit->>TextEdit: slideGestureX slideGestureY (realtime follow)
TextEdit->>TextEdit: m_totalDistanceX += diffXpos
TextEdit->>TextEdit: m_totalDistanceY += diffYpos
TextEdit->>TextEdit: m_totalDurationX += diffTimeX
TextEdit->>TextEdit: m_totalDurationY += diffTimeY
TextEdit->>TextEdit: m_moveCount++
TextEdit->>TextEdit: update m_lastMouseTimeY
end
User->>TextEdit: mouseReleaseEvent
TextEdit->>TextEdit: check m_gestureAction == GA_slide
TextEdit->>TextEdit: timeSinceLastMove = timestamp - m_lastMouseTimeY
alt timeSinceLastMove < MAX_IDLE_TIME and m_moveCount > 0 and durations > 0
TextEdit->>TextEdit: avgSpeedY = m_totalDistanceY / m_totalDurationY
TextEdit->>TextEdit: avgSpeedX = m_totalDistanceX / m_totalDurationX
TextEdit->>TextEdit: apply INERTIAL_SPEED_MULTIPLIER to avgSpeedY
TextEdit->>TextEdit: clamp speeds to [-MAX_INERTIAL_SPEED, MAX_INERTIAL_SPEED]
TextEdit->>TextEdit: compute durationY durationX (sqrt based)
TextEdit->>TextEdit: cap durations by MAX_INERTIAL_DURATION
TextEdit->>TextEdit: adjust speeds by font size
TextEdit->>TextEdit: compute changeY changeX
TextEdit->>TextEdit: log inertial parameters
TextEdit->>TweenX: startX(0 0 changeX durationX callback)
TextEdit->>TweenY: startY(0 0 changeY durationY callback)
else no valid inertia
TextEdit-->>User: no inertial scroll
end
TextEdit->>TextEdit: reset m_totalDistance* m_totalDuration* m_moveCount
TextEdit->>TextEdit: m_gestureAction = GA_null
deactivate TextEdit
Updated class diagram for TextEdit inertial scrolling membersclassDiagram
class TextEdit {
// inertial state
ulong m_lastMouseTimeY
qreal m_stepSpeedY
qreal m_stepSpeedX
qreal m_totalDistanceY
qreal m_totalDistanceX
ulong m_totalDurationY
ulong m_totalDurationX
int m_moveCount
// event handlers
void mouseMoveEvent(QMouseEvent *e)
void mouseReleaseEvent(QMouseEvent *e)
// scrolling helpers
void slideGestureX(qreal delta)
void slideGestureY(qreal delta)
}
class InertialConfig {
<<static>> qreal MAX_INERTIAL_SPEED
<<static>> qreal MAX_INERTIAL_DURATION
<<static>> qreal INERTIAL_SPEED_MULTIPLIER
<<static>> ulong MAX_IDLE_TIME
}
TextEdit .. InertialConfig : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The inertia trigger condition
m_totalDurationY > 0 && m_totalDurationX > 0inmouseReleaseEventprevents inertial scrolling when the user scrolls only vertically or only horizontally; consider allowing inertia when either axis has valid movement. - The Y-axis average speed is scaled by
INERTIAL_SPEED_MULTIPLIERwhile the X-axis speed is not, which may lead to inconsistent inertial feel between axes; consider applying the multiplier consistently or documenting the intentional asymmetry. - The new
qWarning()log for every inertial scroll start may be too noisy in normal use; consider downgrading it to a debug-level log or guarding it behind a verbosity flag.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The inertia trigger condition `m_totalDurationY > 0 && m_totalDurationX > 0` in `mouseReleaseEvent` prevents inertial scrolling when the user scrolls only vertically or only horizontally; consider allowing inertia when either axis has valid movement.
- The Y-axis average speed is scaled by `INERTIAL_SPEED_MULTIPLIER` while the X-axis speed is not, which may lead to inconsistent inertial feel between axes; consider applying the multiplier consistently or documenting the intentional asymmetry.
- The new `qWarning()` log for every inertial scroll start may be too noisy in normal use; consider downgrading it to a debug-level log or guarding it behind a verbosity flag.
## Individual Comments
### Comment 1
<location path="src/editor/dtextedit.cpp" line_range="6872" />
<code_context>
//add for single refers to the sliding
if (e->type() == QEvent::MouseButtonRelease && e->source() == Qt::MouseEventSynthesizedByQt) {
if (m_gestureAction == GA_slide) {
+ // 检查最后一次移动到现在的时差,如果太长说明有停顿,不触发惯性
+ ulong timeSinceLastMove = e->timestamp() - m_lastMouseTimeY;
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the shared inertia math and state checks into reusable helpers to simplify mouseReleaseEvent and avoid duplicated per-axis logic.
You can keep all current behavior but reduce complexity and duplication by extracting the per-axis math and the trigger condition into helpers.
### 1. Extract per-axis inertia calculation
All the speed/duration/distance math for X/Y is nearly identical. Move it into a single pure helper:
```cpp
struct InertiaAxisResult {
qreal distance = 0.0;
qreal duration = 0.0;
};
static InertiaAxisResult computeInertiaAxis(qreal totalDistance,
qreal totalDuration,
qreal fontPointSize,
bool amplifySpeed)
{
if (totalDuration <= 0)
return {};
qreal avgSpeed = totalDistance / totalDuration;
if (amplifySpeed)
avgSpeed *= INERTIAL_SPEED_MULTIPLIER;
avgSpeed = qBound(-MAX_INERTIAL_SPEED, avgSpeed, MAX_INERTIAL_SPEED);
qreal duration = std::sqrt(std::abs(avgSpeed)) * 1000;
const qreal originalDuration = duration;
duration = std::min(duration, MAX_INERTIAL_DURATION);
qreal adjustedSpeed = avgSpeed / std::sqrt(fontPointSize * 4.0);
qreal distance = adjustedSpeed * std::sqrt(std::abs(adjustedSpeed)) * 100;
if (duration < originalDuration && originalDuration > 0.0)
distance *= (duration / originalDuration);
return { distance, duration };
}
```
Usage in `mouseReleaseEvent`:
```cpp
if (m_gestureAction == GA_slide) {
const ulong timeSinceLastMove = e->timestamp() - m_lastMouseTimeY;
if (shouldTriggerInertia(timeSinceLastMove)) {
const QFont font = this->font();
const qreal fontPt = font.pointSizeF();
InertiaAxisResult y = computeInertiaAxis(m_totalDistanceY, m_totalDurationY,
fontPt, /*amplifySpeed=*/true);
InertiaAxisResult x = computeInertiaAxis(m_totalDistanceX, m_totalDurationX,
fontPt, /*amplifySpeed=*/false);
qWarning() << "Inertial scroll started - Y: duration=" << y.duration
<< "ms, distance=" << y.distance
<< "px | X: duration=" << x.duration
<< "ms, distance=" << x.distance << "px";
tweenX.startX(0, 0, x.distance, x.duration,
std::bind(&TextEdit::slideGestureX, this, std::placeholders::_1));
tweenY.startY(0, 0, y.distance, y.duration,
std::bind(&TextEdit::slideGestureY, this, std::placeholders::_1));
}
resetInertiaAccumulation();
m_gestureAction = GA_null;
}
```
This removes the duplicated speed/duration/distance code and makes future tuning much less error-prone.
### 2. Extract the trigger condition and reset logic
The inline condition:
```cpp
if (timeSinceLastMove < MAX_IDLE_TIME && m_moveCount > 0 &&
m_totalDurationY > 0 && m_totalDurationX > 0) {
...
}
```
can be named and reused:
```cpp
bool TextEdit::shouldTriggerInertia(ulong timeSinceLastMove) const
{
return timeSinceLastMove < MAX_IDLE_TIME
&& m_moveCount > 0
&& m_totalDurationY > 0
&& m_totalDurationX > 0;
}
void TextEdit::resetInertiaAccumulation()
{
m_totalDistanceY = 0.0;
m_totalDistanceX = 0.0;
m_totalDurationY = 0;
m_totalDurationX = 0;
m_moveCount = 0;
}
```
This keeps `mouseReleaseEvent` focused on control flow (press → maybe inertia → reset) instead of mixing it with detailed math and state management, while preserving all existing functionality.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Add inertial scrolling constants (MAX_INERTIAL_SPEED, MAX_INERTIAL_DURATION, INERTIAL_SPEED_MULTIPLIER, MAX_IDLE_TIME) - Replace per-move speed calculation with accumulated distance/time tracking for more accurate overall velocity - Implement idle time check to prevent inertia after pauses - Add duration limiting to maximum 2 seconds for better UX - Calculate average speeds based on total distance over total time - Apply font-based speed adjustment and log final parameters - Reset accumulation variables after releasing mouse button Log: fix(editor): enhance inertial scrolling with improved speed calculation Bug: https://pms.uniontech.com/bug-view-359379.html
暂不需要,目前只有触控在使用这部分逻辑 |
deepin pr auto review代码审查意见1. 语法和逻辑1.1 变量初始化
1.2 惯性滑动逻辑
1.3 时间戳比较ulong timeSinceLastMove = e->timestamp() - m_lastMouseTimeY;
2. 代码质量2.1 魔法数字
2.2 日志级别qWarning() << "Inertial scroll started - Y: speed=" << avgSpeedY << "px/ms, duration=" << durationY << "ms, distance=" << changeY << "px | X: speed=" << avgSpeedX << "px/ms, duration=" << durationX << "ms, distance=" << changeX << "px";
2.3 代码重复
3. 代码性能3.1 不必要的计算qreal originalDurationY = durationY;
durationY = qMin(durationY, MAX_INERTIAL_DURATION);
// ...
if (durationY < originalDurationY) {
changeY *= (durationY / originalDurationY);
}
3.2 频繁的数学计算
4. 代码安全4.1 边界检查if (timeSinceLastMove < MAX_IDLE_TIME && m_moveCount > 0 && m_totalDurationY > 0 && m_totalDurationX > 0) {
4.2 类型转换qreal avgSpeedY = static_cast<qreal>(m_totalDistanceY) / static_cast<qreal>(m_totalDurationY);
5. 改进建议5.1 提取惯性计算为独立函数struct InertialParams {
qreal speed;
qreal duration;
qreal distance;
};
InertialParams calculateInertialParams(qreal totalDistance, ulong totalDuration, const QFont &font) {
InertialParams params;
// 计算逻辑...
return params;
}5.2 添加方向变化检测// 在累加距离前检查方向变化
if ((m_totalDistanceY * diffYpos) < 0) {
// 方向改变,重置累加值
m_totalDistanceY = 0;
m_totalDurationY = 0;
}
m_totalDistanceY += diffYpos;5.3 优化时间差计算// 使用更安全的时间差计算
qint64 timeSinceLastMove = static_cast<qint64>(e->timestamp()) - static_cast<qint64>(m_lastMouseTimeY);
if (timeSinceLastMove < 0) {
// 处理时间戳回绕情况
timeSinceLastMove += ULONG_MAX; // 或其他适当的处理
}5.4 使用常量替代魔法数字static const qreal INERTIAL_DISTANCE_FACTOR = 100.0;
static const qreal FONT_ADJUSTMENT_FACTOR = 4.0;
static const qreal TIME_SCALE_FACTOR = 1000.0;总结这段代码实现了文本编辑器的惯性滑动功能,整体逻辑基本正确,但在边界条件处理、代码复用和性能优化方面还有改进空间。建议主要关注以下几点:
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, pppanghu77 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
0cd12fb
into
linuxdeepin:release/eagle
Log: fix(editor): enhance inertial scrolling with improved speed calculation
Bug: https://pms.uniontech.com/bug-view-359379.html
Summary by Sourcery
Improve editor inertial scrolling behavior by basing inertia on accumulated movement metrics and enforcing limits for speed, duration, and idle time.
Bug Fixes:
Enhancements: