EmulationStation icon indicating copy to clipboard operation
EmulationStation copied to clipboard

Fix stuttering when starting to scroll a Game List

Open XenuIsWatching opened this issue 4 years ago • 5 comments
trafficstars

Emulations station would stutter a lot when scorlling through game lists that had artwork with. This was due to it performing multiple reads of the screenshots, marquees, video, etc... For example, even if you were scrolling by 1, it would read these data 3 times. Each time it was calling the function updateInfoPanel which can kick of some File I/O which can be expensive (especially when you have the data on a NAS)

This gets it down to just 1 read. It also changes the behavior a little (for the better). There are three states in an input for scolling through a list: a rising edge where the button is pressed, a held high state where the button is held, and a falling edge where the button is released.

In the case where it was just press quickly, it was doing a read when the button was first pressed of the next item in the list. Then when it was released it would do another read of the next item in the list TWICE! The behavior now is to blank the info panel when the button is pressed, and then perform ONE read of the screenshot, etc. when the button is released.

Fixes #753

XenuIsWatching avatar May 21 '21 07:05 XenuIsWatching

I have this set to a draft because I want to run through some more test cases to make sure I didn't break anything else...

I also have before/after change videos demonstrating the noticeable improvement that will upload later.

XenuIsWatching avatar May 21 '21 07:05 XenuIsWatching

First: Thanks for documenting the variables. +1 Makes sense to me. However, I guess the wording needs some review (e.g. surplus "the"). My suggestion: Why not renaming mScrollTier to mScrollGear and mScrollVelocity to mScrollStep (as it is literally a fixed value and not a quotient over time)? This way mScrollGearAccumulator would almost explain itself.

Second: I found a small regression in the DetailGamelistView.

Large gamelist (> 1 Screen): Single step: Fade out/fade in (differs from master: no fading) Page step: Fade out/fade in (differs from master: no fading)

Short list (< 1 Screen): Single step: Fade out/fade in (differs from master: no fading) Page step: No fading / hard skip (ok as in master)

These are ok: Continous scrolling / Dimmed with Initials: ok (same as in master) Roll-over (i.e. pushing up at first list item, pushing down on last list item): ok (same as in master)

The introduced fadeout/fadein for single actions make the ES feel less snappy IMHO.

Gemba avatar May 22 '21 17:05 Gemba

First: Thanks for documenting the variables. +1 Makes sense to me. However, I guess the wording needs some review (e.g. surplus "the"). My suggestion: Why not renaming mScrollTier to mScrollGear and mScrollVelocity to mScrollStep (as it is literally a fixed value and not a quotient over time)? This way mScrollGearAccumulator would almost explain itself.

mScrollVelocity is really the direction you are scrolling. '-1forup, 1 for down, and 0 for stop scrolling. I could be renaming variables all over the place for clairty :/ ... but I didn't want to go down that rabbit hole. That could be rather a separate pull request, as I want this to stay focused on the issue at hand rather than updating the code for easier maintance and readability.

Second: I found a small regression in the DetailGamelistView.

Large gamelist (> 1 Screen): Single step: Fade out/fade in (differs from master: no fading) Page step: Fade out/fade in (differs from master: no fading)

Thanks for testing this out! About the single step, please see my comment below. "same reason as above

Short list (< 1 Screen): Single step: Fade out/fade in (differs from master: no fading) Page step: No fading / hard skip (ok as in master)

Again, Thanks for testing this out! Single step, same as above... Page step, hmm... that's interesting that it was different behavior. I'll try to reproduce it, and investigate why that is happening.

These are ok: Continous scrolling / Dimmed with Initials: ok (same as in master) Roll-over (i.e. pushing up at first list item, pushing down on last list item): ok (same as in master)

There is a bug I noticed in master with the Continuous scrolling, and then hitting the very end of the list. When it hits the very end of the list, It will load the info panel, even if the user hasn't let go of the button. But then once the user lets go of the button, it will reload the info panel again. You can notice this if you have a long description, and you hold the button until it begins scrolling after a brief moment. When you let go of it, the description will reset back to beginning indicateing it reloaded.

But this is very minor bug IMO...

The introduced fadeout/fadein for single actions make the ES feel less snappy IMHO.

I thought this at first, and I did experiment with this using that isSingleScrolling function I mentioned earlier. What I did was just force it to retain the state of the info panel when the button was first pressed, and then blank once it is known that the user is holding it down (scrolling by at least 2). But IMHO, I actually felt the opposite and found that this implementation felt less snappier. My thought went like this....

I press down a button, I see the next item selected... then I see the previous item still on the screen (albeit very briefly) wondering what's going on asking myself "Why am I still seeing old information on the screen, I already see the game selected?", and then I release the button, I finally see the update info after very brief moment again (hitting the file system). All of this happens in less than quarter a second...

For the case I have implemented, my thought went like this...

I press down a button, I see the info panel fade out (and I think to myself it must be doing something and I feel like it's moving), and then I finally release the button, I see the update info (which hits the file system) after very brief moment again. Again, all of this happens in less than quarter a second...

This is just IMHO as well, but if there is consensus from the maintainers/reviewers of this project, then I can easily make it perform the former action.

XenuIsWatching avatar May 22 '21 19:05 XenuIsWatching

I decided to go a simpler route which did no introduce blank in between single scrolling. All this does is record what it just wrote to the info panel, so that when it gets called again (for example, on a button release). it won't waste time updating the panel.

I found that in my system this function can take a while by adding in my own timers ( with <chrono>). I compared both my change and the base code. In the base code it calls it twice for button releases. I found that the first call (when the button is first pressed (rising edge), the time for this function can quite a long time under certain conditions in my system (50ms to 1000ms), but the subsequent calls that happened on the button release would take less (5ms to 35ms each). I can only speculate why the subsequent calls are so much shorter, but I would guess there is either some caching going on or my hard drive controller is doing some "magic". These subsequent calls are just rereading the same data and completely redundant and a waste of time.

Logs of my timing are below... retropie_infopanel_mod_time.txt retropie_infopanel_base_time.txt

XenuIsWatching avatar May 27 '21 06:05 XenuIsWatching

I tried the suggested change on a installation with the ProfilingUtil (#716). As expected there was not a significant effect measurable (Rpi4 with UASP USB to SATA adapter):

es-app/src/views/gamelist/DetailedGameListView.cpp
Commit: 52c04d7
Message          	       Calls	  Total Time	    Avg Time	    Min Time	    Max Time	 Internal Total Time	   Internal Avg Time
updateInfoPanel()	         316	    9.973043	    0.031560	    0.000001	    0.108529	            9.973043	            0.031560

With this PR:
Message          	       Calls	  Total Time	    Avg Time	    Min Time	    Max Time	 Internal Total Time	   Internal Avg Time
updateInfoPanel()	         314	   10.098609	    0.032161	    0.000001	    0.104449	           10.098609	            0.032161

Could you @XenuIsWatching pls try on your setup with the NAS? And: Are you using a wired or wireless connection to your NAS?

To use the profile, in a nutshell: cmake . -DPROFILING=on -DRPI=on -DGL=on, #include "utils/ProfilingUtil.h" in object under test, then ProfileScope("..."); in the method to profile. Start ES with --debug.

Gemba avatar Sep 02 '22 15:09 Gemba