Instancing data not working as expected on Metal/MacOS

[jay072]

I'm not able to use texcoord2 to pass additional per-instance data to a shader. It works fine on Windows/DX11. When looking into the graphics debugger on Xcode I'm seeing that the additional data is appearing as "uchar2" rather than the expected "float4" type.

I've modified the instancing example to demonstrate my issue here. In this modified example, all of the cubes should appear white if the instancing data is corrected passed to the shader. On Mac, these cubes are rendering black.

[bkaradzic]

cc @attilaz

[attilaz]

I think there is a mismatch in attributes.

In cpp you are using bgfx::Attrib::Color0, but in shader you are using a_color1. They should be both 0 or 1. https://github.com/jay072/bgfx/blob/master/examples/05-instancing/instancing.cpp#L25 https://github.com/jay072/bgfx/blob/master/examples/05-instancing/vs_instancing.sc#L21

This creates the uchar2 in the binding, not the instance parameter.

The cubes have the colors defined here https://github.com/jay072/bgfx/blob/master/examples/05-instancing/instancing.cpp#L36 after fixing the mismatch. I am not sure why they should be white.

[jay072]

Hi.

I’m trying to pass the data from line 220 https://github.com/jay072/bgfx/blob/master/examples/05-instancing/instancing.cpp#L220 to the a_color1 attribute. That’s why it should be white ( and this works as expected in windows)

i_data5 would of been a more suiting name but this isn’t accepted by Bgfx.

I'm under the impression that my varying.def.sc means that the a_color1 attribute should definitely be populated with the per-instance data

[attilaz]

@jay072 Ok. I got it now. @bkaradzic But I don't see how this should work. The only thing why a_color1 : TEXCOORD2 bound as an instance parameter is that using the d3d semantics convention of instance attributes. (TEXCOORD7 = i_data0, TEXCOORD6 = i_data1, etc.) But these semantics are non-existant in gl/metal. They are using attribute names i_data0, i_data1 directly.

D3D11 maps instance attributes like this https://github.com/bkaradzic/bgfx/blob/b9ab564c47386b5683a56a1d60ac4c4e3a0761bd/src/renderer_d3d11.cpp#L2635 , and there is no "bounds check" for available instance parameters, so I think there should be check somewhere that only 5 instance parameters is supported right now.

Of course bgfx could support more i_dataX, but I think supporting a_color1 as an instance parameter for metal needs more info like a vertexLayout for instancebuffer.

[jay072]

Is there a particular reason why i_data5 isn't in s_allowedVertexShaderInputs? https://github.com/bkaradzic/bgfx/blob/82f56b59876b756c0b26dac4edf9ba384da23396/tools/shaderc/shaderc.cpp#L179

Would adding i_data5 here be enough?

[attilaz]

@jay072 You should check i_data0 for the whole bgfx. (https://github.com/bkaradzic/bgfx/search?p=2&q=i_data0&unscoped_q=i_data0) It seems like renderer_mtl.mm and renderer_gl.cpp needs change too. Also change BGFX_CONFIG_MAX_INSTANCE_DATA_COUNT which is defined as 5 here https://github.com/bkaradzic/bgfx/blob/82f56b59876b756c0b26dac4edf9ba384da23396/src/config.h#L311

https://github.com/bkaradzic/bgfx/search?q=BGFX_CONFIG_MAX_INSTANCE_DATA_COUNT&unscoped_q=BGFX_CONFIG_MAX_INSTANCE_DATA_COUNT

[bkaradzic]

5 x 16 = 80 bytes of data per instance... Do you really need more than that? In that case use of buffers would be better.

[jay072]

Thanks for the suggestion! Is there any examples of how to do that I can take a look at?

[bkaradzic]

You add BUFFER_RO to your vertex shader and read it directly from it.