Improve automatic scrolling of text in Lyrics applet

Review Request #104935 - Created May 13, 2012 and submitted

Information
Alexander Potashev
amarok
Reviewers
amarok
lfranchi, rickc
The former scrolling algorithm was to scroll text continuously
while playing a song. In this case the first lines of lyrics hide
very quickly - often before they are sung.

Again, with the old algorithm the scrollbar position is 0 at the
beginning of the song (0:00), and it reaches the maximum possible
position exactly when the song ends.

The new algorithm tries to keep the currently "played" line
at center of the Lyrics applet window. To achieve this, we
target scrollbar position to be (-pageStep/2) at the beginning
of the song (0:00), and to reach position (maximum + pageStep/2)
exactly when the song ends.

`pageStep` is the height of Lyrics applet window expressed in the
units used by the scrollbar. Therefore `pageStep/2` is an offset
that can be added/substracted to move down/up by half a page in
the Lyrics applet.

Here is a formula to transform a scrollbar position used in the old
algorithm to the scrollbar position when the new algorithm is used:

    new_pos = pos / maximum * (maximum + pageStep) - pageStep / 2

We make the following changes to the algorithm:

1. Multiply by (maximum + pageStep) instead of `maximum` to fulfill
the multiplication factor from the above formula.

2. Additionally, substract `pageStep/2`, as defined by the above
formula.

3. Avoid using floating point math (the "double" data type) to improve
performance.

4. Re-read the current position of the scrollbar when saving
`oldSliderPosition`, because scrollbar positions calculated using
the above formula may be out of the allowed range of the scrollbar
and "truncated", i.e. adjusted to fit the scrollbar range.
When truncated, `vbar->value()` differs from `newSliderPosition`.
Works as expected.
Commit Hook
Alexander Potashev
Review request changed

Status: Closed (submitted)

Matěj Laitl
I wanted to do exactly the same for ages, thanks for the patch! Only one small point - using floating point arithmetics is slightly slower, but in infrequent cases like this the slowdown is absolutelny below measurable margin - other things like repainting are thousands times slower. But I'm fine leaving it like this if you can ensure the rounding error won't be noticeable.
  1. I understand that getting rid of floating point arithmetics does not make much different, but the code looks cleaner without those casts to `double`.
    
    The rounding error will be the same as in the old code: Even before this patch, the resulting scrollbar position had to be rounded to an integer.
Loading...