render by points error on M1 Mac Metal

[Xayah-Hina]

Describe the bug example-01-cubes cannot render primitives by points (BGFX_STATE_PT_POINTS) on M1 mac, metal

To Reproduce

1. Run example-01-cubes on M1 mac, metal
2. switch Primitive Topology Combo to Points

Screenshots

[swilson007]

I've run into this as well. I know the general solution, but I'm not sure where it would go as a fix within bgfx. Basically on Metal the point size needs to be a vertex shader output. From Apple: The vertex shader must provide [[point_size]], or the point size is undefined. https://developer.apple.com/documentation/metal/mtlprimitivetype/point

I was able to hack into the compiled metal vertex shader for cubes and "fix" the issue by adding two lines, thus I directly modified bgfx/examples/runtime/shaders/metal/vs_cubes.bin and then ran the demo.

Here's the shader code change. Two lines were added, note the ADDED comments

using namespace metal;

struct _Global
{
    float4x4 u_modelViewProj;
};

struct xlatMtlMain_out
{
    float4 _entryPointOutput_v_color0 [[user(locn0)]];
    float4 gl_Position [[position]];
    float pointSize [[point_size]]; // ADDED
};

struct xlatMtlMain_in
{
    float4 a_color0 [[attribute(0)]];
    float3 a_position [[attribute(1)]];
};

vertex xlatMtlMain_out xlatMtlMain(xlatMtlMain_in in [[stage_in]], constant _Global& _mtl_u [[buffer(0)]])
{
    xlatMtlMain_out out = {};
    out.gl_Position = _mtl_u.u_modelViewProj * float4(in.a_position, 1.0);
    out._entryPointOutput_v_color0 = in.a_color0;
    out.pointSize = 1;                  // ADDED
    return out;
}
...

I don't know for certain why Intel MacOS doesn't show this problem but ARM does. On Intel Metal it seems that this value defaults to 1 even when it's not set, but on ARM Metal its just a garbage value. So maybe just some type of defaulting in the Metal driver.

Here's the screenshot on my M1 Mac with my shader hack:

[bkaradzic]

Yeah, this is actually more complicated issue to fix. Since requires runtime shader patching depending on rasterizer state. In all other renderers point size defaults to 1 (even when point size is not supported) and point rendering can be turned on, but in Metal it appears that this requires explicit shader change in order to turn on point rendering.

[swilson007]

Note that all the topology modes in the demo work properly with that same shader hack enabled, so in the case where it's not rasterizing points it seems to just be ignored. A solution that always adds the point size to the out variable in the shader might be acceptable. And if there was some way to enable/disable that support for each shader even better. Like it defaults to disabled but you can add a shader equivalent of #pragma enable point-size to your shader code. Anyway... just a thought.

[bkaradzic]

A solution that always adds the point size to the out variable in the shader might be acceptable.

Ah that's good to know. Check spirv-cross code, since that's what outputs Metal shaders.

[swilson007]

I looked into this a little bit. Unfortunately I'm very new to bgfx and fairly new to shaders so my background knowledge is limited. It looks like there's support in spirv-cross already for point-size, as it has a BuiltInPointSize type that appears to output the right things for a metal shader.

There's also a facility for this in glsl, and that's a predefined float named gl_PointSize. https://registry.khronos.org/OpenGL-Refpages/gl4/html/gl_PointSize.xhtml. But that builtin doesn't look to be supported by shaderc. If I add something like

void main()
{
    gl_Position = mul(u_modelViewProj, vec4(a_position, 1.0) );
    gl_PointSize = 1;
}

to my vertex shader and run it through shaderc I get this error: 'gl_PointSize' : unknown variable.

It looks like HLSL has a PSIZE facility for point size but it was deprecated/ignored/removed (I'm not sure which) in DX11. There's a thread here talking about it https://github.com/KhronosGroup/glslang/issues/1154.

So yea, it seems like the challenge here is getting the gl_PointSize variable to be handled by shaderc such that it makes it into the spirv-cross internals properly, but it's unclear (to me) how that would be accomplished given the HLSL intermediate step.

My own use of point drawing has just been for occasional debugging, so I can't really afford to spend more time on it given that it's not an actual feature I'll use in production. Maybe we can get Apple to just change their API to specify that point-size defaults to 1 instead of being undefined. I'll get on the phone to Tim Cook right away :)

[bkaradzic]

I looked into this a little bit. Unfortunately I'm very new to bgfx and fairly new to shaders so my background knowledge is limited.

This is actually excellent way to get familiar with all of that... You already figured out all requirements to fix for problem, now you need to figure out how to do it properly.

So yea, it seems like the challenge here is getting the gl_PointSize variable to be handled by shaderc such that it makes it into the spirv-cross internals properly, but it's unclear (to me) how that would be accomplished given the HLSL intermediate step.

Check shaderc source. It already does insertion/patching shaders before passing it them to spirv-cross. Maybe you could insert gl_PointSize = 1; into every vertex shader, at the end of main function?