refactor brush transform evaluation code to reduce amount of unneeded evaluations to 0

improvable: don't evaluateBRep on shader changed, perhaps more
fix: brush vertex manip: no vertexModeFree() after identity transform (mouse forth and back / rotate dialog 0 / insertion w/o move) = selects vertices on undo
fix: brush vertex manip: tex vertex lock: broken for 220
use brush::vertexmode code to delete selected brush components

libs/scenelib.h

@@ -49,7 +49,7 @@ virtual void setSelectedComponents( bool select, SelectionSystem::EComponentMode
 virtual void testSelectComponents( Selector& selector, SelectionTest& test, SelectionSystem::EComponentMode mode ) = 0;
 };
 
-typedef Callback1<const Vector3&> Vector3Callback;
+typedef std::function<void( const Vector3& )> Vector3Callback;
 
 class ComponentEditable
 {

libs/transformlib.h

@@ -145,7 +145,6 @@ Callback m_changed;
 Callback m_apply;
 TransformModifierType m_type;
 public:
-bool m_transformFrozen = true;
 
 TransformModifier( const Callback& changed, const Callback& apply ) :
 	m_translation( c_translation_identity ),
@@ -181,13 +180,85 @@ void setSkew( const Skew& value ){
 void freezeTransform(){
 	if ( !isIdentity() ) {
 		m_apply();
-		m_translation = c_translation_identity;
+		setIdentity();
-		m_rotation = c_rotation_identity;
-		m_scale = c_scale_identity;
-		m_skew = c_skew_identity;
-		m_transformFrozen = true;
 		m_changed();
 	}
+}
+const Translation& getTranslation() const {
+	return m_translation;
+}
+const Rotation& getRotation() const {
+	return m_rotation;
+}
+const Scale& getScale() const {
+	return m_scale;
+}
+const Skew& getSkew() const {
+	return m_skew;
+}
+Matrix4 calculateTransform() const {
+	return matrix4_transform_for_components( getTranslation(), getRotation(), getScale(), getSkew() );
+}
+private:
+bool isIdentity() const {
+	return m_translation == c_translation_identity
+		 && m_rotation == c_rotation_identity
+		 && m_scale == c_scale_identity
+		 && m_skew == c_skew_identity;
+}
+void setIdentity(){
+	m_translation = c_translation_identity;
+	m_rotation = c_rotation_identity;
+	m_scale = c_scale_identity;
+	m_skew = c_skew_identity;
+}
+};
+
+class BrushTransformModifier : public Transformable
+{
+Translation m_translation;
+Rotation m_rotation;
+Scale m_scale;
+Skew m_skew;
+Callback m_changed;
+Callback m_apply;
+TransformModifierType m_type;
+public:
+bool m_transformFrozen = true;
+
+BrushTransformModifier( const Callback& changed, const Callback& apply ) :
+	m_translation( c_translation_identity ),
+	m_rotation( c_quaternion_identity ),
+	m_scale( c_scale_identity ),
+	m_skew( c_skew_identity ),
+	m_changed( changed ),
+	m_apply( apply ),
+	m_type( TRANSFORM_PRIMITIVE ){
+}
+void setType( TransformModifierType type ){
+	m_type = type;
+}
+TransformModifierType getType() const {
+	return m_type;
+}
+void setTranslation( const Translation& value ){
+	m_translation = value;
+	m_changed();
+}
+void setRotation( const Rotation& value ){
+	m_rotation = value;
+	m_changed();
+}
+void setScale( const Scale& value ){
+	m_scale = value;
+	m_changed();
+}
+void setSkew( const Skew& value ){
+	m_skew = value;
+	m_changed();
+}
+void freezeTransform(){
+	m_apply();
 	m_transformFrozen = true;
 }
 const Translation& getTranslation() const {
@@ -211,6 +282,12 @@ bool isIdentity() const {
 		 && m_scale == c_scale_identity
 		 && m_skew == c_skew_identity;
 }
+void setIdentity(){
+	m_translation = c_translation_identity;
+	m_rotation = c_rotation_identity;
+	m_scale = c_scale_identity;
+	m_skew = c_skew_identity;
+}
 };
 
 

radiant/brush.cpp

@@ -106,7 +106,9 @@ inline bool Brush_isBounded( const Brush& brush ){
 }
 
 void Brush::buildBRep(){
-	const bool _vertexModeOn = m_vertexModeOn;
+	globalOutputStream() << "  buildBRep start\n";
+
+	m_BRep_evaluation = true;
 
 	bool degenerate = buildWindings();
 
@@ -308,17 +310,16 @@ void Brush::buildBRep(){
 			}
 		}
 
-		//  :faceleg:     start  move  end
+		if( m_vertexModeOn ){
-		// m_vertexModeOn   1     1     0
+			for( const auto& i : m_vertexModeVertices )
-		//  _vertexModeOn   0     1     1
+				if( i.m_selected )
-		if( _vertexModeOn || m_vertexModeOn ){
+					for ( Observers::iterator o = m_observers.begin(); o != m_observers.end(); ++o )
-			if( !( !m_vertexModeOn && _vertexModeOn && GlobalSelectionSystem().Mode() == SelectionSystem::ePrimitive ) ) //don't select in the end of g_bTmpComponentMode
+						( *o )->vertex_select( i.m_vertexTransformed );
-				for( const auto& i : m_vertexModeVertices )
-					if( i.m_selected )
-						for ( Observers::iterator o = m_observers.begin(); o != m_observers.end(); ++o )
-							( *o )->vertex_select( i.m_vertexTransformed );
 		}
+		globalOutputStream() << m_vertexModeOn << " m_vertexModeOn\n";
 	}
+	m_BRep_evaluation = false;
+	globalOutputStream() << "  buildBRep end\n";
 }
 
 
@@ -462,7 +463,8 @@ const Face* vertex_mode_find_common_face( const Brush::VertexModeVertex& v1, con
 }
 
 #include "quickhull/QuickHull.hpp"
-void Brush::vertexModeBuildHull( bool allTransformed ){
+void Brush::vertexModeBuildHull( bool allTransformed /*= false*/ ){
+	globalOutputStream() << "  vertexModeBuildHull\n";
 	quickhull::QuickHull<double> quickhull;
 	std::vector<quickhull::Vector3<double>> pointCloud;
 	pointCloud.reserve( m_vertexModeVertices.size() );
@@ -507,6 +509,7 @@ void Brush::vertexModeBuildHull( bool allTransformed ){
 		for( const auto& i : vertexModePlanes ){
 			const Face& face = *i.m_face;
 			if( i.m_transformed ){
+				TextureProjection projection( face.getTexdef().m_projection );
 				if( g_brush_textureVertexlock_enabled ){
 					Matrix4 local2tex;
 					Texdef_Construct_local2tex( face.getTexdef().m_projection, face.getShader().width(), face.getShader().height(), face.getPlane().plane3().normal(), local2tex );
@@ -514,14 +517,12 @@ void Brush::vertexModeBuildHull( bool allTransformed ){
 												matrix4_transformed_point( local2tex, i.m_v[1]->m_vertex ),
 												matrix4_transformed_point( local2tex, i.m_v[2]->m_vertex ) };
 					const DoubleVector3 points[3]{ i.m_v[0]->m_vertexTransformed, i.m_v[1]->m_vertexTransformed, i.m_v[2]->m_vertexTransformed };
-					TextureProjection projection;
 					Texdef_from_ST( projection, points, st, face.getShader().width(), face.getShader().height() );
-					projection.m_brushprimit_texdef.removeScale( face.getShader().width(), face.getShader().height() );
-
-					addPlane( i.m_v[0]->m_vertexTransformed, i.m_v[1]->m_vertexTransformed, i.m_v[2]->m_vertexTransformed, face.GetShader(), projection );
 				}
-				else{
+				Face* newFace = addPlane( i.m_v[0]->m_vertexTransformed, i.m_v[1]->m_vertexTransformed, i.m_v[2]->m_vertexTransformed, face.GetShader(), TextureProjection() );
-					addPlane( i.m_v[0]->m_vertexTransformed, i.m_v[1]->m_vertexTransformed, i.m_v[2]->m_vertexTransformed, face.GetShader(), face.getTexdef().normalised() );
+				if( newFace ){
+					newFace->getTexdef().m_projection = projection; //set TextureProjection later, addPlane() resets Valve220 basis
+					newFace->revertTexdef();
 				}
 			}
 			else{

radiant/brush.h

@@ -1609,6 +1609,7 @@ Callback m_boundsChanged;
 
 mutable bool m_planeChanged;   // b-rep evaluation required
 mutable bool m_transformChanged;   // transform evaluation required
+bool m_BRep_evaluation = false; //mutex for invalidation
 // ----
 
 public:
@@ -1776,12 +1777,13 @@ void updateFiltered(){
 
 // observer
 void planeChanged(){
-	/* m_planeChanged vs m_transformChanged relationship is important
+//	globalOutputStream() << "  planeChanged\n";
-	b4 (w/o one) cycling dependency occured:
+	/* m_BRep_evaluation mutex prevents cyclic dependency:
 	transformModifier.set ; transformChanged() ; planeChanged() ; pivotChanged() ; sceneChangeNotify() ;
 	sceneRender() ; localAABB ; evaluateBRep ; buildBRep() ; evaluateTransform ; !!!problem starts here!!!! planeChanged() ; pivotChanged() ; sceneChangeNotify() ;
 	sceneRender() ; localAABB ; evaluateBRep ; buildBRep() ; */
-	if( !m_transformChanged ){
+	if( !m_BRep_evaluation ){
+								globalOutputStream() << "  planeChangedOK\n";
 		m_planeChanged = true;
 		aabbChanged();
 		m_lightsChanged();
@@ -1789,7 +1791,7 @@ void planeChanged(){
 }
 void shaderChanged(){
 	updateFiltered();
-	planeChanged();
+	planeChanged(); ///isn't too much for shader changed only?
 }
 
 void evaluateBRep() const {
@@ -1800,18 +1802,18 @@ void evaluateBRep() const {
 }
 
 void transformChanged(){
-	//m_transformChanged = true;
+	globalOutputStream() << "  transformChanged()\n";
 	planeChanged();
-	m_transformChanged = true; //experimental fix of cyclic dependency
+	m_transformChanged = true;
 }
 typedef MemberCaller<Brush, &Brush::transformChanged> TransformChangedCaller;
 
 void evaluateTransform(){
 	if ( m_transformChanged ) {
-		//m_transformChanged = false;
+			globalOutputStream() << "  Brush::evaluateTransform()\n";
 		revertTransform();
 		m_evaluateTransform();
-		m_transformChanged = false; //experimental fix of cyclic dependency
+		m_transformChanged = false;
 	}
 }
 const Matrix4& localToParent() const {
@@ -1865,16 +1867,19 @@ void snapto( float snap ){
 //	}
 }
 void revertTransform(){
+	globalOutputStream() << " revertTransform \n";
 	for ( Faces::iterator i = m_faces.begin(); i != m_faces.end(); ++i )
 	{
 		( *i )->revertTransform();
 	}
 }
 void freezeTransform(){
+	globalOutputStream() << "  freezeTransform\n";
 	for ( Faces::iterator i = m_faces.begin(); i != m_faces.end(); ++i )
 	{
 		( *i )->freezeTransform();
 	}
+	m_transformChanged = false;
 }
 
 	class VertexModeVertex
@@ -1894,12 +1899,14 @@ VertexModeVertices m_vertexModeVertices;
 bool m_vertexModeOn{false};
 
 void vertexModeInit(){
+	globalOutputStream() << " vertexModeInit\n";
 	m_vertexModeOn = true;
 	m_vertexModeVertices.clear();
 	undoSave();
 }
 
 void vertexModeFree(){
+	globalOutputStream() << "  vMF\n";
 	m_vertexModeOn = false;
 //	m_vertexModeVertices.clear(); //keep, as it is required by buildBRep() after this call
 }
@@ -3187,7 +3194,7 @@ void selectVerticesOnFaces( const FaceInstances_ptrs& faceinstances ){
 	while ( faceVertex.getFace() != m_vertex->m_faceVertex.getFace() );
 }
 void gather( Brush::VertexModeVertices& vertexModeVertices ) const {
-	vertexModeVertices.push_back( Brush::VertexModeVertex( m_vertex->getFace().getWinding()[m_vertex->m_faceVertex.getVertex()].vertex, isSelected() ) );
+	vertexModeVertices.emplace_back( m_vertex->getFace().getWinding()[m_vertex->m_faceVertex.getVertex()].vertex, isSelected() );
 	FaceVertexId faceVertex = m_vertex->m_faceVertex;
 	do
 	{
@@ -3268,7 +3275,7 @@ static Shader* m_state_selpoint;
 
 const LightList* m_lightList;
 
-TransformModifier m_transform;
+BrushTransformModifier m_transform;
 
 BrushInstance( const BrushInstance& other ); // NOT COPYABLE
 BrushInstance& operator=( const BrushInstance& other ); // NOT ASSIGNABLE
@@ -3409,12 +3416,14 @@ void vertex_select( const Vector3& vertex ){
 
 void vertex_snap( const float snap, bool all ){
 	m_brush.vertexModeInit();
+	m_brush.m_vertexModeVertices.reserve( m_vertexInstances.size() );
 	for ( const auto& i : m_vertexInstances ){
 		i.gather( m_brush.m_vertexModeVertices );
 	}
 	m_brush.vertexModeSnap( snap, all );
 	m_brush.evaluateBRep();
 	m_brush.vertexModeFree();
+	m_brush.freezeTransform();
 }
 
 void vertex_snap( const float snap ){
@@ -3791,6 +3800,7 @@ void gatherSelectedComponents( const Vector3Callback& callback ) const {
 void insert_vertices( const Brush::VertexModeVertices& vertexModeVertices ){
 	if( !m_vertexInstances.empty() ){
 		m_brush.vertexModeInit();
+		m_brush.m_vertexModeVertices.reserve( m_vertexInstances.size() + 2 );
 		for ( const auto& i : m_vertexInstances ){
 			i.gather( m_brush.m_vertexModeVertices );
 		}
@@ -3806,6 +3816,35 @@ void insert_vertices( const Brush::VertexModeVertices& vertexModeVertices ){
 	}
 }
 
+void remove_vertices(){
+	if( !m_vertexInstances.empty() ){
+		m_brush.vertexModeInit();
+		Brush::VertexModeVertices v;
+		v.reserve( m_vertexInstances.size() );
+		for ( const auto& i : m_vertexInstances ){
+			i.gather( v );
+			if( v.back().m_selected )
+				v.pop_back();
+		}
+		std::vector<bool> ok( v.size(), true );
+		gatherSelectedComponents( [&]( const Vector3 & value ) {
+			for( std::size_t i = 0; i < v.size(); ++i )
+				if( vector3_length_squared( v[i].m_vertex - value ) < 0.05 * 0.05 )
+					ok[i] = false;
+		} );
+
+		m_brush.m_vertexModeVertices.reserve( v.size() );
+		for( std::size_t i = 0; i < v.size(); ++i ){
+			if( ok[i] )
+				m_brush.m_vertexModeVertices.push_back( v[i] );
+		}
+		m_brush.vertexModeBuildHull();
+		m_brush.evaluateBRep();
+		m_brush.vertexModeFree();
+		m_brush.freezeTransform();
+	}
+}
+
 void snapComponents( float snap ){
 	for ( const auto& fi : m_faceInstances ){
 		if( fi.selectedComponents( SelectionSystem::eVertex ) ){
@@ -3818,8 +3857,9 @@ void snapComponents( float snap ){
 		fi.snapComponents( snap );
 }
 void evaluateTransform(){
-	if( m_transform.m_transformFrozen )
+	globalOutputStream() << " evaluateTransform\n";
-		m_brush.vertexModeFree();
+	if( m_transform.m_transformFrozen && m_transform.isIdentity() )
+		return;
 	if( m_transform.m_transformFrozen && !m_transform.isIdentity() ){ /* new transform */
 		m_transform.m_transformFrozen = false;
 		for( auto& i : m_faceInstances )
@@ -3829,6 +3869,7 @@ void evaluateTransform(){
 			for ( const auto& i : m_faceInstances ){
 				if( i.selectedComponents( SelectionSystem::eVertex ) ){
 					m_brush.vertexModeInit();
+					m_brush.m_vertexModeVertices.reserve( m_vertexInstances.size() );
 					for ( const auto& i : m_vertexInstances ){
 						i.gather( m_brush.m_vertexModeVertices );
 					}
@@ -3839,6 +3880,7 @@ void evaluateTransform(){
 	}
 
 	const Matrix4 matrix( m_transform.calculateTransform() );
+	globalOutputStream() << matrix << " matrix\n";
 
 	if ( m_transform.getType() == TRANSFORM_PRIMITIVE ) {
 		m_brush.transform( matrix );
@@ -3859,9 +3901,17 @@ void evaluateTransform(){
 	}
 }
 void applyTransform(){
-	m_brush.revertTransform();
+	if( !m_transform.isIdentity() ){
-	evaluateTransform();
+				globalOutputStream() << "     applyTransform\n";
-	m_brush.freezeTransform();
+		if( m_transform.m_transformFrozen ){ //not yet unfrozen by evaluateTransform(), so evaluate
+//			m_brush.revertTransform();
+//			evaluateTransform();
+			m_brush.evaluateBRep();
+		}
+		m_brush.freezeTransform();
+		m_transform.setIdentity();
+	}
+	m_brush.vertexModeFree();
 }
 typedef MemberCaller<BrushInstance, &BrushInstance::applyTransform> ApplyTransformCaller;
 

radiant/csg.cpp

@@ -1030,14 +1030,10 @@ public:
 class Scene_gatherSelectedComponents : public scene::Graph::Walker
 {
 MergeVertices& m_mergeVertices;
-void call( const Vector3& value ) const {
-	m_mergeVertices.insert( value );
-}
-typedef ConstMemberCaller1<Scene_gatherSelectedComponents, const Vector3&, &Scene_gatherSelectedComponents::call> Caller;
 const Vector3Callback m_callback;
 public:
 Scene_gatherSelectedComponents( MergeVertices& mergeVertices )
-	: m_mergeVertices( mergeVertices ), m_callback( Vector3Callback( Scene_gatherSelectedComponents::Caller( *this ) ) ){
+	: m_mergeVertices( mergeVertices ), m_callback( [this]( const Vector3& value ){ m_mergeVertices.insert( value ); } ){
 }
 bool pre( const scene::Path& path, scene::Instance& instance ) const {
 	if ( path.top().get().visible() ) {
@@ -1266,7 +1262,7 @@ void CSG_WrapMerge(){
 }
 
 
-
+#if 0
 class find_instance_to_DeleteComponents : public SelectionSystem::Visitor
 {
 public:
@@ -1346,6 +1342,12 @@ void CSG_DeleteComponents(){
 		}
 	}
 }
+#else
+
+void CSG_DeleteComponents(){
+	Scene_forEachSelectedBrush( []( BrushInstance& brush ){ brush.remove_vertices(); } );
+}
+#endif
 
 
 

radiant/selection.cpp

@@ -3985,7 +3985,8 @@ void testSelect( const View& view, const Matrix4& pivot2world ){
 					m_freeDragXY_Z.set0( g_vector3_identity );
 				}
 				else if( GlobalSelectionSystem().ComponentMode() == SelectionSystem::eVertex ){
-					m_dragSelected = scene_insert_brush_vertices( view, m_freeDragXY_Z );
+					m_dragSelected = g_bTmpComponentMode = scene_insert_brush_vertices( view, m_freeDragXY_Z ); //hack: indicating not a tmp mode
+					return;
 				}
 			}
 		}
@@ -5529,6 +5530,7 @@ bool pre( const scene::Path& path, scene::Instance& instance ) const {
 };
 
 void RadiantSelectionSystem::freezeTransforms(){
+	globalOutputStream() << " RadiantSelectionSystem::freezeTransforms() \n";
 	GlobalSceneGraph().traverse( FreezeTransforms() );
 }