glDrawsElements Minimal Experiment

include/IECoreGL/MeshPrimitive.h

@@ -35,6 +35,7 @@
 #ifndef IECOREGL_MESHPRIMITIVE_H
 #define IECOREGL_MESHPRIMITIVE_H
 
+#include "IECoreGL/Buffer.h"
 #include "IECoreGL/Export.h"
 #include "IECoreGL/Primitive.h"
 
@@ -51,7 +52,7 @@ class IECOREGL_API MeshPrimitive : public Primitive
 
 		IE_CORE_DECLARERUNTIMETYPEDEXTENSION( IECoreGL::MeshPrimitive, MeshPrimitiveTypeId, Primitive );
 
-		MeshPrimitive( unsigned numTriangles );
+		MeshPrimitive( IECore::ConstIntVectorDataPtr meshIndices );
 		~MeshPrimitive() override;
 
 		Imath::Box3f bound() const override;

src/IECoreGL/Buffer.cpp

@@ -101,6 +101,11 @@ GLuint Buffer::buffer() const
 
 size_t Buffer::size() const
 {
+	// \todo : This is a nasty trap to leave lying around in this code - someone looking at the API
+	// could think that was safe to call while preparing data for rendering a frame, where it will introduce
+	// a pipeline stall that could likely have a pretty significant impact. If we want to support this, we
+	// should just store the size we called glBufferData with.
+
 	ScopedBinding binding( *this, GL_ARRAY_BUFFER );
 	int result = 0;
 	glGetBufferParameteriv( GL_ARRAY_BUFFER, GL_BUFFER_SIZE, &result );

src/IECoreGL/MeshPrimitive.cpp

@@ -36,6 +36,7 @@
 
 #include "IECoreGL/GL.h"
 #include "IECoreGL/State.h"
+#include "IECoreGL/CachedConverter.h"
 
 #include "IECore/DespatchTypedData.h"
 
@@ -57,11 +58,12 @@ class MeshPrimitive::MemberData : public IECore::RefCounted
 
 	public :
 
-		MemberData( unsigned numTriangles ) : numTriangles( numTriangles )
+		MemberData( IECore::ConstIntVectorDataPtr meshIndices ) : meshIndices( meshIndices )
 		{
 		}
 
-		unsigned numTriangles;
+		IECore::ConstIntVectorDataPtr meshIndices;
+		ConstBufferPtr meshIndicesGL;
 		Imath::Box3f bound;
 
 };
@@ -72,8 +74,8 @@ class MeshPrimitive::MemberData : public IECore::RefCounted
 
 IE_CORE_DEFINERUNTIMETYPED( MeshPrimitive );
 
-MeshPrimitive::MeshPrimitive( unsigned numTriangles )
-	:	m_memberData( new MemberData( numTriangles ) )
+MeshPrimitive::MeshPrimitive( IECore::ConstIntVectorDataPtr meshIndices )
+	:	m_memberData( new MemberData( meshIndices ) )
 {
 }
 
@@ -114,7 +116,24 @@ void MeshPrimitive::addPrimitiveVariable( const std::string &name, const IECoreS
 
 void MeshPrimitive::renderInstances( size_t numInstances ) const
 {
-	glDrawArraysInstancedARB( GL_TRIANGLES, 0, m_memberData->numTriangles * 3, numInstances );
+	if( !m_memberData->meshIndicesGL )
+	{
+		// \todo : The use of defaultCachedConverter here is very inefficient - the mesh indices have already
+		// been expanded from verticesPerFace to an explicit index list before being passed in ... any time
+		// we find the index list in the cache, the time we previously spent recomputing the expanded list
+		// was wasted.
+		// The same problem exists for primvars - we first fully expand the data out to FaceVarying in
+		// ToGLMeshConverter, and then we look up if it's in the cache and that work has already been done.
+		// The solution would be passing in more context to this convert call about how it's supposed to be
+		// converted, so we could just pass in the verticesPerFace, but indicate that they need to be converted
+		// to expanded indices. I had considered just adding a "Purpose" enum to this call, John suggested
+		// it should take some sort of function pointer, so this cache could be used for anything. Either
+		// way, it now seems complicated enough that we're not going to tackle it now.
+		m_memberData->meshIndicesGL = IECore::runTimeCast<const Buffer>( CachedConverter::defaultCachedConverter()->convert( m_memberData->meshIndices.get() ) );
+	}
+
+	Buffer::ScopedBinding binding( *m_memberData->meshIndicesGL, GL_ELEMENT_ARRAY_BUFFER );
+	glDrawElementsInstancedARB( GL_TRIANGLES, m_memberData->meshIndices->readable().size(), GL_UNSIGNED_INT, 0, numInstances );
 }
 
 Imath::Box3f MeshPrimitive::bound() const

src/IECoreGL/ToGLMeshConverter.cpp

@@ -34,6 +34,7 @@
 
 #include "IECoreGL/ToGLMeshConverter.h"
 
+#include "IECoreGL/CachedConverter.h"
 #include "IECoreGL/MeshPrimitive.h"
 
 #include "IECoreScene/FaceVaryingPromotionOp.h"
@@ -49,6 +50,49 @@
 
 using namespace IECoreGL;
 
+namespace {
+
+void triangulateMeshIndices(
+    const IECoreScene::MeshPrimitive *mesh,
+    std::vector<int> &newIds
+)
+{
+	const std::vector<int> &verticesPerFace = mesh->verticesPerFace()->readable();
+
+	newIds.clear();
+
+	int numTris = 0;
+	for( auto n : verticesPerFace )
+	{
+		numTris += n - 2;
+	}
+
+	newIds.reserve( numTris * 3 );
+
+	int faceVertexIdStart = 0;
+	for( int faceIdx = 0; faceIdx < (int)verticesPerFace.size(); faceIdx++ )
+	{
+		int numFaceVerts = verticesPerFace[ faceIdx ];
+
+		const int i0 = faceVertexIdStart + 0;
+
+		for( int i = 1; i < numFaceVerts - 1; i++ )
+		{
+			const int i1 = faceVertexIdStart + i;
+			const int i2 = faceVertexIdStart + i + 1;
+
+			/// Store the indices required to rebuild the facevarying primvars
+			newIds.push_back( i0 );
+			newIds.push_back( i1 );
+			newIds.push_back( i2 );
+		}
+
+		faceVertexIdStart += numFaceVerts;
+	}
+}
+
+} // namespace
+
 IE_CORE_DEFINERUNTIMETYPED( ToGLMeshConverter );
 
 ToGLConverter::ConverterDescription<ToGLMeshConverter> ToGLMeshConverter::g_description;
@@ -74,6 +118,9 @@ IECore::RunTimeTypedPtr ToGLMeshConverter::doConversion( IECore::ConstObjectPtr
 
 	if( mesh->variables.find( "N" )==mesh->variables.end() )
 	{
+		// \todo - this is a weird place for this - you don't always need normals in order to render a mesh.
+		// Pretty wasteful to be running a normalsOp on a mesh if you're using a shader that doesn't need normals.
+
 		// the mesh has no normals - we need to explicitly add some. if it's a polygon
 		// mesh (interpolation==linear) then we add per-face normals for a faceted look
 		// and if it's a subdivision mesh we add smooth per-vertex normals.
@@ -86,14 +133,15 @@ IECore::RunTimeTypedPtr ToGLMeshConverter::doConversion( IECore::ConstObjectPtr
 		normalOp->operate();
 	}
 
-	mesh = IECoreScene::MeshAlgo::triangulate( mesh.get() );
-
 	IECoreScene::FaceVaryingPromotionOpPtr faceVaryingOp = new IECoreScene::FaceVaryingPromotionOp;
 	faceVaryingOp->inputParameter()->setValue( mesh );
 	faceVaryingOp->copyParameter()->setTypedValue( false );
 	faceVaryingOp->operate();
 
-	MeshPrimitivePtr glMesh = new MeshPrimitive( mesh->numFaces() );
+	IECore::IntVectorDataPtr meshIndices = new IECore::IntVectorData();
+	triangulateMeshIndices( mesh.get(), IECoreScene::PrimitiveVariable::FaceVarying, meshIndices->writable(), nullptr );
+
+	MeshPrimitivePtr glMesh = new MeshPrimitive( meshIndices );
 
 	for ( IECoreScene::PrimitiveVariableMap::iterator pIt = mesh->variables.begin(); pIt != mesh->variables.end(); ++pIt )
 	{