Fix slowdown issue
ClosedPublic

Authored by helfferich on Jan 10 2018, 12:45 PM.

Details

Reviewers
aacid
Group Reviewers
KDE Games
Summary

The bug has been discussed on the mailing list: The speed of the ball decreases instead of increasing at some point in the game. The reason is that the QML-timer has a minimum resolution of 16ms (i.e. 60 fps). Now, instead of firing the timer at a higher interval, the evaluation (Move balls -> detect Collisions) is split into several substeps. The number of substeps is doubled instead of cutting the timer interval in half.

Test Plan

I have played the game extensively. The bug does no longer occur. I collected speed-up and slow-down bonuses both with a single and with multiple balls.

Diff Detail

Repository
R393 KBreakout
Lint
Lint Skipped
Unit
Unit Tests Skipped
helfferich requested review of this revision.Jan 10 2018, 12:45 PM
helfferich created this revision.

Thanks for picking up the topic with a patch. :)
I might be able to test this tonight but not sure.
Anyway, here are just some things I saw while scrolling over the patch.

src/qml/logic.js
38

divided

435

I don't think this comment is necessary. :)

OK, tested it and it works as in the ball does not slow down anymore.
I cannot really argue about the implementation because I did not understand why this switching is needed in the first place. :D

helfferich updated this revision to Diff 25115.Jan 10 2018, 7:08 PM

Thanks for catching the typo. About the comment: You are of course perfectly right :D.

Do you mean switching the interval/substeps/speed? Or switching from a change in timer interval to the substeps?

I'll try to explain:

  1. Why change the interval? The basic loop is: Move ball -> Check for collisions -> Change direction, perform actions, according to collisions, etc. -> Move ball -> ... The speed determines how far the ball is moved during the "Move ball". If the ball is moved too far, the collision detection might be inaccurate (E.g. the extreme case is the ball moving through a brick with no detected collision). To avoid such bugs, we move the ball only half the distance, but twice as often.
  2. Why use substeps now? In the original code, this was done by cutting the gameTimer interval in half. However, this broke during the port to QML, because QML timer have a minimum interval of 16ms. Cutting it in half to 8ms does not have any effect. The alternative approach is to keep the gameTimer interval, but perform the basic loop twice.

I hope this made it a little bit more clear. I am still wondering if the new approach has performance issues (moving the balls sequentially, instead of in parallel), but I don't think it matters as the collision detection, which is much more costly, is done sequentially as well.

Do you mean switching the interval/substeps/speed? Or switching from a change in timer interval to the substeps?

I'll try to explain:

  1. Why change the interval? The basic loop is: Move ball -> Check for collisions -> Change direction, perform actions, according to collisions, etc. -> Move ball -> ... The speed determines how far the ball is moved during the "Move ball". If the ball is moved too far, the collision detection might be inaccurate (E.g. the extreme case is the ball moving through a brick with no detected collision). To avoid such bugs, we move the ball only half the distance, but twice as often.
  2. Why use substeps now? In the original code, this was done by cutting the gameTimer interval in half. However, this broke during the port to QML, because QML timer have a minimum interval of 16ms. Cutting it in half to 8ms does not have any effect. The alternative approach is to keep the gameTimer interval, but perform the basic loop twice.

Thanks for the explanation. That made it more clear. :)

I hope this made it a little bit more clear. I am still wondering if the new approach has performance issues (moving the balls sequentially, instead of in parallel), but I don't think it matters as the collision detection, which is much more costly, is done sequentially as well.

I played on a five years old laptop and when the "cut" came, the ball felt more smooth. In general it is better than before the change here.

aacid added a subscriber: aacid.Jan 10 2018, 10:01 PM

Looks sensible, only minor comment is that you should be using === instead of == in JavaScript code since it's marginally faster, but not that it matters much.

To me you can commit it but i won't give you the "Accept" yet in case someone else wants to commit, if no one beats me to it i'll add it in a week.

aacid accepted this revision.Jan 17 2018, 5:29 PM
This revision is now accepted and ready to land.Jan 17 2018, 5:29 PM
helfferich closed this revision.Jan 20 2018, 8:29 PM

Pushed to master and Applications/17.12