From e868d8e03c6fd89ea6ba3d08b8329d1482b5471a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 28 May 2012 19:41:30 +0200 Subject: [PATCH] Deprecating AbstractShaderProgram::bind*Location(). Preferred workflow is to specify attribute location explicitly in the shader. The functions remains here as some old Intel systems don't support the required extension ARB_explicit_attrib_location (and it's not in GL 3.0 either). Also updated and fixed the documentation. Function bindAttribute() was renamed to bindAttributeLocation() to be consistent with bindFragmentDataLocation(). PhongShader now uses explicit attribute location. --- src/AbstractShaderProgram.cpp | 2 +- src/AbstractShaderProgram.h | 72 ++++++++++++++++++++++------------- src/Shaders/PhongShader.cpp | 3 -- src/Shaders/PhongShader.vert | 4 +- 4 files changed, 48 insertions(+), 33 deletions(-) diff --git a/src/AbstractShaderProgram.cpp b/src/AbstractShaderProgram.cpp index d27779996..d53ad29b7 100644 --- a/src/AbstractShaderProgram.cpp +++ b/src/AbstractShaderProgram.cpp @@ -38,7 +38,7 @@ bool AbstractShaderProgram::attachShader(Shader& shader) { return true; } -void AbstractShaderProgram::bindAttribute(GLuint location, const string& name) { +void AbstractShaderProgram::bindAttributeLocation(GLuint location, const string& name) { CORRADE_ASSERT(state == Initialized, "AbstractShaderProgram: attribute cannot be bound after linking.", ) glBindAttribLocation(program, location, name.c_str()); diff --git a/src/AbstractShaderProgram.h b/src/AbstractShaderProgram.h index 917083915..c248f6136 100644 --- a/src/AbstractShaderProgram.h +++ b/src/AbstractShaderProgram.h @@ -30,6 +30,8 @@ namespace Magnum { /** @brief Base class for shaders +@section AbstractShaderProgramSubclassing Subclassing workflow + This class is designed to be used via subclassing. Subclasses define these functions and properties: - **Attribute location** typedefs defining locations and types for attribute @@ -39,19 +41,13 @@ typedef Attribute<0, Vector4> Vertex; typedef Attribute<1, Vector3> Normal; typedef Attribute<2, Vector2> TextureCoords; @endcode - See also bindAttribute(). - - **Constructor**, which attaches particular shaders, links the program, - binds attribute locations and gets uniform locations, for example: + - **Constructor**, which attaches particular shaders, links the program and + gets uniform locations, for example: @code // Load shaders from file and attach them to the program attachShader(Shader::fromFile(Shader::Vertex, "PhongShader.vert")); attachShader(Shader::fromFile(Shader::Fragment, "PhongShader.frag")); -// Bind attribute names to IDs -bindAttribute(Vertex::Location, "vertex"); -bindAttribute(Normal::Location, "normal"); -bindAttribute(TextureCoords::Location, "textureCoords"); - // Link link(); @@ -61,28 +57,43 @@ projectionMatrixUniform = uniformLocation("projectionMatrix"); // more uniforms like light location, colors etc. @endcode - **Uniform binding functions**, which set shader uniforms using - setUniform() and setUniformArray() functions. Example: + setUniform() functions. Example: @code void setTransformationMatrixUniform(const Matrix4& matrix) { setUniform(transformationMatrixUniform, matrix); } @endcode -Basic workflow with AbstractShaderProgram subclasses is: instancing the class -(once at the beginning), then in every frame calling use(), setting uniforms -and calling Mesh::draw() (see its documentation for more). +@subsection AbstractShaderProgramAttributeLocation Binding attribute location +The preferred workflow is to specify attribute location for vertex shader +input attributes and fragment shader output attributes explicitly in the +shader code, e.g.: +@code +layout(location = 0) in vec4 vertex; +layout(location = 1) in vec3 normal; +layout(location = 2) in vec2 textureCoords; +@endcode +@requires_gl33 Extension @extension{ARB,explicit_attrib_location} (for + explicit attribute location instead of using bindAttributeLocation()) -@section MultipleFragmentOutputs Multiple fragment shader outputs -If your shader uses multiple fragment outputs, you can use -bindFragmentDataLocation() *before linking* to bind their names to desired -location, e.g.: +If you don't have the required extension, you can use functions bindAttributeLocation() +and bindFragmentDataLocation() between attaching of shaders and linking the +program: @code -bindFragmentDataLocation(0, "color"); +// Shaders attached... + +bindAttributeLocation(Vertex::Location, "vertex"); +bindAttributeLocation(Normal::Location, "normal"); +bindAttributeLocation(TextureCoords::Location, "textureCoords"); + +// Link... @endcode -You should then clearly state in the documentation which output is on what -position, so the user can set framebuffer attachments for them using -Framebuffer::mapForDraw() or Framebuffer::mapDefaultForDraw(). +@section AbstractShaderProgramRenderingWorkflow Rendering workflow + +Basic workflow with AbstractShaderProgram subclasses is: instancing the class +(once at the beginning), then in every frame calling use(), setting uniforms +and calling Mesh::draw() (see its documentation for more). @todo Uniform arrays support @@ -152,19 +163,27 @@ class MAGNUM_EXPORT AbstractShaderProgram { * @param name Attribute name * * Binds attribute to location which is be used later for binding - * vertex buffers. - * @note This function should be called between loadShader() calls - * and link(). + * vertex buffers. Preferred usage is to + * @note This function should be called between attachShader() calls + * and link(). + * @deprecated Preferred usage is to specify attribute location + * explicitly in the shader instead of using this function. See + * @ref AbstractShaderProgramAttributeLocation "class documentation" + * for more information. */ - void bindAttribute(GLuint location, const std::string& name); + void bindAttributeLocation(GLuint location, const std::string& name); /** * @brief Bind fragment data to given location * @param location Location * @param name Fragment output variable name * - * @note This function should be called between loadShader() calls - * and link(). + * @note This function should be called between attachShader() calls + * and link(). + * @deprecated Preferred usage is to specify attribute location + * explicitly in the shader instead of using this function. See + * @ref AbstractShaderProgramAttributeLocation "class documentation" + * for more information. * * @requires_gl30 Extension @extension{EXT,gpu_shader4} */ @@ -212,7 +231,6 @@ class MAGNUM_EXPORT AbstractShaderProgram { glUniform4fv(location, 1, value.data()); } - /** @copydoc setUniform(GLint, GLfloat) */ inline void setUniform(GLint location, GLint value) { glUniform1i(location, value); diff --git a/src/Shaders/PhongShader.cpp b/src/Shaders/PhongShader.cpp index be54c0117..67500d9f2 100644 --- a/src/Shaders/PhongShader.cpp +++ b/src/Shaders/PhongShader.cpp @@ -24,9 +24,6 @@ PhongShader::PhongShader() { attachShader(Shader::fromData(Shader::Type::Vertex, rs.get("PhongShader.vert"))); attachShader(Shader::fromData(Shader::Type::Fragment, rs.get("PhongShader.frag"))); - bindAttribute(Vertex::Location, "vertex"); - bindAttribute(Normal::Location, "normal"); - link(); ambientColorUniform = uniformLocation("ambientColor"); diff --git a/src/Shaders/PhongShader.vert b/src/Shaders/PhongShader.vert index 5b73ccd79..19755516a 100644 --- a/src/Shaders/PhongShader.vert +++ b/src/Shaders/PhongShader.vert @@ -4,8 +4,8 @@ uniform mat4 transformationMatrix; uniform mat4 projectionMatrix; uniform vec3 light; -in vec4 vertex; -in vec3 normal; +layout(location = 0) in vec4 vertex; +layout(location = 1) in vec3 normal; out vec3 transformedNormal; out vec3 lightDirection;