[0.4.0] [Ion Fury] Rendering/visual Glitches

[ReaperAA]

I am sure you devs are already aware of the rendering/visual glitches. Still I wanted to point out all the visual glitches (or oddities) that I have encountered so far (in case there are any that devs are not aware of). I may update the list as I discover more.

1. The skies are completely black. However in some case, like in start of zone 3, when I look at the sky from certain angles/positions, the sky sometimes appear (but bugged or having HOMs of sorts)
2. At the very start of the game, the outside area (visible when the building is destroyed) does not render correctly. The far away areas appear blacked.
3. The tinting effects(when taking damage or picking up items) is not correct. It is different from IF on Eduke32.

[mjr4077au]

I know the skybox code is gone and that's why the skies are always black, but the rendering issues in the first level were caused in this commit.

Reverted locally and compiled, playing without issues. Probably not the "right" fix, but might give some insight as to why this is occurring.

[Graf Zahl]

The depth test needs to remain, of course. This means that some leftover data in the depth buffer wasn't cleared before drawing the scene.

[mjr4077au]

The function that calls 'polymost_drawrooms()' is 'renderDrawRoomsQ16' in engine.cpp.

Immedately after 'renderDrawRoomsQ16' calls 'polymost_drawrooms()', it calls 'videoBeginDrawing()'. However, 'polymost_drawrooms()' also calls 'polymost_drawrooms()'. Could this be some kind of overlap? The function isn't taking parameters, so does that mean it's doing the same thing every call, or is it relying on globals?

engine.cpp code for 'renderDrawRoomsQ16':

Code: Select all

#ifdef USE_OPENGL
    //============================================================================= //POLYMOST BEGINS
    polymost_drawrooms();

    if (videoGetRenderMode() != REND_CLASSIC)
        return inpreparemirror;
    //============================================================================= //POLYMOST ENDS
#endif

    videoBeginDrawing(); //{{{

polymost.cpp code for 'polymost_drawrooms()':

Code: Select all

void polymost_drawrooms()
{
    if (videoGetRenderMode() == REND_CLASSIC) return;

    polymost_outputGLDebugMessage(3, "polymost_drawrooms()");

    videoBeginDrawing();

[Graf Zahl]

This could be moved before the check, but if you look closer at the 'if's, you'll notice that it cannot enter both of these code paths.
However, since the software rendering path is only used by camera textures, this will eventually be removed once the backend can do this itself as a hardware rendering feature.

[mjr4077au]

The if statements there are only doing some kind of return? I can't see any curly braces, etc. Irrespective of that, my understanding of the engine and how it's functioning is quite low, and it's hard to read with so many ifdef guards etc.

For what it's worth, the glDepthFunc in JFDuke3D and EDuke32 in polymost_drawrooms() is set to "GL_ALWAYS", and I hypothesise that changing it to match Raze would result in a similar experience. Broken by design?

Is it worth considering matching upstream for the sake of playability, or leave it broken as a reminder that this needs doing/fixing better?

[Gez]

The if statements there are only doing some kind of return?

Yeah, and return means that it doesn't execute the code there's after.

So when you have this:

Code: Select all

void polymost_drawrooms()
{
    if (videoGetRenderMode() == REND_CLASSIC) return;

    polymost_outputGLDebugMessage(3, "polymost_drawrooms()");

    videoBeginDrawing();

It's basically tantamount to this:

Code: Select all

void polymost_drawrooms()
{
    if (videoGetRenderMode() != REND_CLASSIC)
    {
        polymost_outputGLDebugMessage(3, "polymost_drawrooms()");

        videoBeginDrawing();
    }

Which means that the bigger block becomes:

Code: Select all

#ifdef USE_OPENGL
    //============================================================================= //POLYMOST BEGINS
    if (videoGetRenderMode() != REND_CLASSIC)
    {
        polymost_outputGLDebugMessage(3, "polymost_drawrooms()");

        videoBeginDrawing();
    }

    if (videoGetRenderMode() != REND_CLASSIC)
        return inpreparemirror;
    //============================================================================= //POLYMOST ENDS
#endif

    videoBeginDrawing(); //{{{

Which then becomes:

Code: Select all

    if (videoGetRenderMode() != REND_CLASSIC)
    {
        polymost_outputGLDebugMessage(3, "polymost_drawrooms()");

        videoBeginDrawing();

        return inpreparemirror;
    }
    else
    {
        videoBeginDrawing(); 
    }

(Of course for brevity and convenience I skipped any and all instructions that happen after the quoted tidbits. The TL;DR is that it's going to call videoBeginDrawing() either way, and it's not going to call it twice.

[mjr4077au]

I didn't read the code enough before commenting. The function polymost_drawrooms() that's calling videoBeginDrawing() also calls videoEndDrawing() before it ends, so when renderDrawRoomsQ16 calls videoBeginDrawing() after polymost_drawrooms(), it's a new instance of videoBeginDrawing().

When I saw videoBeginDrawing() straight after polymost_drawrooms(), as well as one of the first lines in polymost_drawrooms(), I thought it doing the same thing twice and possibly filling a buffer or something because it's being called without any parameters or objects passed to it, but now I realise it must be just working off global variables or something.

I have much to learn, I haven't had great exposure to lower level languages and have only ever worked with interpreted ones like Powershell and a bit of PHP.

[Gez]

Again:

Code: Select all

if (a) return;
doSomething;

is the same as:

Code: Select all

if (!a)
{
   doSomething();
}

Because if you return, you do not execute any later instruction in the function. If you have a and you return, you do not doSomething(). So it's the logical equivalent of only doingSomething() if not a.

[mjr4077au]

Thanks, and sorry I didn't note the ! when I was reading this pre-coffee.