Merge pull request #453 from johnhaddon/wireframeWithVertexShaders

IECoreGL : Fixes relating to custom vertex shaders

Author: andrewkaufman

include/IECoreGL/Primitive.h

@@ -96,6 +96,23 @@ class IECOREGL_API Primitive : public Renderable
 		/// Most classes will not need to override this method - reasons for overriding would be
 		/// to substitute in custom geometry or vertex shaders and/or to bind in attributes
 		/// not already specified with addUniformAttribute() or addVertexAttribute().
+		///
+		/// \todo We need to rethink this mechanism. The problem is that we've ended up using
+		/// this method for two things - firstly to get a ShaderSetup where all the primitive
+		/// variables are bound (good), and secondly we've abused it to actually change the
+		/// shader in PointsPrimitive and CurvePrimitive. Asking for a setup for one shader and getting
+		/// back a setup for another doesn't make a great deal of sense. There are several
+		/// competing sources of source code for shaders :
+		///
+		/// - The user-provided source coming through Renderer::shader().
+		/// - The vertex and geometry shaders that PointsPrimitive and CurvesPrimitive need
+		///   to insert.
+		/// - The constant fragment shader that Primitive needs to insert to do wireframe
+		///   shading etc.
+		/// - The ID fragment shader needed for the Selector.
+		///
+		/// We should redesign our API so that we first resolve these requirements to generate
+		/// a shader, and then use shaderSetup() just to apply primitive variables to it.
 		virtual const Shader::Setup *shaderSetup( const Shader *shader, State *state ) const;
 		/// Adds the primitive variables held by this Primitive to the specified Shader::Setup.
 		/// Vertex attributes will be prefixed as specified, and for each vertex attribute

include/IECoreGL/Selector.h

@@ -119,14 +119,18 @@ class IECOREGL_API Selector : boost::noncopyable
 
 		/// The IDRender mode requires a shader which takes a name
 		/// via a "uniform uint ieCoreGLName" parameter and outputs it
-		/// via an "out uint ieCoreGLNameOut" fragment output.
-		/// Typically one is set up automatically in baseState(), but
-		/// if rendering must be performed with an alternative shader
-		/// then it may be passed via this function.
+		/// via an "out uint ieCoreGLNameOut" fragment output. The
+		/// defaultIDShader() meets these criteria and will be bound
+		/// automatically when the selector constructs in IDRender mode.
+		/// Note however, that if any other shaders are subsequently
+		/// bound while the selector is active, they too must meet the
+		/// same critera.
 		void pushIDShader( const IECoreGL::Shader *idShader );
-		
-		/// Revert to previous ID shader:
+		/// Reverts to the previous ID shader.
 		void popIDShader();
+
+		/// A shader suitable for use in IDRender mode.
+		static const Shader *defaultIDShader();
 
 		/// Returns the currently active Selector - this may be used
 		/// in drawing code to retrieve a selector to call loadName()

include/IECoreGL/ShaderStateComponent.h

@@ -76,6 +76,9 @@ class IECOREGL_API ShaderStateComponent : public StateComponent
 		ShaderLoader *shaderLoader();
 		TextureLoader *textureLoader();
 
+		/// Returns a hash to uniquely identify this shader state.
+		IECore::MurmurHash hash() const;
+
 		/// Returns a Shader::Setup object for binding the shader. This function can
 		/// only be called from a thread with a valid GL context.
 		Shader::Setup *shaderSetup();

src/IECoreGL/Primitive.cpp

@@ -44,6 +44,7 @@
 #include "IECoreGL/Exception.h"
 #include "IECoreGL/TypedStateComponent.h"
 #include "IECoreGL/ShaderStateComponent.h"
+#include "IECoreGL/ShaderLoader.h"
 #include "IECoreGL/Shader.h"
 #include "IECoreGL/NumericTraits.h"
 #include "IECoreGL/UniformFunctions.h"
@@ -74,66 +75,60 @@ IECOREGL_TYPEDSTATECOMPONENT_SPECIALISEANDINSTANTIATE( Primitive::PointWidth, Pr
 IECOREGL_TYPEDSTATECOMPONENT_SPECIALISEANDINSTANTIATE( Primitive::Selectable, PrimitiveSelectableTypeId, bool, true );
 IECOREGL_TYPEDSTATECOMPONENT_SPECIALISEANDINSTANTIATE( Primitive::TransparencySort, PrimitiveTransparencySortStateComponentTypeId, bool, true );
 
-}
+} // namespace IECoreGL
 
 //////////////////////////////////////////////////////////////////////////
-// Primitive implementation
+// Utilities
 //////////////////////////////////////////////////////////////////////////
 
-IE_CORE_DEFINERUNTIMETYPED( Primitive );
-
-Primitive::Primitive()
+namespace
 {
-}
 
-Primitive::~Primitive()
+struct FlatConstant
 {
-}
 
-void Primitive::addPrimitiveVariable( const std::string &name, const IECore::PrimitiveVariable &primVar )
-{
-	if ( primVar.interpolation == IECore::PrimitiveVariable::Constant )
+	FlatConstant( const IECore::MurmurHash &hash, const Shader::ConstSetupPtr &shaderSetup = Shader::ConstSetupPtr() )
+		:	hash( hash ), shaderSetup( shaderSetup )
 	{
-		addUniformAttribute( name, primVar.data );
 	}
-	else
+
+	IECore::MurmurHash hash;
+	Shader::ConstSetupPtr shaderSetup;
+
+	bool operator < ( const FlatConstant &rhs )
 	{
-		addVertexAttribute( name, primVar.data );
+		return hash < rhs.hash;
 	}
-}
 
-static Shader *constant2()
+};
+
+// Makes a constant shader by taking the vertex and geometry
+// shader from the state and combining them with a flat fragment
+// shader.
+const Shader::Setup *flatConstantShaderSetup( State *state, bool forIDRender )
 {
+	ShaderStateComponent *shaderStateComponent = state->get<ShaderStateComponent>();
 
-	static const char *vertexSource =
-		
-		"#if __VERSION__ <= 120\n"
-		"#define in attribute\n"
-		"#define out varying\n"
-		"#endif\n"
-		""
-		"in vec3 vertexP;"
-		""
-		"void main()"
-		"{"
-		"	gl_Position = gl_ProjectionMatrix * gl_ModelViewMatrix * vec4( vertexP, 1 );"
-		"}";
-		
-	static const char *fragmentSource =
-	
-		"void main()"
-		"{"
-			"gl_FragColor = vec4( 1, 1, 1, 1 );"
-		"}";
+	// Get a hash to represent the shader we're about to make.
+	IECore::MurmurHash hash = shaderStateComponent->hash();
+	hash.append( forIDRender );
 
-	static ShaderPtr s = new Shader( vertexSource, fragmentSource );
-	return s.get();
-}
+	// If we've made an equivalent shader before, then
+	// just return it.
+	static vector<FlatConstant> flatConstants;
+	vector<FlatConstant>::iterator it = lower_bound(
+		flatConstants.begin(),
+		flatConstants.end(),
+		hash
+	);
+	if( it != flatConstants.end() && it->hash == hash )
+	{
+		return it->shaderSetup.get();
+	}
 
-static Shader *flatConstant()
-{
+	// If we haven't, then make one.
 
-	static const char *fragmentSource =
+	static const std::string constantFragmentSource =
 
 		"uniform vec3 Cs;" // get colour from uniform Cs, bypassing vertexCs
 		""
@@ -142,10 +137,48 @@ static Shader *flatConstant()
 		"	gl_FragColor = vec4( Cs, 1 );"
 		"}";
 
-	static ShaderPtr s = new Shader( "", fragmentSource );
-	return s.get();
+	const std::string &fragmentSource = forIDRender ? Selector::defaultIDShader()->fragmentSource() : constantFragmentSource;
+
+	const Shader *originalShader = shaderStateComponent->shaderSetup()->shader();
+	ShaderLoader *shaderLoader = shaderStateComponent->shaderLoader();
+	ConstShaderPtr shader = shaderLoader->create( originalShader->vertexSource(), originalShader->geometrySource(), fragmentSource );
+	Shader::SetupPtr shaderSetup = new Shader::Setup( shader );
+	shaderStateComponent->addParametersToShaderSetup( shaderSetup.get() );
+
+	// Put it in our store so we don't have to remake it next time.
+
+	flatConstants.insert( it, FlatConstant( hash, shaderSetup.get() ) );
+
+	return shaderSetup.get();
 }
 
+} // namespace
+
+//////////////////////////////////////////////////////////////////////////
+// Primitive implementation
+//////////////////////////////////////////////////////////////////////////
+
+IE_CORE_DEFINERUNTIMETYPED( Primitive );
+
+Primitive::Primitive()
+{
+}
+
+Primitive::~Primitive()
+{
+}
+
+void Primitive::addPrimitiveVariable( const std::string &name, const IECore::PrimitiveVariable &primVar )
+{
+	if ( primVar.interpolation == IECore::PrimitiveVariable::Constant )
+	{
+		addUniformAttribute( name, primVar.data );
+	}
+	else
+	{
+		addVertexAttribute( name, primVar.data );
+	}
+}
 
 void Primitive::render( State *state ) const
 {
@@ -163,9 +196,10 @@ void Primitive::render( State *state ) const
 	// with a simple shader and early out.
 	if( currentSelector && currentSelector->mode() == Selector::GLSelect )
 	{
-		const Shader *constantShader = constant2();
-		const Shader::Setup *constantSetup = shaderSetup( constantShader, state );
-		Shader::Setup::ScopedBinding constantBinding( *constantSetup );
+		const Shader::Setup *uniformSetup = flatConstantShaderSetup( state, /* forIDRender = */ false );
+		Shader::Setup::ScopedBinding uniformBinding( *uniformSetup );
+		const Shader::Setup *primitiveSetup = shaderSetup( uniformSetup->shader(), state );
+		Shader::Setup::ScopedBinding primitiveBinding( *primitiveSetup );
 		render( state, Primitive::DrawSolid::staticTypeId() );
 		return;
 	}
@@ -177,8 +211,18 @@ void Primitive::render( State *state ) const
 	{
 		glDepthMask( !depthSortRequested( state ) );
 		// the state itself will have a shader with some nice uniform parameter
-		// values. we are responsible for binding this setup.
-		Shader::Setup *uniformSetup = state->get<ShaderStateComponent>()->shaderSetup();
+		// values. we are responsible for binding this setup. unless we're performing
+		// an id render for selection, in which case we're responsible for binding an
+		// id shader.
+		const Shader::Setup *uniformSetup = NULL;
+		if( currentSelector && currentSelector->mode() == Selector::IDRender )
+		{
+			uniformSetup = flatConstantShaderSetup( state, /* forIDRender = */ true );
+		}
+		else
+		{
+			uniformSetup = state->get<ShaderStateComponent>()->shaderSetup();
+		}
 		Shader::Setup::ScopedBinding uniformBinding( *uniformSetup );
 		// we then bind our own setup on top, adding in the parameters
 		// stored on the primitive itself.
@@ -210,18 +254,18 @@ void Primitive::render( State *state ) const
 	}
 
 	// get a constant shader suitable for drawing wireframes, points etc.
-	const Shader *constantShader = flatConstant();
-	if( currentSelector && currentSelector->mode() == Selector::IDRender )
-	{
-		// if we're in IDRender mode, then the constant shader is unsuitable,
-		// and we should instead use the ID shader we've been given.
-		constantShader = state->get<ShaderStateComponent>()->shaderSetup()->shader();
-	}
-	
-	const Shader::Setup *constantSetup = shaderSetup( constantShader, state );
-	Shader::Setup::ScopedBinding constantBinding( *constantSetup );
+	// we do this by taking the current shader from the state and overriding
+	// just the fragment shader within it - we want to keep any vertex or
+	// geometry shader the user has specified. again, if we're performing an
+	// id render, we're responsible for binding an id shader instead.
+	const Shader::Setup *uniformSetup = flatConstantShaderSetup( state, currentSelector && currentSelector->mode() == Selector::IDRender );
+	Shader::Setup::ScopedBinding uniformBinding( *uniformSetup );
+
+	const Shader::Setup *primitiveSetup = shaderSetup( uniformSetup->shader(), state );
+	Shader::Setup::ScopedBinding primitiveBinding( *primitiveSetup );
+
 	GLint csIndex = -1;
-	if( const Shader::Parameter *csParameter = constantSetup->shader()->csParameter() )
+	if( const Shader::Parameter *csParameter = uniformSetup->shader()->csParameter() )
 	{
 		csIndex = csParameter->location;
 	}