Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCMemcardManager : icon drawing takes to much cpu time #8346

Open
wants to merge 2 commits into
base: master
from

Conversation

@DacoTaco
Copy link

commented Sep 4, 2019

as described in the issue tracker, if you load a bit of a bigger memory card that has a larger list of saves ( 39 in my raw ) the redrawing of the icons takes waaay to much cpu time and it semi kills the window.

i fixed this by

  1. skipping unloaded memory card slots.
  2. skipping icons that don't have an animation or don't need updating
  3. only redrawing icons currently being displayed in the table.

this made the window usable for me, and dropped the cpu time from ~30% to ~4%

this will probably conflict with a PR from AdmiralCurtiss (#8304) , and i'll fix it after his stuff is merged

@DacoTaco DacoTaco force-pushed the DacoTaco:master branch 2 times, most recently from 9c54c24 to 1eb637e Sep 4, 2019

GCMemcardManager performance boost
only update the icons that are visable on screen. this makes the window
respond faster to input and reduces its cpu usage by ~50% (at least with
my big memory card dump)

@DacoTaco DacoTaco force-pushed the DacoTaco:master branch from 4f36323 to de4e0a0 Sep 5, 2019

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Is it redrawing only on changing animations or is it still possible to draw the same frame multiple times? If not, I think that's where you could potentially gain the most time.

@DacoTaco

This comment has been minimized.

Copy link
Author

commented Sep 5, 2019

it's redrawing the icons that have animations and are currently being displayed in the table's viewport.
not redrawing single frame icons was the first thing i changed, and it helped, but not much.
only redrawing those in the viewport changed the most with my big memory card.

i noticed that with the PR from AdmiralCurtiss ( 8304 ) and this together cpu time went down to ~7% or something, which is a whole lot better and usable.

@CookiePLMonster

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Sure, but what I mean (without looking at the code in detail) is - is redrawing done only when the icon frame changes, or "always"? It would probably be best to only trigger redraw once frame changes, if it's not done this way already.

@DacoTaco

This comment has been minimized.

Copy link
Author

commented Sep 5, 2019

that would indeed be better, but i think thats a change that could only be done in either AdmiralCurtiss' PR or after his PR is merged (which will conflict with this one anyway) as his PR fixes the icon animation speed and the 3 frame limit the current code has.

either case , now that i think about it its better to do his PR first and then fix this to his code, so i can introduce that then

@DacoTaco

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

Sure, but what I mean (without looking at the code in detail) is - is redrawing done only when the icon frame changes, or "always"? It would probably be best to only trigger redraw once frame changes, if it's not done this way already.

i tried this on the branch this PR is on (so if merged it isn't forgotten to be implemented) and it seems to already reduce cpu time on my system with 14 animated icons on screen.
i didn't expect much change before the whole rework, but its good none the less.

i also did a quick implementation of it after the PR of admiralCurtiss (not in this branch) and seems to work there too

@DacoTaco DacoTaco force-pushed the DacoTaco:master branch from 57a60be to 2dfeed6 Sep 6, 2019

don't redraw icon if frame didn't change
seems to reduce cpu time to ~3%

@DacoTaco DacoTaco force-pushed the DacoTaco:master branch from c101d42 to 85fadb5 Sep 6, 2019

@CookiePLMonster
Copy link
Contributor

left a comment

Not a huge fan of autos for values which are obvious to be integers, but that's not a problem.

@DacoTaco

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

its my habit of almost always using var in C# i guess
ill keep that in mind

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.