From 28ec4e4fb8506b7a042f5a58eefc70c00c59c925 Mon Sep 17 00:00:00 2001 From: baldurk Date: Tue, 26 Jun 2018 14:20:28 +0100 Subject: [PATCH] Remove bool returns from GL hook functions that cannot meaningfully fail * In particular, populating hooks can't fail because we're just fetching as many function pointers as we can. Any missing functions have to be handled elsewhere. * This simplifies codepaths and makes it easier to track what's going on without needing to thinkg about failure conditions that can never happen. --- renderdoc/driver/gl/gl_hooks_egl.cpp | 30 +++++-------------- renderdoc/driver/gl/gl_hooks_linux.cpp | 23 ++++++-------- renderdoc/driver/gl/gl_hooks_linux_shared.cpp | 4 +-- renderdoc/driver/gl/gl_hooks_linux_shared.h | 2 +- renderdoc/driver/gl/gl_hooks_win32.cpp | 16 +++++----- 5 files changed, 26 insertions(+), 49 deletions(-) diff --git a/renderdoc/driver/gl/gl_hooks_egl.cpp b/renderdoc/driver/gl/gl_hooks_egl.cpp index b4f08a631..ff6076493 100644 --- a/renderdoc/driver/gl/gl_hooks_egl.cpp +++ b/renderdoc/driver/gl/gl_hooks_egl.cpp @@ -37,8 +37,6 @@ class EGLHook : LibraryHook, public GLPlatform public: EGLHook() { - m_HasHooks = false; - m_GLDriver = NULL; m_PopulatedHooks = false; @@ -57,7 +55,7 @@ public: { if(!m_PopulatedHooks) { - m_PopulatedHooks = PopulateHooks(); + PopulateHooks(); SharedCheckContext(); } } @@ -174,25 +172,18 @@ public: set m_Contexts; bool m_PopulatedHooks; - bool m_HasHooks; - bool SetupHooks() + void SetupHooks() { - bool success = true; - if(!real.IsInitialized()) { bool symbols_ok = real.LoadSymbolsFrom(libGLdlsymHandle); if(!symbols_ok) - { RDCWARN("Unable to load some of the EGL API functions, may cause problems"); - success = false; - } } - return success; } - bool PopulateHooks(); + void PopulateHooks(); } eglhooks; // everything below here needs to have C linkage @@ -436,22 +427,17 @@ bool EGLHook::CreateHooks(const char *libName) #endif } - bool success = PopulateHooks(); - - if(!success) - return false; - - m_HasHooks = true; + PopulateHooks(); return true; } -bool EGLHook::PopulateHooks() +void EGLHook::PopulateHooks() { SetupHooks(); if(m_PopulatedHooks) - return true; + return; // dlsym can return GL symbols during a GLES context bool dlsymFirst = false; @@ -461,14 +447,14 @@ bool EGLHook::PopulateHooks() dlsymFirst = true; #endif - m_PopulatedHooks = SharedPopulateHooks(dlsymFirst, [](const char *funcName) { + SharedPopulateHooks(dlsymFirst, [](const char *funcName) { // on some android devices we need to hook dlsym, but eglGetProcAddress might call dlsym so we // need to ensure we return the 'real' pointers ScopedSuppressHooking suppress; return (void *)eglGetProcAddress(funcName); }); - return m_PopulatedHooks; + m_PopulatedHooks = true; } void PopulateEGLFunctions() diff --git a/renderdoc/driver/gl/gl_hooks_linux.cpp b/renderdoc/driver/gl/gl_hooks_linux.cpp index ff58c5dff..07232e583 100644 --- a/renderdoc/driver/gl/gl_hooks_linux.cpp +++ b/renderdoc/driver/gl/gl_hooks_linux.cpp @@ -99,12 +99,7 @@ public: m_GLXWindowMap.erase(it); } - void PopulateGLFunctions() - { - if(!m_PopulatedHooks) - m_PopulatedHooks = PopulateHooks(); - } - + void PopulateGLFunctions() { PopulateHooks(); } void SetupExportedFunctions() { // in the replay application we need to call SetupHooks to ensure that all of our exported @@ -432,7 +427,7 @@ public: (const GLubyte *)"glXCreateContextAttribsARB"); } - bool PopulateHooks(); + void PopulateHooks(); } glhooks; void OpenGLHook::libHooked(void *realLib) @@ -838,22 +833,22 @@ __attribute__((visibility("default"))) VkResult vk_icdNegotiateLoaderLayerInterf }; // extern "C" -bool OpenGLHook::PopulateHooks() +void OpenGLHook::PopulateHooks() { + if(m_PopulatedHooks) + return; + + m_PopulatedHooks = true; + SetupHooks(); glXGetProcAddress((const GLubyte *)"glXCreateContextAttribsARB"); - bool ret = SharedPopulateHooks(true, [](const char *funcName) { + SharedPopulateHooks(true, [](const char *funcName) { return (void *)glXGetProcAddress((const GLubyte *)funcName); }); - if(!ret) - return false; - SharedCheckContext(); - - return true; } void PopulateGLFunctions() diff --git a/renderdoc/driver/gl/gl_hooks_linux_shared.cpp b/renderdoc/driver/gl/gl_hooks_linux_shared.cpp index 4e3725c10..3609efdf5 100644 --- a/renderdoc/driver/gl/gl_hooks_linux_shared.cpp +++ b/renderdoc/driver/gl/gl_hooks_linux_shared.cpp @@ -1416,7 +1416,7 @@ void *SharedLookupFuncPtr(const char *func, void *realFunc) return realFunc; } -bool SharedPopulateHooks(bool dlsymFirst, void *(*lookupFunc)(const char *)) +void SharedPopulateHooks(bool dlsymFirst, void *(*lookupFunc)(const char *)) { #undef HookInit #define HookInit(function) \ @@ -1457,8 +1457,6 @@ bool SharedPopulateHooks(bool dlsymFirst, void *(*lookupFunc)(const char *)) DLLExportHooks(); HookCheckGLExtensions(); - - return true; } void SharedCheckContext() diff --git a/renderdoc/driver/gl/gl_hooks_linux_shared.h b/renderdoc/driver/gl/gl_hooks_linux_shared.h index b94e20207..8b802ca19 100644 --- a/renderdoc/driver/gl/gl_hooks_linux_shared.h +++ b/renderdoc/driver/gl/gl_hooks_linux_shared.h @@ -31,7 +31,7 @@ void CloneDisplay(Display *dpy); #endif void *SharedLookupFuncPtr(const char *func, void *realFunc); -bool SharedPopulateHooks(bool dlsymFirst, void *(*lookupFunc)(const char *)); +void SharedPopulateHooks(bool dlsymFirst, void *(*lookupFunc)(const char *)); void SharedCheckContext(); void PosixHookFunctions(); diff --git a/renderdoc/driver/gl/gl_hooks_win32.cpp b/renderdoc/driver/gl/gl_hooks_win32.cpp index aa7514c9c..b02ad6762 100644 --- a/renderdoc/driver/gl/gl_hooks_win32.cpp +++ b/renderdoc/driver/gl/gl_hooks_win32.cpp @@ -688,12 +688,7 @@ public: static OpenGLHook glhooks; - void PopulateGLFunctions() - { - if(!m_PopulatedHooks) - m_PopulatedHooks = PopulateHooks(); - } - + void PopulateGLFunctions() { PopulateHooks(); } void MakeContextCurrent(GLWindowingData data) { if(wglMakeCurrent_hook()) @@ -1385,8 +1380,13 @@ private: DLLExportHooks(); } - bool PopulateHooks() + void PopulateHooks() { + if(m_PopulatedHooks) + return; + + m_PopulatedHooks = true; + void *moduleHandle = Process::LoadModule("opengl32.dll"); if(wglGetProcAddress_hook() == NULL) @@ -1415,8 +1415,6 @@ private: // see gl_emulated.cpp GL.EmulateUnsupportedFunctions(); GL.EmulateRequiredExtensions(); - - return true; } DefineDLLExportHooks();