[Carmack] Sprites + Wrapped Midtextures + 3DFloors = Holes

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
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

[Carmack] Sprites + Wrapped Midtextures + 3DFloors = Holes

Post by Xaser »

Apologies for the title. It was a mouthful.

The gist is that in the Carmack renderer, if you have both 3D floors and wrapped midtextures visible in a scene, sprites will punch holes through the textures. Visuals are better than words, so here's a test wad and a screenshot:
https://static.angryscience.net/pub/doo ... rapbug.wad
Image

This is a bit vague/sketchy, and the test wad is weird, but this is basically an issue we ran into during Square E2 development that I've reproduced here, so I'm unsure of the complete cause -- just that this particular recipe yields the symptoms.

I haven't gone version-spelunking yet to figure out when this last worked, but it used to in some quasi-recent GZDoom build. I've confirmed it broken on GZDoom v3.2.5 and the latest devbuild at time of writing (g3.3pre-400-g3228751). It happens from a fresh config (after switching to Software), so hopefully that helps.
User avatar
Rachael
Posts: 13558
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Rachael »

This may be a side effect of me attempting to fix another bug with the exact same stuff - midtextures (in this case, fog boundaries, but they are still midtextures from the renderer point of view) with 3D floors in the same scene.

Basically the only way this can be fixed is either disable 3D floors completely, or redo them from the ground-up. In their current form, 3D floors are a black magic that are more difficult to understand than most of the early Silverman BUILD code.

In more technical terms, what is happening is the 3D floor code will redraw a scene above and below it, sometimes ambiguously selectively, including midtextures, even when the midtextures don't need it.

Honestly, I do not believe this can be fixed without a complete rewrite of the entire 3D floor system in the Carmack renderer.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

Fog boundaries are causing the same problem -- I'll see if I can augment the test wad later. For Square, we can work around the wrapped midtextures if we must, but the fog is going to prove much more problematic.

[STEALTH-EDIT] Not bumping this 'cause there's nothing new to report on isolating the issue yet, but ignore the note above about fog boundaries. There was a buggy scene in the map that I thought was a fog boundary, but was actually a translucent solid color wrapped midtexture -- i.e. the same problem as the OP with no differences. Apologies for the red herring.

I did some quick release version tests and it seems to have broken between GZDoom v2.1.1 and v2.2.0. Need to get to bed tonight, but I'll see if I can zero in on a commit sometime soonish.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

Well, dang. I didn't expect "soonish" to turn into "literally a whole month", but here we are. :P

I've nailed the commit to this one. It's an old commit by randi from way before the fog boundary stuff was touched, so fortunately that doesn't have anything to do with it.

In master, the offending line is line 158 of swrenderer/line/r_renderdrawsegment.cpp. Removing the "if (!wrap)" effectively reverts randi's change and removes the holes. I'm not sure what this was attempting to fix, but the side effects are pretty severe.

I can submit a PR if reverting this is acceptable. Either way, it'd help to have an expert or two around weigh in on what this fix was supposed to do to see if we can arrive at a more proper solution.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

PR submitted anyhow. Here it is.

FWIW, I haven't been able to locate any references to the bug Randi fixed (outside of that commit), but we're using wrapped midtextures and 3D Floors all over the place in Adventures of Square E2, and everything worked just fine for us on the older versions.

Either way, the situation got way worse after that change, so I'm highly in favor of reverting it (i.e. accepting the PR). Naturally. :P
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49067
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Graf Zahl »

I'll wait for dpJudas to drop a line before merging it.
dpJudas
 
 
Posts: 3040
Joined: Sat May 28, 2016 1:01 pm

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by dpJudas »

I can't really understand much of the code in that area. If I could, I would have rewritten it as I have no doubt that it could be significantly simplified. If we revert it, maybe we'll get a bug report for the other case and that will give us a better idea of what Randi was trying to fix.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

The lack of an identifiable bug report is what led me to shrug and make the PR anyway. Can't test something without a test case, and it's not like reverting the fix-attempt is going to make the situation any worse. :P

Seems like we're good for merging, then?
User avatar
Rachael
Posts: 13558
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Rachael »

Admittedly I am a bit leery of merging this right away with a new release right around the corner. Sometimes it's better to live with a glitch that's been there for a long time, than to blindly fix a glitch that may have been part of a fix for something much more serious.

The visual defects may be covering up a much more serious bug - and without an identifiable bug report we'll never really know.

Also I have been playing around with code in similar places to try and fix this issue, and it's also had mixed results - not really fixing the problem except for in one mapset, and breaking in another.

I'd say we can go with the merge if we want to delay the release another week or so for testing.
User avatar
drfrag
Vintage GZDoom Developer
Posts: 3141
Joined: Fri Apr 23, 2004 3:51 am
Location: Spain
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by drfrag »

Well, the commit says "Fixed: Tiled midtextures could go up past the ceiling when a 3D floor is in view", may be there wasn't any bug report.
That commit is in ZDoom32 and it's also affected. I know it's unrelated but the old truecolor renderer from QZDoom 0.0 alpha also shows this, may be a bad translation?
Screenshot_Doom_20180307_195917.png
User avatar
Rachael
Posts: 13558
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Rachael »

Those are broken 1-col drawers. They were fixed in a later commit.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

What can I do to help get this merged before release? I can try and play around with trying to reproducing whatever issue Randi fixed, though it'll be a bit guessworky without some assistance.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49067
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Graf Zahl »

Xaser wrote:What can I do to help get this merged before release?
This won't get merged before a release, unless someone can prove beyond reasonable doubt that the issue the reverted commit was supposed to fix wasn't worth fixing and this change doesn't break anything.
dpJudas
 
 
Posts: 3040
Joined: Sat May 28, 2016 1:01 pm

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by dpJudas »

Xaser wrote:What can I do to help get this merged before release? I can try and play around with trying to reproducing whatever issue Randi fixed, though it'll be a bit guessworky without some assistance.
I'm not sure how much this will help, but I'll try explain what the code is doing and why (as far as I understand it):

Initially the renderer walks the BSP tree and draws all solid walls. As it does this it keeps track on what has already been drawn vertically as min/max values for each pixel column on the screen. Essentially this is the top of the mid wall stored in 'ceilingclip' arrays and the bottom in 'floorclip' arrays. For each two-sided midtexture it creates a DrawSegment for the wall, but it does not draw it yet. As part of this creation it copies the clip arrays into 'sprtopclip' and 'sprbottomclip' on the DrawSegment.

Once the renderer is done drawing all the solid walls and planes, it draws the sprites. The sprites are drawn back to front, but before each sprite is drawn it checks if a draw segment is behind the sprite. If it is, then it draws only the part of the draw segment covered by the sprite. It then marks that part of the draw segment drawn by filling 'sprtopclip' with values beyond any 'sprbottomclip' value, thus effectively causing everything in that range to be fully clipped. That's what the 'fillshort' lines are about.

After it is done drawing all the sprites it iterates through all the draw segments again and draws them all back to front. All the places that were covered by sprites have the clip arrays modifies so they skip those blocks so sprites don't get overwritten in this last draw call.

This is Carmack's original (brain dead) way of sorting segments and sprites. On top of this 3D floors were added. I'm not 100% sure of the attempted algorithm here, but for each 3D floor height it draws only sprites above or below that floor level. Since it reuses the original sprite drawing loops, it runs into the problem with sprtopclip being cleared as part of the drawing as it now calls it multiple times. That's where the 'bkup' array enters the picture. The code attempts to take a backup of the array, so that other code can mess up the array and then it can copy back the original. The code you changed made it always copy back the backup. Apparently Randi identified a situation where restoring the array wasn't good for 3D floors.

That's about as far as I can help. The exact details of 3D floor segment/sprite clipping eludes me and for softpoly I opted for a much easier way: split/sort sprites by their subsector before drawing them. That way you have the BSP guaranteeing that wall segment is in front all sprites drawn so far and behind the rest.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol

Post by Xaser »

Thanks for the breakdown! That helps a bit -- it's still quite arcane, but it at least identifies a few troublesome spots.
dpJudas wrote:If [a draw segment is behind a sprite], then it draws only the part of the draw segment covered by the sprite.
The obvious bit, I suppose: this is pretty clearly the step that's getting "skipped." The "if(!wrap)" check is unconditionally disabling the bkup restore, meaning none of these behind-sprite draws happen at all since sprtopclip's been trashed. This part makes sense.

The mystery, then, is trying to figure out exactly what case Randi found in which restoring from bkup was a bad idea. We really don't have anything to go off of aside from the commit description ("Tiled midtextures could go up past the ceiling when a 3D floor is in view"), but judging from that alone, here's an educated guess of sorts:

The big scary 3D floor pass in RenderTranslucentPass starts from the top of the screen, iterates through all 3D floors from top to bottom, and re-renders the scene until it hits the player's viewz. At this point, it flip-flops and starts rendering the remaining 3D floors from the bottom up. Seems like a setup for a classic off-by-one error: either on the first, last, or "middle" pass (the one where it switches from top-down to bottom-up), sprtopclip gets filled with [something bad] from bkup that causes the drawsegment to render infinitely upward. Presuming that's what Randi meant in that commit desc, of course. ;)

This is all pure conjecture unless we can actually trigger Randi's bug though. I'll muck about with Doom Builder soonish to see if I can produce anything that matches the description.
Post Reply

Return to “Closed Bugs [GZDoom]”