Skip to content

switch from GL_TRIANGLE_FAN to GL_TRIANGLE_STRIP #538

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.

Sign up for GitHub

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

Merged
nschimme merged 1 commit into from
Jun 5, 2026
+17 −4
Merged

switch from GL_TRIANGLE_FAN to GL_TRIANGLE_STRIP #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/opengl/legacy/SimpleMesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
void Legacy::drawRoomQuad(Functions &gl, const GLsizei numInstances)
{
// The shader uses gl_VertexID to generate quad vertices [0..3]
gl.glDrawArraysInstanced(GL_TRIANGLE_FAN, 0, 4, numInstances);
gl.glDrawArraysInstanced(GL_TRIANGLE_STRIP, 0, 4, numInstances);
}
19 changes: 16 additions & 3 deletions src/resources/shaders/legacy/room/tex/acolor/vert.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,22 @@ out vec3 vTexCoord;

void main()
{
// ccw-order assumes it's a triangle fan (as opposed to a triangle strip)
const ivec3[4] ioffsets_ccw = ivec3[4](ivec3(0, 0, 0), ivec3(1, 0, 0), ivec3(1, 1, 0), ivec3(0, 1, 0));
ivec3 ioffset = ioffsets_ccw[gl_VertexID];
#if 0
// GL_TRIANGLE_FAN
// 3--2
// | /| Triangles 012 and 023 both use CCW order.
// |/ | Notice that the four vertices are in CCW order.
// 0--1
const ivec3[4] ioffsets = ivec3[4](ivec3(0, 0, 0), ivec3(1, 0, 0), ivec3(1, 1, 0), ivec3(0, 1, 0)); // fan
#else
// GL_TRIANGLE_STRIP
// 2--3 Note: Triangle strips alternate CCW/CW winding on every other triangle. This means...
// |\ | triangle 012 is CCW order, but triangle 123 is CW order (backwards).
// | \| Keep in mind that OpenGL does not actually draw the triangle backwards,
// 0--1 so it does not affect glFrontFace() or the gl_FrontFacing variable.
Comment on lines +28 to +29

@sourcery-ai sourcery-ai Bot Jun 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: Clarify the impact of alternating winding on glFrontFace/gl_FrontFacing.

The note about triangle strips not affecting glFrontFace() / gl_FrontFacing is inaccurate. Alternating winding in strips does change which triangles are classified as front-facing/back-facing under the current glFrontFace setting, which in turn impacts gl_FrontFacing and face culling. Please reword this to avoid misleading anyone relying on those behaviors.

const ivec3[4] ioffsets = ivec3[4](ivec3(0, 0, 0), ivec3(1, 0, 0), ivec3(0, 1, 0), ivec3(1, 1, 0)); // strip
#endif
ivec3 ioffset = ioffsets[gl_VertexID];

int texZ = aVertTexCol.w & 0xFF;
int colorId = (aVertTexCol.w >> 8) % MAX_NAMED_COLORS;
Expand Down
Loading