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

[Rachael]

A really good map set to test 3D floors and sprites/midtex's with is Unloved and Unloved 2. The extensive 3D floor usage in these mod makes it an ideal candidate for bug troubleshooting with the software 3D floor system, as the problems occur in here in droves. It's been my experience that no matter what I do to fix it, something else breaks.

Make sure to run 'git blame' over the code, as well, because there may be modifications from me in that code from about a year or so ago. You'll likely want to revert those commits.

A couple good places:

Unloved - MAP01 (-246, 372, 67)
Unloved 2 - MAP08 (48, 309, -1334)

[Graf Zahl]

Hi-Tech Hell is also a great thing to test 3D floors. Since it's made for Doom Legacy it doesn't have any slopes that may cause problems.

[dpJudas]

I took a look at the code and analyzed it a bit further.

bkup is always a copy of the original ceilingclip and never changes.

RenderDrawSegment::Render itself gets called in two situations: in RenderTranslucentPass::DrawMaskedSingle and VisibleSprite::Render.

The visible sprite render is called first and here fake3D does not have the FAKE3D_REFRESHCLIP flag yet. The flow is thus as follows:

1) Calls RenderDrawSegment::Render for each draw segment behind a sprite. Draws the segment columns behind the sprite. Fills those ranges in 'sprtopclip' with 'viewheight'.
2) Calls RenderDrawSegment::Render for all draw segments. This draws the full segment, except for those ranges cleared in step #1.
3) At the end of the second RenderDrawSegment::Render call it copies back the backup. This ensures the next call to DrawMaskedSingle begins from fresh.

Randi's fix basically says that #3 should not begin from fresh if it is a wrapped midtex. Why would that ever be needed? At what height should a wrapped midtex end? The ceiling?

[Rachael]

I wonder if it might be one of the many things that never actually got finished in the renderer before and during the floating point rewrite.

[Xaser]

bkup is always a copy of the original ceilingclip and never changes.

...

Randi's fix basically says that #3 should not begin from fresh if it is a wrapped midtex. Why would that ever be needed? At what height should a wrapped midtex end? The ceiling?

Yeah, the more I noodle this, the less it makes sense. If sprtopclip is restored from bkup, then the real ceiling clipping should be correct. Here's the place in RenderDrawSegment::RenderWall that pulls mceilingclip from sprtopclip.

If Randi instead meant that midtextures were bleeding into the ceilings of 3D floors, then you'd think that the culprit would live here. Does anything seem awry here versus the non-wrapped-midtex version of the same code?

It feels more and more like we're chasing a ghost.

[dpJudas]

OK, I've decided that what Randi was doing made no sense whatsoever. I don't know what bug she was trying to fix, but the fix was certainly incorrect. I effectively committed your change to master, but I didn't do it via your PR as I took the opportunity to move the backup copy code closer to its correct location.

Now on to eradicate the fake3D completely - probably the stupidiest variable in all of the swrenderer. It packs two enums into one integer and then also uses bits for booleans for the goal of saving 36 bytes of system memory..

[Xaser]

Thanks! Just made a fresh build from master and everything's A-OK here. Good to see the code infinitely less janky now too.

I'll keep an eye out for any stray midtex phantoms in the future, just for pseudoscience's sake, but I doubt there's anything to be found.

[EDIT] This can be closed as [Fixed] now.

[Rachael]

Well it looks a lot better, but I can see the original issue Randi was trying to address. (That didn't actually get addressed with her fix)

In MAP08 of Unloved2 in the hospital, if 3D floors are present, midtextures can no longer clip at all, they are always drawn as if clipping is turned off. The compat options are already set such that midtextures should always clip, and this was done because the mapset was originally for the OpenGL renderer, before we had 3D floors and truecolor and stuff.

[dpJudas]

Fixed the midtexture clipping error in MAP08 of Unloved2.

[drfrag]

Those are broken 1-col drawers. They were fixed in a later commit.

Thanks very much, i guess mystery solved then. If only i could port the fix from LLVM to C++... First i'll need to find out which drawer i'd need to fix.

Fixed the midtexture clipping error in MAP08 of Unloved2.

Well done!