Skip to content

Commit

Permalink
Fix memory leak in Shader maintenance code
Browse files Browse the repository at this point in the history
  • Loading branch information
9prady9 committed Aug 15, 2016
1 parent f07f44f commit 6aaf3e7
Show file tree
Hide file tree
Showing 16 changed files with 222 additions and 196 deletions.
46 changes: 21 additions & 25 deletions src/backend/opengl/chart_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,9 @@ AbstractChart::AbstractChart(const int pLeftMargin, const int pRightMargin,
mLeftMargin(pLeftMargin), mRightMargin(pRightMargin),
mTopMargin(pTopMargin), mBottomMargin(pBottomMargin),
mXMax(1), mXMin(0), mYMax(1), mYMin(0), mZMax(1), mZMin(0),
mXTitle("X-Axis"), mYTitle("Y-Axis"), mZTitle("Z-Axis"),
mDecorVBO(-1), mBorderProgram(-1), mSpriteProgram(-1),
mXTitle("X-Axis"), mYTitle("Y-Axis"), mZTitle("Z-Axis"), mDecorVBO(-1),
mBorderProgram(glsl::chart_vs.c_str(), glsl::chart_fs.c_str()),
mSpriteProgram(glsl::chart_vs.c_str(), glsl::tick_fs.c_str()),
mBorderAttribPointIndex(-1), mBorderUniformColorIndex(-1),
mBorderUniformMatIndex(-1), mSpriteUniformMatIndex(-1),
mSpriteUniformTickcolorIndex(-1), mSpriteUniformTickaxisIndex(-1),
Expand All @@ -137,16 +138,13 @@ AbstractChart::AbstractChart(const int pLeftMargin, const int pRightMargin,
* are loaded into the shared Font object */
getChartFont();

mBorderProgram = initShaders(glsl::chart_vs.c_str(), glsl::chart_fs.c_str());
mSpriteProgram = initShaders(glsl::chart_vs.c_str(), glsl::tick_fs.c_str());
mBorderAttribPointIndex = mBorderProgram.getAttributeLocation("point");
mBorderUniformColorIndex = mBorderProgram.getUniformLocation("color");
mBorderUniformMatIndex = mBorderProgram.getUniformLocation("transform");

mBorderAttribPointIndex = glGetAttribLocation (mBorderProgram, "point");
mBorderUniformColorIndex = glGetUniformLocation(mBorderProgram, "color");
mBorderUniformMatIndex = glGetUniformLocation(mBorderProgram, "transform");

mSpriteUniformTickcolorIndex = glGetUniformLocation(mSpriteProgram, "tick_color");
mSpriteUniformMatIndex = glGetUniformLocation(mSpriteProgram, "transform");
mSpriteUniformTickaxisIndex = glGetUniformLocation(mSpriteProgram, "isYAxis");
mSpriteUniformTickcolorIndex = mSpriteProgram.getUniformLocation("tick_color");
mSpriteUniformMatIndex = mSpriteProgram.getUniformLocation("transform");
mSpriteUniformTickaxisIndex = mSpriteProgram.getUniformLocation("isYAxis");

CheckGL("End AbstractChart::AbstractChart");
}
Expand All @@ -159,8 +157,6 @@ AbstractChart::~AbstractChart()
glDeleteVertexArrays(1, &vao);
}
glDeleteBuffers(1, &mDecorVBO);
glDeleteProgram(mBorderProgram);
glDeleteProgram(mSpriteProgram);
CheckGL("End AbstractChart::~AbstractChart");
}

Expand Down Expand Up @@ -404,11 +400,11 @@ void chart2d_impl::render(const int pWindowId,

/* Draw grid */
chart2d_impl::bindResources(pWindowId);
glUseProgram(mBorderProgram);
mBorderProgram.bind();
glUniformMatrix4fv(mBorderUniformMatIndex, 1, GL_FALSE, glm::value_ptr(trans));
glUniform4fv(mBorderUniformColorIndex, 1, GRAY);
glDrawArrays(GL_LINES, 4+2*mTickCount, 4*mTickCount);
glUseProgram(0);
mBorderProgram.unbind();
chart2d_impl::unbindResources();

glEnable(GL_SCISSOR_TEST);
Expand All @@ -422,17 +418,17 @@ void chart2d_impl::render(const int pWindowId,

chart2d_impl::bindResources(pWindowId);

glUseProgram(mBorderProgram);
mBorderProgram.bind();
glUniformMatrix4fv(mBorderUniformMatIndex, 1, GL_FALSE, glm::value_ptr(trans));
glUniform4fv(mBorderUniformColorIndex, 1, BLACK);
/* Draw borders */
glDrawArrays(GL_LINE_LOOP, 0, 4);
glUseProgram(0);
mBorderProgram.unbind();

/* bind the sprite shader program to
* draw ticks on x and y axes */
glPointSize((GLfloat)mTickSize);
glUseProgram(mSpriteProgram);
mSpriteProgram.bind();

glUniform4fv(mSpriteUniformTickcolorIndex, 1, BLACK);
glUniformMatrix4fv(mSpriteUniformMatIndex, 1, GL_FALSE, glm::value_ptr(trans));
Expand All @@ -443,7 +439,7 @@ void chart2d_impl::render(const int pWindowId,
glUniform1i(mSpriteUniformTickaxisIndex, 0);
glDrawArrays(GL_POINTS, 4+mTickCount, mTickCount);

glUseProgram(0);
mSpriteProgram.unbind();
glPointSize(1);
chart2d_impl::unbindResources();

Expand Down Expand Up @@ -729,11 +725,11 @@ void chart3d_impl::render(const int pWindowId,

/* draw grid */
chart3d_impl::bindResources(pWindowId);
glUseProgram(mBorderProgram);
mBorderProgram.bind();
glUniformMatrix4fv(mBorderUniformMatIndex, 1, GL_FALSE, glm::value_ptr(PVM));
glUniform4fv(mBorderUniformColorIndex, 1, GRAY);
glDrawArrays(GL_LINES, 6+3*mTickCount, 12*mTickCount);
glUseProgram(0);
mBorderProgram.unbind();
chart3d_impl::unbindResources();

glEnable(GL_SCISSOR_TEST);
Expand All @@ -749,17 +745,17 @@ void chart3d_impl::render(const int pWindowId,
/* Draw borders */
chart3d_impl::bindResources(pWindowId);

glUseProgram(mBorderProgram);
mBorderProgram.bind();
glUniformMatrix4fv(mBorderUniformMatIndex, 1, GL_FALSE, glm::value_ptr(PVM));
glUniform4fv(mBorderUniformColorIndex, 1, BLACK);
glDrawArrays(GL_LINES, 0, 6);
glUseProgram(0);
mBorderProgram.unbind();

/* bind the sprite shader program to
* draw ticks on x and y axes */
glEnable(GL_PROGRAM_POINT_SIZE);
glPointSize((GLfloat)mTickSize);
glUseProgram(mSpriteProgram);
mSpriteProgram.bind();

glUniform4fv(mSpriteUniformTickcolorIndex, 1, BLACK);
glUniformMatrix4fv(mSpriteUniformMatIndex, 1, GL_FALSE, glm::value_ptr(PVM));
Expand All @@ -773,7 +769,7 @@ void chart3d_impl::render(const int pWindowId,
glUniform1i(mSpriteUniformTickaxisIndex, 0);
glDrawArrays(GL_POINTS, 6 + (2*mTickCount), mTickCount);

glUseProgram(0);
mSpriteProgram.unbind();
glPointSize((GLfloat)1);
glDisable(GL_PROGRAM_POINT_SIZE);

Expand Down
4 changes: 2 additions & 2 deletions src/backend/opengl/chart_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class AbstractChart : public AbstractRenderable {
std::string mZTitle;
/* OpenGL Objects */
gl::GLuint mDecorVBO;
gl::GLuint mBorderProgram;
gl::GLuint mSpriteProgram;
ShaderProgram mBorderProgram;
ShaderProgram mSpriteProgram;
/* shader uniform variable locations */
gl::GLuint mBorderAttribPointIndex;
gl::GLuint mBorderUniformColorIndex;
Expand Down
56 changes: 52 additions & 4 deletions src/backend/opengl/common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,60 @@ Shaders loadShaders(const char* pVertexShaderSrc,
return out;
}

GLuint initShaders(const char* pVertShaderSrc, const char* pFragShaderSrc, const char* pGeomShaderSrc)
namespace opengl
{

ShaderProgram::ShaderProgram(const char* pVertShaderSrc,
const char* pFragShaderSrc,
const char* pGeomShaderSrc)
: mVertex(0), mFragment(0), mGeometry(0), mProgram(0)
{
Shaders shrds = loadShaders(pVertShaderSrc, pFragShaderSrc, pGeomShaderSrc);
GLuint shaderProgram = glCreateProgram();
attachAndLinkProgram(shaderProgram, shrds);
return shaderProgram;
mProgram = glCreateProgram();
attachAndLinkProgram(mProgram, shrds);
mVertex = shrds.vertex;
mFragment = shrds.fragment;
mGeometry = shrds.geometry;
}

ShaderProgram::~ShaderProgram()
{
if (mVertex ) glDeleteShader ( mVertex );
if (mFragment) glDeleteShader ( mFragment);
if (mGeometry) glDeleteShader ( mGeometry);
if (mProgram ) glDeleteProgram( mProgram );
}

gl::GLuint ShaderProgram::getProgramId() const
{
return mProgram;
}

GLuint ShaderProgram::getUniformLocation(const char* pAttributeName)
{
return gl::glGetUniformLocation(mProgram, pAttributeName);
}

GLuint ShaderProgram::getUniformBlockIndex(const char* pAttributeName)
{
return gl::glGetUniformBlockIndex(mProgram, pAttributeName);
}

GLuint ShaderProgram::getAttributeLocation(const char* pAttributeName)
{
return gl::glGetAttribLocation(mProgram, pAttributeName);
}

void ShaderProgram::bind()
{
glUseProgram(mProgram);
}

void ShaderProgram::unbind()
{
glUseProgram(0);
}

}

float clampTo01(const float pValue)
Expand Down
34 changes: 24 additions & 10 deletions src/backend/opengl/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,6 @@ gl::GLenum ctype2gl(const fg::ChannelFormat pMode);
*/
gl::GLenum ictype2gl(const fg::ChannelFormat pMode);

/* Compile OpenGL GLSL vertex and fragment shader sources
*
* @pVertShaderSrc is the vertex shader source code string
* @pFragShaderSrc is the vertex shader source code string
* @pGeomShaderSrc is the vertex shader source code string
*
* @return GLSL program unique identifier for given shader duo
*/
gl::GLuint initShaders(const char* pVertShaderSrc, const char* pFragShaderSrc, const char* pGeomShaderSrc=NULL);

/* Create OpenGL buffer object
*
* @pTarget should be either GL_ARRAY_BUFFER or GL_ELEMENT_ARRAY_BUFFER
Expand Down Expand Up @@ -141,6 +131,30 @@ typedef unsigned int uint;
typedef unsigned short ushort;
typedef unsigned char uchar;

class ShaderProgram {
private:
gl::GLuint mVertex;
gl::GLuint mFragment;
gl::GLuint mGeometry;
gl::GLuint mProgram;

public:
ShaderProgram(const char* pVertShaderSrc,
const char* pFragShaderSrc,
const char* pGeomShaderSrc=NULL);

~ShaderProgram();

gl::GLuint getProgramId() const;

gl::GLuint getUniformLocation(const char* pAttributeName);
gl::GLuint getUniformBlockIndex(const char* pAttributeName);
gl::GLuint getAttributeLocation(const char* pAttributeName);

void bind();
void unbind();
};

/* Basic renderable class
*
* Any object that is renderable to a window should inherit from this
Expand Down
19 changes: 9 additions & 10 deletions src/backend/opengl/font_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,22 +257,21 @@ void font_impl::destroyGLResources()
}

font_impl::font_impl()
: mTTFfile(""), mIsFontLoaded(false), mAtlas(new FontAtlas(1024, 1024, 1)),
mVBO(0), mProgram(0), mOrthoW(1), mOrthoH(1)
: mTTFfile(""), mIsFontLoaded(false), mAtlas(new FontAtlas(1024, 1024, 1)), mVBO(0),
mProgram(glsl::font_vs.c_str(), glsl::font_fs.c_str()),
mOrthoW(1), mOrthoH(1)
{
mProgram = initShaders(glsl::font_vs.c_str(), glsl::font_fs.c_str());
mPMatIndex = glGetUniformLocation(mProgram, "projectionMatrix");
mMMatIndex = glGetUniformLocation(mProgram, "modelViewMatrix");
mTexIndex = glGetUniformLocation(mProgram, "tex");
mClrIndex = glGetUniformLocation(mProgram, "textColor");
mPMatIndex = mProgram.getUniformLocation("projectionMatrix");
mMMatIndex = mProgram.getUniformLocation("modelViewMatrix");
mTexIndex = mProgram.getUniformLocation("tex");
mClrIndex = mProgram.getUniformLocation("textColor");

mGlyphLists.resize(MAX_FONT_SIZE-MIN_FONT_SIZE+1, GlyphList());
}

font_impl::~font_impl()
{
destroyGLResources();
if (mProgram) glDeleteProgram(mProgram);
}

void font_impl::setOthro2D(int pWidth, int pHeight)
Expand Down Expand Up @@ -428,7 +427,7 @@ void font_impl::render(int pWindowId,
glDepthFunc(GL_ALWAYS);
glEnable(GL_BLEND);
glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
glUseProgram(mProgram);
mProgram.bind();

glUniformMatrix4fv(mPMatIndex, 1, GL_FALSE, (GLfloat*)&mProjMat);
glUniform4fv(mClrIndex, 1, pColor);
Expand Down Expand Up @@ -482,7 +481,7 @@ void font_impl::render(int pWindowId,

unbindResources();

glUseProgram(0);
mProgram.unbind();
glDisable(GL_BLEND);
glDepthMask(GL_TRUE);
glDepthFunc(GL_LESS);
Expand Down
2 changes: 1 addition & 1 deletion src/backend/opengl/font_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class font_impl {
bool mIsFontLoaded;
FontAtlas* mAtlas;
GLuint mVBO;
GLuint mProgram;
ShaderProgram mProgram;
int mOrthoW;
int mOrthoH;

Expand Down
36 changes: 14 additions & 22 deletions src/backend/opengl/histogram_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,29 +70,25 @@ void histogram_impl::unbindResources() const

histogram_impl::histogram_impl(const uint pNBins, const fg::dtype pDataType)
: mDataType(pDataType), mGLType(dtype2gl(mDataType)), mNBins(pNBins),
mProgram(0), mYMaxIndex(-1), mNBinsIndex(-1), mMatIndex(-1), mPointIndex(-1),
mProgram(glsl::histogram_vs.c_str(), glsl::histogram_fs.c_str()),
mYMaxIndex(-1), mNBinsIndex(-1), mMatIndex(-1), mPointIndex(-1),
mFreqIndex(-1), mColorIndex(-1), mAlphaIndex(-1), mPVCIndex(-1), mPVAIndex(-1),
mBColorIndex(-1)
{
CheckGL("Begin histogram_impl::histogram_impl");
mIsPVCOn = false;
mIsPVAOn = false;

setColor(0.8f, 0.6f, 0.0f, 1.0f);
mLegend = std::string("");

mProgram = initShaders(glsl::histogram_vs.c_str(), glsl::histogram_fs.c_str());

mYMaxIndex = glGetUniformLocation(mProgram, "ymax" );
mNBinsIndex = glGetUniformLocation(mProgram, "nbins" );
mMatIndex = glGetUniformLocation(mProgram, "transform");
mPVCIndex = glGetUniformLocation(mProgram, "isPVCOn" );
mPVAIndex = glGetUniformLocation(mProgram, "isPVAOn" );
mBColorIndex = glGetUniformLocation(mProgram, "barColor" );
mPointIndex = glGetAttribLocation (mProgram, "point" );
mFreqIndex = glGetAttribLocation (mProgram, "freq" );
mColorIndex = glGetAttribLocation (mProgram, "color" );
mAlphaIndex = glGetAttribLocation (mProgram, "alpha" );
mYMaxIndex = mProgram.getUniformLocation("ymax" );
mNBinsIndex = mProgram.getUniformLocation("nbins" );
mMatIndex = mProgram.getUniformLocation("transform");
mPVCIndex = mProgram.getUniformLocation("isPVCOn" );
mPVAIndex = mProgram.getUniformLocation("isPVAOn" );
mBColorIndex = mProgram.getUniformLocation("barColor" );
mPointIndex = mProgram.getAttributeLocation("point");
mFreqIndex = mProgram.getAttributeLocation("freq" );
mColorIndex = mProgram.getAttributeLocation("color");
mAlphaIndex = mProgram.getAttributeLocation("alpha");

mVBOSize = mNBins;
mCBOSize = 3*mVBOSize;
Expand Down Expand Up @@ -127,10 +123,6 @@ histogram_impl::~histogram_impl()
GLuint vao = it->second;
glDeleteVertexArrays(1, &vao);
}
glDeleteBuffers(1, &mVBO);
glDeleteBuffers(1, &mCBO);
glDeleteBuffers(1, &mABO);
glDeleteProgram(mProgram);
CheckGL("End histogram_impl::~histogram_impl");
}

Expand All @@ -143,7 +135,7 @@ void histogram_impl::render(const int pWindowId,
glEnable(GL_BLEND);
glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);

glUseProgram(mProgram);
mProgram.bind();

glUniform1f(mYMaxIndex, mRange[3]);
glUniform1f(mNBinsIndex, (GLfloat)mNBins);
Expand All @@ -159,7 +151,7 @@ void histogram_impl::render(const int pWindowId,
glDrawArraysInstanced(GL_TRIANGLE_FAN, 0, 4, mNBins);
histogram_impl::unbindResources();

glUseProgram(0);
mProgram.unbind();
glDisable(GL_BLEND);
glDepthMask(GL_TRUE);
CheckGL("End histogram_impl::render");
Expand Down
2 changes: 1 addition & 1 deletion src/backend/opengl/histogram_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class histogram_impl : public AbstractRenderable {
gl::GLenum mGLType;
gl::GLuint mNBins;
/* OpenGL Objects */
gl::GLuint mProgram;
ShaderProgram mProgram;
/* internal shader attributes for mProgram
* shader program to render histogram bars for each
* bin*/
Expand Down
Loading

0 comments on commit 6aaf3e7

Please sign in to comment.