From f6f2edb90682ed2e507162265fc82ab250b7ea81 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Fri, 14 Aug 2026 12:18:13 +0200 Subject: [PATCH] CAD: the Plane card refuses a method it cannot build, instead of quietly building another one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every method in CadDocument's plane dispatch falls back to offset_angle_plane() when its references are missing. Picking Tangent and confirming with nothing selected therefore produced an OFFSET plane, announced as "Plane added — pick it as a sketch plane". The user asked for one construction and silently received a different one, with nothing on screen to reveal the substitution. Validate at the GUI boundary instead: Angle needs an edge, Midplane two faces (and not the same face twice — that yields a plane coincident with the face, which is well-defined and useless), Tangent a face, Two-edges two edges. Offset and Coincident are unchanged: both are meaningful with no reference, since they fall back to the base plane by design. on_add_plane() now returns false when it refuses, and confirm_tool() skips close_tool() in that case — a refusal that also threw away the picks the user had already made would be worse than the bug. The kernel keeps fallback_offset(): it must return something. It should just never be reachable from a user gesture without a warning. Verified on the Xvfb rig: Tangent with no pick refuses and creates no feature (it created one before), the card stays open with the type preserved, Midplane with no faces refuses with its own message, and Offset with no picks still creates a plane as it always did. Co-Authored-By: Claude Opus 5 (1M context) --- src/slic3r/GUI/DesignPanel.cpp | 38 ++++++++++++++++++++++++++++++++-- src/slic3r/GUI/DesignPanel.hpp | 2 +- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index 5401f15573..460597ff95 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -6007,8 +6007,41 @@ void DesignPanel::arm_coordsys_pick(CoordSysPick target) m_status->Refresh(); } -void DesignPanel::on_add_plane() +bool DesignPanel::on_add_plane() { + // Refuse here rather than let the kernel substitute. Every method in CadDocument's plane + // dispatch falls back to offset_angle_plane() when its references are missing, so picking + // Tangent and confirming with nothing selected used to produce an offset plane reported as + // a success — the user asks for one construction and silently receives another. The kernel + // keeps its fallback (it must return SOMETHING), but no user gesture should reach it. + auto refuse = [this](const wxString& why) { + m_status->SetForegroundColour(wxColour(235, 110, 110)); + set_status(why); + m_status->Refresh(); + }; + switch ((PlaneType)m_plane_type->GetSelection()) { + case PlaneType::Angle: + if (m_pl_edgeA < 0) { refuse(_L("An angled plane needs an edge to tilt about — pick Edge A")); return false; } + break; + case PlaneType::Midplane: + if (m_pl_faceA < 0 || m_pl_faceB < 0) { refuse(_L("A midplane needs two faces — pick Face A and Face B")); return false; } + if (m_pl_faceA == m_pl_faceB && m_pl_faceA_body == m_pl_faceB_body) { + // Well-defined but useless: the midplane of a face with itself is that same face. + refuse(_L("Face A and Face B are the same face — a midplane needs two different faces")); + return false; + } + break; + case PlaneType::Tangent: + // The face must also be cylindrical; that check stays in the kernel, which has the geometry. + if (m_pl_faceA < 0) { refuse(_L("A tangent plane needs a cylindrical face — pick Face A")); return false; } + break; + case PlaneType::TwoEdges: + if (m_pl_edgeA < 0 || m_pl_edgeB < 0) { refuse(_L("This plane needs two edges — pick Edge A and Edge B")); return false; } + break; + default: + break; // Offset and Coincident are meaningful with no reference: they use the base plane + } + m_feature_counter++; int idx = m_doc.add_plane(m_plane_base->GetSelection(), m_plane_offset->GetValue(), m_plane_tilt->GetValue(), m_plane_tilt_axis->GetSelection(), @@ -6018,6 +6051,7 @@ void DesignPanel::on_add_plane() m_status->SetForegroundColour(wxNullColour); set_status(_L("Plane added — pick it as a sketch plane")); refresh_tree(); + return true; } void DesignPanel::on_add_axis() @@ -10244,7 +10278,7 @@ void DesignPanel::confirm_tool() case Tool::Revolve: on_add_revolve(); break; case Tool::Sweep: on_add_sweep(); break; case Tool::Pattern: on_add_pattern(); break; - case Tool::Plane: on_add_plane(); break; + case Tool::Plane: if (!on_add_plane()) return; break; // refused: keep the card and its picks case Tool::Loft: on_add_loft(); break; case Tool::Draft: on_add_draft(); break; case Tool::Boolean: on_add_boolean(); break; diff --git a/src/slic3r/GUI/DesignPanel.hpp b/src/slic3r/GUI/DesignPanel.hpp index 29345e472f..0afbddd190 100644 --- a/src/slic3r/GUI/DesignPanel.hpp +++ b/src/slic3r/GUI/DesignPanel.hpp @@ -97,7 +97,7 @@ private: void on_add_sweep(); void on_add_loft(); void on_add_pattern(); - void on_add_plane(); + bool on_add_plane(); // false = refused, card stays open void arm_plane_pick(PlanePick target); // Plane tool: next solid pick fills this reference void apply_plane_refs(CadFeature& f) const; // copy type + face/edge refs + sizes from the card void refresh_plane_labels(); // update the 4 pick labels from the captured refs