feat: Mesh API for index and vertex buffer reading #2858
Conversation
…) to transpose a buffer into a different definition
|
I'm done, added a helper to write to the buffer directly regardless of the data type it is made up of; Model.Meshes[0].Draw.VertexBuffers[0].AsReadable(Services, out VertexBufferHelper helper, out int count);
// Write to colors if that semantic already exist in the buffer, otherwise returns false
helper.Write<ColorSemantic, Vector4, MultColor>(new MultColor(){ Color = Color.Gray });
// Upload changes to the GPU
Model.Meshes[0].Draw.VertexBuffers[0].Buffer.Recreate(helper.DataOuter);
private struct MultColor : VertexBufferHelper.IWriter<Vector4>
{
public Color Color;
public unsafe void Write<TConversion, TSource>(byte* sourcePointer, int elementCount, int stride)
where TConversion : IConversion<TSource, Vector4>, IConversion<Vector4, TSource>
where TSource : unmanaged
{
for (byte* end = sourcePointer + elementCount * stride; sourcePointer < end; sourcePointer += stride)
{
TConversion.Convert(*(TSource*)sourcePointer, out var val);
val *= (Vector4)Color;
TConversion.Convert(val, out *(TSource*)sourcePointer);
}
}
}if someone could take a look at this one it would be great. |
| public struct UShort4(ushort x, ushort y, ushort z, ushort w) | ||
| { | ||
| public ushort X = x, Y = y, Z = z, W = w; | ||
| } | ||
|
|
||
| public struct Byte4(byte x, byte y, byte z, byte w) | ||
| { | ||
| public byte X = x, Y = y, Z = z, W = w; | ||
| } |
There was a problem hiding this comment.
Maybe these special vector types are better in another namespace.
I don't think the new Stride.Graphics.Semantic is appropriate, as these types can be used for data packing apart from shader/vertex semantics.
The Stride.Mathematics would be the first option, but as they are not math types (like vectors) I've been thinking if it would make sense to have a namespace for these types of packed vectors, like it existed in XNA (and now MonoGame). Look here for an example.
There was a problem hiding this comment.
That's a very good idea, do you think I should mirror the pixel format naming scheme for those types then ?
There was a problem hiding this comment.
That's an idea I had for a future PR, but if we start it here the better. I (or anyone that wants to contribute) can add the remaining packed vector types that make sense later.
But for now, I think for simple types without much packing restrictions, Byte4 is more expressive in what it stores than Rgba32 or whatever it is called in MonoGame.
Do you think it would make sense to have a Byte4 and later a R8G8B8A8 and have easy conversions between the two? Or better deciding on a single name and documenting it or letting the user discover the type she needs?
There was a problem hiding this comment.
I can see having those two type living in the same namespace be a bit awkward - Maybe we could introduce R8G8B8A8 inside some new Stride.Graphics.Format namespace and have its analogous type, Byte4, inside Stride.Core.Mathematics regardless - we do have some weirder types like Size3 after all.
I'm saying this but I'm definitely not convinced of this suggestion either though, I would actually rather you tell me what you would rather have and I'll just adopt it as I don't think I ultimately have a strong grasp on if any of our propositions would be preferable from a graphics programmer's perspective.
There was a problem hiding this comment.
I'd like to separate the mathematical concepts (vectors, matrices, planes, etc.) from the data type / memory concepts. Here, Size3 would be a mathematical concept (a vector of three dimensions specifying a size. The weird thing is that it consists of just integers, so it is a size for arrays, matrices, textures, etc.), while I see Byte4 as a data/memory type (a type consisting of 4 bytes, with a very specific size, padding, etc.)
For reference, that was also what XNA (and now MonoGame) did, mathematical types in the base namespace, and the "packed vectors" were just special types for when you need to operate on the data of textures, vertex buffers, etc. That's why they were in a separate namespace.
Taking this into account, I vote to have them in a separate namespace. First, so it not polutes the Intellisense with weirdly named types, and second, so people that uses those types know that they are importing them from a specialized namespace, for specific uses.
There was a problem hiding this comment.
Oh, and another thing:
The Stride.Core.Mathematics package can be used independently, and it should contain only math-related things.
And I see some of those packed vector types as more graphics-interop-related (specially the ones mirroring pixel formats), so another reason to have them into its own namespace nested in Stride.Graphics.
There was a problem hiding this comment.
Done, moved them to Stride.Graphics.Interop, let me know what you think
Ethereal77
left a comment
Ethereal77
left a comment
There was a problem hiding this comment.
Impressive! This has the potential to ease the management of indices and vertices by a lot. Nice job! 👏
Left some comments to discuss.
|
Sorry for the delay. I'm quite busy lately. I like what you are trying to do here, Eideren. I feel like it is abusing the type system a bit however 😁. I know Norbo does similar things in Bepu. I have some comments:
As an aside, what do you use to check the JIT assembly (apart from SharpLab)? The snippet of the comment above fails because |
Right now I'm just trying to cover the most common types, UNorm4Byte is what assimp spits out when your model comes with vertex color from what I can tell.
That would be nice but the issue is that they aren't interpreted in the same way, a byte4 is clearly a integer type, whereas a UNormByte is treated like a floating point number. It's kind of a more compressed half. The user should not have to deal with that since we're automatically converting the type for them behind the scene for all the other kinds of types.
This was supposed to reflect how stride interprets the Color semantic; stride/sources/engine/Stride.Graphics/VertexElement.cs Lines 491 to 530 in 4903592 But looks like I confused Color4 with Color, I'll correct this one tomorrow.
You can use the native dotnet one or Disasmo if you're lazy, both of them require a fairly straightforward solution, which stride's isn't, so your best bet is to create a temporary solution with the logic you want to check out. |
|
Actually, godbolt seems like it has improved since the last time I tried it. If I force inline the generic call into the caller it does significantly improve the codegen. If you're not against the example I've shown above I think I can make it work. |
But it is a 32-bit type with 4 bytes. It's just the way you can interpret those values as unsigned and normalized, not the internal representation. Look at how we do in
For a first version I think that's sensible. Even though it would be nice to use this for all kind of packed values (pixels, vertices, etc.), I would prefer to expand it later if needed than making a convoluted mess now just to support everything and the kitchen sink 😀
In your example, I assume I ask because I think I'm not getting why you need to have both Sorry for being a pain 😁, but can you post a simple snippet of how you would read, for example, a color semantic of type I don't want to slow you with too many questions, as this seems to be a pretty interesting concept to me. What about doing some kind of prototype where we can discuss and iterate? |
|
Welp, looked into it more thoroughly and the jit still isn't quite there yet I'm afraid, conversion from large to smaller data types has too much added overhead compared to manual conversion Here's were I'll leave things off for now https://github.com/Eideren/VectorConversion
That datatype was replaced with |
I'm not really opposed to having data types that represent UNorm values. Just that, as it is a concept separated from the memory layout and more related to how you operate on the data, it can lead to an explosion of types. But I really don't know what would be preferable. Having a few UNorm types for when data is always going to be UNorm does not seem so bad to me. My initial fear was that having |
|
Makes sense, thankfully this became a non-issue once Color got involved, we can figure it out once there is a need to cover more datatypes. |
Eideren
changed the title
…d#2858 Reverts the method to what it was before stride3d#2858. This fix is not necessary on main branch where it was already applied by 8e791ad. However that fix is not applicable on 4.2 branch because it needs .net10.
2 participants
PR Details
Adding in an API to read mesh data efficiently.
Why
A vertex buffer, like its name implies, contain the vertices of a mesh.
Vertex buffers do not have a standardized layout, each mesh may have its own layout and data type it uses for their vertices. Some have blend weights, or tangents, while others only have positions - they may also use different data types, for example Half4 positions, 4byte color ...
All of those permutations and subtleties make reading a mesh's vertex buffer tedious and error prone.
Index buffers are significantly easier, but still easy to mess up given that they may be defined as a 16bit or 32bit indices and that buffers may or may not use the whole data - they have offset and lengths.
There's also the lack of documentation around the fact that vertex buffers are not kept in cpu memory, they are uploaded as soon as they are deserialized. Reading it requires loading it from disk or the gpu, both of which are definitely not straightforward.
Example
Quick example for the most common method:
The semantic generic used there informs the API as to what it should read from the vertices, but it also helps in preventing user error; they enforce the use of the right type for a given semantic -
TextureCoordinateSemanticonly acceptsVector2as the span type, providing anything else would show a syntax error.There is also a
Read<>method which provides user with a direct access to the buffer and all the tools they may need to deal with the issues outlined above. When they have other needs than just a dumb copy.There are other examples baked into the documentation for the API.
Related Issue
None
Types of changes
Checklist