Skip to content

Validate BSP vertex input #1653

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

Merged
merged 2 commits into from
Apr 26, 2025
Merged

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Apr 19, 2025

Requires #1652 because it has some other fixes in this area, I didn't test this pr without those.

Apparently, q3map2 sometimes produces garbage values for lightmap uvs in vertices, like nan or just garbage like -1e38 which later turns into inf in patch meshes. Most of these don't use the lightmapping shader, however from my testing some do.

This fixes a crash in MergeDuplicateVertices(), which was happening because of these garbages values, since they always compare as false.

@VReaperV VReaperV force-pushed the fix-bsp-input branch 2 times, most recently from d3ce144 to b11c9cf Compare April 20, 2025 09:33
q3map2 produces garbage lightmap values on some surfaces, so do this to make sure we don't get `nan` or `inf`.
Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Out of curiosity how do the NaNs break things so badly? I wouldn't have though just failing to obey the hash/equality contract could cause that.

@VReaperV
Copy link
Contributor Author

Out of curiosity how do the NaNs break things so badly? I wouldn't have though just failing to obey the hash/equality contract could cause that.

It caused some out of bounds writes. The code assumed that there would, at most, be as many unique vertices as there are vertices in the .bsp, so I kept the hunk allocation that was already there for vertices/indices. With failing equality check though, it generated more vertices than there were oriinally.

@VReaperV VReaperV merged commit 36196cc into DaemonEngine:master Apr 26, 2025
9 checks passed
@VReaperV VReaperV deleted the fix-bsp-input branch April 26, 2025 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants