[Carmack] Sprites + Wrapped Midtextures + 3DFloors = Holes
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.
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.
[Carmack] Sprites + Wrapped Midtextures + 3DFloors = Holes
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
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.
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
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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
[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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
Well, dang. I didn't expect "soonish" to turn into "literally a whole month", but here we are.
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.
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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
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.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
I'll wait for dpJudas to drop a line before merging it.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
Seems like we're good for merging, then?
Seems like we're good for merging, then?
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
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.
- drfrag
- Vintage GZDoom Developer
- Posts: 3141
- Joined: Fri Apr 23, 2004 3:51 am
- Location: Spain
- Contact:
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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?
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?
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
Those are broken 1-col drawers. They were fixed in a later commit.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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.Xaser wrote:What can I do to help get this merged before release?
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
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):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.
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.
Re: [Carmack] Sprites + Wrapped Midtextures + 3DFloors = Hol
Thanks for the breakdown! That helps a bit -- it's still quite arcane, but it at least identifies a few troublesome spots.
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.
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.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 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.