[GameList][Performance] Redundant file accesses to Game Info when scrolling by 1

[XenuIsWatching]

I noticed significant stuttering on my system when scrolling on my game lists. I put the debugger on it, and found that it was spending most of it's time in VideoGameListView::updateInfoPanel() which makes sense as this is doing file access to the video, marquee, and images to be displayed... but what I found odd was that it was called 3 times for the same game (performing file accesses each time!) and when I only scrolled by 1 (which seems rather unnecessary). It was called once by the input for the up or down button, then it was called twice again for by the stop scrolling.

It calls from the up or down button from here:

EmulationStation/es-app/src/components/TextListComponent.h

Line 283 in c6bbd38

listInput(1);

Which then calls listInput(delta) then calls scroll(), when then calls onCursorChanged() which then calls the updateInfoPanel() later on.

The stop scrolling is from here:

EmulationStation/es-app/src/components/TextListComponent.h

Line 313 in c6bbd38

stopScrolling();

This function is even more strange to me... It calls listInput(0) which will call the onCursorChanged() function calling updateInfoPanel(), but then it performs a direct call to onCursorChanged() right after finishing listInput(0).

VideoGameListView::updateInfoPanel()

EmulationStation/es-app/src/views/gamelist/VideoGameListView.cpp

Line 247 in c6bbd38

void VideoGameListView::updateInfoPanel()

I tried to think to myself.. "How can I fix this?", but as I was reading through the lower portions of the code, but more I just began to see spaghetti 🍝 (or I just need to spend more time understanding it) and wasn't quite sure how to re-architect it in order to fix these redundant files calls without breaking anything else as I believe this code is used with more than just Game Lists.

Anyways, I'm raising this issue just so it gets visibility from those who may have insights

[Gemba]

Thanks for the detailed analysis.

From a more general perspective it is expected to have at least two events onCursorChanged() for a short button press (i.e. not press and hold). One for the rising edge (released -> pressed) IList::listinput(1 /* in general some value != 0 */) and
one for the failing edge (pressed -> released) IList::listIpnut(0).

But more than two are too much.

Some educated guesses:

1. One onCursorChanged(CURSOR_STOPPED) call is surplus to my understanding. Can you check, if you want to, what happens if you remove it from IList::stopScrolling() method? (May introduce regression in SystemView, ImageGridComponent, ComponentList (used e.g. in Scraping from within ES)).

2. In TextListComponent::input() after the stopScrolling()-call the input() method should be exited/jumped out with return true, to avoid further processing by subseqent GUI Components and possible additional events. I assume ImageGridComponent::input() copy-pasted this from TextListComponent::input(), so it should be added there also if tests are successful.

[XenuIsWatching]

For the rising edge, I would imagine that the ideal case would be for to retain the information in the info panel (another option would be to clear it, but I would lean against this from a ux point of view because its so quick).

For the held state, it clears the info panel which makes sense.

Only for the falling edge, I would expect it to perform the 'expensive' operation of reading the file system for screenshots, marquees, and etc.
But it doesn't really make sense for it to be calling onCursorChanged() twice here.

Currently the onCursorChanged() function just always calls the updateInfoPanel() no matter the CursorState called to it with. There is a check within updateInfoPanel() which checks to see if it is in the "Press and Hold" state and will blank the info panel through the function isScrolling().

1. IList::stopScrolling() will call listInput(0) update the infopanel (which will blank it), but then after that it will reset all the scrolling values to 0. Then will call onCursorChanged() again, and will update the infopanel with screenshots, videos, etc because all the scrolling values have just been resetted. Removing onCursorChanged() within IList::stopScrolling() will just leave the info panel blank in the Press and Held State. It might make sense to remove the call with in listInput(0) instead.
2. [Still need to look into]

[XenuIsWatching]

I have started experimenting on my branch

https://github.com/XenuIsWatching/EmulationStation/tree/optimize_game_list