From 283198c3190e3913f39eeaf59d5e98b1e1daa7ae Mon Sep 17 00:00:00 2001 From: Garux Date: Mon, 24 Feb 2025 02:52:23 +0500 Subject: [PATCH] * fix tiny structural brushes breaking bsp tree --- docs/changelog-custom.txt | 1 + .../q3map2/tiny_structural_brush/README.txt | 18 ++++ .../maps/tiny_structural_brush.map | 95 +++++++++++++++++++ tools/quake3/common/qmath.h | 2 +- tools/quake3/q3map2/brush.cpp | 17 ++-- tools/quake3/q3map2/facebsp.cpp | 10 +- tools/quake3/q3map2/surface.cpp | 2 +- 7 files changed, 130 insertions(+), 15 deletions(-) create mode 100644 regression_tests/q3map2/tiny_structural_brush/README.txt create mode 100644 regression_tests/q3map2/tiny_structural_brush/maps/tiny_structural_brush.map diff --git a/docs/changelog-custom.txt b/docs/changelog-custom.txt index 1fe6d3f8..157b4172 100644 --- a/docs/changelog-custom.txt +++ b/docs/changelog-custom.txt @@ -180,6 +180,7 @@ Q3map2: * -globalflag : add surface flag to every bsp shader. Reusable, e.g. -globalflag slick -globalflag nodamage. * fix deformvertexes autosprite2 on brushes and models * tighten FilterPatchIntoTree() regarding to even (pink) points + * fix tiny structural brushes breaking bsp tree diff --git a/regression_tests/q3map2/tiny_structural_brush/README.txt b/regression_tests/q3map2/tiny_structural_brush/README.txt new file mode 100644 index 00000000..ae9ac651 --- /dev/null +++ b/regression_tests/q3map2/tiny_structural_brush/README.txt @@ -0,0 +1,18 @@ +DESCRIPTION OF PROBLEM: +======================= + +Tiny brush windings are removed by FixWindingAccu(), which may result in +non convex set of faces in a brush. Using such set in FaceBSP() produces +fucked up tree with unexpected behavior. In sample map half of the box +is invisible, as if there is antiportal brush. + +To trigger the bug, compile the map with -bsp -meta args. + + +SOLUTION TO PROBLEM: +==================== + +For now basic test 'non-empty windings number < 4' catches all reported cases, +while, of course, it's not sufficient to detect all possible cases. +Catch in MakeStructuralBSPFaceList(), report to user, mark brush detail +to be also skipped in MakeVisibleBSPFaceList() later. diff --git a/regression_tests/q3map2/tiny_structural_brush/maps/tiny_structural_brush.map b/regression_tests/q3map2/tiny_structural_brush/maps/tiny_structural_brush.map new file mode 100644 index 00000000..6125fbbd --- /dev/null +++ b/regression_tests/q3map2/tiny_structural_brush/maps/tiny_structural_brush.map @@ -0,0 +1,95 @@ + +// entity 0 +{ +"classname" "worldspawn" +"_blocksize" "0" +// brush 0 +{ +brushDef +{ +( 1024.1 0 8 ) ( 1024 0.1 8 ) ( 1024.1 0.1 8 ) ( ( 0.015625 0 -0.533203125 ) ( 0 0.015625 -0.298828125 ) ) common/caulk 0 0 0 +( 1024 0.1 8 ) ( 1024 0.1 0 ) ( 1024.1 0.1 8 ) ( ( 0.015625 0 0.298828125 ) ( 0 0.015625 0.234375 ) ) common/caulk 0 0 0 +( 1024 0.1 0 ) ( 1024.1 0 0 ) ( 1024.1 0.1 0 ) ( ( 0.015625 0 -0.533203125 ) ( 0 0.015625 0.298828125 ) ) common/caulk 0 0 0 +( 1024.1 0 0 ) ( 1024.1 0.1 8 ) ( 1024.1 0.1 0 ) ( ( 0.015625 0 -0.533203125 ) ( 0 0.015625 0.234375 ) ) common/caulk 0 0 0 +( 1024 0 8 ) ( 1024.1 0 8 ) ( 1024.1 0 0 ) ( ( 0.015625 0 -0.298828125 ) ( 0 0.015625 0.234375 ) ) common/caulk 0 0 0 +( 1024 0.1 0 ) ( 1024 0 8 ) ( 1024 0 0 ) ( ( 0.015625 0 0.533203125 ) ( 0 0.015625 0.234375 ) ) common/caulk 0 0 0 +} +} +// brush 1 +{ +brushDef +{ +( 1280 0 512 ) ( 1280 -256 512 ) ( 768 0 512 ) ( ( 0.015625 0 0 ) ( 0 0.015625 -0 ) ) common/caulk 0 0 0 +( 1280 512 256 ) ( 768 512 256 ) ( 1280 512 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1536 0 256 ) ( 1536 0 -256 ) ( 1536 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -512 -256 ) ( 768 -512 256 ) ( 1280 -512 -256 ) ( ( 0.015625 0 -0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 512 -256 -256 ) ( 512 0 -256 ) ( 512 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 0 256 ) ( 1280 -256 256 ) ( 1280 0 256 ) ( ( 0.0078125 0 0 ) ( 0 0.00390625 0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +// brush 2 +{ +brushDef +{ +( 1280 0 256 ) ( 1280 -256 256 ) ( 768 0 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 -0 ) ) common/caulk 0 0 0 +( 1280 512 256 ) ( 768 512 256 ) ( 1280 512 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1536 0 256 ) ( 1536 0 -256 ) ( 1536 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -512 ) ( 1280 -256 -512 ) ( 768 0 -512 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 512 -256 -256 ) ( 512 0 -256 ) ( 512 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1280 256 -256 ) ( 768 256 256 ) ( 1280 256 256 ) ( ( 0.0078125 0 -0 ) ( 0 0.00390625 0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +// brush 3 +{ +brushDef +{ +( 1280 0 256 ) ( 1280 -256 256 ) ( 768 0 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 -0 ) ) common/caulk 0 0 0 +( 1280 256 256 ) ( 768 256 256 ) ( 1280 256 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1536 0 256 ) ( 1536 0 -256 ) ( 1536 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -512 ) ( 1280 -256 -512 ) ( 768 0 -512 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -512 -256 ) ( 768 -512 256 ) ( 1280 -512 -256 ) ( ( 0.015625 0 -0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1280 -256 256 ) ( 1280 0 -256 ) ( 1280 0 256 ) ( ( 0.0078125 0 0 ) ( 0 0.00390625 0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +// brush 4 +{ +brushDef +{ +( 1280 256 256 ) ( 768 256 256 ) ( 1280 256 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1280 0 256 ) ( 1280 0 -256 ) ( 1280 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -512 ) ( 1280 -256 -512 ) ( 768 0 -512 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -512 -256 ) ( 768 -512 256 ) ( 1280 -512 -256 ) ( ( 0.015625 0 -0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 512 -256 -256 ) ( 512 0 -256 ) ( 512 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 0 -256 ) ( 1280 -256 -256 ) ( 768 -256 -256 ) ( ( 0.0078125 0 0 ) ( 0 0.00390625 -0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +// brush 5 +{ +brushDef +{ +( 1280 0 256 ) ( 1280 -256 256 ) ( 768 0 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 -0 ) ) common/caulk 0 0 0 +( 1280 0 256 ) ( 1280 0 -256 ) ( 1280 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -256 ) ( 1280 -256 -256 ) ( 768 0 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -512 -256 ) ( 768 -512 256 ) ( 1280 -512 -256 ) ( ( 0.015625 0 -0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 512 -256 -256 ) ( 512 0 -256 ) ( 512 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 1280 -256 -256 ) ( 768 -256 256 ) ( 768 -256 -256 ) ( ( 0.0078125 0 0 ) ( 0 0.00390625 0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +// brush 6 +{ +brushDef +{ +( 1280 0 256 ) ( 1280 -256 256 ) ( 768 0 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 -0 ) ) common/caulk 0 0 0 +( 1280 256 256 ) ( 768 256 256 ) ( 1280 256 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -256 ) ( 1280 -256 -256 ) ( 768 0 -256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 -256 ) ( 768 -256 256 ) ( 1280 -256 -256 ) ( ( 0.015625 0 -0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 512 -256 -256 ) ( 512 0 -256 ) ( 512 -256 256 ) ( ( 0.015625 0 0 ) ( 0 0.015625 0 ) ) common/caulk 0 0 0 +( 768 -256 256 ) ( 768 0 -256 ) ( 768 -256 -256 ) ( ( 0.0078125 0 0 ) ( 0 0.00390625 0 ) ) DF2_baronshed/00cab02 0 0 0 +} +} +} +// entity 1 +{ +"classname" "info_player_deathmatch" +"origin" "1184 0 0" +} diff --git a/tools/quake3/common/qmath.h b/tools/quake3/common/qmath.h index 6f74cfb2..08b033aa 100644 --- a/tools/quake3/common/qmath.h +++ b/tools/quake3/common/qmath.h @@ -79,7 +79,7 @@ struct MinMax___ return other.maxs.x() >= mins.x() && other.maxs.y() >= mins.y() && other.maxs.z() >= mins.z() && other.mins.x() <= maxs.x() && other.mins.y() <= maxs.y() && other.mins.z() <= maxs.z(); } - // true, if other is completely enclosed by this + // true, if other is completely enclosed by this //! implicitly requires this->valid() or zero volume template bool surrounds( const MinMax___& other ) const { return other.mins.x() >= mins.x() && other.mins.y() >= mins.y() && other.mins.z() >= mins.z() diff --git a/tools/quake3/q3map2/brush.cpp b/tools/quake3/q3map2/brush.cpp index 8dc2e331..1f0d2041 100644 --- a/tools/quake3/q3map2/brush.cpp +++ b/tools/quake3/q3map2/brush.cpp @@ -487,20 +487,16 @@ static int FilterBrushIntoTree_r( brush_t&& b, node_t *node ){ void FilterDetailBrushesIntoTree( const entity_t& e, tree_t& tree ){ int c_unique = 0, c_clusters = 0; - /* note it */ Sys_FPrintf( SYS_VRB, "--- FilterDetailBrushesIntoTree ---\n" ); /* walk the list of brushes */ - c_unique = 0; - c_clusters = 0; for ( const brush_t& b : e.brushes ) { - if ( !b.detail ) { - continue; + if ( b.detail ) { + c_unique++; + c_clusters += FilterBrushIntoTree_r( brush_t( b ), tree.headnode ); } - c_unique++; - c_clusters += FilterBrushIntoTree_r( brush_t( b ), tree.headnode ); } /* emit some statistics */ @@ -521,11 +517,10 @@ void FilterStructuralBrushesIntoTree( const entity_t& e, tree_t& tree ) { Sys_FPrintf( SYS_VRB, "--- FilterStructuralBrushesIntoTree ---\n" ); for ( const brush_t& b : e.brushes ) { - if ( b.detail ) { - continue; + if ( !b.detail ) { + c_unique++; + c_clusters += FilterBrushIntoTree_r( brush_t( b ), tree.headnode ); } - c_unique++; - c_clusters += FilterBrushIntoTree_r( brush_t( b ), tree.headnode ); } /* emit some statistics */ diff --git a/tools/quake3/q3map2/facebsp.cpp b/tools/quake3/q3map2/facebsp.cpp index 305bd8cd..9df61872 100644 --- a/tools/quake3/q3map2/facebsp.cpp +++ b/tools/quake3/q3map2/facebsp.cpp @@ -47,7 +47,7 @@ static void SelectSplitPlaneNum( const node_t *node, const facelist_t& list, int /* ydnar: set some defaults */ - *splitPlaneNum = -1; /* leaf */ + *splitPlaneNum = PLANENUM_LEAF; /* leaf */ *compileFlags = 0; /* ydnar 2002-06-24: changed this to split on z-axis as well */ @@ -184,7 +184,7 @@ static void BuildFaceTree_r( node_t *node, facelist_t& list ){ SelectSplitPlaneNum( node, list, &splitPlaneNum, &compileFlags ); /* if we don't have any more faces, this is a node */ - if ( splitPlaneNum == -1 ) { + if ( splitPlaneNum == PLANENUM_LEAF ) { node->planenum = PLANENUM_LEAF; node->has_structural_children = false; c_faceLeafs++; @@ -346,6 +346,12 @@ facelist_t MakeStructuralBSPFaceList( const brushlist_t& list ){ continue; } + if( std::ranges::count_if( b.sides, []( const side_t& side ){ return !side.winding.empty(); } ) < 4 ){ + xml_Select( "ignoring malformed structural brush: sides < 4: would break bsp tree", b.entityNum, b.brushNum, false ); + const_cast( b ).detail = true; + continue; + } + for ( const side_t& s : b.sides ) { /* get winding */ diff --git a/tools/quake3/q3map2/surface.cpp b/tools/quake3/q3map2/surface.cpp index 2a179616..3256fe67 100644 --- a/tools/quake3/q3map2/surface.cpp +++ b/tools/quake3/q3map2/surface.cpp @@ -1882,7 +1882,7 @@ static int FilterPatchIntoTree( mapDrawSurface_t *ds, tree_t& tree ){ const Vector3& p7 = ds->verts[( y + 2 ) * ds->patchWidth + ( x + 1 )].xyz; const Vector3& p8 = ds->verts[( y + 2 ) * ds->patchWidth + ( x + 2 )].xyz; - // add 4 invariant points 12 of those which are used to calculate subdivisionless patch LoD + // add 4 invariant points + 12 of those which are used to calculate subdivisionless patch LoD // convex hull defined by them guaranteedly encompasses any patch LoD std::array points = { p0, p2, p6, p8, vector3_mid( p0, p1 ), vector3_mid( p1, p2 ),