Hi,
The current code doesn’t deal with OpenGL errors. Since they are quite sparse, probably we have been lucky ignoring them, but it can be crashy.
For example, if a glDisableClientState() fails, you’ll have a “pointer to buffer” still enabled, leading to a crash on the next glDrawArray call (and good luck to debug this).
Usually the good method to check for OpenGL error is to call glGetError() after each GL call. Since this can be very painful, I propose the following code:
// Example usage code
{
GLGuard guard = MakeGLGuard(glDisable, GL_TEXTURE_2D);
glEnable(GL_TEXTURE_2D);
// Some gl call here we don't actually check for error
[...]
} // If any of the GL call fails, the guard will wake up and disable the texture state
// Another example
{
GLGuard guard = MakeGLGuard(glDisableClientState, GL_TEXCOORD_ARRAY);
glEnableClientState(GL_TEXCOORD_ARRAY);
glTexPointer(...);
glDrawArrays(...);
guard.act(); // Force disabling the state anyway, regardless of the error state (so you don't write thing twice)
}
The GLGuard implementation:
struct GLGuardBase
{
void act() const noexcept { dismissed = false; forced = true; }
bool logOpenGLError() const noexcept
{
static String commonErrorCode[] = { "Invalid enum", "Invalid value", "Invalid operation", "Stack overflow", "Stack underflow", "Out of memory" };
GLenum error = glGetError();
if (error)
{
Logger::outputDebugString(TRANS("OpenGL Error: ") + String((error - GL_INVALID_ENUM < numElementsInArray(commonErrorCode)) ? commonErrorCode[error - GL_INVALID_ENUM] : String("Unknown error")));
return true;
}
return false;
}
// Construction and destruction is not allowed except for child classes
protected:
GLGuardBase() : dismissed(true), forced(false) {}
/** Copy constructor, undo the initial scope guard behavior */
GLGuardBase(const GLGuardBase& other) : dismissed(other.dismissed) { other.dismissed = false; other.forced = false; }
/** Destructor
It is not virtual because from point of declaration to point to destruction
it is a child class which is created and the compiler directly call the right
child destructor (it also avoid setting up and using virtual function pointer) */
~GLGuardBase() {}
// Member
protected:
/** When set to true, the destructor ignore the asked clean up */
mutable bool dismissed, forced;
private:
/** GL Guard are not moveable */
GLGuardBase& operator= (const GLGuardBase&);
};
// Very very important to use this instead of the former
typedef const GLGuardBase & GLGuard;
/** Real implementation for a function with no parameter */
template <typename Fun>
class GLGuardImpl0 : public GLGuardBase
{
Fun function;
// Construction and destruction
public:
/** Unique constructor */
GLGuardImpl0(const Fun& fun) : function(fun) {}
/** Destructor - catch any pending exception to continue cleaning for any other guard */
~GLGuardImpl0() { if (forced || (!dismissed && logOpenGLError())) try{ function(); } catch (...) {} }
};
/** Real implementation for a function with 1 parameter */
template <typename Fun, typename Parm>
class GLGuardImpl1 : public GLGuardBase
{
Fun function;
const Parm parameter;
// Construction and destruction
public:
/** Unique constructor */
GLGuardImpl1(const Fun& fun, const Parm& parm) : function(fun), parameter(parm) {}
/** Destructor - catch any pending exception to continue cleaning for any other guard */
~GLGuardImpl1() { if (forced || (!dismissed && logOpenGLError())) try{ function(parameter); } catch (...) {} }
};
/** Real implementation for a function with 2 parameter */
template <typename Fun, typename Parm, typename Parm2>
class GLGuardImpl2 : public GLGuardBase
{
Fun function;
const Parm param1;
const Parm2 param2;
// Construction and destruction
public:
/** Unique constructor */
GLGuardImpl2(const Fun& fun, const Parm& parm1, const Parm2& parm2) : function(fun), param1(parm1), param2(parm2) {}
/** Destructor - catch any pending exception to continue cleaning for any other guard */
~GLGuardImpl2() { if (forced || (!dismissed && logOpenGLError())) try{ function(param1, param2); } catch (...) {} }
};
template <typename Fun>
inline GLGuardImpl0<Fun*> MakeGLGuard(const Fun& fun) { return GLGuardImpl0<Fun*>(fun); }
template <typename Fun, typename Parm>
inline GLGuardImpl1<Fun*, Parm> MakeGLGuard(const Fun& fun, const Parm& parm) { return GLGuardImpl1<Fun*, Parm>(fun, parm); }
template <typename Fun, typename Parm, typename Parm2>
inline GLGuardImpl2<Fun*, Parm, Parm2> MakeGLGuard(const Fun& fun, const Parm& parm, const Parm2& parm2) { return GLGuardImpl2<Fun*, Parm, Parm2>(fun, parm, parm2); }
If it’s hard to understand, I’ll explain in few words how it works.
- The basic idea is to create a templated object whose destructor calls a function pointer set by the overloaded MakeGLGuard function.
- All these objects derives from a GLGuardBase so they advertize a common interface, but no virtual table is used (for performance). This is possible because when taking a const reference on a temporary object, the compiler will use the scope of the reference to extend the life of the temporary. So when you do: “const Obj & a = Obj(“c”);”, the temporary “c” destructor will be called when “a” goes out of scope. This is the hardest part to understand, but you are already using it every day when you use const reference in any function’s parameter.
- You must use the “GLGuard” typedef in order to see the above behaviour (or the more explicit and painful “const GLGuardBase &”).
If you think it’s worth it, or have any comment on this code, please do so, I put this code in public domain.