Daemon icon indicating copy to clipboard operation
Daemon copied to clipboard

'Generic quad' is mistakenly added into other draw calls

Open slipher opened this issue 1 year ago • 2 comments

I have found that the generic_VBO vertices are sometimes rendered with other shaders than the ones it is intended to be used with. Out of curiosity, I checked to see if this would have any visible consequences. I tested with the following patch:

diff --git a/src/engine/renderer/tr_backend.cpp b/src/engine/renderer/tr_backend.cpp
index 92ede239f..0d774768b 100644
--- a/src/engine/renderer/tr_backend.cpp
+++ b/src/engine/renderer/tr_backend.cpp
@@ -720,6 +720,11 @@ void GL_VertexAttribPointers( uint32_t attribBits )
 				frame = glState.vertexAttribsOldFrame;
 			}
 
+			if ( !(glState.currentVBO->attribBits & bit ) )
+			{
+				Log::Notice("%s lacking in %s", attributeNames[ i ], glState.currentVBO->name);
+			}
+
 			glVertexAttribPointer( i, layout->numComponents, layout->componentType, layout->normalize, layout->stride, BUFFER_OFFSET( layout->ofs + ( frame * layout->frameOffset + base ) ) );
 			glState.vertexAttribPointersSet |= bit;
 		}
diff --git a/src/engine/renderer/tr_vbo.cpp b/src/engine/renderer/tr_vbo.cpp
index accdc89d1..4cfeef779 100644
--- a/src/engine/renderer/tr_vbo.cpp
+++ b/src/engine/renderer/tr_vbo.cpp
@@ -906,9 +906,12 @@ static void R_InitGenericVBOs() {
 	VectorCopy( v3, verts[3].xyz );
 	
 	for ( int i = 0; i < 4; i++ ) {
-		verts[i].color = Color::White;
+		verts[i].color = Color::Magenta;
 		verts[i].texCoords[0] = floatToHalf( i < 2 ? 0.0f : 1.0f );
 		verts[i].texCoords[1] = floatToHalf( i > 0 && i < 3 ? 1.0f : 0.0f );
+
+		vec4_t orent{1, 0, 0, 20};
+		floatToHalf(orent, verts[i].spriteOrientation);
 	}
 	surface->vbo = R_CreateStaticVBO2( "generic_VBO", surface->numVerts, verts, ATTR_POSITION | ATTR_TEXCOORD | ATTR_COLOR );
 
  • The color is changed to magenta to make it identifiable as this and not some other bug
  • Something is set for qtangents/spriteOrientation to avoid randomness from uninitialized memory
  • The log message shows whether the bug is active

Also I used the commands /cg_motionblurMinSpeed -1; cg_flyspeed 30; setviewpos 0.890782 -0.764073 1.764857 101.173088 57.002563. Depending on the map, r_drawWorld 0 or noclip may be needed. If you don't see the message attr_QTangent lacking in generic_VBO, the bug is not active and you may have to restart or change map a few times.

unvanquished_2024-06-19_160809_000

Voila, a little font sprite sheet is drawn at the origin. Sometimes I also got a base beacon or a solid rectangle.

I think I can fix this by adding back some of the cleanup code to Tess_InstantQuad which was removed when it was changed to use a VBO. Like tess.multiDrawPrimitives = 0; tess.numVertexes = 0; etc.

slipher avatar Jun 19 '24 21:06 slipher

I think I can fix this by adding back some of the cleanup code to Tess_InstantQuad which was removed when it was changed to use a VBO. Like tess.multiDrawPrimitives = 0; tess.numVertexes = 0; etc.

Those are still present though?

Also, attr_QTangent wasn't used by the old code either, so the bug might've already been there.

VReaperV avatar Jun 20 '24 09:06 VReaperV

I think I can fix this by adding back some of the cleanup code to Tess_InstantQuad which was removed when it was changed to use a VBO. Like tess.multiDrawPrimitives = 0; tess.numVertexes = 0; etc.

Those are still present though?

Nope. 99f79719 removes the buffer cleanup code from Tess_InstantQuad since it no longer calls Tess_DrawElements, which is because the MVP matrix wasn't set for the shader yet. The DrawElements calls were moved to the InstantQuad callsites. But the callsites don't do the cleanup stuff.

I have a working fix based on having Tess_InstantQuad set the MVP shader uniform itself. This is done by passing a reference of type u_ModelViewProjectionMatrix. Then the function can queue up the verts, set the MVP, draw, and clean up all in one.

slipher avatar Jun 20 '24 16:06 slipher