-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Index buffer specialization #568
Conversation
Awesome this looks good to me. At some point (not in this pr) I think we should make a decision on "u16 vs u32" as the default (and where that default is applied). Ex: if half of the meshes with a given material use u16 and the other half use u32, will the memory reduction savings outweigh the state change cost (because specialization results in different pipelines for u16 vs u32). would it be cheaper to just use u32 everywhere to allow increased batching/instancing? Should all shapes use u32 for that reason? |
(i dont expect you to come up with all of the answers. im just thinking out loud) |
Yeah so I've been pondering that stuff as well. I can ask around in a games-tech slack I'm in what people do in production these days -- I'm not sure how much benefit there is to using u16 for indices. Seems to me we should definitely make everything in the engine use one type by default to limit state changes between pipelines for sure, and at this point I'm leaning towards u16 standard for all the standard stuff provided by the engine and just having u32 there supported if people want to do things with it. |
That would be much appreciated! Im thinking if theres no significant difference, then we might as well use u32 as the default so things "just work" in basically every case. My main issue with u16 as the default is that when it breaks down, it can be silently / subtly broken. And there will be a "class of thing" where u16 can't (always) be the default (like icospheres). I'd want some protections against the "overflow" case to prevent silent breakage like we saw on discord. |
Alright so basically what I've gathered is that it's possible you could have a small perf impact in situations where you're not bound by pixel shader compute -- i.e. shadow map passes and other simple shaders. But even in that case it would be single digit percentage improvement. It can be helpful for memory savings as well especially on mobile platforms or places where you're very memory starved, but I don't think we're really going to be experiencing that at the moment. So, I'd propose to move forward with U32 as default for the time being. |
Cool that makes sense to me. I like that as its fool-proof in general and we can optimize later if we need to. |
I agree with the u32 as default part, because after all functionally > performance. But it still should be easy to switch back to u16, since asking arround on twitter with this issue revealed that there are indeed performance implications on mobile and u16 still is sorta standard. (see: https://twitter.com/IRCSS/status/1309453536045027328) And just because its very cheap doesnt mean its free, and I'm kinda worried about taking a lot of small perf hits stacking up to "hm that code just slow". |
Agreed @julhe ... it should be fairly easy, at this point changing between the two is a matter of just a few lines of code from saying U16 -> U32 basically, as seen in the PR I just opened/mentioned :D |
index buffer specialization
Allows the use of U32 indices in Mesh index buffers in addition to the usual U16 indices.
Note: I use U32 indices for Icosphere here both as a test but also because there could be a reasonable case where you surpass the U16 space with a highly tesselated icosphere.