diff --git a/src/render/PointArray.cpp b/src/render/PointArray.cpp index 546726a9..e93c99a3 100644 --- a/src/render/PointArray.cpp +++ b/src/render/PointArray.cpp @@ -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; @@ -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); @@ -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); diff --git a/src/render/View3D.cpp b/src/render/View3D.cpp index 1927cbea..b5ea4aa8 100644 --- a/src/render/View3D.cpp +++ b/src/render/View3D.cpp @@ -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) diff --git a/src/render/View3D.h b/src/render/View3D.h index 88cd1c45..13df926b 100644 --- a/src/render/View3D.h +++ b/src/render/View3D.h @@ -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); diff --git a/src/render/glutil.cpp b/src/render/glutil.cpp index 90c738d9..a46f7261 100644 --- a/src/render/glutil.cpp +++ b/src/render/glutil.cpp @@ -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; @@ -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); } diff --git a/src/render/glutil.h b/src/render/glutil.h index 676b4e6d..dcf1d2ae 100644 --- a/src/render/glutil.h +++ b/src/render/glutil.h @@ -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 @@ -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(); }