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

Vessels with a large number of crew are slow (presumably animation/portrait updating) #84

Closed
JonnyOThan opened this issue Aug 12, 2022 · 3 comments
Labels
kspPerformance Possible performance improvement in KSP

Comments

@JonnyOThan
Copy link
Contributor

JonnyOThan commented Aug 12, 2022

Saw a report from discord that performance tanks for vessels with 190 kerbals, but it's fine in map mode or without crew. This hints that it's caused by animations or something to do with the portrait system. It should be possible to limit that cost to just the kerbals that are going to be drawn.

what i noticed is that during uncrewed test flights the performance had absolutely no problems,but with the crew of 190 in it the performance is terrible,what i think is going on is that when not in map view it loads all the kerbals and their constant movements,killing performance

@gotmachine gotmachine added the kspPerformance Possible performance improvement in KSP label Aug 13, 2022
@gotmachine
Copy link
Contributor

gotmachine commented Aug 13, 2022

Yeah, the portrait related lag has been an issue since quite some time.

I gave a look some time ago, the issue is somewhere in the component and coroutine spaghetti portrait handling mess causing inactive portraits to be updated when they aren't shown under certain circumstances, but I never got to the bottom of it.

image

@gotmachine
Copy link
Contributor

gotmachine commented Apr 30, 2023

This is fighting me real hard, the thing is probably one of the worst piece of spaghetti code KSP has...

The issue seems to be that KerbalPortrait.OnEnable() is forcefully enabling all portraits, even hidden ones.
KerbalPortrait is a component living down in the flight UI hierarchy, as an indirect child of the PortraitViewport GameObject, which gets de-activated when switching to map view (and probably in other cases like IVA mode), then re-activated when getting back to flight view.

Activating it then trigger KerbalPortrait.OnEnable(), forcefully enabling all portraits, starting the coroutine handling kerbals animations and portrait renders to a rendertexture, regardless of if the portrait is shown.

There is a separate callback handling activation/deactivation of the portraits that aren't shown when cycling (RectContainment_OnContainmentChanged). The RectContainmentDetector thing is using a pooling approach checking if the portrait rect is visible or not and fire that event, and it maintain some state, but trying to use said state to prevent portrait activation from KerbalPortrait.OnEnable() seemingly has the side effect of preventing correct initialization of the some other stuff, I guess because it prevent the initial OnEnable() call when the component is instantiated, resulting in inactive portraits never becoming activated (or rather, getting stuck in "noise mode") when adding additional slots (but cycling them somehow fix it, I guess because that trigger the OnContainmentChanged callback).

I will have to dig further, maybe there is a simpler approach to fix this...

Edit : actually the issue of portraits getting stuck in "noise mode" seems to happen in stock too, but it's hard to reproduce consistently...

gotmachine added a commit that referenced this issue Apr 30, 2023
…- 1.12.5] Prevent non-visible crew portraits from being rendered after a switch back from the map view (and other cases), causing a significant perf hit when there are many kerbals in the vessel.
@gotmachine
Copy link
Contributor

Fix implemented in 1.28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kspPerformance Possible performance improvement in KSP
Development

No branches or pull requests

2 participants