Fixes #1272 - Improve GLDebugDesktop by reducing manual overrides

[MeFisto94]

Use a reflection-based approach to call checkError() after every call to the openGL API to reduce Code Duplication and increase Maintainability, while also fixing the regression caused by GLDebugDesktop extending from GLDebugES and thus making the Renderer think it is on mobile.

As discussed, if I should make this version more dynamic to support GLFbo and GLExt as separate GLDebugDesktop instances, I can do that, but having the if statement probably isn't that bad, but I can see that it is better and more flexible and especially gets rid of the exception in line 73

[MeFisto94]

I forgot to add the changes to LWJGL3, will force-push shortly.

Edit: I think we could do even better and only have GLDebug which can then be invoked for Desktop OR ES (in case of ES, the only thing different is that we have to add GLES_30 as implemented interface and keep GL3 and GL4 off)

[stephengold]

Before we can integrate this, we need to upgrade the NDK version.

[MeFisto94]

Do you talk about the build failure stemming from #1281 ?
If so I can rebase this to master, then the Android build will succeed.

[stephengold]

I was assuming the build failure was something more serious. Go ahead and rebase then, and we'll see.

[pspeed42]

Sorry it's taken me so long to look at this and thanks for being patient. It mostly looks good.

A concern I have is that code that used to create on gl object and then cast it to different fields is now creating three separate proxies.
gl = (GL) GLDebug.createProxy(gl, gl, GL.class, GL2.class, GL3.class, GL4.class);
glext = (GLExt) GLDebug.createProxy(gl, glext, GLExt.class);
glfbo = (GLFbo) GLDebug.createProxy(gl, glfbo, GLFbo.class);

I don't really know how/where these fields are used and haven't had time to look, but any code expecting them to be the same object (or expecting to be able to case glfbo to GL4 or whatever) is going to fail where it would have succeeded before.

Perhaps we are correct to enforce this and I guess it would only break if they turn debugging on... and then they could see their error and correct it.

Otherwise, the changes look good to me.

[MeFisto94]

@pspeed42 Actually I just stumbled over this:
Do you think it would be more wise to lookup the Method object and use equals there instead of comparing the string all the time?

[pspeed42]

Sure it would be slightly faster, I guess, if the method is looked up at proxy creation time. I wasn't overly concerned about it since this is a debugging class.

I'm ok if that's a follow-on improvement, too.

[riccardobl]

What is the status for this?

[MeFisto94]

I need to change the if to be Method based and think about the separate proxies first, so this is mostly a "draft"

[MeFisto94]

I looked through the concern of having multiple proxies and:
In Release Mode, GLEXT and GLFbo are different classes already, so it's just that the old Debug State was mixing them up. The old Debug state still provided separate variables for this, so there is no real reason to assume the above.

That means, I'm fine with breaking code that relied on glfbo = gl = glext only on debug.
I will fix what I commited eariler this day, though.

[MeFisto94]

@pspeed42 Any objections based on the fact that I throw an unchecked exception when glGetError cannot be found?

[pspeed42]

IllegalStateException isn't really the right exception here. If you want to avoid a naked RuntimeException (and that's probably a good idea), IllegalArgumentException is probably better. ie: the caller passed a GL context that is incompatible with this debugger.

[stephengold]

Fixed in v3.3 branch at 4c2c3fa.