Change defaults of SpriteBatch to use VBO instead of VertexArray on gl2/gles2.0 #7486
Conversation
…t fallback for gl2. Use VBO instead. Expose index object from mesh so we can pre-upload indices once for SpriteBatch Changes to SpriteBatchPerformanceTest to be able to easily test each vertex data type
|
Since I'm the jerk who submitted the issue. I pulled down the PR, built it locally, deployed and tested it out. Fixes the issue as initially reported. |
| /** Used to completely override the vertex type used by SpriteBatch. This is useful for picking a specific vertex data type on | ||
| * construction of the sprite batch. Recommended to reset this back to defaultVertexDataType Once the batch has been created | ||
| * with this flag */ | ||
| @Deprecated public static VertexDataType overrideVertexType = null; |
There was a problem hiding this comment.
OK, another Deprecated field; I can sorta see why this exists now. Is there a plan to eventually remove these public deprecated fields?
There was a problem hiding this comment.
These are just for warnings mostly, as you should not touch these without understanding the consequences. There is no safe handling of what is compatible, no errors etc.
Probably would be better to have extra SpriteBatch constructors to create with specific VertexDataTypes, and then handle all incompatibilities there and give sensible warnings and exceptions when someone tries to create with something invalid.
Something that can be addressed in a separate PR as wanted this to be as noninvasive as possible.
tommyettinger
left a comment
tommyettinger
left a comment
There was a problem hiding this comment.
I'm not totally clear on how this is acting differently (and better than before) on GWT than it does on other platforms, but I'm glad it does. I would eventually like to know what the plan is for deprecation in libGDX in general -- do we intend to ever remove these deprecated fields?
|
These deprecated fields can be removed with something like described in comments. Mostly they are used everywhere as warnings of not to use things. Its not often someone will be checking the javadocs for every method/field to see if its experimental or potentially a bit dangerous, but deprecated annotations make these very obvious. As for the difference, Gwts implementation of VertexArray wasn't the same as other platforms. Which is why that bug cropped up. Its not actually a client side vertex array. But should be working better now, because it no longer uses 'vertex arrays', and uses the vbo by default on gl2. With benefits from previous commits to optimize not updating indices, and in general a bit more of a performant approach by having these buffers mostly gpu side, we get a little boost too. |
obigu
left a comment
obigu
left a comment
There was a problem hiding this comment.
Tested on real projects on Android and iOS looks good. Also have run the SpriteBatchPerformanceTest on an Android 8 gl2 and results are Vertex Array 1.2s and VBO 0.65s
Regression in gwt due to VertexArray/IndexArray not being implemented the same on all platforms.
Should fix #7480
After tests, I havent seen an issue in just moving from client side buffers (VA), to VBOs as the new default.
For all other buffer types we can still pre-upload indices, and save us some nice time during rendering.
Test results for lwjgl, lwjlg3, android, ios and gwt below.
Times here are all just relative to each other. Lower the better
#7480
bfe255a