From 13e271c6cb41de2e7d6fbd0c3455eae00c9200c8 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 12 Apr 2024 12:18:41 +0200 Subject: [PATCH] Improve error checks in COpenGL3DriverBase --- irr/src/COGLESDriver.h | 2 + irr/src/COpenGLCoreRenderTarget.h | 12 ++---- irr/src/COpenGLCoreTexture.h | 39 ++++++++++++++---- irr/src/COpenGLDriver.h | 2 + irr/src/OpenGL/Driver.cpp | 68 +++++++++++++++++++++++++------ irr/src/OpenGL/Driver.h | 7 +++- 6 files changed, 99 insertions(+), 31 deletions(-) diff --git a/irr/src/COGLESDriver.h b/irr/src/COGLESDriver.h index 82b4a1793..29e0d94f1 100644 --- a/irr/src/COGLESDriver.h +++ b/irr/src/COGLESDriver.h @@ -15,6 +15,8 @@ #include "COGLESExtensionHandler.h" #include "IContextManager.h" +#define TEST_GL_ERROR(cls) (cls)->testGLError(__LINE__) + namespace irr { namespace video diff --git a/irr/src/COpenGLCoreRenderTarget.h b/irr/src/COpenGLCoreRenderTarget.h index 105cdd702..6bfc98cc0 100644 --- a/irr/src/COpenGLCoreRenderTarget.h +++ b/irr/src/COpenGLCoreRenderTarget.h @@ -174,9 +174,7 @@ public: AssignedTextures[i] = GL_COLOR_ATTACHMENT0 + i; GLenum textarget = currentTexture->getType() == ETT_2D ? GL_TEXTURE_2D : GL_TEXTURE_CUBE_MAP_POSITIVE_X + (int)CubeSurfaces[i]; Driver->irrGlFramebufferTexture2D(GL_FRAMEBUFFER, AssignedTextures[i], textarget, textureID, 0); -#ifdef _DEBUG - Driver->testGLError(__LINE__); -#endif + TEST_GL_ERROR(Driver); } else if (AssignedTextures[i] != GL_NONE) { AssignedTextures[i] = GL_NONE; Driver->irrGlFramebufferTexture2D(GL_FRAMEBUFFER, AssignedTextures[i], GL_TEXTURE_2D, 0, 0); @@ -239,9 +237,7 @@ public: AssignedDepth = false; AssignedStencil = false; } -#ifdef _DEBUG - Driver->testGLError(__LINE__); -#endif + TEST_GL_ERROR(Driver); RequestDepthStencilUpdate = false; } @@ -261,9 +257,7 @@ public: Driver->irrGlDrawBuffers(bufferCount, AssignedTextures.pointer()); } -#ifdef _DEBUG - Driver->testGLError(__LINE__); -#endif + TEST_GL_ERROR(Driver); } #ifdef _DEBUG diff --git a/irr/src/COpenGLCoreTexture.h b/irr/src/COpenGLCoreTexture.h index 45bf2f0be..79a855cc1 100644 --- a/irr/src/COpenGLCoreTexture.h +++ b/irr/src/COpenGLCoreTexture.h @@ -59,6 +59,18 @@ public: if (!InternalFormat) return; +#ifdef _DEBUG + char lbuf[128]; + snprintf_irr(lbuf, sizeof(lbuf), + "COpenGLCoreTexture: Type = %d Size = %dx%d (%dx%d) ColorFormat = %d (%d)%s -> %#06x %#06x %#06x%s", + (int)Type, Size.Width, Size.Height, OriginalSize.Width, OriginalSize.Height, + (int)ColorFormat, (int)OriginalColorFormat, + HasMipMaps ? " +Mip" : "", + InternalFormat, PixelFormat, PixelType, Converter ? " (c)" : "" + ); + os::Printer::log(lbuf, ELL_DEBUG); +#endif + const core::array *tmpImages = &images; if (KeepImage || OriginalSize != Size || OriginalColorFormat != ColorFormat) { @@ -104,6 +116,8 @@ public: } #endif + TEST_GL_ERROR(Driver); + for (u32 i = 0; i < (*tmpImages).size(); ++i) uploadTexture(true, i, 0, (*tmpImages)[i]->getData()); @@ -124,7 +138,7 @@ public: Driver->getCacheHandler()->getTextureCache().set(0, prevTexture); - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); } COpenGLCoreTexture(const io::path &name, const core::dimension2d &size, E_TEXTURE_TYPE type, ECOLOR_FORMAT format, TOpenGLDriver *driver) : @@ -155,6 +169,16 @@ public: return; } +#ifdef _DEBUG + char lbuf[100]; + snprintf_irr(lbuf, sizeof(lbuf), + "COpenGLCoreTexture: RTT Type = %d Size = %dx%d ColorFormat = %d -> %#06x %#06x %#06x%s", + (int)Type, Size.Width, Size.Height, (int)ColorFormat, + InternalFormat, PixelFormat, PixelType, Converter ? " (c)" : "" + ); + os::Printer::log(lbuf, ELL_DEBUG); +#endif + GL.GenTextures(1, &TextureName); const COpenGLCoreTexture *prevTexture = Driver->getCacheHandler()->getTextureCache().get(0); @@ -188,7 +212,7 @@ public: } Driver->getCacheHandler()->getTextureCache().set(0, prevTexture); - if (Driver->testGLError(__LINE__)) { + if (TEST_GL_ERROR(Driver)) { char msg[256]; snprintf_irr(msg, 256, "COpenGLCoreTexture: InternalFormat:0x%04x PixelFormat:0x%04x", (int)InternalFormat, (int)PixelFormat); os::Printer::log(msg, ELL_ERROR); @@ -241,7 +265,7 @@ public: IImage *tmpImage = LockImage; // not sure yet if the size required by glGetTexImage is always correct, if not we might have to allocate a different tmpImage and convert colors later on. Driver->getCacheHandler()->getTextureCache().set(0, this); - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); GLenum tmpTextureType = TextureType; @@ -252,7 +276,7 @@ public: } GL.GetTexImage(tmpTextureType, MipLevelStored, PixelFormat, PixelType, tmpImage->getData()); - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); if (IsRenderTarget && lockFlags == ETLF_FLIP_Y_UP_RTT) { const s32 pitch = tmpImage->getPitch(); @@ -334,7 +358,7 @@ public: } } - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); } return (LockImage) ? getLockImageData(MipLevelStored) : 0; @@ -392,6 +416,7 @@ public: } while (width != 1 || height != 1); } else { Driver->irrGlGenerateMipmap(TextureType); + TEST_GL_ERROR(Driver); } Driver->getCacheHandler()->getTextureCache().set(0, prevTexture); @@ -544,7 +569,7 @@ protected: GL.TexImage2D(tmpTextureType, level, InternalFormat, width, height, 0, PixelFormat, PixelType, tmpData); else GL.TexSubImage2D(tmpTextureType, level, 0, 0, width, height, PixelFormat, PixelType, tmpData); - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); break; default: break; @@ -561,7 +586,7 @@ protected: Driver->irrGlCompressedTexImage2D(tmpTextureType, level, InternalFormat, width, height, 0, dataSize, data); else Driver->irrGlCompressedTexSubImage2D(tmpTextureType, level, 0, 0, width, height, PixelFormat, dataSize, data); - Driver->testGLError(__LINE__); + TEST_GL_ERROR(Driver); break; default: break; diff --git a/irr/src/COpenGLDriver.h b/irr/src/COpenGLDriver.h index 6fe8ac37d..bca1055da 100644 --- a/irr/src/COpenGLDriver.h +++ b/irr/src/COpenGLDriver.h @@ -22,6 +22,8 @@ class CIrrDeviceMacOSX; #include "COpenGLExtensionHandler.h" #include "IContextManager.h" +#define TEST_GL_ERROR(cls) (cls)->testGLError(__LINE__) + namespace irr { diff --git a/irr/src/OpenGL/Driver.cpp b/irr/src/OpenGL/Driver.cpp index 3d6a908ee..bf9334e15 100644 --- a/irr/src/OpenGL/Driver.cpp +++ b/irr/src/OpenGL/Driver.cpp @@ -134,7 +134,18 @@ void APIENTRY COpenGL3DriverBase::debugCb(GLenum source, GLenum type, GLuint id, void COpenGL3DriverBase::debugCb(GLenum source, GLenum type, GLuint id, GLenum severity, GLsizei length, const GLchar *message) { - printf("%04x %04x %x %x %.*s\n", source, type, id, severity, length, message); + // shader compiler can be very noisy + if (source == GL_DEBUG_SOURCE_SHADER_COMPILER && severity == GL_DEBUG_SEVERITY_NOTIFICATION) + return; + + ELOG_LEVEL ll = ELL_INFORMATION; + if (severity == GL_DEBUG_SEVERITY_HIGH) + ll = ELL_ERROR; + else if (severity == GL_DEBUG_SEVERITY_MEDIUM) + ll = ELL_WARNING; + char buf[256]; + snprintf_irr(buf, sizeof(buf), "%04x %04x %.*s", source, type, length, message); + os::Printer::log("GL", buf, ll); } COpenGL3DriverBase::COpenGL3DriverBase(const SIrrlichtCreationParameters ¶ms, io::IFileSystem *io, IContextManager *contextManager) : @@ -283,7 +294,7 @@ bool COpenGL3DriverBase::genericDriverInit(const core::dimension2d &screenS // This fixes problems with intermediate changes to the material during texture load. ResetRenderStates = true; - testGLError(__LINE__); + TEST_GL_ERROR(this); return true; } @@ -514,7 +525,7 @@ bool COpenGL3DriverBase::updateVertexHardwareBuffer(SHWBufferLink_opengl *HWBuff GL.BindBuffer(GL_ARRAY_BUFFER, 0); - return (!testGLError(__LINE__)); + return (!TEST_GL_ERROR(this)); } bool COpenGL3DriverBase::updateIndexHardwareBuffer(SHWBufferLink_opengl *HWBuffer) @@ -571,7 +582,7 @@ bool COpenGL3DriverBase::updateIndexHardwareBuffer(SHWBufferLink_opengl *HWBuffe GL.BindBuffer(GL_ELEMENT_ARRAY_BUFFER, 0); - return (!testGLError(__LINE__)); + return (!TEST_GL_ERROR(this)); } //! updates hardware buffer if needed @@ -845,7 +856,7 @@ void COpenGL3DriverBase::draw2DImage(const video::ITexture *texture, const core: if (clipRect) GL.Disable(GL_SCISSOR_TEST); - testGLError(__LINE__); + TEST_GL_ERROR(this); } void COpenGL3DriverBase::draw2DImage(const video::ITexture *texture, u32 layer, bool flip) @@ -1126,28 +1137,60 @@ void COpenGL3DriverBase::setMaterial(const SMaterial &material) } //! prints error if an error happened. -bool COpenGL3DriverBase::testGLError(int code) +bool COpenGL3DriverBase::testGLError(const char *file, int line) { if (!EnableErrorTest) return false; GLenum g = GL.GetError(); + const char *err = nullptr; switch (g) { case GL_NO_ERROR: return false; case GL_INVALID_ENUM: - os::Printer::log("GL_INVALID_ENUM", core::stringc(code).c_str(), ELL_ERROR); + err = "GL_INVALID_ENUM"; break; case GL_INVALID_VALUE: - os::Printer::log("GL_INVALID_VALUE", core::stringc(code).c_str(), ELL_ERROR); + err = "GL_INVALID_VALUE"; break; case GL_INVALID_OPERATION: - os::Printer::log("GL_INVALID_OPERATION", core::stringc(code).c_str(), ELL_ERROR); + err = "GL_INVALID_OPERATION"; + break; + case GL_STACK_OVERFLOW: + err = "GL_STACK_OVERFLOW"; + break; + case GL_STACK_UNDERFLOW: + err = "GL_STACK_UNDERFLOW"; break; case GL_OUT_OF_MEMORY: - os::Printer::log("GL_OUT_OF_MEMORY", core::stringc(code).c_str(), ELL_ERROR); + err = "GL_OUT_OF_MEMORY"; break; + case GL_INVALID_FRAMEBUFFER_OPERATION: + err = "GL_INVALID_FRAMEBUFFER_OPERATION"; + break; +#ifdef GL_VERSION_4_5 + case GL_CONTEXT_LOST: + err = "GL_CONTEXT_LOST"; + break; +#endif }; + + // Empty the error queue, see + bool multiple = false; + while (GL.GetError() != GL_NO_ERROR) + multiple = true; + + // basename + for (char sep : {'/', '\\'}) { + const char *tmp = strrchr(file, sep); + if (tmp) + file = tmp+1; + } + + char buf[80]; + snprintf_irr(buf, sizeof(buf), "%s %s:%d%s", + err, file, line, multiple ? " (older errors exist)" : ""); + os::Printer::log(buf, ELL_ERROR); return true; } @@ -1797,7 +1840,7 @@ IImage *COpenGL3DriverBase::createScreenShot(video::ECOLOR_FORMAT format, video: } GL.ReadPixels(0, 0, ScreenSize.Width, ScreenSize.Height, internalformat, type, pixels); - testGLError(__LINE__); + TEST_GL_ERROR(this); // opengl images are horizontally flipped, so we have to fix that here. const s32 pitch = newImage->getPitch(); @@ -1825,11 +1868,10 @@ IImage *COpenGL3DriverBase::createScreenShot(video::ECOLOR_FORMAT format, video: } } - if (testGLError(__LINE__)) { + if (TEST_GL_ERROR(this)) { newImage->drop(); return 0; } - testGLError(__LINE__); return newImage; } diff --git a/irr/src/OpenGL/Driver.h b/irr/src/OpenGL/Driver.h index 0807542f5..3b2026611 100644 --- a/irr/src/OpenGL/Driver.h +++ b/irr/src/OpenGL/Driver.h @@ -16,6 +16,8 @@ #include "ExtensionHandler.h" #include "IContextManager.h" +#define TEST_GL_ERROR(cls) (cls)->testGLError(__FILE__, __LINE__) + namespace irr { namespace video @@ -221,8 +223,9 @@ public: //! Returns an image created from the last rendered frame. IImage *createScreenShot(video::ECOLOR_FORMAT format = video::ECF_UNKNOWN, video::E_RENDER_TARGET target = video::ERT_FRAME_BUFFER) override; - //! checks if an OpenGL error has happened and prints it (+ some internal code which is usually the line number) - bool testGLError(int code = 0); + //! checks if an OpenGL error has happened and prints it, use via TEST_GL_ERROR(). + // Does *nothing* unless in debug mode. + bool testGLError(const char *file, int line); //! Set/unset a clipping plane. bool setClipPlane(u32 index, const core::plane3df &plane, bool enable = false) override;