64-bit OpenGL holes

Splitting discussion from #38 (closed)

What I observed:

• By the time HWR_RenderPlane() is called, the extrasubsector[].polyplane->poly coordinates are already fudged up
• HWR_SubsecPoly() produces poly coordinates in 32-bit that are more consistent with subsector coordinates (but not always). However, in 64-bit, more often the poly coordinates do not match the subsector coordinates. But more importantly, the 64-bit output does not match the 32-bit output.
• 32 and 64-bit are consistent in the values they process, until this code block in CutOutSubsecPoly(), below. Two more identical code blocks exist at the end of SplitPoly()

I don't know if fixing this inconsistency would fix the holes, but it's rather glaring. I'm not completely educated on what this code is doing -- it's called from WalkBSPNode(), a recursive function -- but I'll bang at it with a hammer later this week...

Coordinate printouts and test EXE with log messages. See 64-gl-log branch for log messages.

For reference:

• Sapphire Falls' missing triangle is subsector 79
• GFZ2's missing triangle is subsector 1222

	// there was a split
	if (ps >= 0)
	{
		//need 2 points
		if (pe >= 0)
		{
			// generate FRONT poly
			temppoly = HWR_AllocPoly(nump);
			pv = temppoly->pts;
			*pv++ = vs;
			*pv++ = ve;
			do
			{
				if (++ps == poly->numpts)
					ps = 0;
				*pv++ = poly->pts[ps]; // <------------- FIRST INCONSISTENCY IN 64-BIT
			} while (ps != pe);
			HWR_FreePoly(poly);
			poly = temppoly;
		}
		//hmmm... maybe we should NOT accept this, but this happens
		// only when the cut is not needed it seems (when the cut
		// line is aligned to one of the borders of the poly, and
		// only some times..)
		else
			skipcut++;
		//    I_Error("CutOutPoly: only one point for split line (%d %d) %d", ps, pe, debugpos);
	}
}
return poly; // <------------ THIS GETS FED INTO EXTRASUBSECTOR[], LATER

changed the description

changed the description

Another 32/64 difference: by the time the code reaches here in SplitPoly(), poly->pts[] is off by a few decimal places. This happens earlier than the behavior split in the top post.

Example is from subsector 28 (bsp == 32796), Sapphire Falls.

The 32/64 GL coordinate difference happens at subsector 29. So subsector 28 is the prior unit in which things start breaking.

poly->pts[] failure in SplitPoly():

for (i = 0; i < poly->numpts; i++)
{
	j = i + 1;
	if (j == poly->numpts) j = 0;

	// start & end points
	pv = fracdivline(bsp, &poly->pts[i], &poly->pts[j]); // <----------- poly->pts is off by a few decimals

32:

64:

This causes a break here in fracdivline(), where the returned polyvertex will be NULL

// first check the frac along the polygon segment,
// (do not accept hit with the extensions)
num = (v2x - v1x)*v2dy + (v1y - v2y)*v2dx;
frac = num / den;
if (frac < 0 || frac > 1) // <------------- 32: can be 0.9999927283; 64: can be 1.000000347437
	return NULL;

When the result of fracdivline() is NULL, that can prevent pe from being incremented in SplitPoly(), which means nptback/front will be inaccurate, which means not enough memory will be allocated when HWR_AllocPoly() is called.

pe increment failure in SplitPoly():

if (pv) // <-------- 32: this passes; 64: this FAILS
{
	if (ps < 0)
	{
		// first point
		ps = i;
		vs = *pv;
		fracs = bspfrac;
	}
	else
	{
		//the partition line traverse a junction between two segments
		// or the two points are so close, they can be considered as one
		// thus, don't accept, since split 2 must be another vertex
		if (SameVertice(pv, &lastpv))
		{
			if (pe < 0)
			{
				ps = i;
				psonline = 1;
			}
			else
			{
				pe = i; // <---------- 64: we don't get here!
				peonline = 1;
			}
		}

HWR_AllocPoly() call failure in SplitPoly():

// number of points on each side, _not_ counting those
// that may lie just one the line
nptback  = pe - ps - peonline; // <--------------------- 32: nptback == 2; 64: nptback == 3
nptfront = poly->numpts - peonline - psonline - nptback;

if (nptback > 0)
	*backpoly = HWR_AllocPoly(2 + nptback); // <------------- 32: backpoly->pts == 4; 64: backpoly->pts == 5
else
	*backpoly = NULL;
if (nptfront)
	*frontpoly = HWR_AllocPoly(2 + nptfront); // <------------- 32: backpoly->pts == 5; 64: backpoly->pts == 4
else
	*frontpoly = NULL;

In all, the wrong 64-bit values is definitely allocation-related, and HWR_AllocPoly() is not being called with the correct number of points to allocate.

Follow-up questions:

• poly is sourced from previous calls to WalkBSPTree(). Where do the fraction inconsistencies start?
  • May have to step-by-step from subsector 0

may want to halt that thinking a moment, WalkBSPNode isn't going in order of subsectors at all, it's going by the BSP tree of nodes and subsectors; they could be in any order!

WalkBSPNode starts at node numnodes-1, checking its two children, which could be either another node or a subsector. Rinse and repeat if it's a node, stop recursing if it's a subsector, etc etc

You're right, I'm doing a mind-numbing breakpoint exercise and it's getting me nowhere :X

I do wonder if double calculations differ between 32 and 64 (answer: yes, yes, yes). I'm wondering if this really is the culprit code, and it's messing up somewhere in the WalkBSPNode() maze

static polyvertex_t *fracdivline(fdivline_t *bsp, polyvertex_t *v1,
	polyvertex_t *v2)
{
	static polyvertex_t pt;
	double frac;
	double num;
	double den;
	double v1x,v1y,v1dx,v1dy;
	double v2x,v2y,v2dx,v2dy;

	// a segment of a polygon
	v1x  = v1->x;
	v1y  = v1->y;
	v1dx = v2->x - v1->x;
	v1dy = v2->y - v1->y;

	// the bsp partition line
	v2x  = bsp->x;
	v2y  = bsp->y;
	v2dx = bsp->dx;
	v2dy = bsp->dy;

	den = v2dy*v1dx - v2dx*v1dy;
	if (den == 0)
		return NULL;       // parallel

	// first check the frac along the polygon segment,
	// (do not accept hit with the extensions)
	num = (v2x - v1x)*v2dy + (v1y - v2y)*v2dx;
	frac = num / den;
	if (frac < 0 || frac > 1)
		return NULL;

Interesting, huh.

On a related note, I noticed Doom Legacy changed a lot with hw_bsp.c on their end, including with handling the floating point numbers ...for instance in that function, they no longer check den == 0 directly (should be 0.0 anyway), but instead check fabs(den) < 1.0E-36f. Wonder if that would affect anything or not for us

https://sourceforge.net/p/doomlegacy/svn/HEAD/tree/legacy_one/trunk/src/hardware/hw_bsp.c

From Discord:

Compile 32-bit builds with -msse -mfpmath=sse, which forces SSE instructions for floating point math, and this happens:

This shows that the fracdivline() logic is not portable. This could affect us for 2.2, if we optimize the builds for Pentium 4+.

IDK if 64-bit can be compiled NOT to use SSE logic for floating point. Otherwise, this function needs to be fixed.

now, if we try to compile SRB2 with -Wfloat-equal...

Since with diff FPU modes, the results will be OFF...

command.c: In function ‘Setvalue’: command.c:1093:8: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] r_data.c: In function ‘R_CreateColormap’: r_data.c:1223:22: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] sdl/mixer_sound.c: In function ‘I_PlaySong’: sdl/mixer_sound.c:775:49: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] sdl/mixer_sound.c:782:17: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c: In function ‘GLPerspective’: hardware/r_opengl/r_opengl.c:605:14: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c:605:32: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c:605:52: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c: In function ‘GLProject’: hardware/r_opengl/r_opengl.c:644:12: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c: In function ‘DrawMD2Ex’: hardware/r_opengl/r_opengl.c:1989:26: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/r_opengl/r_opengl.c: In function ‘SetTransform’: hardware/r_opengl/r_opengl.c:2072:75: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c: In function ‘HWR_DrawFixedPatch’: hardware/hw_draw.c:230:25: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c:237:26: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c: In function ‘HWR_DrawCroppedPatch’: hardware/hw_draw.c:378:25: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c:385:26: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c: In function ‘HWR_DrawFill’: hardware/hw_draw.c:849:24: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_draw.c:856:25: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_bsp.c: In function ‘fracdivline’: hardware/hw_bsp.c:196:10: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_bsp.c: In function ‘SameVertice’: hardware/hw_bsp.c:257:12: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_bsp.c:259:12: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_main.c: In function ‘HWR_DrawSpriteShadow’: hardware/hw_main.c:4140:30: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_main.c: In function ‘HWR_SplitSprite’: hardware/hw_main.c:4308:30: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_main.c: In function ‘HWR_DrawSprite’: hardware/hw_main.c:4588:30: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_main.c: In function ‘HWR_SortVisSprites’: hardware/hw_main.c:4834:20: warning: comparing floating point with == or != is unsafe [-Wfloat-equal] hardware/hw_main.c: In function ‘HWR_SetViewSize’: hardware/hw_main.c:5755:19: warning: comparing floating point with == or != is unsafe [-Wfloat-equal]

closed