Review Request: Improve automatic scrolling of text in Lyrics applet

[Matěj Laitl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104935/#review13805
-----------------------------------------------------------

Ship it!


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.

- Matěj Laitl


On May 13, 2012, 11:42 p.m., Alexander Potashev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104935/
> -----------------------------------------------------------
> 
> (Updated May 13, 2012, 11:42 p.m.)
> 
> 
> Review request for Amarok, Leo Franchi and Rick W. Chen.
> 
> 
> Description
> -------
> 
> 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`.
> 
> 
> Diffs
> -----
> 
>   src/context/applets/lyrics/LyricsApplet.cpp 9720db0 
> 
> Diff: http://git.reviewboard.kde.org/r/104935/diff/
> 
> 
> Testing
> -------
> 
> Works as expected.
> 
> 
> Thanks,
> 
> Alexander Potashev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120514/f2b52173/attachment-0001.html>