bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.15k stars 1.95k forks source link

Issues in Metal renderer with gl_vertexID based draw calls #3007

Open EvilTrev opened 1 year ago

EvilTrev commented 1 year ago

Describe the bug

Metal renderer suffers from issues when using gl_VertexID based draw calls with no index or vertex buffers. Part of the issue appears to be caused by draw.m_streamMask which contains the value 0xFF in such calls and logic to loop the stream that exceeds BGFX_CONFIG_MAX_VERTEX_STREAMS. This appears to cause writing to currentState.m_stream[idx] that is outside the range and corrupts memory. However, there appears to be more going wrong here as I'm getting erratic validation errors caused by trying to bind nil vertex buffers to metal with vertex offsets greater than zero. In short, it doesn't seem to play nice with gl_VertexID only draw calls.

To Reproduce

Steps to reproduce the behavior:

  1. Create a simple app that only renders a fullscreen triangle using only gl_VertexID and no vertex or index buffers. Launch a few times with validation enabled.
// Procedural fullscreen coverage using gl_VertexID
bgfx::setState( BGFX_STATE_WRITE_RGB | BGFX_STATE_WRITE_A );
bgfx::setVertexCount( 3 );
bgfx::submit( 0, m_fullscreenProgram );
void main()
{
    // UV from vertexID
    int vid = int( gl_VertexID );
    float u = ( vid == 0 || vid == 3 ) ? -1.0 : 1.0;
    float v = ( vid < 2 ) ? 1.0 : -1.0;
    vec2 uv = vec2( u, v );

    // Position from UV
    gl_Position = vec4( uv * 2.0 - 1.0, 0.0, 1.0 );
}
EvilTrev commented 1 year ago

I was able to prevent my crashes and validation issues with these two defensive changes in renderer_mtl.mm, however it should probably be addressed in some other way:

{
    const uint32_t ntz = bx::uint32_cnttz(streamMask);
    streamMask >>= ntz;
    idx         += ntz;

    // HACK! Fix out of bounds writing
    if ( idx >= BGFX_CONFIG_MAX_VERTEX_STREAMS ) break;

    .........

    // HACK! Enforce nil buffer and zero offset in draw calls that don't have buffers
    rce.setVertexBuffer( draw.m_streamMask != 0xFF ? vb.m_ptr : nil, draw.m_streamMask != 0xFF ? offset : 0x00, idx+1);
}