From e8f5e78811e4586118c8c44513fb006b81c4e720 Mon Sep 17 00:00:00 2001 From: baldurk Date: Sun, 30 Nov 2014 15:31:31 +0000 Subject: [PATCH] Check extensions by enum and centralised checking for 'hacks' * I need to double check the cubemap texture size query thing, I think I'm getting different results on a different nvidia driver (and maybe card). --- renderdoc/driver/gl/gl_common.cpp | 137 +++++++++++++++++++------ renderdoc/driver/gl/gl_common.h | 24 ++++- renderdoc/driver/gl/gl_driver.cpp | 46 ++++----- renderdoc/driver/gl/gl_manager.cpp | 10 +- renderdoc/driver/gl/gl_renderstate.cpp | 2 +- renderdoc/driver/gl/gl_shader_refl.cpp | 2 +- 6 files changed, 155 insertions(+), 66 deletions(-) diff --git a/renderdoc/driver/gl/gl_common.cpp b/renderdoc/driver/gl/gl_common.cpp index 9fb411381..042a37dbf 100644 --- a/renderdoc/driver/gl/gl_common.cpp +++ b/renderdoc/driver/gl/gl_common.cpp @@ -45,28 +45,122 @@ namespace TrackedResource namespace ExtensionSupport { - static vector extensions; + static bool extensions[ExtensionSupported_Count]; + static bool VendorChecks[VendorCheck_Count]; }; int GLCoreVersion = 0; -void UpdateExtensionSupport(const GLHookSet &gl) +void DoVendorChecks(const GLHookSet &gl) { - ExtensionSupport::extensions.clear(); GLint numExts = 0; gl.glGetIntegerv(eGL_NUM_EXTENSIONS, &numExts); - ExtensionSupport::extensions.resize(numExts); + + RDCEraseEl(ExtensionSupport::extensions); + RDCEraseEl(ExtensionSupport::VendorChecks); + for(int i=0; i < numExts; i++) - ExtensionSupport::extensions[i] = (const char *)gl.glGetStringi(eGL_EXTENSIONS, (GLuint)i); + { + const char *ext = (const char *)gl.glGetStringi(eGL_EXTENSIONS, (GLuint)i); + + if(ext == NULL || !ext[0] || !ext[1] || !ext[2] || !ext[3]) continue; + + ext += 3; + +#define EXT_CHECK(extname) if(!strcmp(ext, STRINGIZE(extname))) ExtensionSupport::extensions[CONCAT(ExtensionSupported_, extname)] = true; + + EXT_CHECK(ARB_clip_control) + EXT_CHECK(ARB_enhanced_layouts) + +#undef EXT_CHECK + } + + ////////////////////////////////////////////////////////// + // version/driver/vendor specific hacks and checks go here + // doing these in a central place means they're all documented and + // can be removed ASAP from a single place. + // It also means any work done to figure them out is only ever done + // in one place, when first activating a new context, so hopefully + // shouldn't interfere with the running program + + + // The linux AMD driver doesn't recognise GL_VERTEX_BINDING_BUFFER. + // However it has a "two wrongs make a right" type deal. Instead of returning the buffer that the + // i'th index is bound to (as above, vbslot) for GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, it returns the i'th + // vertex buffer which is exactly what we wanted from GL_VERTEX_BINDING_BUFFER! + // see: http://devgurus.amd.com/message/1306745#1306745 + + { + // clear all error flags. + GLenum err = gl.glGetError(); + while(err != eGL_NONE) err = gl.glGetError(); + + GLint dummy = 0; + gl.glGetIntegeri_v(eGL_VERTEX_BINDING_BUFFER, 0, &dummy); + err = gl.glGetError(); + + if(err != eGL_NONE) + { + // if we got an error trying to query that, we should enable this hack + ExtensionSupport::VendorChecks[VendorCheck_AMD_vertex_buffer_query] = true; + + RDCWARN("Using AMD hack to avoid GL_VERTEX_BINDING_BUFFER"); + } + } + + { + // We need to determine if GL_TEXTURE_COMPRESSED_IMAGE_SIZE for a compressed cubemap face target + // will return the size of the whole cubemap, or just one face. Since we fetch the cubemap + // data face-by-face the distinction is important. + // So we create a 4x4 cubemap with no mips that's DXT1 (BC1) compressed, which is 0.5 bytes per pixel. + // So 4*4*0.5 = 8 bytes per face. If the returned size is 8 or 48 we can determine which result the + // query returns. It's probably safe to assume it's consistent then for all sizes and formats of + // cubemaps. + // I'm not sure what the correct answer is, intuitively it feels like when you query for the size of + // a single face target, it should give you the size of that face. The spec doesn't seem to say + // though + + GLuint prevtex = 0; // should almost certainly be 0, but let's be careful anyway. + gl.glGetIntegerv(eGL_TEXTURE_BINDING_CUBE_MAP, (GLint *)&prevtex); + + GLuint dummy = 0; + gl.glGenTextures(1, &dummy); + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, dummy); + + gl.glBindTexture(eGL_TEXTURE_CUBE_MAP, prevtex); + + gl.glTextureStorage2DEXT(dummy, eGL_TEXTURE_CUBE_MAP, 1, eGL_COMPRESSED_RGBA_S3TC_DXT1_EXT, 4, 4); + + GLint compSize = 0; + gl.glGetTextureLevelParameterivEXT(dummy, eGL_TEXTURE_CUBE_MAP_POSITIVE_X, 0, eGL_TEXTURE_COMPRESSED_IMAGE_SIZE, &compSize); + + if(compSize == 8) + { + ExtensionSupport::VendorChecks[VendorCheck_EXT_compressed_cube_size] = false; + } + else if(compSize == 48) + { + ExtensionSupport::VendorChecks[VendorCheck_EXT_compressed_cube_size] = true; + } + else + { + RDCERR("Unexpected compressed size of +X face of BC1 compressed 4x4 cubemap mip 0! %d", compSize); + } + + gl.glDeleteTextures(1, &dummy); + } } -bool ExtensionSupported(const char *ext) +bool ExtensionSupported(ExtensionCheckEnum ext) { - for(size_t i=0; i < ExtensionSupport::extensions.size(); i++) - if(ExtensionSupport::extensions[i] == ext) - return true; + if(ext < 0 || ext >= ExtensionSupported_Count) return false; + return ExtensionSupport::extensions[ext]; +} - return false; +bool VendorCheck(VendorCheckEnum vc) +{ + if(vc < 0 || vc >= VendorCheck_Count) return false; + return ExtensionSupport::VendorChecks[vc]; } size_t BufferIdx(GLenum buf) @@ -172,30 +266,9 @@ GLenum ShaderEnum(size_t idx) GLuint GetBoundVertexBuffer(const GLHookSet &gl, GLuint i) { - static int hackAMDBugVBBinding = -1; - - // linux AMD driver doesn't recognise GL_VERTEX_BINDING_BUFFER, see below. - if(hackAMDBugVBBinding == -1) - { - GLenum err = gl.glGetError(); - while(err != eGL_NONE) err = gl.glGetError(); - GLint dummy = 0; - gl.glGetIntegeri_v(eGL_VERTEX_BINDING_BUFFER, 0, &dummy); - err = gl.glGetError(); - - hackAMDBugVBBinding = (err == eGL_NONE ? 0 : 1); - - if(hackAMDBugVBBinding) - RDCWARN("Using AMD hack to avoid GL_VERTEX_BINDING_BUFFER"); - } - GLuint buffer = 0; - // the linux AMD driver has a "two wrongs make a right" type deal. Instead of returning the buffer that the - // i'th index is bound to (as above, vbslot) for GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, it returns the i'th - // vertex buffer which is exactly what we wanted from GL_VERTEX_BINDING_BUFFER! - // see: http://devgurus.amd.com/message/1306745#1306745 - if(hackAMDBugVBBinding) + if(VendorCheck(VendorCheck_AMD_vertex_buffer_query)) gl.glGetVertexAttribiv(i, eGL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, (GLint *)&buffer); else gl.glGetIntegeri_v(eGL_VERTEX_BINDING_BUFFER, i, (GLint *)&buffer); diff --git a/renderdoc/driver/gl/gl_common.h b/renderdoc/driver/gl/gl_common.h index ae6629ae7..665ac6731 100644 --- a/renderdoc/driver/gl/gl_common.h +++ b/renderdoc/driver/gl/gl_common.h @@ -85,8 +85,28 @@ GLenum MakeGLFormat(WrappedOpenGL &gl, GLenum target, ResourceFormat fmt); GLuint GetBoundVertexBuffer(const GLHookSet &gl, GLuint idx); extern int GLCoreVersion; -void UpdateExtensionSupport(const GLHookSet &gl); -bool ExtensionSupported(const char *ext); + +// extensions we know we want to check for are precached, indexd by this enum +enum ExtensionCheckEnum +{ + ExtensionSupported_ARB_enhanced_layouts = 0, + ExtensionSupported_ARB_clip_control, + ExtensionSupported_Count, +}; +bool ExtensionSupported(ExtensionCheckEnum ext); + +// for some things we need to know how a specific implementation behaves to work around it +// or adjust things. We centralise that here (similar to extensions) +enum VendorCheckEnum +{ + VendorCheck_AMD_vertex_buffer_query, + VendorCheck_EXT_compressed_cube_size, + VendorCheck_Count, +}; +bool VendorCheck(VendorCheckEnum ext); + +// fills out the extension supported array and the version-specific checks above +void DoVendorChecks(const GLHookSet &gl); #include "serialise/serialiser.h" #include "core/core.h" diff --git a/renderdoc/driver/gl/gl_driver.cpp b/renderdoc/driver/gl/gl_driver.cpp index f3bfcc95b..ad5d466a9 100644 --- a/renderdoc/driver/gl/gl_driver.cpp +++ b/renderdoc/driver/gl/gl_driver.cpp @@ -605,35 +605,35 @@ void WrappedOpenGL::ActivateContext(void *windowHandle, void *contextHandle) if(contextHandle) { - const GLHookSet &gl = m_Real; - - if(gl.glDebugMessageCallback && RenderDoc::Inst().GetCaptureOptions().DebugDeviceMode) - { - gl.glDebugMessageCallback(&DebugSnoopStatic, this); - gl.glEnable(eGL_DEBUG_OUTPUT_SYNCHRONOUS); - } - - if(gl.glGetIntegerv) - { - GLint mj = 0, mn = 0; - gl.glGetIntegerv(eGL_MAJOR_VERSION, &mj); - gl.glGetIntegerv(eGL_MINOR_VERSION, &mn); - - int ver = mj*10 + mn; - - if(ver > GLCoreVersion) - { - GLCoreVersion = ver; - UpdateExtensionSupport(gl); - } - } - FontData &font = m_Fonts[contextHandle]; if(!font.built) { font.built = true; + const GLHookSet &gl = m_Real; + + if(gl.glDebugMessageCallback && RenderDoc::Inst().GetCaptureOptions().DebugDeviceMode) + { + gl.glDebugMessageCallback(&DebugSnoopStatic, this); + gl.glEnable(eGL_DEBUG_OUTPUT_SYNCHRONOUS); + } + + if(gl.glGetIntegerv) + { + GLint mj = 0, mn = 0; + gl.glGetIntegerv(eGL_MAJOR_VERSION, &mj); + gl.glGetIntegerv(eGL_MINOR_VERSION, &mn); + + int ver = mj*10 + mn; + + if(ver > GLCoreVersion) + { + GLCoreVersion = ver; + DoVendorChecks(gl); + } + } + if(gl.glGenTextures && gl.glTextureStorage2DEXT && gl.glTextureSubImage2DEXT && gl.glGenVertexArrays && gl.glBindVertexArray && gl.glGenBuffers && gl.glNamedBufferStorageEXT && diff --git a/renderdoc/driver/gl/gl_manager.cpp b/renderdoc/driver/gl/gl_manager.cpp index bfdc0a923..327ee3b7f 100644 --- a/renderdoc/driver/gl/gl_manager.cpp +++ b/renderdoc/driver/gl/gl_manager.cpp @@ -448,14 +448,10 @@ bool GLResourceManager::Serialise_InitialState(GLResource res) size_t size = compSize; - // cubemaps return the compressed image size for the whole texture, but we read it + // sometimes cubemaps return the compressed image size for the whole texture, but we read it // face by face - // - // Disabled since it seems nvidia doesn't return the whole image size, but just the size per face, - // and it's not clear which is right (or how to handle that anyway). Worst case this means we serialise out - // too much data (since only the first face of data will be used). - //if(t == eGL_TEXTURE_CUBE_MAP) - // size /= 6; + if(VendorCheck(VendorCheck_EXT_compressed_cube_size) && t == eGL_TEXTURE_CUBE_MAP) + size /= 6; byte *buf = new byte[size]; diff --git a/renderdoc/driver/gl/gl_renderstate.cpp b/renderdoc/driver/gl/gl_renderstate.cpp index 12ca21174..7b0cc0fcf 100644 --- a/renderdoc/driver/gl/gl_renderstate.cpp +++ b/renderdoc/driver/gl/gl_renderstate.cpp @@ -114,7 +114,7 @@ void GLRenderState::FetchState() m_Real->glGetFloatv(eGL_POINT_SIZE, &PointSize); m_Real->glGetIntegerv(eGL_PRIMITIVE_RESTART_INDEX, (GLint *)&PrimitiveRestartIndex); - if(GLCoreVersion >= 45 || ExtensionSupported("GL_ARB_clip_control")) + if(GLCoreVersion >= 45 || ExtensionSupported(ExtensionSupported_ARB_clip_control)) { m_Real->glGetIntegerv(eGL_CLIP_ORIGIN, (GLint *)&ClipOrigin); m_Real->glGetIntegerv(eGL_CLIP_DEPTH_MODE, (GLint *)&ClipDepth); diff --git a/renderdoc/driver/gl/gl_shader_refl.cpp b/renderdoc/driver/gl/gl_shader_refl.cpp index 0d72cdb5a..ea445cf22 100644 --- a/renderdoc/driver/gl/gl_shader_refl.cpp +++ b/renderdoc/driver/gl/gl_shader_refl.cpp @@ -877,7 +877,7 @@ void MakeShaderReflection(const GLHookSet &gl, GLenum shadType, GLuint sepProg, GLsizei numProps = (GLsizei)ARRAY_COUNT(props); // GL_LOCATION_COMPONENT not supported on core <4.4 (or without GL_ARB_enhanced_layouts) - if(!ExtensionSupported("GL_ARB_enhanced_layouts") && GLCoreVersion < 44) + if(!ExtensionSupported(ExtensionSupported_ARB_enhanced_layouts) && GLCoreVersion < 44) numProps--; gl.glGetProgramResourceiv(sepProg, sigEnum, i, numProps, props, numProps, NULL, values);