Design: clicking the same face twice takes the body, and the status line moves onto the viewport

A click could point at a face, an edge or a vertex, but never at the body those
belong to: offer_selection_kind() can only return BodySolid when all three are
clear, which a viewport click never produces. The rubber band was the only door,
and the status line said "face 5 selected" while the user believed they had taken
the body. A second click on the SAME sub-element now escalates to it (snaporca-gem).

Not the pick cycle that was removed in bc2b741ce9 -- that one was silent and three
deep, so no click had a predictable meaning. Here the status line names the next
click before you make it, and a further click just takes the face under the cursor
again, which needs no teaching. Double-click is untouched: wx sends Down/Up/DClick/Up
and only the first Up carries a pending press, so a fast double-click still zooms to
fit and picks once.

The status line itself moved to the base of the viewport. In the side panel it was
clipped at ~73 characters with no warning and no wrap -- set_status()'s Wrap() never
took effect (snaporca-8cc) -- which silently length-limited every hint in the tab; the
first version of this change lost a clause to it. m_status is kept, hidden, as the
owner of the text and its colour, and the line is drawn in a bottom-left twin of the
readout HUD where there is a whole window's width.

Three defects found driving it on the rig, none of which the build could see:

  * the HUD as a wxFrame took the WM's keyboard focus every time it was raised, and
    the canvas then received NO key events -- every sketch shortcut silently dead.
    Caught with SNAPORCA_KEYTRACE: shift+S logged a line, the following R logged
    nothing. It is a wxPopupWindow now, which cannot be focused. SetFocus() on the
    canvas does not fix it: focus was on another toplevel.
  * zero vertical padding fits the popup tighter than the font's line box and clips
    the glyphs; 6 (what the readout uses) reads as a two-line box. 3 is right.
  * "has a caller chosen a colour?" compared the label's foreground against its
    PARENT's, which differ by default, so every line counted as chosen and the
    neutral text came out the panel's dark grey -- invisible on a dark chip. Compare
    against the colour the label was created with, captured before any caller writes.

Verified on both rigs against fresh binaries: sketch -> extrude -> click face ->
click again -> whole body tinted, offer opens with the body rows live and Create /
Add material correctly greyed. Keyboard drives the whole sequence.

Filed and NOT fixed here: snaporca-od0 -- a bare-plate click does not deselect the
solid, so "click away, click back" escalates. Pre-existing; clearing there would also
drop the face the Thicken/Shell/Draft cards hold, which needs its own pass.

Refs: snaporca-gem, snaporca-8cc, snaporca-od0
This commit is contained in:
Tommaso Bianchi
2026-08-02 12:12:50 +02:00
parent 7e5994b8cb
commit c32aa3f8ba
5 changed files with 137 additions and 13 deletions
+59
View File
@@ -132,6 +132,36 @@ DesignCanvas::DesignCanvas(wxWindow* parent)
}
m_sketch_tool.on_readout = [this](const std::string& s) { set_readout(s); };
// Bottom-LEFT twin, carrying the status line. Top-level for the same reason as the readout
// (a child widget is hidden by the GL surface) but a wxPopupWindow rather than a wxFrame,
// because a popup cannot take keyboard focus. The readout gets away with a frame only
// because it appears mid-gesture and the next input is the mouse; this one is up
// permanently and is re-raised on every status change. As a frame it took the WM's focus
// each time and the canvas stopped receiving keys at all — every sketch shortcut silently
// dead, which reads as a broken tool. Do not "simplify" it back to a wxFrame.
// Its colour is set per message — the panel decides whether a line is neutral or an error.
{
wxWindow* top = wxGetTopLevelParent(m_canvas_widget);
m_status_hud = new wxPopupWindow(top, wxBORDER_NONE);
m_status_hud->SetBackgroundColour(wxColour(28, 30, 34));
m_status_hud_label = new wxStaticText(m_status_hud, wxID_ANY, wxEmptyString);
auto* ss = new wxBoxSizer(wxHORIZONTAL);
// The line never wraps — there is a whole window's width down here — so the chip is
// ONE LINE tall. Spacers rather than a wxALL border because the two axes want
// different numbers: roomy at the sides so it reads as a label, and just enough top
// and bottom to clear the descenders. Zero vertical clips the glyphs; 6 (what the
// readout chip uses) makes it look like a two-line box.
ss->AddSpacer(10);
ss->Add(m_status_hud_label, 0, wxTOP | wxBOTTOM, 3);
ss->AddSpacer(10);
m_status_hud->SetSizerAndFit(ss);
m_status_hud->Hide();
}
// A floating frame does not follow its parent, so the anchor has to be recomputed whenever
// the canvas changes size. The readout HUD gets away without this because it is transient;
// the status line is on screen almost permanently and would visibly detach.
m_canvas_widget->Bind(wxEVT_SIZE, [this](wxSizeEvent& e) { place_status_hud(); e.Skip(); });
refresh_bed();
m_canvas->bind_event_handlers();
@@ -924,6 +954,35 @@ void DesignCanvas::set_readout(const std::string& text)
m_hud->Raise();
}
void DesignCanvas::set_status_text(const wxString& text, const wxColour& colour)
{
if (!m_status_hud || !m_status_hud_label || !m_canvas_widget) return;
if (text == m_status_hud_last && colour == m_status_hud_colour) return;
m_status_hud_last = text;
m_status_hud_colour = colour;
if (text.IsEmpty()) { m_status_hud->Hide(); return; }
m_status_hud_label->SetForegroundColour(colour);
m_status_hud_label->SetLabel(text);
m_status_hud->Fit();
place_status_hud();
}
void DesignCanvas::place_status_hud()
{
if (!m_status_hud || !m_canvas_widget || m_status_hud_last.IsEmpty()) return;
const wxSize cs = m_canvas_widget->GetClientSize();
const wxSize hs = m_status_hud->GetSize();
// Clear of the view cube and the two round view buttons, which own the bottom-left corner.
const int kLeftInset = m_canvas_widget->FromDIP(190);
const wxPoint bl = m_canvas_widget->ClientToScreen(
wxPoint(kLeftInset, cs.GetHeight() - hs.GetHeight() - 12));
// No Raise() and no focus juggling: a popup neither takes focus nor falls behind. This was
// caught with SNAPORCA_KEYTRACE — shift+S logged a line, the following R logged nothing, and
// the only thing between them was the first status update showing this window.
if (!m_status_hud->IsShown()) m_status_hud->Show(); // Show before Move (GTK ignores pre-map Move)
m_status_hud->Move(bl);
}
void DesignCanvas::set_body_highlight(bool on)
{
if (m_body_selected == on) return;
+15
View File
@@ -2,6 +2,7 @@
#define slic3r_DesignCanvas_hpp_
#include <wx/panel.h>
#include <wx/popupwin.h>
#include <functional>
#include <memory>
@@ -181,6 +182,10 @@ public:
void set_datum_planes(std::vector<SketchPlane> planes,
std::vector<Vec2d> sizes = {}); // draw datum/reference planes (u/v extents)
void set_body_highlight(bool on); // tint the solid when its feature is tree-selected
// The status line, shown along the BASE OF THE VIEWPORT rather than in the side panel:
// the panel clips it at ~73 characters with no warning (snaporca-8cc), the viewport's
// bottom margin has the whole window width to spare. Empty text hides it.
void set_status_text(const wxString& text, const wxColour& colour);
void set_operand_bodies(int target_body, int tool_body); // -1,-1 clears
void set_body_translucent(bool on); // render the solid see-through (fillet/chamfer preview)
void set_body_hidden(bool on); // preview-only: hide base bodies, show only the result ghost
@@ -300,6 +305,16 @@ private:
wxStaticText* m_hud_label{nullptr};
std::string m_hud_last;
void set_readout(const std::string& text);
// Bottom-LEFT viewport HUD: the selection / tool status line, written by DesignPanel.
// A wxPopupWindow, NOT the wxFrame the readout HUD uses: a frame accepts keyboard focus,
// and this one is on screen permanently and re-raised on every status change, so it stole
// the keyboard from the canvas and killed every sketch shortcut in the tab.
wxPopupWindow* m_status_hud{nullptr};
wxStaticText* m_status_hud_label{nullptr};
wxString m_status_hud_last;
wxColour m_status_hud_colour;
void place_status_hud(); // re-anchors to the canvas corner (also on resize)
std::function<void(const SketchProfile&, const SketchPlane&)> m_on_sketch_commit;
std::function<void(const std::vector<SketchEntity>&,
const std::vector<SketchEntityConstraintDef>&,
+33 -7
View File
@@ -3056,6 +3056,8 @@ DesignPanel::DesignPanel(wxWindow* parent)
const int cm = FromDIP(SidebarProps::ContentMargin());
m_status = new wxStaticText(m_form, wxID_ANY, "");
m_status->Hide(); // storage only — the line is drawn over the viewport, see set_status()
m_status_default_fg = m_status->GetForegroundColour(); // capture BEFORE any caller writes
root->Add(m_status, 0, wxLEFT | wxRIGHT | wxBOTTOM, 12);
// DoF / constraint-state readout (P3). Dedicated line so it never clobbers the
@@ -3413,10 +3415,21 @@ DesignPanel::DesignPanel(wxWindow* parent)
m_status->SetForegroundColour(wxNullColour);
const int nb = int(m_doc.bodies.size());
const wxString bodytag = (nb > 1) ? wxString::Format(_L("Body %d "), body + 1) : wxString();
set_status(level == 4 ? bodytag + _L("vertex selected")
: level == 1 ? bodytag + _L("selected (whole body)")
: level == 2 ? bodytag + wxString::Format(_L("face %d selected — right-click to push/pull it"), face)
: level == 3 ? bodytag + wxString::Format(_L("edge %d selected — Fillet/Chamfer to dress it"), edge)
// Each sub-element line ends by naming the NEXT click (snaporca-gem). Escalation to the
// whole body is a gesture nothing on screen would otherwise reveal, and the status line
// is the only surface that can teach it at the moment it applies. It REPLACES the old
// per-level verb hints ("right-click to push/pull it", "Fillet/Chamfer to dress it")
// rather than joining them: the line is clipped at the panel edge past ~55 characters
// (set_status's Wrap() does not take effect — snaporca-8cc), and those verbs are shown
// with their icons in the offer anyway, while this gesture is shown nowhere else.
// Both clauses fit now that the line is drawn over the viewport instead of squeezed
// into the panel. Say "what applies to it", never "verbs" — that is this codebase's
// word for a tool-offer entry, not a word the drawing office uses, and the plane and
// bodies-list lines already say it the right way.
set_status(level == 4 ? bodytag + _L("vertex selected — click again for the whole body")
: level == 1 ? bodytag + _L("selected (whole body) — right-click for what applies to it")
: level == 2 ? bodytag + wxString::Format(_L("face %d selected — right-click to push/pull it, or click again for the whole body"), face)
: level == 3 ? bodytag + wxString::Format(_L("edge %d selected — Fillet/Chamfer to dress it, or click again for the whole body"), edge)
: _L("Nothing selected"));
m_status->Refresh();
});
@@ -5285,9 +5298,22 @@ void DesignPanel::set_status(const wxString& text)
{
if (m_status == nullptr) return;
m_status->SetLabel(text); // the ONE place that may call SetLabel directly
const int w = m_status->GetParent() ? m_status->GetParent()->GetClientSize().x - 24 : 420;
m_status->Wrap(w > 120 ? w : 420);
m_status->Refresh();
// m_status is HIDDEN and kept only as the owner of the text and its colour — every caller
// sets the colour on it just before calling here, so this stays the one place that knows
// both. What the user reads is drawn along the BASE OF THE VIEWPORT: in the panel the line
// was clipped at ~73 characters with no warning and no wrap (Wrap() never took effect —
// snaporca-8cc), which silently length-limited every hint in the tab. The viewport's bottom
// margin has the whole window width, so a sentence can be a sentence.
if (m_viewport != nullptr) {
// wxNullColour means "no opinion", and the dark default text colour is nearly invisible
// on the dark HUD; only a colour a caller actually chose (the error red, the plane-pick
// green) is carried over. Compared against the colour the label was CREATED with —
// comparing against the parent's foreground instead reported "chosen" for every line,
// and the neutral text came out the panel's grey.
const wxColour fg = m_status->GetForegroundColour();
m_viewport->set_status_text(text, fg != m_status_default_fg ? fg
: wxColour(0xDD, 0xE1, 0xE6));
}
}
wxMenuItem* DesignPanel::append_offer_item(wxMenu* menu, int id, const wxString& text,
+4
View File
@@ -779,6 +779,10 @@ private:
static int tree_icon_for(CadFeatureType t);
wxStaticText* m_status{nullptr};
// m_status's foreground as created, captured before any caller touches it. Callers signal
// "no opinion" by setting wxNullColour, which restores exactly this — so it is the only
// reliable way to tell a chosen colour (the error red) from the default. See set_status().
wxColour m_status_default_fg;
wxStaticText* m_dof_status{nullptr}; // DoF / constraint-state readout (P3)
int m_feature_counter{0};
+26 -6
View File
@@ -3007,17 +3007,17 @@ bool DesignSketchTool::handle_solid_click(GLCanvas3D& canvas, const wxMouseEvent
return std::hypot(wx - t * vx, wy - t * vy);
};
// What was selected BEFORE this pick — the escalation below is the only thing that reads
// it, and everything from here on overwrites it.
const SolidSel prev_kind = m_solid_sel;
const int prev_body = m_sel_body, prev_face = m_sel_face, prev_edge = m_sel_edge;
const Vec3d prev_vtx = m_sel_vertex_pt;
m_sel_body = best_body;
m_sel_face = best_face;
m_sel_edge = -1;
m_sel_edge_pts.clear();
// WHOLE-BODY selection is deliberately NOT bound here. Double-click is already zoom-to-fit
// (see the LeftDClick branch at the top of on_mouse) and this pick runs on LeftUp, where
// LeftDClick() is never true — a double-click branch here would be dead code that reads as
// a working feature. The body gesture is the rubber band, which is its own piece of work;
// until it lands, bodies are selected from the Bodies list. Written down rather than
// half-done, because a selection model with a silent hole in it is how we got the cycle.
{
const TopoDS_Shape& bshape = (*m_solid_bodies)[m_sel_body].shape;
const TopoDS_Face face = GeometryEngine::face_by_index(bshape, m_sel_face);
@@ -3059,6 +3059,26 @@ bool DesignSketchTool::handle_solid_click(GLCanvas3D& canvas, const wxMouseEvent
m_solid_sel = SolidSel::Face;
}
}
// CLICK AGAIN ON THE SAME THING -> THE WHOLE BODY (snaporca-gem). Pointing at a face and
// pointing at its body are different intents, and until now only the rubber band could
// express the second one — so the status line said "face 0 selected" while the user
// believed they had taken the body, and every body verb had to opt into the face kinds to
// stay reachable. One more click on the SAME sub-element escalates.
//
// This is not the pick cycle that was removed (bc2b741ce9). That one was silent and three
// deep, so no click had a predictable meaning. Here the escalation is announced by the
// status line BEFORE you make the click, and a further click just takes the face under the
// cursor again — the ordinary meaning of clicking a face, which needs no teaching.
//
// Double-click is safe: wx sends Down/Up/DClick/Up, and only the first Up carries a
// pending press, so a fast double-click zooms to fit and picks ONCE. Escalation needs two
// separate clicks, the same "click, pause, click" distinction a file manager uses.
if (m_solid_sel == prev_kind && m_sel_body == prev_body && m_sel_face == prev_face
&& m_sel_edge == prev_edge
&& (m_solid_sel != SolidSel::Vertex || (m_sel_vertex_pt - prev_vtx).norm() < 1e-9)) {
select_body(m_sel_body); // clears face/edge/vertex, tints the whole body
dp_pick_trace("re-pick -> escalated to whole body %d", m_sel_body);
}
dp_pick_trace("pick -> sel=%d body=%d face=%d edge=%d",
int(m_solid_sel), m_sel_body, m_sel_face, m_sel_edge);
if (on_solid_selection_changed)