ppy/osu-framework

`SpriteIcon`s are unnecessary expensive

EVAST9919 opened this issue · 7 comments

Currently SpriteIcon is basically a container with 2 Sprites - one for icon intelf and one for it's shadow, which is an overkill if you have many icons in the scene. This can be relatively easy improved by moving shadows into draw node instead.
Note that making SpriteIcon : Sprite is a no-go because current implementation implies that they are always centered, fit inside the drawable and texture shouldn't be allowed to be set outside. Thus a new custom drawable should be implemented.

I've made a basic implementation and performance gains are evident. (ignore icons aspect ratios for now, didn't want to spend much time in case this is not-needed/not an huge issue)
Master:
icons-master
My implementation:
Снимок экрана (407)

Can you show demonstrable performance gains in real usages, not in the "spam every font awesome icon that exists" test scene?

I'm not necessarily rejecting this but using the absolute worst case to argue that this gives "significant" perf gains seems like a stretch.

Probably bad wording from my side, that's for sure, but I've never said that this will be a game changer or anything.
I understand that since this won't have a huge impact on the game it's automatically low-priority, but if it can be improved - why not?

Again, not rejecting this. Just wanting to find out how real the perf gains are to properly prioritise fixing this.

This is probably the best it's gonna get.

master proposal
song-select-old song-select-new

Performance gain is nice, but why aren't the icons sized the same on both? It's not just proportional upscaling, they're clearly stretched on the second screenshot.

Performance gain is nice, but why aren't the icons sized the same on both? It's not just proportional upscaling, they're clearly stretched on the second screenshot.

As said in the OP

(ignore icons aspect ratios for now, didn't want to spend much time in case this is not-needed/not an huge issue)

FillMode.Fit math isn't there yet

Please don't use screenshots of frame rates at single frames to compare performance. At very least, show a frame graph with full history showing sustained benefits.

Or better, make a test scene and don't include the whole of song select in the background. Or use the framework icon test scene.