Another crash with wallsprites

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
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Another crash with wallsprites

Post by drfrag »

It crashes in MAP02 of austerity.pk3 with noclip at the northernmost area (when exiting the playable area). I've tested with vid_rendermode 0.
It's version 3.0 BTW. https://www.realm667.com/index.php/en/p ... ty-tribute
It's an incomplete refactoring in the wallsprites code and some code is missing now below the prepare lightning comment. I don't know what dpJudas was trying to achieve.
There's some similar code in r_decal.cpp (below "// Decals that are added to the scene must fade to black").
It's old, since https://github.com/coelckers/gzdoom/com ... ff60739f23

Code: Select all

		// Prepare lighting

		// Decals that are added to the scene must fade to black.
		ColormapLight cmlight;
		if (spr->RenderStyle == LegacyRenderStyles[STYLE_Add] && cmlight.BaseColormap->Fade != 0)
		{
			cmlight.BaseColormap = GetSpecialLights(cmlight.BaseColormap->Color, 0, cmlight.BaseColormap->Desaturate);
		}
Of course cmlight.BaseColormap is nullptr.
User avatar
Rachael
Admin
Posts: 12907
Joined: Tue Jan 13, 2004 1:31 pm
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Preferred Pronouns: She/Her

Re: Another crash with wallsprites

Post by Rachael »

Can you please give exact map coordinates where the sprite is visible from so that I can verify the fix?
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Re: Another crash with wallsprites

Post by drfrag »

Code: Select all

Current map: map02

viewx = 837.951460
viewy = 5205.998836
viewz = -118.065502
viewangle = 90.878906

Code: Select all

Current map: map02

viewx = 838.414319
viewy = 2943.900698
viewz = 153.332968
viewangle = 90.000000
Just walk to the north until it crashes, now i've seen it happens before reaching the top too (at the door with the archvile).
dpJudas
 
 
Posts: 2861
Joined: Sat May 28, 2016 1:01 pm

Re: Another crash with wallsprites

Post by dpJudas »

About what I've been trying to achieve in the SW renderer is always one out of two things:

1) Either it is the futile attempt at doing 'separation of concerns' in a codebase nobody ever heard about it for 20+ years. I.e. notice just how many places the light calculation code is copy-pasta in that codebase. I'm trying to move things closer and closer to their true home. Basically I want to make the renderer first figure out what it can see, then setup a drawing of the object (wall, flat or sprite, light and renderstyle), then tell the drawer to do it. Light calculations should be shared as that setup, not be copy pasta 50 places in the code. The clip list management likewise needs to happen in classes meant to only do that. The wall coordinates needs to be by itself, etc etc. Normally I would just rewrite this kind of code, but in Doom's case I don't fully understand the reasons why #23 of the 50 copy-pastas are arranged a little bit different, has a check missing, and so on. Is it because I'm looking at a bug where someone forgot to update the copy pasta? Or are all the other places missing that check? Or is the check redundant? Is there a special case rule that needs to be accounted for? Ah it is lovely to work with this kind of code. :)

2) Or, I'm trying to isolate the code for multithreading purposes. In simplified terms, in order to multithread things I need things done so you can see them as what would be called a Task in modern frameworks. That means certain things needs to be done at certain cutoff points in order for me to be able to move the next processing to another thread. I also need the data in question to be clustered right so that I can move a single struct for the task input or output.

In this particular commit I've been doing #1. However, the larger reason I might have been doing it was to achieve #2 in a series of multiple commits.
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Re: Another crash with wallsprites

Post by drfrag »

Thanks, interesting.
About the fix that just prevents the crash, i was trying to restore the original behavior (if it even worked). Of course cmlight.BaseColormap will always be null so that code will never execute.
dpJudas
 
 
Posts: 2861
Joined: Sat May 28, 2016 1:01 pm

Re: Another crash with wallsprites

Post by dpJudas »

You need to be more clear about what the original behavior was, visually, that is. :)
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Re: Another crash with wallsprites

Post by drfrag »

Thanks but i don't see any wallsprite. I've done a code analysis instead to reconstruct the missing part. It compiles, it doesn't crash and seems to make sense so it must be good. :) What do you think?
https://github.com/drfrag666/gzdoom/com ... 2a03133e10
User avatar
Rachael
Admin
Posts: 12907
Joined: Tue Jan 13, 2004 1:31 pm
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Preferred Pronouns: She/Her

Re: Another crash with wallsprites

Post by Rachael »

I'd still put a null check there just in case. Even if it's impossible to be null now, that doesn't change that the code could change in the future such that it could be null, or even worse, sometimes mysteriously being null leading to a bug that cannot be reliably traced. Other than that, it looks good to me and should be PR'd. :)
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Re: Another crash with wallsprites

Post by drfrag »

User avatar
Rachael
Admin
Posts: 12907
Joined: Tue Jan 13, 2004 1:31 pm
Discord: Rachael#3767
Twitch ID: madamerachelle
Github ID: madame-rachelle
Preferred Pronouns: She/Her

Re: Another crash with wallsprites

Post by Rachael »

Thank you! :)
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3110
Joined: Fri Apr 23, 2004 3:51 am
Discord: drfrag#3555
Github ID: drfrag666
Location: Spain

Re: Another crash with wallsprites

Post by drfrag »

You're welcome.

Return to “Closed Bugs”