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

Fix insane transform allocations in new leaderboard display #29228

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Aug 1, 2024

Noticed on the live build. On master, there was enough going on here to cause severe stutters when scrolling the new leaderboard display (GC related).

osu.2024-08-01.at.07.39.59.mp4

I've modified the logic to much more correctly only perform any expensive operations when a change needs to be made. This not only improves performance butt also means the transforms aren't re-applied multiple times (causing delayed animations).

No need to include profiling results, video should give enough context.

Copy link
Member

@Joehuu Joehuu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to behave visually the same.

rankLabelOverlay.FadeIn(transition_duration, Easing.OutQuint);
else
rankLabelOverlay.FadeOut(transition_duration, Easing.OutQuint);
}

protected override bool OnInvalidate(Invalidation invalidation, InvalidationSource source)
Copy link
Contributor

@smoogipoo smoogipoo Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future reference, OnInvalidate should not reference any drawable values (like DrawWidth) anyway. I can't remember the exact reason for this but it was something along the lines of stack overflow due to recursive validation+invalidation, or stale values.

Edit: It did schedule it which would make things more safe, I misread it.

@smoogipoo smoogipoo merged commit 8a10d63 into ppy:master Aug 2, 2024
11 of 13 checks passed
@peppy peppy deleted the fix-leaderboard-v2-insane-allocs branch August 7, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants