[osg-users] Concerns about quads, quad strips, polygons

[Paul Martz]

Hi Robert -- After looking at your recent check in of support for quads, 
quad strips, and polygons in GL ES 1 and 2, I have some performance 
concerns.

Consider this code snippet for DrawArrays::draw()...

void DrawArrays::draw(State& state, bool) const
{
#if defined(OSG_GLES1_AVAILABLE) || defined(OSG_GLES2_AVAILABLE)
     GLenum mode = _mode;
     if (_mode==GL_QUADS)
     {
         state.drawQuads(_first, _count, _numInstances);
         return;
     }
     else if (mode==GL_POLYGON)
     {
         mode = GL_TRIANGLE_FAN;
     }
     else if (mode==GL_QUAD_STRIP)
     {
         mode = GL_TRIANGLE_STRIP;
     }

     if (_numInstances>=1) 
state.glDrawArraysInstanced(mode,_first,_count, _numInstances);
     else glDrawArrays(mode,_first,_count);
#else
     if (_numInstances>=1) 
state.glDrawArraysInstanced(_mode,_first,_count, _numInstances);
     else glDrawArrays(_mode,_first,_count);
#endif
}

Note that if you're not on GL ES 1 or 2, you would take the #else path, 
which would result in the status quo performance. However, if you're on 
GL ES 1 or 2, you now have the cost of a cascading if / else if / else 
if inflicted per PrimitiveSet during draw -- even if you are using a 
supported primitive type such as triangles.

For GL 3, the easy route would be for me to add GL3 to the #if and take 
this same code path, but I don't want to handicap performance on GL 3. 
So I'm hoping you might consider a different approach.

Could we, for example, ignore these unsupported primitive types during 
draw for GL ES 1, 2, and GL3, and require an OSG app running on those 
platforms to use a NodeVisitor to convert their scene graph over to 
supported primitive types? This way, the PrimitiveSet ::draw routines 
could be restored to their previous lean versions, resulting in optimal 
performance.

Apps that choose not to use the NodeVisitor would generate GL errors in 
this case, and PrimitiveSets with unsupported modes would simply not 
render. Presumably they'd see the error output and take actions to run 
the necessary NodeVisitor to resolve the issue.

(If we wanted even better performance, we could probably eliminate the 
number of instances check and just always use the instanced variant, as 
the behavior for numInstances=1 is identical to using the non-instanced 
variant.)

Thanks for giving this some thought.
    -Paul