From c45f84edf44a4fe79958c6dc9170cf5f5ed88b66 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Fri, 14 Aug 2026 09:15:51 +0200 Subject: [PATCH] =?UTF-8?q?CAD:=20three=20Design-tab=20fixes=20=E2=80=94?= =?UTF-8?q?=20consumed=20holed=20loop,=20constraint=20rows,=20hover=20ghos?= =?UTF-8?q?t?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. A consumed loop WITH HOLES was never dropped from the sketch overlay. sync_sketch_display compares each region's entity list against what a per-loop extrude stored, but rebuilt the candidate from the region's OWN entities while the extrude stores the region's entities PLUS every hole's (see selected_loop_entities). For a plate with one bore that is 4 against 5, so the match never fired and the extruded rectangle stayed drawn on top of the solid it had become. Adds region_entity_indices_with_holes, which returns the same order the extrude uses, and compares against that; a matching region now drops its holes' entities too. Hole-less regions are unaffected. This is also the artefact that made a correct plate-with-a-bore read as a plate with a plug in it during rig testing. 2. The Constraints card drew its header and its first row on top of each other. The rows and the delete buttons were parented to m_form rather than m_cards, so they were laid out in the wrong window's coordinate space and started at the card's top edge. Re-parented; nothing else about the card changed. 3. The mate hover ghost never appeared. The highlight handler asked for a repaint with request_repaint(), which only queues a Refresh — and a wxMenu popup runs its own modal loop, so the paint was not serviced until the menu closed, by which time the ghost had been dropped. Adds DesignCanvas::repaint_now(), which flushes the paint immediately, mirroring the m_status->Update() the status line in the same function already needed for the same reason. Delegated to opencode (DeepSeek V4 Pro) and reviewed by diff. Fix 1 needed an accessor on DesignSketchTool because region_loops/RegionLoop are private — that was outside the file list it was given, and it said so rather than working around it. Verified on the rig: a rectangle-plus-circle sketch extrudes to a plate with a bore and NO overlay left on top of it, and the Constraints card shows its header clear of eight readable rows. --- src/slic3r/GUI/DesignCanvas.cpp | 13 +++++++++++++ src/slic3r/GUI/DesignCanvas.hpp | 9 +++++++++ src/slic3r/GUI/DesignPanel.cpp | 12 ++++++------ src/slic3r/GUI/DesignSketchTool.cpp | 16 ++++++++++++++++ src/slic3r/GUI/DesignSketchTool.hpp | 4 ++++ 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/slic3r/GUI/DesignCanvas.cpp b/src/slic3r/GUI/DesignCanvas.cpp index 2296896a37..43ccacb67e 100644 --- a/src/slic3r/GUI/DesignCanvas.cpp +++ b/src/slic3r/GUI/DesignCanvas.cpp @@ -282,6 +282,14 @@ void DesignCanvas::force_repaint() }); } +void DesignCanvas::repaint_now() +{ + if (m_canvas == nullptr || m_canvas_widget == nullptr) + return; + request_repaint(); // mark dirty + Refresh (hardware GL) or render (software GL) + m_canvas_widget->Update(); // service the pending paint immediately (a modal popup owns the loop) +} + void DesignCanvas::reload(bool keep_view) { m_canvas->reset_volumes(); @@ -593,6 +601,11 @@ std::vector> DesignCanvas::region_entity_indices(const std::vec return m_sketch_tool.region_entity_indices(ents); } +std::vector> DesignCanvas::region_entity_indices_with_holes(const std::vector& ents) const +{ + return m_sketch_tool.region_entity_indices_with_holes(ents); +} + void DesignCanvas::clear_loop_pick() { m_sketch_tool.clear_display_pick(); diff --git a/src/slic3r/GUI/DesignCanvas.hpp b/src/slic3r/GUI/DesignCanvas.hpp index 833902b9cd..7d7487325e 100644 --- a/src/slic3r/GUI/DesignCanvas.hpp +++ b/src/slic3r/GUI/DesignCanvas.hpp @@ -83,6 +83,11 @@ public: void set_on_display_sketch_activated(std::function cb); // committed sketch DOUBLE-clicked: edit it std::vector selected_loop_entities() const; // entities of the click-selected loop std::vector> region_entity_indices(const std::vector& ents) const; + // Like region_entity_indices, but each region's entry is its OWN entities followed by the + // entities of each of its holes — the same order selected_loop_entities() hands the kernel. + // A per-loop extrude of a region WITH holes stores exactly this, so this is the shape a + // consumed loop must be compared against. + std::vector> region_entity_indices_with_holes(const std::vector& ents) const; void clear_loop_pick(); // drop the click-selected loop highlight (e.g. after extrude) void set_loop_pick(int feature, int region); // adopt a loop pick made before the commit void set_escalate_on_repick(bool on); // off while a card has armed a face/edge pick @@ -284,6 +289,10 @@ public: // being shown is dropped on hardware GL and no wxEVT_PAINT ever follows, leaving the // canvas blank until another tab switch forces an expose. void force_repaint(); + // Repaint synchronously, for use while a modal popup (the offer menu) owns the event loop: + // a queued Refresh() is not serviced until the popup closes, so a hover ghost drawn behind it + // would never appear. Mirrors DesignPanel's m_status->Update() flush. + void repaint_now(); private: void reload(bool keep_view); diff --git a/src/slic3r/GUI/DesignPanel.cpp b/src/slic3r/GUI/DesignPanel.cpp index 1428db0ca0..f701534272 100644 --- a/src/slic3r/GUI/DesignPanel.cpp +++ b/src/slic3r/GUI/DesignPanel.cpp @@ -4079,7 +4079,7 @@ void DesignPanel::sync_sketch_display() // Drop the entities of any loop already extruded; keep the rest (other // loops + non-loop entities) so they stay visible and selectable. std::vector drop(f.entities.size(), 0); - for (const std::vector& loop : m_viewport->region_entity_indices(f.entities)) { + for (const std::vector& loop : m_viewport->region_entity_indices_with_holes(f.entities)) { std::vector es; for (int ei : loop) if (ei >= 0 && ei < int(f.entities.size())) es.push_back(f.entities[ei]); @@ -5768,7 +5768,7 @@ void DesignPanel::show_offer_menu(const wxPoint& screen_pos) if (!mate_ghost) return; m_viewport->clear_preview(); m_viewport->set_body_hidden(false); - m_viewport->request_repaint(); + m_viewport->repaint_now(); mate_ghost = false; }; menu.Bind(wxEVT_MENU_HIGHLIGHT, @@ -5779,7 +5779,7 @@ void DesignPanel::show_offer_menu(const wxPoint& screen_pos) if (i < 0 || i >= int(opts.size()) || !opts[i].viable) { drop_ghost(); return; } std::string err; mate_ghost = show_mate_ghost(opts[i].kind, cs_a, cs_b, 0.0, 0.0, false, err); - m_viewport->request_repaint(); + m_viewport->repaint_now(); // synchronous: the popup owns the loop, a queued repaint is never serviced }); menu.Bind(wxEVT_MENU, [this, cs_a, cs_b, opts, mate_base, drop_ghost](wxCommandEvent& e) { @@ -7137,7 +7137,7 @@ void DesignPanel::rebuild_constraint_list() m_hdr_constraints->SetLabel(wxString::Format(_L("Constraints (%d)"), int(cons.size()))); if (cons.empty()) { - auto* none = new wxStaticText(m_form, wxID_ANY, _L("No constraints yet")); + auto* none = new wxStaticText(m_cards, wxID_ANY, _L("No constraints yet")); none->SetForegroundColour(dp_sec_text()); m_constraint_rows->Add(none, 0, wxTOP, 4); } @@ -7145,12 +7145,12 @@ void DesignPanel::rebuild_constraint_list() auto* row = new wxBoxSizer(wxHORIZONTAL); // Delete button first (fixed left position, always visible — long labels can // horizontally scroll but ✗ stays put and clickable). BMP-safe ✗ glyph. - auto* del = new wxButton(m_form, wxID_ANY, wxString::FromUTF8("✗"), + auto* del = new wxButton(m_cards, wxID_ANY, wxString::FromUTF8("✗"), wxDefaultPosition, wxSize(26, -1)); del->SetToolTip(_L("Delete constraint")); del->Bind(wxEVT_BUTTON, [this, i](wxCommandEvent&) { delete_constraint(i); }); // Clickable label: selecting it highlights the referenced entities. - auto* lbl = new wxButton(m_form, wxID_ANY, constraint_label(cons[i]), + auto* lbl = new wxButton(m_cards, wxID_ANY, constraint_label(cons[i]), wxDefaultPosition, wxDefaultSize, wxBU_LEFT | wxBORDER_NONE); lbl->Bind(wxEVT_BUTTON, [this, i](wxCommandEvent&) { highlight_constraint_entities(i); }); row->Add(del, 0, wxALIGN_CENTER_VERTICAL | wxRIGHT, 6); diff --git a/src/slic3r/GUI/DesignSketchTool.cpp b/src/slic3r/GUI/DesignSketchTool.cpp index c05a77fd12..bcfd6bd311 100644 --- a/src/slic3r/GUI/DesignSketchTool.cpp +++ b/src/slic3r/GUI/DesignSketchTool.cpp @@ -2780,6 +2780,22 @@ DesignSketchTool::region_entity_indices(const std::vector& ents) c return out; } +std::vector> +DesignSketchTool::region_entity_indices_with_holes(const std::vector& ents) const +{ + const std::vector loops = region_loops(ents); + std::vector> out; + out.reserve(loops.size()); + for (const RegionLoop& r : loops) { + std::vector ids = r.ents; + for (int h : r.holes) + if (h >= 0 && h < int(loops.size())) + ids.insert(ids.end(), loops[h].ents.begin(), loops[h].ents.end()); + out.push_back(std::move(ids)); + } + return out; +} + // Nearest stroke, and the enclosing region if the cursor is inside one, for ONE committed // sketch. Factored out so the single-click and double-click paths cannot drift apart: they must // agree about what is under the pointer or one of them will act on something else. diff --git a/src/slic3r/GUI/DesignSketchTool.hpp b/src/slic3r/GUI/DesignSketchTool.hpp index db89f88295..0f80134cc5 100644 --- a/src/slic3r/GUI/DesignSketchTool.hpp +++ b/src/slic3r/GUI/DesignSketchTool.hpp @@ -205,6 +205,10 @@ public: // Per closed loop, the indices into `ents` that form it (for hiding already-extruded // loops from the committed-sketch overlay). std::vector> region_entity_indices(const std::vector& ents) const; + // Same, but each region's entry is its OWN entities followed by the entities of each of its + // holes, in that order — the exact list a per-loop extrude of a region WITH holes stores + // (see selected_loop_entities()). Needed to match a consumed loop against its source sketch. + std::vector> region_entity_indices_with_holes(const std::vector& ents) const; void clear_display_pick() { m_display_pick = -1; m_display_pick_region = -1; } // Adopt a loop pick the tool did not make itself. The live-sketch path resolves the region // BEFORE the sketch is committed, so once finish_sketch() has turned it into a display