Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect lightmap rendering #232

Open
hemebond opened this issue Dec 27, 2024 · 4 comments · May be fixed by #233
Open

Incorrect lightmap rendering #232

hemebond opened this issue Dec 27, 2024 · 4 comments · May be fixed by #233

Comments

@hemebond
Copy link
Contributor

I've noticed lightmaps are not rendering correctly on maps. This has affected my test maps and also Winter Hellscape shown here:

dp20241227214429-00

---- bspinfo / ericw-tools brushbsp ----
---------------------
id1/maps/xm01.bsp
LoadBSPFile: 'id1/maps/xm01.bsp'
BSP is version Quake BSP bsp29
INFO: using built-in palette.

count   lump name    byte size 
     63 models             4032
   3850 planes            77000
  11092 vertexes         133104
   4978 nodes            119472
   2290 texinfos          91600
  10202 faces            204040
  10744 clipnodes         85952
   3063 leafs             85764
  13066 marksurfaces      26132
  22328 edges             89312
  42814 surfedges        171256
    158 textures        1210116
        lightdata        381676
        visdata          244222
        entdata           42106

The same map lightmap looks fine in FTEQW and Ironwail. Affects both the built-in lightmap and the .lit file.

@Baker7
Copy link

Baker7 commented Dec 28, 2024

model_brush.c -> Mod_Q1BSP_LoadFaces

#define DotProductDouble(a,b) ((double)(a)[0](double)(b)[0]+(double)(a)[1](double)(b)[1]+(double)(a)[2]*(double)(b)[2])

			val = DotProductDouble((loadmodel->surfmesh.data_vertex3f + 3 * surface->num_firstvertex) + i * 3, surface->lightmapinfo->texinfo->vecs[j]) + (double)surface->lightmapinfo->texinfo->vecs[j][3];
			texmins[j] = min(texmins[j], val);
			texmaxs[j] = max(texmaxs[j], val);

@hemebond
Copy link
Contributor Author

Ooo, a precision error. Thank you @Baker7, that has definitely improved it. Did I need to switch to the DotProductDouble macro elsewhere in the loop or function?

dp20241228203321-00

@hemebond
Copy link
Contributor Author

Okay, I replaced other calls to the macro and it's now matching what Ironwail shows me.

dp20241228212139-00

Thank you, @Baker7, you're a genius. Writing up a PR now.

@Baker7
Copy link

Baker7 commented Dec 28, 2024

I thought the whole way to fix the issue was just that single line and with your screenshots, obviously implementing that in the entire function is the correct and full way to implement the fix.

The fix I previously had in Zircon seemed to work in the example maps that I had tried out, but this map proves me to be a really good test case and I have now extended it to the whole function like what you did.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants