* improve CSG Subtract results via more defined splits order

improve CSG Subtract performance (evaluate aabb before brush copy and buildBrep())
Garux · Feb 16, 2025 · 12faf99 · 12faf99
1 parent 2abab59
commit 12faf99
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 26 deletions.
diff --git a/contrib/bobtoolz/DEntity.cpp b/contrib/bobtoolz/DEntity.cpp
@@ -344,13 +344,13 @@ bool DEntity::LoadFromEntity( scene::Node& ent, const LoadOptions options ) {
 }
 
 void DEntity::RemoveNonCheckBrushes( const std::vector<CopiedString>& exclusionList ){
-	brushList.erase( std::remove_if( brushList.begin(), brushList.end(), [&]( DBrush *brush ){
+	std::erase_if( brushList, [&]( DBrush *brush ){
 		if ( std::any_of( exclusionList.cbegin(), exclusionList.cend(), [brush]( const CopiedString& tex ){ return brush->HasTexture( tex.c_str() ); } ) ) {
 			delete brush;
 			return true;
 		}
 		return false;
-	} ), brushList.end() );
+	} );
 }
 
 void DEntity::ResetChecks( const std::vector<CopiedString>& exclusionList ){

diff --git a/docs/changelog-custom.txt b/docs/changelog-custom.txt
@@ -1111,6 +1111,7 @@ misc...
 	* fix surface inspector updates on patch texturing changes, on UV Tool edits
 	* add prefs->Build->Region Box Shader (shader to use on the box generated around region)
 	* add bobToolz.explodeEntity: create group entity from each primitive of existing group entity
+	* improve CSG Subtract results via more defined splits order
 
 
 

diff --git a/radiant/csg.cpp b/radiant/csg.cpp
@@ -419,7 +419,7 @@ class BrushDeleteSelected : public scene::Graph::Walker
 		return true;
 	}
 	void post( const scene::Path& path, scene::Instance& instance ) const {
-		if ( Brush* brush = Node_getBrush( path.top() ) ) {
+		if ( Node_isBrush( path.top() ) ) {
 			if ( path.top().get().visible()
 			  && Instance_isSelected( instance )
 			  && path.top().get_pointer() != m_keepNode ) {
@@ -549,32 +549,27 @@ brushsplit_t Brush_classifyPlane( const Brush& brush, const Plane3& plane ){
 	return split;
 }
 
-bool Brush_subtract( const Brush& brush, const Brush& other, brush_vector_t& ret_fragments ){
+bool Brush_subtract( const Brush& brush, const Brush& other, const std::vector<const Face *>& otherfaces, brush_vector_t& ret_fragments ){
 	if ( aabb_intersects_aabb( brush.localAABB(), other.localAABB() ) ) {
 		brush_vector_t fragments;
 		fragments.reserve( other.size() );
 		Brush back( brush );
 
-		for ( Brush::const_iterator i( other.begin() ); i != other.end(); ++i )
+		for ( const Face* face : otherfaces )
 		{
-			if ( ( *i )->contributes() ) {
-				brushsplit_t split = Brush_classifyPlane( back, ( *i )->plane3() );
-				if ( split.counts[ePlaneFront] != 0
-				  && split.counts[ePlaneBack] != 0 ) {
-					fragments.push_back( new Brush( back ) );
-					Face* newFace = fragments.back()->addFace( *( *i ) );
-					if ( newFace != 0 ) {
-						newFace->flipWinding();
-					}
-					back.addFace( *( *i ) );
-				}
-				else if ( split.counts[ePlaneBack] == 0 ) {
-					for ( brush_vector_t::iterator i = fragments.begin(); i != fragments.end(); ++i )
-					{
-						delete( *i );
-					}
-					return false;
+			brushsplit_t split = Brush_classifyPlane( back, face->plane3() );
+			if ( split.counts[ePlaneFront] != 0
+			  && split.counts[ePlaneBack] != 0 ) {
+				fragments.push_back( new Brush( back ) );
+				if ( Face* newFace = fragments.back()->addFace( *face ) ) {
+					newFace->flipWinding();
 				}
+				back.addFace( *face );
+			}
+			else if ( split.counts[ePlaneBack] == 0 ) {
+				for ( Brush* frag : fragments )
+					delete( frag );
+				return false;
 			}
 		}
 		ret_fragments.insert( ret_fragments.end(), fragments.begin(), fragments.end() );
@@ -586,30 +581,91 @@ bool Brush_subtract( const Brush& brush, const Brush& other, brush_vector_t& ret
 class SubtractBrushesFromUnselected : public scene::Graph::Walker
 {
 	const brush_vector_t& m_brushlist;
+	std::vector<std::vector<const Face *>> m_brushfaces; // m_brushfaces.size() == m_brushlist.size()
 	std::size_t& m_before;
 	std::size_t& m_after;
 	mutable bool m_eraseParent = false;
 	scene::Node* m_world = Map_FindWorldspawn( g_map );
 public:
 	SubtractBrushesFromUnselected( const brush_vector_t& brushlist, std::size_t& before, std::size_t& after )
-		: m_brushlist( brushlist ), m_before( before ), m_after( after ){
+		: m_brushlist( brushlist ), m_before( before ), m_after( after )
+	{
+		std::vector<ProjectionAxis> doneProjections;
+		doneProjections.reserve( 3 );
+		m_brushfaces.reserve( m_brushlist.size() );
+		for( const Brush* brush : m_brushlist )
+		{
+			doneProjections.clear();
+			auto& faces = m_brushfaces.emplace_back();
+			faces.reserve( brush->size() );
+			for( const Face* face : *brush )
+				if( face->contributes() )
+					faces.push_back( face );
+
+			std::ranges::sort( faces, []( const Face* a, const Face* b ){
+				const DoubleVector3& n1 = a->getPlane().plane3().normal();
+				const DoubleVector3& n2 = b->getPlane().plane3().normal();
+				const ProjectionAxis p1 = projectionaxis_for_normal( n1 );
+				const ProjectionAxis p2 = projectionaxis_for_normal( n2 );
+				return float_equal_epsilon( fabs( n1[ p1 ] ), fabs( n2[ p2 ] ), c_PLANE_NORMAL_EPSILON )
+				? p1 > p2 // Z > Y > X
+				: fabs( n1[ p1 ] ) > fabs( n2[ p2 ] ); // or most axial
+			} );
+
+			auto it = faces.cbegin(), found = it; // traverse projections and craft more fortunate splits order in non trivial cases
+			while( ( found = std::ranges::find_if( faces, [&doneProjections]( const Face* face ){
+				return std::ranges::find( doneProjections, projectionaxis_for_normal( face->getPlane().plane3().normal() ) ) == doneProjections.cend();
+			} ) ) != faces.cend() ){
+				auto moveit = [&]( decltype( it ) move ){ // moves face forward to prioritize
+					const Face* face = *move;
+					faces.erase( move );
+					faces.insert( it++, face ); // postincrement: 'it' ends up right after the insertion
+				};
+				moveit( found );
+				const DoubleVector3& n1 = ( *( it - 1 ) )->getPlane().plane3().normal();
+				const ProjectionAxis p1 = projectionaxis_for_normal( n1 );
+				doneProjections.push_back( p1 );
+				// find opposite face
+				auto more = faces.cend();
+				double bestmax = 0;
+				double bestdot = 1;
+				for( auto face = it; face != faces.cend(); ++face ){
+					const DoubleVector3& n2 = ( *face )->getPlane().plane3().normal();
+					const ProjectionAxis p2 = projectionaxis_for_normal( n2 );
+					if( p1 == p2 // same projection
+					&& n1[p1] * n2[p2] < 0 // opposite projection facing
+					&& ( fabs( n2[p2] ) > bestmax + c_PLANE_NORMAL_EPSILON // definitely better proj direction
+					   || ( fabs( n2[p2] ) > bestmax - c_PLANE_NORMAL_EPSILON // or similar proj direction
+					     && vector3_dot( n1, n2 ) < bestdot ) ) ){ // + more opposing normal direction
+						bestmax = fabs( n2[p2] );
+						bestdot = vector3_dot( n1, n2 );
+						more = face;
+					}
+				}
+				if( more != faces.end() ){
+					moveit( more );
+				}
+			}
+		}
 	}
 	bool pre( const scene::Path& path, scene::Instance& instance ) const {
 		return path.top().get().visible();
 	}
 	void post( const scene::Path& path, scene::Instance& instance ) const {
 		if ( Brush* thebrush = Node_getBrush( path.top() ) ) {
-			if ( path.top().get().visible() && !Instance_isSelected( instance ) ) {
+			if ( path.top().get().visible() && !Instance_isSelected( instance )
+			&& std::any_of( m_brushlist.cbegin(), m_brushlist.cend(),
+			[thebrush]( const Brush *b ){ return aabb_intersects_aabb( thebrush->localAABB(), b->localAABB() ); } ) ) {
 				brush_vector_t buffer[2];
 				bool swap = false;
 				Brush* original = new Brush( *thebrush );
 				buffer[swap].push_back( original );
 
-				for ( const Brush *subbrush : m_brushlist )
+				for ( size_t i = 0; i < m_brushlist.size(); ++i )
 				{
 					for ( Brush *brush : buffer[swap] )
 					{
-						if ( Brush_subtract( *brush, *subbrush, buffer[!swap] ) ) {
+						if ( Brush_subtract( *brush, *m_brushlist[i], m_brushfaces[i], buffer[!swap] ) ) {
 							delete brush;
 						}
 						else