Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance: make TConsole scrolling smoother #4445

Merged

Conversation

SlySven
Copy link
Member

@SlySven SlySven commented Dec 9, 2020

Make it scroll by only 1 line of text per mouse wheel click instead of three lines. To compensate for the slower nature of the scrolling now, make the scrolling faster (to a half the number of lines in the upper TTextEdit pane per step instead of one line) when the control key is pressed during the wheel event; this thus mimics the same sort of speed up in the map zoom and scrolling through the selected room number/name listing in the 2D mapper.

This should close #4437 .

It also fixes a potential bug in that the TTextEdit::updateScreenView() looked to be sending the "screenheight" for both the upper and lower panes to the same variable in the Host class - when they are not going to be the same value.

It also removes the use of the obsolete (int) QWheelEvent::delta() in favour of the recommended (QPoint) QWheelEvent::angleDelta() in the qlWidget class.

Signed-off-by: Stephen Lyons slysven@virginmedia.com

Make it scroll by only 1 line of text per mouse wheel click instead of
three lines. To compensate for the slower nature of the scrolling now, make
the scrolling faster (to a half the number of lines in the upper
`TTextEdit` pane per step instead of one line) when the control key is
pressed during the wheel event; this thus mimics the same sort of speed
up in the map zoom and scrolling through the selected room number/name
listing in the 2D mapper.

This should close Mudlet#4437 .

It also fixes a potential bug in that the `TTextEdit::updateScreenView()`
looked to be sending the "screenheight" for both the upper and lower panes
to the same variable in the `Host` class - when they are not going to be
the same value.

It also removes the use of the obsolete `(int) QWheelEvent::delta()`
in favour of the recommended `(QPoint) QWheelEvent::angleDelta()` in the
`qlWidget` class.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
@SlySven SlySven requested a review from a team as a code owner December 9, 2020 00:14
@SlySven SlySven requested a review from a team as a code owner December 9, 2020 00:14
@add-deployment-links
Copy link

add-deployment-links bot commented Dec 9, 2020

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

@atari2600tim
Copy link
Contributor

atari2600tim commented Dec 9, 2020

Pasted in lua for i=1,1000 do print("demo "..i) end to start off with a numbered buffer.
With my 14 font size, screen starts off with lines 975 to 1000. Experimenting with a Logitech M570 (clicky wheel) and laptop touchpad.

Held fingertip at bottom of wheel range
A: slowly move it up until finger is flat against top
B: zip back down

status quo:
A: got up to 961 or 958
B: got back down to 970

demo:
A: got up to 968 or 967
B: got back down to bottom, far enough to end the split

Repeated that a few times, status quo shows a net gain of 10 lines or so. Managed to move 100 lines up just wiggling finger back and forth between same 2 spots at different speeds a few times. With new build, moving back and forth between 2 spots brings it back and forth between the same 2 spots. Moving faster just gets you there faster. Scrolling to an exact line is much easier now. Very nice.

From the bottom, scroll up to line 500.

status quo: 33 rolls up (at comfortable speed)
demo: 57 rolls up, or 6 with control key

So it's more exact, but feels pretty different, slower if you're wanting to go back far and not aware of the control key thing. Adjusting multiplier would be judgement call for testing.

Tried two fingers on the touchpad of my laptop...
A: move up across the whole trackpad slowly in about 3 seconds
B: back down fast as can

status quo:
A: got up to line 478
B: got back down to 568. Visually it is still moving some after my fingers are off of it. I believe that my mouse driver or OS or whatever is emulating a wheel that keeps spinning until I touch to catch it.

demo:
A: Oh! it isn't moving at all unless I move pretty quick. I think it's throwing away the moves that are less than a line. Steps are 120 points, maybe my trackpad is picking up 119 or smaller points of movement several times in a row without building up. Holding control key for the multiplier allows it to detect the movement though.

@atari2600tim
Copy link
Contributor

atari2600tim commented Dec 9, 2020

I think it's throwing away the moves that are less than a line.

I would have an outside variable something vaguely like this, which would be held in-between the events and range from -119 to 119 saying how much wheel movement happened so far but didn't result in scrolling any lines:

int accumulatedY = 0;
void wheelEvent(mousewheel){
  int linesToMove;
  accumulatedY += ( mousewheel.deltaY() * ( controlKey ? 10 : 1) );
  linesToMove = accumulatedY/120; // integer division, 120 = 15 degrees in units of eighths
  if(abs(linesToMove) < 1) { mousewheel->handled(); return; } // moved but less than 1 line
  accumulatedY -= linesToMove*120; // note that we're handling a certain amount right now but keep remainder saved for next time
  if(linesToMove > 0) { scrollUp(linesToMove); }
  else{ scrollDown(linesToMove); }
}

Then if you move your device by for example 90 points four times in a row then you'll move by 3 lines rather than 0:
1: reach 90 points, move 0 lines, remainder 90 points
2: reach 180 points, move 1 line, subtract 120 for remainder 60 points
3: reach 150 points, move 1 line, subtract 120 for remainder 30 points
4: reach 120 points, move 1 line, subtract 120 for remainder 0 points

@SlySven
Copy link
Member Author

SlySven commented Dec 9, 2020

I haven't looked at touch pad considerations in this - but IIRC the Qt documentation does make mention of some hi-resolution devices not behaving the same as a clicky mouse scroll wheel, lemme see if I can find it...

Ah yeah, it is in the obvious place as part of the QWheelEvent documentation - specifically for the angeDeleta() method:

Returns the relative amount that the wheel was rotated, in eighths of a degree. A positive value indicates that the wheel was rotated forwards away from the user; a negative value indicates that the wheel was rotated backwards toward the user. angleDelta().y() provides the angle through which the common vertical mouse wheel was rotated since the previous event. angleDelta().x() provides the angle through which the horizontal mouse wheel was rotated, if the mouse has a horizontal wheel; otherwise it stays at zero. Some mice allow the user to tilt the wheel to perform horizontal scrolling, and some touchpads support a horizontal scrolling gesture; that will also appear in angleDelta().x().

Most mouse types work in steps of 15 degrees, in which case the delta value is a multiple of 120; i.e., 120 units * 1/8 = 15 degrees.

However, some mice have finer-resolution wheels and send delta values that are less than 120 units (less than 15 degrees). To support this possibility, you can either cumulatively add the delta values from events until the value of 120 is reached, then scroll the widget, or you can partially scroll the widget in response to each wheel event. But to provide a more native feel, you should prefer pixelDelta() on platforms where it's available.

Example:

void MyWidget::wheelEvent(QWheelEvent *event)
{
    QPoint numPixels = event->pixelDelta();
    QPoint numDegrees = event->angleDelta() / 8;

    if (!numPixels.isNull()) {
        scrollWithPixels(numPixels);
    } else if (!numDegrees.isNull()) {
        QPoint numSteps = numDegrees / 15;
        scrollWithDegrees(numSteps);
    }

    event->accept();
}

Note: On platforms that support scrolling phases, the delta may be null when:

  • scrolling is about to begin, but the distance did not yet change (Qt::ScrollBegin),
  • or scrolling has ended and the distance did not change anymore (Qt::ScrollEnd).

See also pixelDelta().

I am going to have to look at this again and see what I can do after a good thunk...

@vadi2 vadi2 assigned SlySven and unassigned vadi2 Dec 11, 2020
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Bring in recent upstream changes.

Signed-off by: Stephen Lyons <slysven@virginmedia.com>
@atari2600tim
Copy link
Contributor

Tried it out in same way as before using trackpad on Logitech k400r, dragged super slow and made progress as intended. I didn't look too close at the code but feels much better and can easily scroll to exact line.

I do notice that it can not scroll horizontal. But the release build doesn't scroll horizonal either like it did with #4085.

The oldest one on the snapshots page doesn't scroll horizontal either. That got broken at some point more than 14 days ago, but more recent than the last build in my download folder, Mudlet-4.9.1-testing-pr3992-fd96e822-windows from Oct 23.

Copy link
Member

@vadi2 vadi2 left a comment

Choose a reason for hiding this comment

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

Feels good!

e->ignore();
// Make the speed up be half of the (upper pane) lines - need to round it so
// that a decimal part does not make the end +/- value for up/down different
// in magitude:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// in magitude:
// in magnitude:

// Store the (rounded) remainder
mMouseWheelRemainder = QPoint(delta.x() - (15 * xDelta), delta.y() - (15 * yDelta));

qDebug().noquote().nospace() << "TTextEdit::wheelEvent(...) INFO - in \""
Copy link
Member

Choose a reason for hiding this comment

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

This can be removed, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

W.I.P. on the horizontal scrolling aspect at the moment and I am using that. Mayhaps I should move that to a separate PR...

Copy link
Member

Choose a reason for hiding this comment

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

Agree, another PR is good

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
@vadi2
Copy link
Member

vadi2 commented Dec 14, 2020

Thanks for working on this 👍

@SlySven SlySven merged commit 95c497f into Mudlet:development Dec 14, 2020
@SlySven SlySven deleted the Enhance_makeTConsoleScrollingSmoother branch December 14, 2020 18:12
@atari2600tim
Copy link
Contributor

I think that this was the last instance of delta() to switch over to angleDelta() and this closes #498.

Chris7 pushed a commit to Chris7/Mudlet that referenced this pull request Jan 2, 2022
Make it scroll by only 1 line of text per mouse wheel click instead of
three lines. To compensate for the slower nature of the scrolling now, make
the scrolling faster (to a half the number of lines in the upper
`TTextEdit` pane per step instead of one line) when the control key is
pressed during the wheel event; this thus mimics the same sort of speed
up in the map zoom and scrolling through the selected room number/name
listing in the 2D mapper.

This should close Mudlet#4437 .

It also fixes a potential bug in that the `TTextEdit::updateScreenView()`
looked to be sending the "screenheight" for both the upper and lower panes
to the same variable in the `Host` class - when they are not going to be
the same value.

It also removes the use of the obsolete `(int) QWheelEvent::delta()`
in favour of the recommended `(QPoint) QWheelEvent::angleDelta()` in the
`qlWidget` class.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scroll smoother instead of always 3 lines
3 participants