Skip to content

Commit

Permalink
Merge pull request #130 from c42f/opengl-framebuffer-cleanup
Browse files Browse the repository at this point in the history 
Opengl framebuffer cleanup
  • Loading branch information
@c42f
c42f authored Sep 6, 2016
2 parents 38fbb0b + 6cfb4fd commit 30cf200
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 98 deletions.
93 changes: 47 additions & 46 deletions src/render/PointArray.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,18 @@ DrawCount PointArray::drawPoints(QGLShaderProgram& prog, const TransformState& t
for (size_t i = 0; i < attributes.size(); ++i)
{
if (attributes[i])
prog.enableAttributeArray(attributes[i]->location);
glEnableVertexAttribArray(attributes[i]->location);
}

// Compute number of bytes required to store all attributes of a vertex, in
// bytes.
size_t perVertexBytes = 0;
for (size_t i = 0; i < m_fields.size(); ++i)
{
const GeomField &field = m_fields[i];
unsigned int arraySize = field.spec.arraySize();
unsigned int vecSize = field.spec.vectorSize();
perVertexBytes += arraySize * vecSize * field.spec.elsize; //sizeof(glBaseType(field.spec));
}

DrawCount drawCount;
Expand Down Expand Up @@ -634,85 +645,75 @@ DrawCount PointArray::drawPoints(QGLShaderProgram& prog, const TransformState& t
}
continue;
}
size_t idx = node->beginIndex;
if (!incrementalDraw)
node->nextBeginIndex = node->beginIndex;

DrawCount nodeDrawCount = node->drawCount(relCamera, quality, incrementalDraw);
drawCount += nodeDrawCount;

idx = node->nextBeginIndex;
if (nodeDrawCount.numVertices == 0)
continue;

if (m_fields.size() < 1)
continue;

long bufferSize = 0;
for (size_t i = 0; i < m_fields.size(); ++i)
{
const GeomField &field = m_fields[i];
unsigned int arraySize = field.spec.arraySize();
unsigned int vecSize = field.spec.vectorSize();

// tfm::printfln("FIELD-NAME: %s", field.name);
// tfm::printfln("AS: %i, VS: %i, FSS: %i, FSES: %i, GLBTFSS: %i", arraySize, vecSize, field.spec.size(), field.spec.elsize, sizeof(glBaseType(field.spec)));

bufferSize += arraySize * vecSize * field.spec.elsize; //sizeof(glBaseType(field.spec));
}

bufferSize = bufferSize * (GLsizei)nodeDrawCount.numVertices;
// Create a new uninitialized buffer for the current node, reserving
// enough space for the entire set of vertex attributes which will be
// passed to the shader.
//
// (This new memory area will be bound to the "point_buffer" VBO until
// the memory is orphaned by calling glBufferData() next time through
// the loop. The orphaned memory should be cleaned up by the driver,
// and this may actually be quite efficient, see
// http://stackoverflow.com/questions/25111565/how-to-deallocate-glbufferdata-memory
// http://hacksoflife.blogspot.com.au/2015/06/glmapbuffer-no-longer-cool.html )
GLsizeiptr nodeBufferSize = perVertexBytes * nodeDrawCount.numVertices;
glBufferData(GL_ARRAY_BUFFER, nodeBufferSize, NULL, GL_STREAM_DRAW);

// TODO: might be able to do something more efficient here, for example use glBufferSubData to avoid re-allocation of memory by glBufferData
// INITIALIZE THE BUFFER TO FULL SIZE
// tfm::printfln("INIT BUFFER: %i, BS: %i", vbo, bufferSize);

glBufferData(GL_ARRAY_BUFFER, (GLsizeiptr)bufferSize, NULL, GL_STREAM_DRAW);
/// ========================================================================
/// ========================================================================
GLintptr bufferOffset = 0;

for (size_t i = 0, k = 0; i < m_fields.size(); k+=m_fields[i].spec.arraySize(), ++i)
{
const GeomField& field = m_fields[i];
int arraySize = field.spec.arraySize();
int vecSize = field.spec.vectorSize();

// TODO: should use a single data-array that isn't split into vertex / normal / color / etc. sections, but has interleaved data ?
// OpenGL has a stride value in glVertexAttribPointer for exactly this purpose, which should be used for better efficiency
// here we write only the current attribute data into this the buffer (e.g. all positions, then all colors)
bufferSize = arraySize * vecSize * field.spec.elsize * (GLsizei)nodeDrawCount.numVertices; //sizeof(glBaseType(field.spec))

char* bufferData = field.data.get() + idx*field.spec.size();
glBufferSubData(GL_ARRAY_BUFFER, bufferOffset, bufferSize, bufferData);

// tfm::printfln("UPDATE BUFFER: %i, BS: %i", vbo, bufferSize);

// TODO?: Could use a single data-array that isn't split into
// vertex / normal / color / etc. sections, but has interleaved
// data ? OpenGL has a stride value in glVertexAttribPointer for
// exactly this purpose, which should be used for better efficiency
// here we write only the current attribute data into this the
// buffer (e.g. all positions, then all colors)
GLsizeiptr fieldBufferSize = arraySize * vecSize * field.spec.elsize * nodeDrawCount.numVertices;

// Upload raw data for `field` to the appropriate part of the buffer.
char* bufferData = field.data.get() + node->nextBeginIndex*field.spec.size();
glBufferSubData(GL_ARRAY_BUFFER, bufferOffset, fieldBufferSize, bufferData);

// Tell OpenGL how to interpret the buffer of raw data which was
// just uploaded. This should be a single call, but OpenGL spec
// insanity says we need `arraySize` calls (though arraySize=1
// for most usage.)
for (int j = 0; j < arraySize; ++j)
{
const ShaderAttribute* attr = attributes[k+j];
if (!attr)
continue;

// we have to create an intermediate buffer offsets for glVertexAttribPointer, but we can still upload the whole data array earlier !?
GLintptr intermediate_bufferOffset = bufferOffset + j*field.spec.elsize;
GLintptr arrayElementOffset = bufferOffset + j*field.spec.elsize;

if (attr->baseType == TypeSpec::Int || attr->baseType == TypeSpec::Uint)
{
glVertexAttribIPointer(attr->location, vecSize,
glBaseType(field.spec), 0, (const GLvoid *)intermediate_bufferOffset);
glVertexAttribIPointer(attr->location, vecSize, glBaseType(field.spec),
0, (const GLvoid *)arrayElementOffset);
}
else
{
glVertexAttribPointer(attr->location, vecSize,
glBaseType(field.spec),
field.spec.fixedPoint, 0, (const GLvoid *)intermediate_bufferOffset);
glVertexAttribPointer(attr->location, vecSize, glBaseType(field.spec),
field.spec.fixedPoint, 0, (const GLvoid *)arrayElementOffset);
}

glEnableVertexAttribArray(attr->location);
}

bufferOffset += bufferSize;
bufferOffset += fieldBufferSize;
}

glDrawArrays(GL_POINTS, 0, (GLsizei)nodeDrawCount.numVertices);
Expand All @@ -725,7 +726,7 @@ DrawCount PointArray::drawPoints(QGLShaderProgram& prog, const TransformState& t
for (size_t i = 0; i < attributes.size(); ++i)
{
if (attributes[i])
prog.disableAttributeArray(attributes[i]->location);
glDisableVertexAttribArray(attributes[i]->location);
}

glBindBuffer(GL_ARRAY_BUFFER, 0);
Expand Down
42 changes: 0 additions & 42 deletions src/render/View3D.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,52 +319,10 @@ void View3D::resizeGL(int w, int h)
m_camera.setViewport(QRect(0,0,double(w)/dPR,double(h)/dPR));

m_incrementalFramebuffer.init(w,h);

glCheckError();
}


unsigned int View3D::allocIncrementalFramebuffer(int w, int h) const
{
if (w < 1 || h < 1)
return 0;

//should we really delete this every time?
GLuint fb;
glDeleteFramebuffers(1, &fb);

const QGLFormat fmt = context()->format();

glGenFramebuffers(1, &fb);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fb);

// TODO:
// * Should we use multisampling 1 to avoid binding to a texture?

// Intel HD 3000 driver doesn't like the multisampling mode that Qt 4.8 uses
// for samples==1, so work around it by forcing 0, if possible

// requires OpenGL 4
//glFramebufferParameteri(GL_DRAW_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_WIDTH, w);
//glFramebufferParameteri(GL_DRAW_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_HEIGHT, h);
//glFramebufferParameteri(GL_DRAW_FRAMEBUFFER, GL_FRAMEBUFFER_DEFAULT_SAMPLES, fmt.samples() > 1 ? fmt.samples() : 0);

GLuint c_buff;
glGenRenderbuffers(1, &c_buff);
glBindRenderbuffer(GL_RENDERBUFFER, c_buff);
glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA8, w, h);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_RENDERBUFFER, c_buff);

GLuint z_buff;
glGenRenderbuffers(1, &z_buff);
glBindRenderbuffer(GL_RENDERBUFFER, z_buff);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_COMPONENT24, w, h);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, z_buff);

return fb;
}


void View3D::paintGL()
{
if (m_badOpenGL)
Expand Down
1 change: 0 additions & 1 deletion src/render/View3D.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ class View3D : public QGLWidget

private:
double getDevicePixelRatio();
unsigned int allocIncrementalFramebuffer(int w, int h) const;
void initializeGLGeometry(int begin, int end);

void initCursor(float cursorRadius, float centerPointRadius);
Expand Down
16 changes: 10 additions & 6 deletions src/render/glutil.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,16 +307,20 @@ void _glError(const char *file, int line) {
}
}

void _glFrameBufferStatus(GLenum target, const char *file, int line) {
void _glFrameBufferStatus(GLenum target, const char *file, int line)
{
GLenum fbStatus = glCheckFramebufferStatus(target);

GLenum fb_status(glCheckFramebufferStatus(target));
if (fbStatus == GL_FRAMEBUFFER_COMPLETE)
return;

std::string status;

switch(fb_status) {
switch(fbStatus) {
case GL_INVALID_ENUM: status="?? (bad target)"; break;
case GL_FRAMEBUFFER_COMPLETE: status="COMPLETE"; break;
case GL_FRAMEBUFFER_UNDEFINED: status="UNDEFINED"; break;
case GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT: status="INCOMPLETE_ATTACHMENT"; break;
case GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT: status="INCOMPLETE_ATTACHMENT"; break;
//case GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT: status="INCOMPLETE_MISSING_ATTACHMENT"; break;
case GL_FRAMEBUFFER_INCOMPLETE_DRAW_BUFFER: status="INCOMPLETE_DRAW_BUFFER"; break;
case GL_FRAMEBUFFER_INCOMPLETE_READ_BUFFER: status="INCOMPLETE_READ_BUFFER"; break;
Expand All @@ -325,9 +329,9 @@ void _glFrameBufferStatus(GLenum target, const char *file, int line) {
//case GL_FRAMEBUFFER_INCOMPLETE_DIMENSIONS: status="INCOMPLETE_DIMENSIONS"; break;
case GL_FRAMEBUFFER_INCOMPLETE_MISSING_ATTACHMENT: status="INCOMPLETE_MISSING_ATTACHMENT"; break;
case GL_FRAMEBUFFER_UNSUPPORTED: status="UNSUPPORTED"; break;
default: status="INCOMPLETE"; break;
default: status="???"; break;
}

tfm::printfln("GL_FRAMEBUFFER_%s - %s:%i", status, file, line);
tfm::printfln("GL_FRAMEBUFFER_%s (%d) - %s:%i", status, fbStatus, file, line);
}

6 changes: 3 additions & 3 deletions src/render/glutil.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ void _glFrameBufferStatus(GLenum target, const char *file, int line);
#ifdef GL_CHECK

#define glCheckError() _glError(__FILE__,__LINE__)
#define glFrameBufferStatus(TARGET) _glFrameBufferStatus(TARGET, __FILE__,__LINE__)
#define glCheckFrameBufferStatus() _glFrameBufferStatus(GL_FRAMEBUFFER, __FILE__,__LINE__)

#else

#define glCheckError()
#define glFrameBufferStatus(TARGET)
#define glCheckFrameBufferStatus()

#endif //GL_CHECK

Expand Down Expand Up @@ -243,7 +243,7 @@ class Framebuffer
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_COMPONENT24, width, height);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, m_zBuf);

glFrameBufferStatus(m_incrementalFramebuffer.id());
glCheckFrameBufferStatus();
glCheckError();
}

Expand Down

0 comments on commit 30cf200

Please sign in to comment.