mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-22 14:50:59 +00:00
fix(imex): code-review nits — null check, cache-key precision, lambda capture
Four quick-fix items surfaced by pre-PR self-review.
GCodeViewer.cpp:1607
Null-check get_curr_plate() before dereferencing. Other call sites in
the file already guard; this was the only unguarded one in the IMEX
layer-preview path. In practice m_plate_list always has a plate, but
the inconsistency is easy to fix and removes the only ungated deref.
PartPlate.cpp:build_imex_cache_key
Cache key for IMEX zone geometry truncated nozzle_clearance_x/y to int
before stringifying — a config change from 30.0 to 30.5 would not
invalidate the cache. Match the *10 precision pattern already used for
imex_carriage_margin so 0.1 mm steps invalidate correctly.
Plater.cpp:select_plate_by_hover_id (right-click popup)
Two issues:
1. Lambda captured `modes` by reference. PopupMenu() is synchronous
today so the reference outlived the menu's event handling, but the
pattern is fragile — anyone refactoring to async Popup() would
silently dangle. Capture by value.
2. Used wxID_HIGHEST + i for menu item IDs — standard wx anti-pattern
because it can collide with other handlers listening in that range.
Allocate per-item IDs via wxNewId() and look up the chosen mode by
finding the event ID in a parallel vector. The lookup becomes O(N)
instead of O(1) but N is small (mode count) and this is clicker
latency, not a hot path.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
1 parent
7b9070d521
commit
efa9d65cb4
3 files changed
+17
-13
No files matched your search
+14
-10
@@ -17984,20 +17984,24 @@ int Plater::select_plate_by_hover_id(int hover_id, bool right_click, bool isModi
|
||||
// Show a popup menu with all modes.
|
||||
wxMenu menu;
|
||||
std::string current = curr_plate->get_imex_mode();
|
||||
std::vector<int> mode_ids;
|
||||
mode_ids.reserve(modes.size());
|
||||
for (size_t i = 0; i < modes.size(); ++i) {
|
||||
wxMenuItem* item = menu.AppendRadioItem(wxID_HIGHEST + (int)i, from_u8(modes[i]));
|
||||
int id = wxNewId();
|
||||
mode_ids.push_back(id);
|
||||
wxMenuItem* item = menu.AppendRadioItem(id, from_u8(modes[i]));
|
||||
if (modes[i] == current)
|
||||
item->Check(true);
|
||||
}
|
||||
menu.Bind(wxEVT_MENU, [this, curr_plate, &modes](wxCommandEvent& e) {
|
||||
int idx = e.GetId() - wxID_HIGHEST;
|
||||
if (idx >= 0 && idx < (int)modes.size()) {
|
||||
take_snapshot("set imex mode");
|
||||
curr_plate->set_imex_mode(modes[idx]);
|
||||
update_project_dirty_from_presets();
|
||||
set_plater_dirty(true);
|
||||
update();
|
||||
}
|
||||
menu.Bind(wxEVT_MENU, [this, curr_plate, modes, mode_ids](wxCommandEvent& e) {
|
||||
auto it = std::find(mode_ids.begin(), mode_ids.end(), e.GetId());
|
||||
if (it == mode_ids.end()) return;
|
||||
const size_t idx = std::distance(mode_ids.begin(), it);
|
||||
take_snapshot("set imex mode");
|
||||
curr_plate->set_imex_mode(modes[idx]);
|
||||
update_project_dirty_from_presets();
|
||||
set_plater_dirty(true);
|
||||
update();
|
||||
});
|
||||
p->view3D->get_canvas3d()->get_wxglcanvas()->PopupMenu(&menu);
|
||||
ret = 1; // signal to caller: popup was shown, suppress plate context menu
|
||||
|
||||
Reference in new issue
Block a user