Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
β¨ What's this?
I started with wanting to support integer vertex attributes.
Doubles looked supported but were broken because of the wrong GL method call and because of a copy-paste error.
Refamiliarising myself with the relevant GL methods, I also realised that integer normalisation works differently than I thought, and decided that it should default to not normalise anything - except Color, which is specifically intended to be normalised.
I also believed there was confusion about how byte sizes were calculated - though then realised it was working correctly after all and undid the changes, replacing the previous dictionary with a switch expression due to likely compiler optimisations.
Caveat
This latter point highlights a bigger issue, which is that the vertex attribute stride is currently calculated as the sum of vertex attribute sizes, which is not necessarily correct and depends on the struct layout and packing settings. Instead, it would be more correct to inspect the structs with reflection and other methods to get the actual byte offsets of fields annotated with attributes for names and format settings and alignment in arrays. I would also like to have an attribute that forces a check, perhaps at compile time or with an analyser, or if needed at runtime, to see if vertices are optimally laid out and aligned, so that they waste no space, no performance due to bad alignment inside arrays, and can be copied to GPU memory AS IS, instead of requiring expensive transformation of the data.
However, none of that is inside the scope of this PR.
π Why do we want this?
π₯ Breaking changes
IVertexData and VertexData public interfaces have changed, requiring all vertices to be updated, though most of them only require making the interface method static, and changing its return type - source compatibility is otherwise largely maintained, as long as the previous optional normalisation parameter was not specified.
The default for all integer attributes is now not to be normalised, whereas before signed and unsigned bytes were.
π¬ Why not another way?
Could have done it without breaking changes, but it would have made interfaces less clear and perhaps even ambiguous.
While the above caveat remains the same, at least this new version is less and clearer code, easier to use, etc. and should not require any more changes until we perhaps redo it all again as indicated in the caveat.