Skip to content
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

Geometry shader support #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gleblebedev
Copy link
Contributor

It seems to be ok for everything except HLSL due to lack of HLSL support in SPIRV-CROSS. I hope you won't consider it as a blocking issue as it is still useful for other platforms.

@mellinoe
Copy link
Collaborator

What does this actually output in the MSL backend? Metal doesn't support geometry shaders at all, so I don't know what the compilation result would or should be. Is it a compute shader of some sort?

Since HLSL is also apparently not supported, I'm a little hesitant to support this. It will only work on a very limited set of targets, as far as I can tell.

@gleblebedev
Copy link
Contributor Author

Let me check the MSL output. I'll post it here.

I hope that khronos group will finish hlsl support one day. It's on the list of things to do at least.

@gleblebedev
Copy link
Contributor Author

#include <metal_stdlib>
#include <simd/simd.h>

using namespace metal;

struct main0_out
{
float4 gl_Position;
};

unknown main0_out main0()
{
main0_out out = {};
for (int _41 = 0; _41 < 3; )
{
out.gl_Position = _31[_41].out.gl_Position;
EmitVertex();
_41++;
continue;
}
EndPrimitive();
return out;
}

@gleblebedev
Copy link
Contributor Author

My point is that since veldrid.spirv is build on top of spirv cross it should provide all spirv cross functionality. The user should be aware of limitations but he should not be deprived from abilities.

@mellinoe
Copy link
Collaborator

Unfortunately, I don't see how that Metal shader will actually compile, so it seems like that is just placeholder output for something that isn't really supported. Combine that with the fact that it doesn't work for HLSL, and this effectively only works on Vulkan and OpenGL.

My point is that since veldrid.spirv is build on top of spirv cross it should provide all spirv cross functionality. The user should be aware of limitations but he should not be deprived from abilities.

I think of Veldrid.SPIRV as the library that hooks up Veldrid and SPIRV-Cross in the most meaningful and portable (e.g. across graphics API's) way possible. This is why some of the public APIs look the way they do. For example, the CompileVertexFragment function is fused because it allows the compiler to generate code and standardized resource identities that all of Veldrid's backends understand.

I would be more comfortable with getting this merged if:

  • The function was changed to a combined "CompileVertexGeometryFragment" function, or there was some explanation for why that wasn't needed (e.g. how resource identities are correct and usable through Veldrid)
  • SPIRV-Cross supported HLSL compilation of geometry shaders upstream. Is there a tracking issue for this that I could follow?

@mellinoe
Copy link
Collaborator

I should also make another important point: I'd rather encourage people to use solutions that are more portable and future-facing, like compute shaders, instead of adding in support for things like this that aren't as well-supported. Geometry shaders are optional in Vulkan, they will probably never be supported with Metal, and I wouldn't be surprised if future web graphics API's leave them out as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants