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

Add GeometryOptimiser, move some stuff there from tr_bsp #1640

Merged
merged 3 commits into from
Apr 13, 2025

Conversation

VReaperV
Copy link
Contributor

@VReaperV VReaperV commented Apr 6, 2025

Requires #1635

This moves some surface merging stuff into GeometryOptimiser.cpp, and adds a function that removes duplicate vertices there. Also removes some more duplicate code, and puts all renderer surfaces into temp memory at the start of R_CreateWorldVBO(), to avoid going over more of them (i. e. it excludes portals, autosprite surfaces, SF_SKIP etc. at the start, rather than every time when iterating through all of the surfaces.

Adds some functionality that will be needed for further improvements to geometry processing with material system (I've done some tests around that, which already show good results, but I need to make it use surfaces of up to a specific limit of triangles to make it work well in all cases).

@VReaperV VReaperV added T-Improvement Improvement for an existing feature A-Renderer T-Cleanup labels Apr 6, 2025
/*
===========================================================================

Daemon BSD Source Code
Copy link
Member

Choose a reason for hiding this comment

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

This file seems to have old code copy and pasted so it should have the GPL license.

vboIdxs[ vboNumIndexes++ ] = vboNumVerts + surfTriangle->indexes[ 1 ];
vboIdxs[ vboNumIndexes++ ] = vboNumVerts + surfTriangle->indexes[ 2 ];
}
// OptimiseMapGeometryMaterial( &s_worldData, numSurfaces, materialVerts, numVerts, materialIdxs, numTriangles * 3 );
Copy link
Member

Choose a reason for hiding this comment

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

What's up with that?

@illwieckz
Copy link
Member

Seeing work for more geometry optimization is great!

Just a thought brought by reading the PR:

For multi-licenses issues, when we have to put new functions into a file with a more permissive license, I think we may add specific comments to split the file without actually doing two files.

I'm thinking about doing those for huge code blocks I entirely rewritten from tr_shader.cpp for example, where some of the code is like, entirely brand new for hundreds of lines.

It's harder to do when the new/old code is intricately mixed (not just whole functions or large code blocks living side to side).

The easiest way is to use the old license when mixing new code with the old license to avoid marking each block, but we may start marking blocks at some point.

@slipher
Copy link
Member

slipher commented Apr 9, 2025

The task of rewriting the entire engine with BSD-licensed code seems a bit hopeless, so personally I just write new code in old files if it would make my life any more difficult to do otherwise. Dealing with multiple licenses in one file sounds painful.

@VReaperV
Copy link
Contributor Author

VReaperV commented Apr 9, 2025

I'll deal with that when #1635 is merged, I'll need to rebase on it anyway.

@VReaperV VReaperV force-pushed the geometry-optimiser branch 6 times, most recently from a4f1447 to b740cd9 Compare April 13, 2025 21:31
Move some surface merging stuff there from `tr_bsp`, add some definitions for material system use.
Also use tempMemory with only the renderable surfaces, to avoid looping over everything and to remove useless checks for `surfaceType`.
@VReaperV VReaperV force-pushed the geometry-optimiser branch 2 times, most recently from cc2749e to 540444b Compare April 13, 2025 21:47
@VReaperV VReaperV force-pushed the geometry-optimiser branch from 540444b to b4a5ad7 Compare April 13, 2025 21:51
@VReaperV
Copy link
Contributor Author

Mac ci is tripping as the variable is incremented in the loop (possibly optmised out, but the next pr ensures it won't be, so, whatever...).

@VReaperV VReaperV merged commit 7160194 into DaemonEngine:master Apr 13, 2025
6 of 9 checks passed
@VReaperV VReaperV deleted the geometry-optimiser branch April 13, 2025 22:01
@slipher
Copy link
Member

slipher commented Apr 13, 2025

Why is this being merged without LGTM? Doesn't seem very urgent.

@VReaperV
Copy link
Contributor Author

I'm simply following what the project head has said.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Renderer T-Cleanup T-Improvement Improvement for an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants