web/gui: add an option for arrow keys scroll acceleration - #10983
web/gui: add an option for arrow keys scroll acceleration#10983SquareRobin wants to merge 5 commits into
Conversation
Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an option to enable scroll acceleration when using arrow keys in the GUI layout viewer. It propagates the new configuration option through the MainWindow, LayoutTabs, and LayoutScroll classes, and implements the acceleration logic in LayoutScroll's key press event handler. The review feedback suggests aligning the newly added member variables and constants with the project's naming conventions (using snake_case_ and kCamelCase). Additionally, it recommends refining the acceleration logic by using floating-point division with rounding to prevent abrupt jumps caused by integer division, and resetting the acceleration counter when the active scroll key changes.
Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
switch to float with static_cast to int in the multiplier calculation Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
Signed-off-by: Igor Sokolov <disk.destroyer@yahoo.com>
gadfort
left a comment
gadfort
left a comment
There was a problem hiding this comment.
I noticed that the scrolling seems to exceed the speed of the rendering in the GUI, probably not a huge problem, but odd.
Also, we are moving away from the GUI into the web viewer, would you want to try and port these changes over there as well?
| int accel_counter_ = 0; | ||
| static constexpr int kAccelLutSize = 20; | ||
| static constexpr int kAccelCounterMax = kAccelLutSize - 1; | ||
| static std::array<int, kAccelLutSize> accel_lut_; |
There was a problem hiding this comment.
I'm not sure how much this being static saves, is there is a way to mark is const that would make it clear that this is not going to change.
| static constexpr int kAccelLutSize = 20; | ||
| static constexpr int kAccelCounterMax = kAccelLutSize - 1; | ||
| static std::array<int, kAccelLutSize> accel_lut_; | ||
| int getScrollAcceleration() const { return accel_lut_[accel_counter_]; }; |
There was a problem hiding this comment.
maybe use std::clamp() to ensure accel_counter is inside the array
| void LayoutScroll::keyPressEvent(QKeyEvent* event) | ||
| { | ||
| switch (event->key()) { | ||
| Qt::Key key = static_cast<Qt::Key>(event->key()); |
| if (accel_counter_ < kAccelCounterMax) { | ||
| accel_counter_++; | ||
| } |
There was a problem hiding this comment.
should this go after getScollAcceleration? It seems like you would ways get index 1 and index 0 is doing nothing.
|
I'll look into porting this to the web interface. As for the slow rendering, making the acceleration curve proportionate to the viewer zoom factor and size should help. |
SquareRobin
changed the title
2 participants
Summary
Scrolling with the arrow keys is convenient, but the acceleration is controlled only by the system. Adding an option for acceleration does not overcomplicate the settings and makes arrow keys scrolling a bit more practical.
Testing with different key repeat settings did not reveal any problems related to different auto-repeat frequencies and delays.
Type of Change
Impact
Adds a checkbox to the "Options" menu.
Verification
./etc/Build.sh).