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

Issues in Metal renderer with gl_vertexID based draw calls #3007

Open
EvilTrev opened this issue Dec 28, 2022 · 1 comment
Open

Issues in Metal renderer with gl_vertexID based draw calls #3007

EvilTrev opened this issue Dec 28, 2022 · 1 comment
Labels

Comments

@EvilTrev
Copy link

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
Copy link
Author

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);
}

@bkaradzic bkaradzic added the bug label Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants