mellinoe / veldrid-samples

Sample projects for Veldrid
https://mellinoe.github.io/veldrid-docs/
121 stars 49 forks source link

VertexElementSemantic usage in the getting started #14

Open xposure opened 5 years ago

xposure commented 5 years ago

Can you shed some light on the VertexElementSemantic usage? In the getting started, both position and color are set as TextureCoordinate, which works. When I try to change it to Position and Color respectively, it crashes with

Message: The parameter is incorrect.

In what case would you use Position and Color then?

mellinoe commented 5 years ago

VertexElementSemantic is only meaningful when using the Direct3D11 backend and directly providing HLSL shader code. In that case, you need to make sure the semantics match up with what is defined in the shader. When using Veldrid.SPIRV (as the Getting Started example does), you should just pass TextureCoordinate for all of the semantics, because that is what the cross-compiler generates for all vertex attributes.

xposure commented 5 years ago

Could you explain how TextureCoordinate doesn't throw an exception though? I'm trying to understand the significance of TextureCoordinate to SPRIV as opposed to the correct values.

I was thinking either... A) Add a value to the semantic for SPRIV B) Allow correct semantic usage instead of TextureCoordinate so that the same code path works with HLSL/SPRIV.

mellinoe commented 5 years ago

SPIRV-Cross generating "TEXCOORD" semantics is just an arbitrary decision. When it outputs HLSL, it has to choose some semantic name for each attribute, and TEXCOORD is a generic enough string to use. But again, it's just an arbitrary string, there's nothing particularly special about it.

B) Allow correct semantic usage instead of TextureCoordinate so that the same code path works with HLSL/SPRIV.

I'm not sure what you mean by this, exactly. You would like to support hand-written HLSL and HLSL cross-compiled from SPIR-V, but use the same vertex semantics?

xposure commented 5 years ago

But again, it's just an arbitrary string, there's nothing particularly special about it.

Is this documented somewhere? I guess to me its just not intuitive, I just assumed the getting started was a typo and upon fixing it, I was presented with an error without explanation or even a hint of what to fix.

If I were to write this code from scratch I would have spent hours trying to figure out why the semantics were incorrect when in fact they were correct. I'm not specifically looking for supporting both, just trying to limit other people running in to the same issue.

An example of this is if you were using HLSL one day and decided to port over to SPRIV, where would I find this documentation that explains this arbitrary choice?

xposure commented 5 years ago

Also if its arbitrary and doesn't really matter, why does it throw an error when its not TextureCoordinate?

mellinoe commented 5 years ago

@xposure It's documented very briefly in the docs for VertexElementDescription.Semantic and VertexElementSemantic:

When using Veldrid.SPIRV to cross-compile a vertex shader to HLSL, all vertex elements will use VertexElementSemantic.TextureCoordinate.

I agree that more could be done to make this more obvious. Is there anywhere in particular you looked first to find this kind of information?

Also if its arbitrary and doesn't really matter, why does it throw an error when its not TextureCoordinate?

By that, I meant that the particular string is arbitrary and doesn't carry any particular meaning. E.g. just because it is "TEXCOORD" doesn't mean the attribute has anything to do with a texture coordinate. The only part that is important is that the strings match exactly when you construct a Pipeline, and only when using the D3D11 backend (others don't use semantic strings to match attributes).

tzachshabtay commented 5 years ago

Can't Veldrid.SpirV tell Veldrid to override the semantic?

For example, if we'll add to Veldrid.Shader a property like:

    public VertexElementSemantic? OverrideSemantic { get; set; }

Then Veldrid.SpirV can put OverrideSemantic = VertexElementSemantic.TextureCoordinate when it creates the shader object, and Veldrid can use this property when using the semantic.

Then the user can put the more intuitive semantic and it will work for both scenarios.

mellinoe commented 5 years ago

@tzachshabtay That's an interesting option. It could potentially be taken even further and Veldrid.SPIRV could provide "overrides" for the entire VertexLayoutDescription[] array, since it has access to all of that information. Then, a ShaderSetDescription could be constructed from a Shader[] alone, assuming that the vertex Shader has the overrides set (otherwise it would generate an error). I'll experiment with that and see how it feels.