Body-focus picking: fail open, and keep the combo and the viewport as one state

Two defects found by auditing the body-focus x-ray path, which shipped compiled
but never exercised. Neither is reachable from the happy path its test plan
walks, which is why compiling it proved nothing.

1. A STALE FOCUS KILLED THE VIEWPORT. The focus is a body INDEX held by the panel
   across recomputes, so it outlives the body it names: delete a body and the
   stored index can point past the end. body_pickable() then rejected EVERY body,
   because none of them equals an index that no longer exists — a viewport that
   silently accepts no clicks at all, with nothing on screen saying why. Out of
   range now means no restriction. Fail open, never dead.

2. THE COMBO AND THE FOCUS COULD DISAGREE. refresh_cs_body_choice() rebuilds the
   Body combo and, when the body list shrank, silently reset the selection to
   "(all)" — while the viewport stayed focused on the old index. Every other body
   kept its 25% alpha and picking stayed restricted to a body that might be gone.
   That is the exact mirror of the open_tool ordering bug this feature already
   fixed once: that one showed "Body N" over an opaque scene, this one shows
   "(all)" over a dimmed one. They are one state and are now written together.

   Guarded on CoordSys being the active tool, since it is the only card that owns
   this focus. In the edit path the function runs BEFORE open_tool with the
   previous tool still active, so the guard is false and the caller's explicit
   set_xray_focus still wins.

Also confirmed while reading, since the header asserts it: set_solid_pick() does
NOT touch m_pick_only_body, so the focus really does survive the mesh feed. That
claim now has a check behind it rather than a comment.

Reviewed and compiled (libslic3r_gui, RC=0); not exercised. snaporca-bgvk.
This commit is contained in:
Tommaso Bianchi
2026-08-12 21:49:08 +02:00
parent 23585382ab
commit 7311cb12cb
2 changed files with 23 additions and 2 deletions
+14 -1
View File
@@ -5694,7 +5694,20 @@ void DesignPanel::refresh_cs_body_choice()
m_cs_body->Append(_L("(all)"));
for (size_t b = 0; b < m_doc.bodies.size(); ++b)
m_cs_body->Append(wxString::Format(_L("Body %d"), int(b) + 1));
m_cs_body->SetSelection(keep > 0 && keep < int(m_cs_body->GetCount()) ? keep : 0);
const int sel = (keep > 0 && keep < int(m_cs_body->GetCount())) ? keep : 0;
m_cs_body->SetSelection(sel);
// The combo and the viewport focus are ONE state, so they must not be written separately.
// When the body list shrinks, `keep` falls out of range and the selection silently drops to
// "(all)" — while the viewport stayed focused on the old index, leaving every other body at
// 25% alpha and picking locked to a body that may no longer exist. That is the exact mirror
// of the open_tool ordering bug (combo says Body N, viewport opaque); this one says "(all)"
// and stays dimmed.
//
// Guarded on the CoordSys card being the ACTIVE tool because it is the only card that owns
// this focus. In the edit path this function runs BEFORE open_tool, with the previous tool
// still active, so the guard is false and the caller's explicit set_xray_focus still wins.
if (m_viewport != nullptr && m_active == Tool::CoordSys)
m_viewport->set_xray_focus(sel - 1);
}
void DesignPanel::reset_coordsys_refs()
+9 -1
View File
@@ -2847,7 +2847,15 @@ Vec3d DesignSketchTool::body_xform_pt(int body, const Vec3d& p) const
bool DesignSketchTool::body_pickable(int b) const
{
if (b < 0) return false;
if (m_pick_only_body >= 0 && b != m_pick_only_body) return false; // body-focus mode
// Body-focus mode. The focus is an INDEX held by the panel across recomputes, so it can
// outlive the body it names — delete a body and the stored index may point past the end.
// A restriction to a body that no longer exists rejects EVERY body, which is a viewport
// that silently accepts no clicks at all: the worst possible failure for a picking mode,
// because nothing on screen says why. Out of range therefore means NO restriction — fail
// open, never dead.
const bool focus_live = m_pick_only_body >= 0 && m_solid_bodies != nullptr
&& m_pick_only_body < int(m_solid_bodies->size());
if (focus_live && b != m_pick_only_body) return false;
if (m_solid_visible == nullptr || b >= int(m_solid_visible->size())) return true;
return (*m_solid_visible)[b];
}