fix: Improve face creation from edges using BRepOffsetAPI_MakeFilling

Co-authored-by: aider (gemini/gemini-2.5-pro) <aider@aider.chat>

resources.qrc

@@ -14,5 +14,7 @@
         <file alias="simple.frag">src/shaders/simple.frag</file>
         <file alias="texture.vert">src/shaders/texture.vert</file>
         <file alias="texture.frag">src/shaders/texture.frag</file>
+        <file alias="lit.vert">src/shaders/lit.vert</file>
+        <file alias="lit.frag">src/shaders/lit.frag</file>
     </qresource>
 </RCC>

src/ApplicationController.cpp

@@ -158,6 +158,9 @@ void ApplicationController::addCircle(const gp_Pnt& center, double radius)
 
 void ApplicationController::endSketch()
 {
+    if (m_activeSketch) {
+        m_activeSketch->buildShape();
+    }
     m_activeSketch = nullptr;
     setActiveTool(ToolType::None);
     emit sketchModeEnded();

src/SketchFeature.cpp

@@ -2,8 +2,24 @@
 #include "SketchObject.h"
 #include "SketchLine.h"
 #include "SketchRectangle.h"
+#include "SketchCircle.h"
+
+#include <BRepBuilderAPI_MakeEdge.hxx>
+#include <BRepBuilderAPI_MakeWire.hxx>
+#include <BRepBuilderAPI_MakeFace.hxx>
+#include <BRep_Builder.hxx>
+#include <TopoDS_Compound.hxx>
+#include <TopoDS.hxx>
+#include <gp_Circ.hxx>
+#include <gp_Ax2.hxx>
+#include <TopExp_Explorer.hxx>
+#include <ShapeAnalysis_FreeBounds.hxx>
+#include <gp_Pln.hxx>
+#include <BRepOffsetAPI_MakeFilling.hxx>
+#include <GeomAbs_Shape.hxx>
 
 #include <QJsonArray>
+#include <QDebug>
 
 SketchFeature::SketchFeature(const QString& name)
     : Feature(name)
@@ -35,6 +51,119 @@ const TopoDS_Shape& SketchFeature::shape() const
     return m_shape;
 }
 
+void SketchFeature::buildShape()
+{
+    m_shape.Nullify();
+    QList<TopoDS_Edge> lineEdges;
+    QList<TopoDS_Face> faces;
+
+    gp_Pln sketchPlane;
+    switch (m_plane) {
+        case SketchPlane::XY: sketchPlane = gp_Pln(gp::Origin(), gp::DY()); break;
+        case SketchPlane::XZ: sketchPlane = gp_Pln(gp::Origin(), gp::DZ()); break;
+        case SketchPlane::YZ: sketchPlane = gp_Pln(gp::Origin(), gp::DX()); break;
+    }
+
+    for (SketchObject* obj : m_objects) {
+        if (auto line = dynamic_cast<SketchLine*>(obj)) {
+            BRepBuilderAPI_MakeEdge makeEdge(line->startPoint(), line->endPoint());
+            if (makeEdge.IsDone()) {
+                lineEdges.append(makeEdge.Edge());
+            }
+        } else if (auto rect = dynamic_cast<SketchRectangle*>(obj)) {
+            const gp_Pnt& c1 = rect->corner1();
+            const gp_Pnt& c2 = rect->corner2();
+            gp_Pnt other_corner1, other_corner2;
+
+            if (m_plane == SketchPlane::XY) {
+                other_corner1.SetCoord(c2.X(), c1.Y(), c1.Z());
+                other_corner2.SetCoord(c1.X(), c1.Y(), c2.Z());
+            } else if (m_plane == SketchPlane::XZ) {
+                other_corner1.SetCoord(c2.X(), c1.Y(), c1.Z());
+                other_corner2.SetCoord(c1.X(), c2.Y(), c1.Z());
+            } else { // YZ
+                other_corner1.SetCoord(c1.X(), c2.Y(), c1.Z());
+                other_corner2.SetCoord(c1.X(), c1.Y(), c2.Z());
+            }
+
+            BRepBuilderAPI_MakeEdge me1(c1, other_corner1);
+            BRepBuilderAPI_MakeEdge me2(other_corner1, c2);
+            BRepBuilderAPI_MakeEdge me3(c2, other_corner2);
+            BRepBuilderAPI_MakeEdge me4(other_corner2, c1);
+            if (me1.IsDone() && me2.IsDone() && me3.IsDone() && me4.IsDone()) {
+                BRepBuilderAPI_MakeWire wireBuilder(me1.Edge(), me2.Edge(), me3.Edge(), me4.Edge());
+                if (wireBuilder.IsDone()) {
+                    BRepBuilderAPI_MakeFace faceBuilder(sketchPlane, wireBuilder.Wire());
+                    if (faceBuilder.IsDone()) {
+                        faces.append(faceBuilder.Face());
+                    }
+                }
+            }
+        } else if (auto circle = dynamic_cast<SketchCircle*>(obj)) {
+            const gp_Pnt& center = circle->center();
+            double radius = circle->radius();
+            gp_Dir normal;
+            switch (m_plane) {
+                case SketchPlane::XY: normal = gp::DY(); break;
+                case SketchPlane::XZ: normal = gp::DZ(); break;
+                case SketchPlane::YZ: normal = gp::DX(); break;
+            }
+            gp_Ax2 axis(center, normal);
+            gp_Circ circ(axis, radius);
+            BRepBuilderAPI_MakeEdge makeEdge(circ);
+            if (makeEdge.IsDone()) {
+                TopoDS_Edge edge = makeEdge.Edge();
+                BRepBuilderAPI_MakeWire wireBuilder(edge);
+                if(wireBuilder.IsDone()) {
+                    BRepBuilderAPI_MakeFace faceBuilder(sketchPlane, wireBuilder.Wire());
+                    if (faceBuilder.IsDone()) {
+                        faces.append(faceBuilder.Face());
+                    }
+                }
+            }
+        }
+    }
+
+    if (!lineEdges.isEmpty()) {
+        qDebug() << "buildShape: processing" << lineEdges.size() << "line edges";
+
+        BRepOffsetAPI_MakeFilling faceMaker;
+        for (const TopoDS_Edge& edge : lineEdges) {
+            faceMaker.Add(edge, GeomAbs_C0);
+        }
+        faceMaker.Build();
+
+        if (faceMaker.IsDone()) {
+            TopExp_Explorer explorer(faceMaker.Shape(), TopAbs_FACE);
+            int facesAdded = 0;
+            for (; explorer.More(); explorer.Next()) {
+                faces.append(TopoDS::Face(explorer.Current()));
+                facesAdded++;
+            }
+            qDebug() << "buildShape: added" << facesAdded << "face(s) using MakeFilling";
+        } else {
+            qDebug() << "buildShape: MakeFilling failed";
+        }
+    }
+
+    qDebug() << "buildShape: total faces created:" << faces.size();
+    if (faces.isEmpty()) {
+        return;
+    }
+
+    if (faces.size() == 1) {
+        m_shape = faces.first();
+    } else {
+        TopoDS_Compound compound;
+        BRep_Builder builder;
+        builder.MakeCompound(compound);
+        for (const auto& face : faces) {
+            builder.Add(compound, face);
+        }
+        m_shape = compound;
+    }
+}
+
 void SketchFeature::addObject(SketchObject* object)
 {
     m_objects.append(object);

src/SketchFeature.h

@@ -26,6 +26,8 @@ public:
 
     const TopoDS_Shape& shape() const;
 
+    void buildShape();
+
     void addObject(SketchObject* object);
     const QList<SketchObject*>& objects() const;
 

src/SketchGrid.cpp

@@ -23,9 +23,9 @@ GridParams getGridParams(float distance)
     } else if (distance > 50.0f) {
         return { 5.0f, 25.0f, 1000 };
     } else if (distance > 10.0f) {
-        return { 1.0f, 5.0f, 100 };
+        return { 1.0f, 5.0f, 1000 };
     } else { // zoomed in
-        return { 0.2f, 1.0f, 10 };
+        return { 0.2f, 1.0f, 1000 };
     }
 }
 } // namespace

src/ViewportWidget.cpp

@@ -30,6 +30,15 @@
 #include <QVector>
 #include <map>
 
+#include <BRepMesh_IncrementalMesh.hxx>
+#include <TopExp_Explorer.hxx>
+#include <TopoDS.hxx>
+#include <TopoDS_Face.hxx>
+#include <Poly_Triangulation.hxx>
+#include <BRep_Tool.hxx>
+#include <gp_Dir.hxx>
+#include <gp_Trsf.hxx>
+
 struct PntComparator {
     bool operator()(const gp_Pnt& a, const gp_Pnt& b) const {
         // A tolerance is needed for floating point comparisons
@@ -72,6 +81,7 @@ ViewportWidget::~ViewportWidget()
 {
     makeCurrent();
     delete m_shaderProgram;
+    delete m_litShaderProgram;
     delete m_viewCube;
     delete m_sketchGrid;
     m_vbo.destroy();
@@ -594,6 +604,9 @@ void ViewportWidget::drawSketch(const SketchFeature* sketch)
 
     if (!lineVertices.isEmpty()) {
         m_vbo.allocate(lineVertices.constData(), lineVertices.size() * sizeof(GLfloat));
+        m_shaderProgram->enableAttributeArray(0);
+        m_shaderProgram->setAttributeBuffer(0, GL_FLOAT, 0, 3, 0);
+        m_shaderProgram->disableAttributeArray(1);
         glDrawArrays(GL_LINES, 0, lineVertices.size() / 3);
     }
 
@@ -649,13 +662,88 @@ void ViewportWidget::drawSketch(const SketchFeature* sketch)
         }
     }
 
+    // Draw faces
+    if (!sketch->shape().IsNull()) {
+        glDisable(GL_CULL_FACE);
+        BRepMesh_IncrementalMesh(sketch->shape(), 0.1, Standard_False, 0.5, Standard_True); // Linear deflection, compute normals
+
+        QVector<GLfloat> faceData;
+        TopExp_Explorer explorer(sketch->shape(), TopAbs_FACE);
+        for (; explorer.More(); explorer.Next()) {
+            TopoDS_Face face = TopoDS::Face(explorer.Current());
+            TopLoc_Location location;
+            Handle(Poly_Triangulation) triangulation = BRep_Tool::Triangulation(face, location);
+
+            if (!triangulation.IsNull()) {
+                gp_Trsf locTrsf = location.Transformation();
+
+                if (triangulation->HasNormals()) {
+                    for (int i = 1; i <= triangulation->NbTriangles(); ++i) {
+                        const Poly_Triangle& triangle = triangulation->Triangle(i);
+                        for (int j = 1; j <= 3; ++j) {
+                            int nodeIdx = triangle.Value(j);
+                            gp_Pnt p = triangulation->Node(nodeIdx).Transformed(location);
+                            gp_Dir n = triangulation->Normal(nodeIdx);
+                            n.Transform(locTrsf);
+
+                            if (face.Orientation() == TopAbs_REVERSED) {
+                                n.Reverse();
+                            }
+                            faceData << p.X() << p.Y() << p.Z();
+                            faceData << n.X() << n.Y() << n.Z();
+                        }
+                    }
+                } else {
+                    for (int i = 1; i <= triangulation->NbTriangles(); ++i) {
+                        const Poly_Triangle& triangle = triangulation->Triangle(i);
+                        gp_Pnt p1 = triangulation->Node(triangle.Value(1)).Transformed(location);
+                        gp_Pnt p2 = triangulation->Node(triangle.Value(2)).Transformed(location);
+                        gp_Pnt p3 = triangulation->Node(triangle.Value(3)).Transformed(location);
+
+                        QVector3D v1(p1.X(), p1.Y(), p1.Z());
+                        QVector3D v2(p2.X(), p2.Y(), p2.Z());
+                        QVector3D v3(p3.X(), p3.Y(), p3.Z());
+                        QVector3D faceNormal = QVector3D::crossProduct(v2 - v1, v3 - v1).normalized();
+                        if (face.Orientation() == TopAbs_REVERSED) {
+                            faceNormal = -faceNormal;
+                        }
+                        faceData << p1.X() << p1.Y() << p1.Z() << faceNormal.x() << faceNormal.y() << faceNormal.z();
+                        faceData << p2.X() << p2.Y() << p2.Z() << faceNormal.x() << faceNormal.y() << faceNormal.z();
+                        faceData << p3.X() << p3.Y() << p3.Z() << faceNormal.x() << faceNormal.y() << faceNormal.z();
+                    }
+                }
+            }
+        }
+
+        if (!faceData.isEmpty()) {
+            m_litShaderProgram->bind();
+            m_litShaderProgram->setUniformValue(m_litProjMatrixLoc, projection);
+            m_litShaderProgram->setUniformValue(m_litMvMatrixLoc, m_camera->modelViewMatrix());
+            m_litShaderProgram->setUniformValue(m_litNormalMatrixLoc, m_camera->modelViewMatrix().normalMatrix());
+
+            m_vbo.bind();
+            m_vbo.allocate(faceData.constData(), faceData.size() * sizeof(GLfloat));
+
+            m_litShaderProgram->enableAttributeArray(0);
+            m_litShaderProgram->setAttributeBuffer(0, GL_FLOAT, 0, 3, 6 * sizeof(GLfloat));
+            m_litShaderProgram->enableAttributeArray(1);
+            m_litShaderProgram->setAttributeBuffer(1, GL_FLOAT, 3 * sizeof(GLfloat), 3, 6 * sizeof(GLfloat));
+
+            glDrawArrays(GL_TRIANGLES, 0, faceData.size() / 6);
+            m_litShaderProgram->disableAttributeArray(1);
+            m_shaderProgram->bind(); // rebind simple shader for subsequent draws
+            m_shaderProgram->setAttributeBuffer(0, GL_FLOAT, 0, 3, 0);
+        }
+        glEnable(GL_CULL_FACE);
+    }
+
     glEnable(GL_DEPTH_TEST);
 }
 
 void ViewportWidget::initShaders()
 {
+    // Simple shader for lines and grids
     m_shaderProgram = new QOpenGLShaderProgram(this);
-
     if (!m_shaderProgram->addShaderFromSourceFile(QOpenGLShader::Vertex, ":/shaders/simple.vert")) {
         qCritical() << "Vertex shader compilation failed:" << m_shaderProgram->log();
         return;
@@ -668,10 +756,27 @@ void ViewportWidget::initShaders()
         qCritical() << "Shader program linking failed:" << m_shaderProgram->log();
         return;
     }
-
     m_projMatrixLoc = m_shaderProgram->uniformLocation("projectionMatrix");
     m_mvMatrixLoc = m_shaderProgram->uniformLocation("modelViewMatrix");
     m_colorLoc = m_shaderProgram->uniformLocation("objectColor");
+
+    // Lit shader for faces
+    m_litShaderProgram = new QOpenGLShaderProgram(this);
+    if (!m_litShaderProgram->addShaderFromSourceFile(QOpenGLShader::Vertex, ":/shaders/lit.vert")) {
+        qCritical() << "Lit vertex shader compilation failed:" << m_litShaderProgram->log();
+        return;
+    }
+    if (!m_litShaderProgram->addShaderFromSourceFile(QOpenGLShader::Fragment, ":/shaders/lit.frag")) {
+        qCritical() << "Lit fragment shader compilation failed:" << m_litShaderProgram->log();
+        return;
+    }
+    if (!m_litShaderProgram->link()) {
+        qCritical() << "Lit shader program linking failed:" << m_litShaderProgram->log();
+        return;
+    }
+    m_litProjMatrixLoc = m_litShaderProgram->uniformLocation("projectionMatrix");
+    m_litMvMatrixLoc = m_litShaderProgram->uniformLocation("modelViewMatrix");
+    m_litNormalMatrixLoc = m_litShaderProgram->uniformLocation("normalMatrix");
 }
 
 void ViewportWidget::drawSelectionPlanes()
@@ -686,6 +791,9 @@ void ViewportWidget::drawSelectionPlanes()
     glDisable(GL_DEPTH_TEST);
 
     m_vbo.bind();
+    m_shaderProgram->enableAttributeArray(0);
+    m_shaderProgram->setAttributeBuffer(0, GL_FLOAT, 0, 3, 0);
+    m_shaderProgram->disableAttributeArray(1);
     QVector<GLfloat> vertices;
 
     auto drawPlane = [&](const QVector<GLfloat>& quadVerts, bool highlighted) {

src/ViewportWidget.h

@@ -97,6 +97,7 @@ private:
 
     QMatrix4x4 projection;
     QOpenGLShaderProgram* m_shaderProgram = nullptr;
+    QOpenGLShaderProgram* m_litShaderProgram = nullptr;
     QOpenGLVertexArrayObject m_vao;
     QOpenGLBuffer m_vbo;
 
@@ -104,6 +105,11 @@ private:
     int m_projMatrixLoc = -1;
     int m_mvMatrixLoc = -1;
     int m_colorLoc = -1;
+
+    // Lit shader uniform locations
+    int m_litProjMatrixLoc = -1;
+    int m_litMvMatrixLoc = -1;
+    int m_litNormalMatrixLoc = -1;
     Camera* m_camera = nullptr;
     ViewCube* m_viewCube;
     SketchGrid* m_sketchGrid = nullptr;

src/shaders/lit.frag

@@ -0,0 +1,33 @@
+#version 330 core
+out vec4 FragColor;
+
+in vec3 FragPos;
+in vec3 Normal;
+
+void main()
+{
+    vec3 viewPos = vec3(0.0, 0.0, 0.0); // View position is origin in view space
+    vec3 lightPos = vec3(0.0, 10.0, 5.0); // Light position in view space
+    vec3 lightColor = vec3(1.0, 1.0, 1.0);
+    vec3 objectColor = vec3(0.5, 0.5, 1.0);
+
+    // Ambient
+    float ambientStrength = 0.3;
+    vec3 ambient = ambientStrength * lightColor;
+
+    // Diffuse
+    vec3 norm = normalize(Normal);
+    vec3 lightDir = normalize(lightPos - FragPos);
+    float diff = max(dot(norm, lightDir), 0.0);
+    vec3 diffuse = diff * lightColor;
+
+    // Specular
+    float specularStrength = 0.5;
+    vec3 viewDir = normalize(viewPos - FragPos);
+    vec3 reflectDir = reflect(-lightDir, norm);
+    float spec = pow(max(dot(viewDir, reflectDir), 0.0), 32);
+    vec3 specular = specularStrength * spec * lightColor;
+
+    vec3 result = (ambient + diffuse + specular) * objectColor;
+    FragColor = vec4(result, 1.0);
+}

src/shaders/lit.vert

@@ -0,0 +1,17 @@
+#version 330 core
+layout (location = 0) in vec3 aPos;
+layout (location = 1) in vec3 aNormal;
+
+out vec3 FragPos;
+out vec3 Normal;
+
+uniform mat4 modelViewMatrix;
+uniform mat4 projectionMatrix;
+uniform mat3 normalMatrix;
+
+void main()
+{
+    FragPos = vec3(modelViewMatrix * vec4(aPos, 1.0));
+    Normal = normalMatrix * aNormal;
+    gl_Position = projectionMatrix * modelViewMatrix * vec4(aPos, 1.0);
+}