void256/nifty-gui — pull request #209 by 3xp0n3nt (2014-01-25 09:31:41+00:00)
Title: Fix rendering bugs & improve error handling.

Note: Various debugging logging statements from
350b20bdb983622bcd67b3c607e0fced8c2b63b6 are still in place until the bug from

Bugfixes:

- The checkTarget method in CoreTexture2D had previously been re-written
to use if statements rather than a switch because the switch cases had
to be converted to methods, which is illegal for a switch statement.
Unfortunately during this re-writing a logic error was made that used
AND's (&&) instead of OR's (||) for the conditions that used to be
cases. This had the effect of throwing a CoreGLException during the
creation of new textures, causing the texture id of a new texture atlas
to be -1, which in turn caused a NullPointerException in
BatchRenderBackendCoreProfileInternal when attempting to manipulate a
non-existent texture atlas. To make matters worse, the exception was
being swallowed by the LoggerShortFormat logging formatter that is used
by the nifty-examples, preventing easy debugging. Change checkTarget
method in CoreTexture2D to use the correct OR (||) logic, and fix the
nifty-examples logging formatter to not swallow the exception (more on
the logging in the "Improved error handling" section). Fixes bug from
#203.

- The glTextImage2D method in CoreTexture2D was ignoring the
internalFormat parameter being passed to this method, and was
assigning the internal field this.texImageInternalFormat to the format
parameter instead, which basically means that the internal texture
format of newly created textures is being assigned to the format of the
pixel data when it should be being assigned to the internalFormat
parameter, which specifies the format of the texture. Of course, this
bug never manifested in normal use of the CoreTexture2D class because
the most common pixel data format, GL_RGBA, just happens to match the
most common internal texture format, GL_RGBA. This bug would have
manifested more clearly if compressed textures were being used, where
the pixel data format would differ from the internal texture format.
This bug appears to have never been documented, and also seems to have
been around for a very long time.

Improved error handling:

- Change LoggerShortFormat logging format used by nifty-examples to
include the stack trace of any exception that is logged. The logger
was swallowing exceptions whole o_O...

- Remove duplicate OpenGL error checking methods from
BatchRenderBackendInternal, BatchRenderBackendCoreProfileInternal, &
CoreTexture2D - use CheckGL class instead as it provides the exact
methods.

- Remove duplicate texture size checking method from
BatchRenderBackendInternal, BatchRenderBackendCoreProfileInternal, &
CoreTexture2D - move it to CheckGL class instead so that all
OpenGL-based internal rendering classes can make use of it. There are
other checking-type methods in CoreTexture2D but they are only used
internally by that class and so haven't been moved to CheckGL.

- Add OpenGL error checking to CoreProfileSaveGLState because it makes a
lot of OpenGL calls.

- Change log warning entries in BatchRenderBackendInternal &
BatchRenderBackendCoreProfileInternal to severe when dealing with
failed texture creation or failed texture updating to make them stand
out more, and this is really a more appropriate level for such
phenomenon, and add more detailed debugging messages to these log
entries.

Miscellaneous :

- Changed order of constructors in CoreTexture2D so they are basically
in order of increasing complexity.

- Remove unused method from CoreTexture2D.

- Rearranged imports in LwjglGL, LwjglInitHelper, & NiftyTest to be in
correct order.

- Remove some unnecessary this keywords.

- Add "Internal implementations" comments in CheckGL & LoggerShortFormat.