From 41808f29c67e447aa88fdb585fe6830b42e3571a Mon Sep 17 00:00:00 2001 From: Kearwood Gilbert Date: Thu, 27 Apr 2017 17:48:12 -0700 Subject: [PATCH] Cleanup warnings and bugs found through static analysis --- kraken/KRAABB.cpp | 4 ++-- kraken/KRAudioManager.cpp | 3 +-- kraken/KRCamera.cpp | 10 +++++--- kraken/KRDirectionalLight.cpp | 4 ++++ kraken/KRMaterial.cpp | 2 +- kraken/KRMesh.cpp | 11 +++++---- kraken/KRNode.cpp | 4 ++-- kraken/KRShader.cpp | 18 ++++++++++----- kraken/KRTexture.cpp | 43 +++++++++++++++++------------------ kraken/KRTextureManager.cpp | 9 -------- kraken/KRTextureTGA.cpp | 8 ------- 11 files changed, 57 insertions(+), 59 deletions(-) diff --git a/kraken/KRAABB.cpp b/kraken/KRAABB.cpp index cd05f92..74002d7 100644 --- a/kraken/KRAABB.cpp +++ b/kraken/KRAABB.cpp @@ -233,11 +233,11 @@ bool KRAABB::intersectsRay(const KRVector3 &v1, const KRVector3 &dir) const quadrant[i] = LEFT; candidatePlane[i] = min.c[i]; inside = false; - }else if (v1.c[i] > max.c[i]) { + } else if(v1.c[i] > max.c[i]) { quadrant[i] = RIGHT; candidatePlane[i] = max.c[i]; inside = false; - }else { + } else { quadrant[i] = MIDDLE; } diff --git a/kraken/KRAudioManager.cpp b/kraken/KRAudioManager.cpp index b264076..7890cf8 100644 --- a/kraken/KRAudioManager.cpp +++ b/kraken/KRAudioManager.cpp @@ -872,12 +872,11 @@ KRVector2 KRAudioManager::getNearestHRTFSample(const KRVector2 &dir) KRVector2 min_direction; bool first = true; - float min_distance; + float min_distance = 360.0f; for(std::vector::iterator itr = m_hrtf_sample_locations.begin(); itr != m_hrtf_sample_locations.end(); itr++) { if(first) { first = false; min_direction = (*itr); - min_distance = 360.0f; } else if((*itr).x == elevation) { float distance = fabs(dir_deg.y - (*itr).y); if(min_distance > distance) { diff --git a/kraken/KRCamera.cpp b/kraken/KRCamera.cpp index ebe184b..26c0f91 100644 --- a/kraken/KRCamera.cpp +++ b/kraken/KRCamera.cpp @@ -107,7 +107,7 @@ void KRCamera::renderFrame(float deltaTime, GLint renderBufferWidth, GLint rende GLint defaultFBO = -1; GLDEBUG(glGetIntegerv(GL_FRAMEBUFFER_BINDING, &defaultFBO)); - + createBuffers(renderBufferWidth, renderBufferHeight); KRScene &scene = getScene(); @@ -498,7 +498,7 @@ void KRCamera::renderFrame(float deltaTime, GLint renderBufferWidth, GLint rende // fprintf(stderr, "VBO Mem: %i Kbyte Texture Mem: %i/%i Kbyte (active/total) Shader Handles: %i Visible Bounds: %i Max Texture LOD: %i\n", (int)m_pContext->getMeshManager()->getMemUsed() / 1024, (int)m_pContext->getTextureManager()->getActiveMemUsed() / 1024, (int)m_pContext->getTextureManager()->getMemUsed() / 1024, (int)m_pContext->getShaderManager()->getShaderHandlesUsed(), (int)m_visibleBounds.size(), m_pContext->getTextureManager()->getLODDimCap()); GL_PUSH_GROUP_MARKER("Post Processing"); - + GLDEBUG(glBindFramebuffer(GL_FRAMEBUFFER, defaultFBO)); renderPost(); m_pContext->getMeshManager()->unbindVBO(); @@ -507,6 +507,7 @@ void KRCamera::renderFrame(float deltaTime, GLint renderBufferWidth, GLint rende #if GL_EXT_discard_framebuffer + GLenum attachments[2] = {GL_DEPTH_ATTACHMENT, GL_COLOR_ATTACHMENT0}; GLDEBUG(glDiscardFramebufferEXT(GL_FRAMEBUFFER, 2, attachments)); #endif @@ -671,7 +672,9 @@ void KRCamera::renderPost() GLDEBUG(glDisable(GL_BLEND)); - +/* + FINDME - Determine if we still need this... + static const GLfloat squareVerticesShadow[3][8] = {{ -1.0f, -1.0f, -0.60f, -1.0f, @@ -688,6 +691,7 @@ void KRCamera::renderPost() 0.00f, -0.60f, 0.40f, -0.60f, }}; + */ GLDEBUG(glViewport(0, 0, m_viewport.getSize().x, m_viewport.getSize().y)); diff --git a/kraken/KRDirectionalLight.cpp b/kraken/KRDirectionalLight.cpp index b68185a..647e84a 100644 --- a/kraken/KRDirectionalLight.cpp +++ b/kraken/KRDirectionalLight.cpp @@ -44,11 +44,15 @@ int KRDirectionalLight::configureShadowBufferViewports(const KRViewport &viewpor const float KRENGINE_SHADOW_BOUNDS_EXTRA_SCALE = 1.25f; // Scale to apply to view frustrum bounds so that we don't need to refresh shadows on every frame int cShadows = 1; for(int iShadow=0; iShadow < cShadows; iShadow++) { + /* + TODO - Determine if we still need this... + GLfloat shadowMinDepths[3][3] = {{0.0f, 0.0f, 0.0f},{0.0f, 0.0f, 0.0f},{0.0f, 0.05f, 0.3f}}; GLfloat shadowMaxDepths[3][3] = {{0.0f, 0.0f, 1.0f},{0.1f, 0.0f, 0.0f},{0.1f, 0.3f, 1.0f}}; float min_depth = 0.0f; float max_depth = 1.0f; + */ KRAABB worldSpacefrustrumSliceBounds = KRAABB(KRVector3(-1.0f, -1.0f, -1.0f), KRVector3(1.0f, 1.0f, 1.0f), KRMat4::Invert(viewport.getViewProjectionMatrix())); worldSpacefrustrumSliceBounds.scale(KRENGINE_SHADOW_BOUNDS_EXTRA_SCALE); diff --git a/kraken/KRMaterial.cpp b/kraken/KRMaterial.cpp index b9b3a84..d576c2a 100644 --- a/kraken/KRMaterial.cpp +++ b/kraken/KRMaterial.cpp @@ -34,7 +34,7 @@ #include "KRMaterial.h" #include "KRTextureManager.h" -#include "KRcontext.h" +#include "KRContext.h" KRMaterial::KRMaterial(KRContext &context, const char *szName) : KRResource(context, szName) { m_name = szName; diff --git a/kraken/KRMesh.cpp b/kraken/KRMesh.cpp index d75f962..4bf67c7 100644 --- a/kraken/KRMesh.cpp +++ b/kraken/KRMesh.cpp @@ -173,11 +173,14 @@ void KRMesh::getMaterials() for(std::vector::iterator itr = m_submeshes.begin(); itr != m_submeshes.end(); itr++) { const char *szMaterialName = (*itr)->szMaterialName; - KRMaterial *pMaterial = getContext().getMaterialManager()->getMaterial(szMaterialName); + KRMaterial *pMaterial = nullptr; + if(*szMaterialName != '\0') { + pMaterial = getContext().getMaterialManager()->getMaterial(szMaterialName); + } m_materials.push_back(pMaterial); if(pMaterial) { m_uniqueMaterials.insert(pMaterial); - } else { + } else if(*szMaterialName != '\0') { KRContext::Log(KRContext::LOG_LEVEL_WARNING, "Missing material: %s", szMaterialName); } } @@ -1456,7 +1459,7 @@ void KRMesh::convertToIndexed() } } - delete szKey; + delete[] szKey; KRContext::Log(KRContext::LOG_LEVEL_INFORMATION, "Convert to indexed, before: %i after: %i (%.2f%% saving)", getHeader()->vertex_count, mi.vertices.size(), ((float)getHeader()->vertex_count - (float)mi.vertices.size()) / (float)getHeader()->vertex_count * 100.0f); @@ -1533,7 +1536,7 @@ void KRMesh::optimizeIndexes() pack_header *header = getHeader(); __uint16_t *index_data = getIndexData(); - unsigned char *vertex_data = getVertexData(); + // unsigned char *vertex_data = getVertexData(); // Uncomment when re-enabling Step 2 below for(int submesh_index=0; submesh_index < header->submesh_count; submesh_index++) { pack_material *submesh = getSubmesh(submesh_index); diff --git a/kraken/KRNode.cpp b/kraken/KRNode.cpp index e979a1a..e78992f 100644 --- a/kraken/KRNode.cpp +++ b/kraken/KRNode.cpp @@ -402,9 +402,9 @@ KRNode *KRNode::LoadXML(KRScene &scene, tinyxml2::XMLElement *e) { const char *szName = e->Attribute("name"); if(strcmp(szElementName, "node") == 0) { new_node = new KRNode(scene, szName); - } if(strcmp(szElementName, "lod_set") == 0) { + } else if(strcmp(szElementName, "lod_set") == 0) { new_node = new KRLODSet(scene, szName); - } if(strcmp(szElementName, "lod_group") == 0) { + } else if(strcmp(szElementName, "lod_group") == 0) { new_node = new KRLODGroup(scene, szName); } else if(strcmp(szElementName, "point_light") == 0) { new_node = new KRPointLight(scene, szName); diff --git a/kraken/KRShader.cpp b/kraken/KRShader.cpp index e4a6f69..4c1dc54 100644 --- a/kraken/KRShader.cpp +++ b/kraken/KRShader.cpp @@ -128,11 +128,12 @@ KRShader::KRShader(KRContext &context, char *szKey, std::string options, std::st GLDEBUG(glCompileShader(vertexShader)); // Report any compile issues to stderr - GLint logLength; + GLint logLength = 0; GLDEBUG(glGetShaderiv(vertexShader, GL_INFO_LOG_LENGTH, &logLength)); if (logLength > 0) { GLchar *log = (GLchar *)malloc(logLength + 1); assert(log != NULL); + log[0] = '\0'; // In case glGetShaderInfoLog fails GLDEBUG(glGetShaderInfoLog(vertexShader, logLength, &logLength, log)); log[logLength] = '\0'; KRContext::Log(KRContext::LOG_LEVEL_ERROR, "KREngine - Failed to compile vertex shader: %s\nShader compile log:\n%s", szKey, log); @@ -146,10 +147,12 @@ KRShader::KRShader(KRContext &context, char *szKey, std::string options, std::st GLDEBUG(glCompileShader(fragShader)); // Report any compile issues to stderr + logLength = 0; // In case glGetShaderiv fails GLDEBUG(glGetShaderiv(fragShader, GL_INFO_LOG_LENGTH, &logLength)); if (logLength > 0) { GLchar *log = (GLchar *)malloc(logLength + 1); assert(log != NULL); + log[0] = '\0'; // In case glGetShaderInfoLog fails GLDEBUG(glGetShaderInfoLog(fragShader, logLength, &logLength, log)); log[logLength] = '\0'; KRContext::Log(KRContext::LOG_LEVEL_ERROR, "KREngine - Failed to compile fragment shader: %s\nShader compile log:\n%s", szKey, log); @@ -181,12 +184,13 @@ KRShader::KRShader(KRContext &context, char *szKey, std::string options, std::st if(link_success != GL_TRUE) { // Report any linking issues to stderr KRContext::Log(KRContext::LOG_LEVEL_ERROR, "KREngine - Failed to link shader program: %s", szKey); - + logLength = 0; // In case glGetProgramiv fails GLDEBUG(glGetProgramiv(m_iProgram, GL_INFO_LOG_LENGTH, &logLength)); if (logLength > 0) { GLchar *log = (GLchar *)malloc(logLength + 1); assert(log != NULL); + log[0] = '\0'; // In case glGetProgramInfoLog fails GLDEBUG(glGetProgramInfoLog(m_iProgram, logLength, &logLength, log)); log[logLength] = '\0'; KRContext::Log(KRContext::LOG_LEVEL_ERROR, "Program link log:\n%s", log); @@ -363,8 +367,8 @@ bool KRShader::bind(KRCamera &camera, const KRViewport &viewport, const KRMat4 & setUniform(KRENGINE_UNIFORM_ABSOLUTE_TIME, getContext().getAbsoluteTime()); int light_directional_count = 0; - int light_point_count = 0; - int light_spot_count = 0; + //int light_point_count = 0; + //int light_spot_count = 0; // TODO - Need to support multiple lights and more light types in forward rendering if(renderPass != KRNode::RENDER_PASS_DEFERRED_LIGHTS && renderPass != KRNode::RENDER_PASS_DEFERRED_GBUFFER && renderPass != KRNode::RENDER_PASS_DEFERRED_OPAQUE && renderPass != KRNode::RENDER_PASS_GENERATE_SHADOWMAPS) { @@ -422,8 +426,8 @@ bool KRShader::bind(KRCamera &camera, const KRViewport &viewport, const KRMat4 & light_directional_count++; } - light_point_count = point_lights.size(); - light_spot_count = spot_lights.size(); + //light_point_count = point_lights.size(); + //light_spot_count = spot_lights.size(); } @@ -566,11 +570,13 @@ bool KRShader::bind(KRCamera &camera, const KRViewport &viewport, const KRMat4 & GLDEBUG(glGetProgramiv(m_iProgram, GL_VALIDATE_STATUS, &validate_status)); if(validate_status != GL_TRUE) { KRContext::Log(KRContext::LOG_LEVEL_ERROR, "KREngine - Failed to validate shader program: %s", m_szKey); + logLength = 0; // In case glGetProgramiv fails GLDEBUG(glGetProgramiv(m_iProgram, GL_INFO_LOG_LENGTH, &logLength)); if (logLength > 0) { GLchar *log = (GLchar *)malloc(logLength + 1); assert(log != NULL); + log[0] = '\0'; // In case glGetProgramInfoLog fails GLDEBUG(glGetProgramInfoLog(m_iProgram, logLength, &logLength, log)); log[logLength] = '\0'; KRContext::Log(KRContext::LOG_LEVEL_ERROR, "Program validate log:\n%s", log); diff --git a/kraken/KRTexture.cpp b/kraken/KRTexture.cpp index c6e1e6f..5c9570e 100644 --- a/kraken/KRTexture.cpp +++ b/kraken/KRTexture.cpp @@ -65,33 +65,32 @@ long KRTexture::getReferencedMemSize() { void KRTexture::resize(int max_dim) { - while(m_handle_lock.test_and_set()); // Spin lock - { - if(m_iHandle == m_iNewHandle) { - if(max_dim == 0) { - m_iNewHandle = 0; - } else { - int target_dim = max_dim; - if(target_dim < m_min_lod_max_dim) target_dim = m_min_lod_max_dim; + while(m_handle_lock.test_and_set()) {}; // Spin lock - if(m_new_lod_max_dim != target_dim || (m_iHandle == 0 && m_iNewHandle == 0)) { - assert(m_newTextureMemUsed == 0); - m_newTextureMemUsed = getMemRequiredForSize(target_dim); - - getContext().getTextureManager()->memoryChanged(m_newTextureMemUsed); - getContext().getTextureManager()->addMemoryTransferredThisFrame(m_newTextureMemUsed); - - if(!createGLTexture(target_dim)) { - getContext().getTextureManager()->memoryChanged(-m_newTextureMemUsed); - m_newTextureMemUsed = 0; - assert(false); // Failed to create the texture - } + if(m_iHandle == m_iNewHandle) { + if(max_dim == 0) { + m_iNewHandle = 0; + } else { + int target_dim = max_dim; + if(target_dim < m_min_lod_max_dim) target_dim = m_min_lod_max_dim; + + if(m_new_lod_max_dim != target_dim || (m_iHandle == 0 && m_iNewHandle == 0)) { + assert(m_newTextureMemUsed == 0); + m_newTextureMemUsed = getMemRequiredForSize(target_dim); + + getContext().getTextureManager()->memoryChanged(m_newTextureMemUsed); + getContext().getTextureManager()->addMemoryTransferredThisFrame(m_newTextureMemUsed); + + if(!createGLTexture(target_dim)) { + getContext().getTextureManager()->memoryChanged(-m_newTextureMemUsed); + m_newTextureMemUsed = 0; + assert(false); // Failed to create the texture } } } - - m_handle_lock.clear(); } + + m_handle_lock.clear(); } GLuint KRTexture::getHandle() { diff --git a/kraken/KRTextureManager.cpp b/kraken/KRTextureManager.cpp index 79ac0ba..5e4ae0c 100644 --- a/kraken/KRTextureManager.cpp +++ b/kraken/KRTextureManager.cpp @@ -148,15 +148,6 @@ KRTexture *KRTextureManager::getTextureCube(const char *szName) { if(itr == m_textures.end()) { // Defer resolving the texture cube until its referenced textures are ready - const GLenum TARGETS[6] = { - GL_TEXTURE_CUBE_MAP_POSITIVE_X, - GL_TEXTURE_CUBE_MAP_NEGATIVE_X, - GL_TEXTURE_CUBE_MAP_POSITIVE_Y, - GL_TEXTURE_CUBE_MAP_NEGATIVE_Y, - GL_TEXTURE_CUBE_MAP_POSITIVE_Z, - GL_TEXTURE_CUBE_MAP_NEGATIVE_Z - }; - const char *SUFFIXES[6] = { "_positive_x", "_negative_x", diff --git a/kraken/KRTextureTGA.cpp b/kraken/KRTextureTGA.cpp index 6f1c237..892d157 100644 --- a/kraken/KRTextureTGA.cpp +++ b/kraken/KRTextureTGA.cpp @@ -95,12 +95,6 @@ bool KRTextureTGA::uploadTexture(GLenum target, int lod_max_dim, int ¤t_lo m_pData->lock(); TGA_HEADER *pHeader = (TGA_HEADER *)m_pData->getStart(); unsigned char *pData = (unsigned char *)pHeader + (long)pHeader->idlength + (long)pHeader->colourmaplength * (long)pHeader->colourmaptype + sizeof(TGA_HEADER); - -#if TARGET_OS_IPHONE - GLenum base_internal_format = GL_BGRA; -#else - GLenum base_internal_format = pHeader->bitsperpixel == 24 ? GL_BGR : GL_BGRA; -#endif GLenum internal_format = GL_RGBA; @@ -115,8 +109,6 @@ bool KRTextureTGA::uploadTexture(GLenum target, int lod_max_dim, int ¤t_lo return false; // Mapped colors not supported } - GLenum err; - switch(pHeader->imagetype) { case 2: // rgb switch(pHeader->bitsperpixel) {