Crash on Gore [Software Renderer]

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
User avatar
Rachael
Posts: 13531
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Crash on Gore [Software Renderer]

Post by Rachael »

I know I shouldn't be playing OpenGL specific mods using the software renderer - but I have my reasons.

Needless to say, the titlemap on Gore crashes in the latest ZDoom builds. It does not crash in ZDoom 2.8.1.

It's reliable about 5-10 seconds in from viewing the titlemap for this crash to occur.

Unfortunately, I can't give too much more details about what's going on here, because even loading it in the debugger I am not sure what.

What appears to happen is in r_drawt.cpp, line 531, *dest is pointing to either a null pointer, or to an object that's been freed. All the other variables seem to be fine.

Screenshot
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

Well, I have no idea, this is too deep in the software renderer.
dpJudas
 
 
Posts: 3037
Joined: Sat May 28, 2016 1:01 pm

Re: Crash on Gore [Software Renderer]

Post by dpJudas »

This function draws a column to the screen at (sx,yl) to (sx,yh). You could add some asserts at the top of the function like this:

assert(sx >= 0 && sx < SCREENWIDTH); // check x range
assert(yh < yl ||(yl >= 0 && yl < SCREENHEIGHT)); // check y start
assert(yh < yl ||(yh >= 0 && yh < SCREENHEIGHT)); // check y end

One or more of those are probably wrong due to some calculation in R_DrawVisSprite.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

One of the y's was way beyond that range, somewhere around 30000. But this is for Randi.
User avatar
Rachael
Posts: 13531
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: Crash on Gore [Software Renderer]

Post by Rachael »

There's nothing to see with Y's that far off screen. I think it should be safe to put in sanity checks on these functions that determines the column position and length, and if both are off screen, to simply return the function and do nothing. I'm going to try that and see what happens.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

We don't even know what the problem is to decide whether to patch the symptoms rather than the cause. It's never a good idea to just add a few range checks to hide a bad calculation elsewhere.
User avatar
Rachael
Posts: 13531
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: Crash on Gore [Software Renderer]

Post by Rachael »

Fair enough then - as far as code submissions go, I will not make any to try to fix this.

I could be wrong, but here's what I have guessed is happening:

This exception is being thrown at a translucent drawer (the add drawer, specifically) - in order to work it has to read from the screen. (or at least the buffer that it is currently using)

For some odd reason, it's also writing (before the crash) to the screen at coordinates clearly out of range without throwing an exception.

Now - why it's off screen in the first place - well - simple - the sprites are really big, are being shown up-close, and naturally have huge Y clip-ends which the masking function theoretically is calling the translucent drawer to handle.

I know it's a bit of a gamble to go off of a theory from someone who has probably a week's experience playing with the software renderer, but I'm going to at least play around with it even if nothing comes of it to see if I can nail this one down.
dpJudas
 
 
Posts: 3037
Joined: Sat May 28, 2016 1:01 pm

Re: Crash on Gore [Software Renderer]

Post by dpJudas »

This part of the code is not trivial to read because it defers the calculations in buffers and uses callbacks to do it. Roughly, the algorithm is as follows:

1. R_DrawVisSprite is called to draw a sprite. Sets up light, blend mode, scaling. At this point the X position has already been calculated elsewhere.
2. Loops through vis->x1 to vis->x2 to draw the sprite. For four columns at a time it:
3. Calls rt_initcols. This resets the working buffer it uses to eventually pass in yl/yh values to rt_addclamp1col.
4. Calls R_DrawMaskedColumnHoriz four times to add a column at a time to the working buffers.
4a. In R_DrawMaskedColumnHoriz it calculates the unclipped position of the sprite for that column (search for "calculate unclipped screen coordinates for post").
4b. Then it clips it vertically according to the min/max values in mfloorclip and mceilingclip. One possibility is that the mfloorclip or mceilingclip values are out of bounds.
4c. Then it clips it based on the texture 'span post'. This code I can barely read, so that gives it a 200% extra chance of being buggy. Especially as there is a double involved there, so its been patched since the floating point change. Or it might be perfectly fine but could still fail if it gets invalid span post info from the texture.
4c. Now it calls hcolfunc_pre to tell rt_drawt functions to copy the data to the working buffers.
4d. Last it inverts the yl/yh ranges if the sprite is flipped.
5. Calls rt_draw4cols to copy the result to the screen. Here it does some sorting to draw things in pairs of four whenever the columns align. Here it calls rt_addclamp1 for one of them that don't and the program crashes.

To summarize, the candidates for this crash are: A) mfloorclip + mceilingclip, B) R_DrawMaskedColumnHoriz's "if (dc_yl <= dc_yh)" stuff juggling of dc_yl and dc_yh, C) FTexture::GetColumn returning invalid span data, D) rt_draw4cols having a bug in its sorting. Of those four, B is the only one doing double stuff, so my bet would be to examine that one first. If it is mfloorclip and mceilingclip it gets really really nasty as those are set elsewhere far far away from this part of the code. You really don't want it to be A. :)
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

dpJudas wrote:If it is mfloorclip and mceilingclip it gets really really nasty as those are set elsewhere far far away from this part of the code. You really don't want it to be A. :)
... and now take a guess why I absolutely refuse to work with this code. This design has blocked me several times to implement some new feature because I was unable to figure out where to set the necessary info.
dpJudas
 
 
Posts: 3037
Joined: Sat May 28, 2016 1:01 pm

Re: Crash on Gore [Software Renderer]

Post by dpJudas »

Hehe, I can't really blame you. I actually tried several times to see if I could refactor the swrender code a little bit. All those branches failed. My last attempt was to rewrite the whole thing (r_swrenderer2.cpp/h in the QZDoom repository). I got as far as having the main rendering working well enough to render most of vanilla Doom/Doom2, but adding fake floors and 3d floors made me conclude the task was just too big.

At this point the only thing I think can really be done to this part of the codebase is to put it all in a single class and just accept its variable usage is too convoluted to split it any further. A shame tho, because I really wanted to throw multiple cores at the bsp walking part.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

That's only one of the problems. Another one is if you cannot track down where a variable is set for a certain task, there's no chance to add a new feature depending on it. This was the reason why I never managed to add per-tier colorizing for walls.
dpJudas
 
 
Posts: 3037
Joined: Sat May 28, 2016 1:01 pm

Re: Crash on Gore [Software Renderer]

Post by dpJudas »

The primary reason I want it in a class is actually to deal with that situation. As long as the codebase uses globals that aren't even declared in headers, it becomes impossible to see which variables effectively act as function arguments and where the variables even came from. Once the variables are members of a class, it gets much easier to attack them one variable at a time. Especially if you can also see the full set of function names at a glance by looking in the header file.
Gez
 
 
Posts: 17833
Joined: Fri Jul 06, 2007 3:22 pm

Re: Crash on Gore [Software Renderer]

Post by Gez »

Graf Zahl wrote:per-tier colorizing for walls.
Do you mean Doom64-style color gradients?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Crash on Gore [Software Renderer]

Post by Graf Zahl »

What I meant was a color equivalent to Transfer_Light and Transfer_WallLight and a per-tier version of Transfer_WallLight. The reason I never managed to do that was the sad fact that the global variable state is such a total mess.
dpJudas
 
 
Posts: 3037
Joined: Sat May 28, 2016 1:01 pm

Re: Crash on Gore [Software Renderer]

Post by dpJudas »

There's barely any global variables in the software renderer: https://github.com/dpjudas/dpDoom/blob/ ... renderer.h :D
Post Reply

Return to “Closed Bugs [GZDoom]”