Advertisement Jump to content
Sign in to follow this  

OpenGL How can I improve these simple OpenGL|ES 2.0 shaders?

This topic is 2506 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Well, I wrote my first program ever that uses shaders! It works and all. It supports multiple lights (up to 10), textures (only 1 per object), and that's about it. It's for a school assignment, so it's not going to be a full-blown engine (and I've already met the requirements for the assignment), but I plan on using the things I'm learning in the future for various projects I work on and so I'd like to try and learn even more if I could. I'm wondering if people with more experience could look over my shaders and how I'm using them and tell me ways I can improve them and what are common "best practices" I should be following.


uniform mat4 modelView;
uniform mat4 projection;

attribute vec4 position;
attribute vec3 vertexNormal;
attribute vec2 vertexTexCoords;

varying vec2 texCoords;
varying vec3 normal;
varying vec4 positionEyeCoords;

void main()
gl_Position = projection * modelView * position;
normal = normalize(vec3(modelView * vec4(vertexNormal, 0.0)));
texCoords = vertexTexCoords;
positionEyeCoords = modelView * position;


uniform vec4 lightPositions[10];
uniform vec4 lightColors[10];
uniform int numLights;

uniform vec4 ambient;

uniform vec4 meshColor;
uniform vec4 meshSpecular;

uniform sampler2D texture;
uniform bool hasTexture;

varying vec2 texCoords;
varying vec3 normal;
varying vec4 positionEyeCoords;

void main()
vec4 ambientInput = meshColor * ambient;
vec4 diffuseInput = vec4(0.0, 0.0, 0.0, 0.0);
vec4 specularInput = vec4(0.0, 0.0, 0.0, 0.0);

// Phong Lighting
for (int i = 0; i < numLights; ++i)
vec3 incident = normalize(vec3(lightPositions - positionEyeCoords));
float d = clamp(dot(incident, normal), 0.0, 1.0); // negatives get clamped to zero
diffuseInput = diffuseInput + lightColors * meshColor * d;

vec3 view = vec3(-positionEyeCoords);
view = normalize(view);
vec3 reflect = normal * 2 * dot(incident, normal) - incident;
reflect = normalize(reflect);
float shiny = clamp(dot(view, reflect), 0.0, 1.0); // negatives get clamped to zero
vec4 phong = meshSpecular;
phong.w = 0.0;
specularInput = specularInput + lightColors * phong * pow(shiny, meshSpecular.w);

vec4 endColor = ambientInput + diffuseInput;
if (hasTexture)
endColor = endColor * texture2D(texture, texCoords);
endColor = endColor + specularInput;
endColor.w = meshColor.w; // preserve transparentcy
gl_FragColor = endColor;


// Notes:
// program is a GLuint that specifies the shader program
// objects is a std::vector<Object>
// Object is simply a class that stores a pointer to a Mesh or Texture object
// I know I shouldn't be calling glGetUniformLocation() each render frame; I should
// be storing the locations after I compile and link the shaders. I will do that at some
// point in the near future.
// lightPositions is a std::vector<Vector4f> representing the light positions
// lightColors is a std::vector<Vector4f> representing the light colors
void Scene::render() const
for (unsigned int i = 0; i < objects.size(); ++i)
const Mesh* mesh = objects.getMesh();
if (mesh != nullptr)
Matrix44f modelView = objects.getTransform(); // Get the object's transform
modelView = camera.getTransform() * modelView; // Camera just represents the game's camera
glUniformMatrix4fv(glGetUniformLocation(program, "modelView"), 1, GL_FALSE, (GLfloat*)&modelView);
glUniformMatrix4fv(glGetUniformLocation(program, "projection"), 1, GL_FALSE, (GLfloat*)&projection);

glUniform4fv(glGetUniformLocation(program, "ambient"), 1, (GLfloat*)&ambient);
int lightCount = std::min(10, (int)lightPositions.size());
if (lightCount > 0)
glUniform4fv(glGetUniformLocation(program, "lightPositions[0]"), lightCount, (GLfloat*)&lightPositions[0]);
glUniform4fv(glGetUniformLocation(program, "lightColors[0]"), lightCount, (GLfloat*)&lightColors[0]);
glUniform1i(glGetUniformLocation(program, "numLights"), lightCount);

const Texture* texture = objects.getTexture();
if (texture != nullptr)
glBindTexture(GL_TEXTURE_2D, texture->getTextureId()); // Get's the OpenGL texture ID representing the texture
glUniform1i(glGetUniformLocation(program, "texture"), 0);
glUniform1i(glGetUniformLocation(program, "hasTexture"), GL_TRUE);
glUniform1i(glGetUniformLocation(program, "hasTexture"), GL_FALSE);

glUniform4fv(glGetUniformLocation(program, "meshColor"), 1, (GLfloat*)&objects.getColor());
glUniform4fv(glGetUniformLocation(program, "meshSpecular"), 1, (GLfloat*)&objects.getSpecular());

const std::vector<Vertexf>& verts = mesh->getVertices();
glVertexAttribPointer(glGetAttribLocation(program, "position"), 4, GL_FLOAT, GL_FALSE, sizeof(Vertexf), &verts[0].v);
glVertexAttribPointer(glGetAttribLocation(program, "vertexNormal"), 3, GL_FLOAT, GL_FALSE, sizeof(Vertexf), &verts[0].n);
glVertexAttribPointer(glGetAttribLocation(program, "vertexTexCoords"), 2, GL_FLOAT, GL_FALSE, sizeof(Vertexf), &verts[0].t);
glEnableVertexAttribArray(glGetAttribLocation(program, "position"));
glEnableVertexAttribArray(glGetAttribLocation(program, "vertexNormal"));
glEnableVertexAttribArray(glGetAttribLocation(program, "vertexTexCoords"));

glDrawArrays(GL_TRIANGLES, 0, verts.size());

Mesh.hpp (simplified)

template <unsigned int, typename T> struct Vector;template <typename T>
// There's also Vector<3, T> and Vector<2, T> defined similarly, but with 3 and 2 components, respectively
struct Vector<4, T>
T v[4];
T x, y, z, w;

// And the usual functions for vectors...

typedef Vector<4, float> Vector4f;

typedef Vector<3, float> Vector3f;
typedef Vector<2, float> Vector2f;

static_assert(sizeof(Vector4f) == 4 * sizeof(float), "Error, Vector4f is begin padded!");
static_assert(sizeof(Vector3f) == 3 * sizeof(float), "Error, Vector3f is begin padded!");
static_assert(sizeof(Vector2f) == 2 * sizeof(float), "Error, Vector2f is begin padded!");

template <typename T>
struct Vertex
Vector<4, T> v; // vertex point
Vector<3, T> n; // normal
Vector<2, T> t; // texture

typedef Vertex<float> Vertexf;

static_assert(sizeof(Vertexf) == sizeof(Vector4f) + sizeof(Vector3f) + sizeof(Vector2f), "Error, Vertexf is being padded!");

class Mesh
Mesh(const std::string& filename); // Just loads a .raw or .obj file into verts

const std::vector<Vertexf>& getVertices() const; // Just returns the member verts

Mesh(const Mesh& m) {}
void operator = (const Mesh& m) {}

std::vector<Vertexf> verts;

I hate to do a code dump, but my instructor is mostly going to just make sure the program does what it needs to and not really give feedback on the quality of the code. So I'm hoping I can get some feedback on the quality and how I can improve it here. The most important code is the shaders. I just included the Mesh and Scene::render() portions to give you an idea how I use those shaders.

Share this post

Link to post
Share on other sites
Sign in to follow this  

  • Advertisement

Important Information

By using, you agree to our community Guidelines, Terms of Use, and Privacy Policy. is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!