fix: STEP part names with accented characters import as numbers (clears 6 warnings) (#15406)

build: clear 295 -Woverloaded-virtual warnings in GUI widgets

Turns three hidden base virtuals into real overrides, clearing 295 of
the 553 -Woverloaded-virtual warnings and taking a full clang-cl build
from 1,264 to 969. Part of #15374.

Search.hpp: SearchDialog::Popup and SearchObjectDialog::Popup took a
wxPoint that neither body ever read, hiding the virtual
wxPopupTransientWindow::Popup(wxWindow*). Both bodies clear the input,
call the base, set focus and refill the list, and SearchObjectDialog
also guards re-entry, so hiding meant none of that ran when the window
was popped through a base pointer. They now override and forward focus.

LabeledStaticBox::SetFont and ScrolledWindow::SetBackgroundColour hid
their base virtuals the same way, so the label metrics recompute and
the child colour propagation only ran for callers holding the concrete
type. Both now override.

Marking a member override makes clang flag every other unmarked
override in the same class, so seven sibling declarations needed the
keyword too. Left unmarked they were worth 481 warnings, which would
have made this a net loss.

MSWDismissUnfocusedPopup is declared only inside #ifdef __WXMSW__ in
wx/popupwin.h, so off Windows there is no base virtual to override and
the keyword would not compile. Both the declarations and the definitions
are guarded, which is how wxWidgets itself declares MSWWindowProc in
wx/nativewin.h and how this repo already handles it in BBLTopbar,
MainFrame, Button, ComboBox and TabCtrl.

ScrolledWindow's constructor left m_userPanel and m_scroll_win
uninitialised unless the style requested a vertical scrollbar, while
SetBackgroundColour dereferences both. No caller hits that today since
every instantiation passes wxVSCROLL, but the override widens who can
reach them, so they are now initialised alongside their siblings.

Co-authored-by: Rodrigo Faselli <162915171+RF47@users.noreply.github.com>

src/libslic3r/AABBTreeIndirect.hpp

@@ -229,7 +229,7 @@ public:
         m_bbox(bbox.min - Point(SCALED_EPSILON, SCALED_EPSILON), bbox.max + Point(SCALED_EPSILON, SCALED_EPSILON)) {}
     size_t             idx() const { return m_idx; }
     const BoundingBox& bbox() const { return m_bbox; }
-    Point              centroid() const { return (m_bbox.min() + m_bbox.max() / 2); }
+    Point              centroid() const { return (m_bbox.min() + m_bbox.max()) / 2; }
 private:
     size_t             m_idx;
     BoundingBox		   m_bbox;

src/libslic3r/Format/OBJ.cpp

@@ -55,9 +55,9 @@ bool load_obj(const char *path, TriangleMesh *meshptr, ObjInfo& obj_info, std::s
                 boost::filesystem::path temp_mtl_path(mtl_file);
                 mtl_path = temp_mtl_path;
             }
-            auto    _mtl_path = mtl_name_is_path ? mtl_abs_path.string().c_str() : mtl_path.string().c_str();
+            const std::string _mtl_path = (mtl_name_is_path ? mtl_abs_path : mtl_path).string();
             if (boost::filesystem::exists(mtl_name_is_path ? mtl_abs_path : mtl_path)) {
-                if (!ObjParser::mtlparse(_mtl_path, mtl_data)) {
+                if (!ObjParser::mtlparse(_mtl_path.c_str(), mtl_data)) {
                     BOOST_LOG_TRIVIAL(error) << "load_obj:load_mtl: failed to parse " << _mtl_path;
                     message = _L("load mtl in obj: failed to parse");
                     return false;

src/libslic3r/Format/STEP.cpp

@@ -111,14 +111,19 @@ bool StepPreProcessor::isUtf8File(const char* path)
 bool StepPreProcessor::isUtf8(const std::string str)
 {
     size_t num = 0;
-    int i = 0;
+    size_t i = 0;
     while (i < str.length()) {
-        if ((str[i] & 0x80) == 0x00) {
+        const unsigned char lead = static_cast<unsigned char>(str[i]);
+        if ((lead & 0x80) == 0x00) {
             i++;
-        } else if ((num = preNum(str[i])) > 2) {
+        // preNum() counts the leading 1 bits, and a multi-byte sequence is 2 to 4
+        // bytes long, so anything outside that range is not a lead byte.
+        } else if ((num = preNum(lead)) >= 2 && num <= 4) {
+            if (i + num > str.length())
+                return false;
             i++;
-            for (int j = 0; j < num - 1; j++) {
-                if ((str[i] & 0xc0) != 0x80)
+            for (size_t j = 0; j < num - 1; j++) {
+                if ((static_cast<unsigned char>(str[i]) & 0xc0) != 0x80)
                     return false;
                 i++;
             }
@@ -132,15 +137,20 @@ bool StepPreProcessor::isUtf8(const std::string str)
 bool StepPreProcessor::isGBK(const std::string str) {
     size_t i = 0;
     while (i < str.length()) {
-        if (str[i] <= 0x7f) {
+        // char is signed here, so every byte compares <= 0x7f unless widened first.
+        const unsigned char lead = static_cast<unsigned char>(str[i]);
+        if (lead <= 0x7f) {
             i++;
             continue;
         } else {
-            if (str[i] >= 0x81 &&
-                str[i] <= 0xfe &&
-                str[i + 1] >= 0x40 &&
-                str[i + 1] <= 0xfe &&
-                str[i + 1] != 0xf7) {
+            if (i + 1 >= str.length())
+                return false;
+            const unsigned char trail = static_cast<unsigned char>(str[i + 1]);
+            if (lead >= 0x81 &&
+                lead <= 0xfe &&
+                trail >= 0x40 &&
+                trail <= 0xfe &&
+                trail != 0xf7) {
                 i += 2;
                 continue;
             }

src/slic3r/GUI/Search.cpp

@@ -681,7 +681,7 @@ SearchDialog::SearchDialog(OptionsSearcher *searcher, Preset::Type type, wxWindo
 
 SearchDialog::~SearchDialog() {}
 
-void SearchDialog::Popup(wxPoint position /*= wxDefaultPosition*/)
+void SearchDialog::Popup(wxWindow *focus /*= nullptr*/)
 {
     /* const std::string& line = searcher->search_string();
      search_line->SetValue(line.empty() ? default_string : from_u8(line));
@@ -696,17 +696,19 @@ void SearchDialog::Popup(wxPoint position /*= wxDefaultPosition*/)
     search_line2->SetValue(wxString(""));
     //const std::string &line = searcher->search_string();
     //searcher->search(into_u8(line), true);
-    PopupWindow::Popup();
+    PopupWindow::Popup(focus);
     search_line2->SetFocus();
     update_list();
 }
 
 
+#ifdef __WXMSW__
 void SearchDialog::MSWDismissUnfocusedPopup()
 {
     Dismiss();
     OnDismiss();
 }
+#endif // __WXMSW__
 
 void SearchDialog::OnDismiss() { }
 
@@ -926,7 +928,7 @@ SearchObjectDialog::SearchObjectDialog(GUI::ObjectList* object_list, wxWindow* p
 
 SearchObjectDialog::~SearchObjectDialog() {}
 
-void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/)
+void SearchObjectDialog::Popup(wxWindow *focus /*= nullptr*/)
 {
     if (m_is_dismissing || this->IsShown()) {
         return;
@@ -937,7 +939,7 @@ void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/)
     // dropdown list, otherwise the text input won't be usable
     m_object_list->SetFocus();
 #endif
-    PopupWindow::Popup();
+    PopupWindow::Popup(focus);
     search_line2->SetFocus();
 
     m_object_list->assembly_plate_object_name();
@@ -945,11 +947,13 @@ void SearchObjectDialog::Popup(wxPoint position /*= wxDefaultPosition*/)
     update_list();
 }
 
+#ifdef __WXMSW__
 void SearchObjectDialog::MSWDismissUnfocusedPopup()
 {
     Dismiss();
     OnDismiss();
 }
+#endif // __WXMSW__
 
 void SearchObjectDialog::OnDismiss() {}
 

src/slic3r/GUI/Search.hpp

@@ -216,10 +216,12 @@ public:
     SearchDialog(OptionsSearcher *searcher, Preset::Type type, wxWindow *parent, TextInput *input, wxWindow *search_btn);
     ~SearchDialog();
 
-    void MSWDismissUnfocusedPopup();
-    void Popup(wxPoint position = wxDefaultPosition);
-    void OnDismiss();
-    void Dismiss();
+#ifdef __WXMSW__
+    void MSWDismissUnfocusedPopup() override;
+#endif // __WXMSW__
+    void Popup(wxWindow *focus = nullptr) override;
+    void OnDismiss() override;
+    void Dismiss() override;
     void Die();
     void msw_rescale();
 
@@ -260,10 +262,12 @@ public:
     SearchObjectDialog(GUI::ObjectList* object_list, wxWindow* parent, TextInput* input);
     ~SearchObjectDialog();
 
-    void MSWDismissUnfocusedPopup();
-    void Popup(wxPoint position = wxDefaultPosition);
-    void OnDismiss();
-    void Dismiss();
+#ifdef __WXMSW__
+    void MSWDismissUnfocusedPopup() override;
+#endif // __WXMSW__
+    void Popup(wxWindow *focus = nullptr) override;
+    void OnDismiss() override;
+    void Dismiss() override;
     void Die();
 
     void OnInputText(wxCommandEvent& event);

src/slic3r/GUI/Widgets/LabeledStaticBox.cpp

@@ -98,7 +98,7 @@ void LabeledStaticBox::SetBorderColor(StateColor const &color)
     Refresh();
 }
 
-void LabeledStaticBox::SetFont(wxFont set_font)
+bool LabeledStaticBox::SetFont(const wxFont &set_font)
 {
     m_font = set_font;
 
@@ -109,6 +109,7 @@ void LabeledStaticBox::SetFont(wxFont set_font)
     m_label_width  = tW;
 
     Refresh();
+    return true;
 }
 
 bool LabeledStaticBox::Enable(bool enable)

src/slic3r/GUI/Widgets/LabeledStaticBox.hpp

@@ -42,7 +42,7 @@ public:
 
     void SetBorderColor(StateColor const &color);
 
-    void SetFont(wxFont set_font);
+    bool SetFont(const wxFont &set_font) override;
 
     bool Enable(bool enable) override;
 

src/slic3r/GUI/Widgets/ScrolledWindow.cpp

@@ -21,6 +21,8 @@ ScrolledWindow::ScrolledWindow(wxWindow *parent, wxWindowID id, wxPoint position
     m_bottomScrollbar    = NULL;
     m_verticalSplitter   = NULL;
     m_horizontalSplitter = NULL;
+    m_userPanel          = NULL;
+    m_scroll_win         = NULL;
 
     m_marginWidth        = marginWidth;
 
@@ -110,12 +112,13 @@ void ScrolledWindow::SetTipColor(wxColour color)
     if (m_bottomScrollbar) m_bottomScrollbar->SetTipColor(color);
 }
 
-void ScrolledWindow::SetBackgroundColour(wxColour color)
+bool ScrolledWindow::SetBackgroundColour(const wxColour &color)
 {
-    wxWindow::SetBackgroundColour(color); 
+    const bool result = wxWindow::SetBackgroundColour(color);
     m_verticalSplitter->SetBackgroundColour(color); 
     m_userPanel->SetBackgroundColour(color);
     m_scroll_win->SetBackgroundColour(color);
+    return result;
 }
 
 void ScrolledWindow::SetMarginColor(wxColour color)

src/slic3r/GUI/Widgets/ScrolledWindow.hpp

@@ -15,7 +15,7 @@ public:
     ScrolledWindow(wxWindow *parent, wxWindowID id, wxPoint position, wxSize size, long style, int marginWidth = 0, int scrollbarWidth = 4, int tipLength = 0);
     void OnMouseWheel(wxMouseEvent &event);
     void SetTipColor(wxColour color);
-    void SetBackgroundColour(wxColour color);
+    bool SetBackgroundColour(const wxColour &color) override;
 
     void         SetMarginColor(wxColour color);
     void         SetScrollbarColor(wxColour color);
@@ -26,7 +26,7 @@ public:
     // wxSplitterWindow* GetVerticalSplitter() { return m_verticalSplitter; }
     // wxSplitterWindow* GetHorizontalSplitter() { return m_horizontalSplitter; }
     bool         IsBothDirections() { return m_bothDirections; }
-    virtual void SetScrollbars(int pixelsPerUnitX, int pixelsPerUnitY, int noUnitsX, int noUnitsY, int xPos = 0, int yPos = 0, bool noRefresh = false);
+    virtual void SetScrollbars(int pixelsPerUnitX, int pixelsPerUnitY, int noUnitsX, int noUnitsY, int xPos = 0, int yPos = 0, bool noRefresh = false) override;
 
 private:
     wxPanel *    m_userPanel; // the panel targeted by the scrolled window

tests/libslic3r/CMakeLists.txt

@@ -29,6 +29,7 @@ add_executable(${_TEST_NAME}_tests
     test_mutable_polygon.cpp
     test_mutable_priority_queue.cpp
     test_nozzle_volume_type.cpp
+    test_step.cpp
     test_stl.cpp
     test_triangle_selector.cpp
     test_meshboolean.cpp

tests/libslic3r/test_step.cpp

@@ -0,0 +1,93 @@
+#include <catch2/catch_all.hpp>
+
+#include <boost/nowide/fstream.hpp>
+
+#include "libslic3r/Model.hpp"
+#include "libslic3r/Format/STEP.hpp"
+#include "test_utils.hpp"
+
+using namespace Slic3r;
+
+static void write_step_line(const std::string &path, const std::string &line)
+{
+    boost::nowide::ofstream file(path, std::ios::binary);
+    file << "ISO-10303-21;\n" << line << "\nEND-ISO-10303-21;\n";
+}
+
+// preprocess() hands back the input path unless it transcoded into a temporary.
+static std::string preprocess_result(const std::string &line)
+{
+    ScopedSlic3rTemporaryDir scratch;
+
+    ScopedTemporaryFile step(".step");
+    write_step_line(step.string(), line);
+
+    std::string      output_path;
+    StepPreProcessor preprocessor;
+    REQUIRE(preprocessor.preprocess(step.string().c_str(), output_path));
+
+    return output_path == step.string() ? "untouched" : "transcoded";
+}
+
+// data/utf8_part_names.step is three boxes written by OCCT's own STEP writer, whose
+// PRODUCT names were then patched to raw UTF-8. Most CAD exporters write non-ASCII names
+// that way rather than in the \X2\ escape form. The third part is ASCII, as a control.
+TEST_CASE("Part names with multi-byte UTF-8 survive import", "[Step]")
+{
+    // getNamedSolids() replaces a name that isUtf8() rejects with a running number.
+    const std::string path = TEST_DATA_DIR PATH_SEPARATOR "utf8_part_names.step";
+
+    Model model;
+    bool  cancel = false;
+    Step  step(path); // no isUtf8Fn, matching how Model::read_from_step builds it
+
+    REQUIRE(step.load() == Step::Step_Status::LOAD_SUCCESS);
+    REQUIRE(step.mesh(&model, cancel, false) == Step::Step_Status::MESH_SUCCESS);
+
+    REQUIRE(model.objects.size() == 1);
+    const ModelObject *object = model.objects.front();
+    REQUIRE(object->volumes.size() == 3);
+    // "ce" is split off, or the hex escape would swallow it as further hex digits.
+    CHECK(object->volumes[0]->name == "pi\xC3\xA8" "ce");
+    CHECK(object->volumes[1]->name == "Geh\xC3\xA4use");
+    CHECK(object->volumes[2]->name == "bracket");
+}
+
+TEST_CASE("isUtf8 recognises two, three and four byte sequences", "[Step]")
+{
+    CHECK(StepPreProcessor::isUtf8("\xC3\xA9"));         // U+00E9
+    CHECK(StepPreProcessor::isUtf8("\xE4\xB8\xAD"));     // U+4E2D
+    CHECK(StepPreProcessor::isUtf8("\xF0\x9F\x94\xA9")); // U+1F529
+    CHECK_FALSE(StepPreProcessor::isUtf8("\x81\x30"));   // 0x81 is not a lead byte
+    CHECK_FALSE(StepPreProcessor::isUtf8("\xC3"));       // truncated sequence
+}
+
+// The only caller of isGBK is preprocess(), which nothing calls today.
+TEST_CASE("Encoding detection decides whether a step file is transcoded", "[Step]")
+{
+    SECTION("UTF-8, so left alone")
+    {
+        // A two byte sequence also satisfies every GBK range, so misdetecting it as
+        // not-UTF-8 sends it to be transcoded.
+        const std::string sequence = GENERATE(std::string("\xC3\xA9"),          // U+00E9
+                                              std::string("\xE4\xB8\xAD"),      // U+4E2D
+                                              std::string("\xF0\x9F\x94\xA9")); // U+1F529
+
+        CHECK(preprocess_result("NAME('" + sequence + "');") == "untouched");
+    }
+
+    SECTION("neither UTF-8 nor GBK, so left alone")
+    {
+        // 0x81 is not a UTF-8 lead byte, and 0x30 is below the 0x40 floor for a GBK trail.
+        CHECK(preprocess_result("NAME('\x81\x30');") == "untouched");
+    }
+
+    SECTION("GBK, so transcoded")
+    {
+        // U+554A in GBK, whose lead byte is not valid UTF-8. Pins the other direction,
+        // since a detector that never reports GBK would pass every case above.
+        CHECK(preprocess_result("NAME('\xB0\xA1');") == "transcoded");
+    }
+
+    SECTION("plain ASCII, so left alone") { CHECK(preprocess_result("NAME('bracket');") == "untouched"); }
+}

tests/test_utils.hpp

@@ -4,6 +4,7 @@
 #include <libslic3r/TriangleMesh.hpp>
 #include <libslic3r/Format/OBJ.hpp>
 #include <libslic3r/SVG.hpp>
+#include <libslic3r/Utils.hpp>
 
 #include <boost/filesystem.hpp>
 
@@ -32,7 +33,7 @@ inline Slic3r::TriangleMesh load_model(const std::string &obj_filename)
 // ---------------------------------------------------------------------------
 
 // Owns a unique path under the system temp dir, "<prefix>-<unique>[<extension>]"
-// (parallel-safe, cross-platform). Shared base for the two RAII temp guards below.
+// (parallel-safe, cross-platform). Shared base for the RAII temp guards below.
 class ScopedTemporaryPath
 {
 public:
@@ -70,6 +71,24 @@ public:
     ~ScopedTemporaryDir() { boost::system::error_code ec; boost::filesystem::remove_all(m_path, ec); }
 };
 
+// A temp directory that is also Slic3r::temporary_dir() for its lifetime. No test
+// process sets that global, so code under test which writes there (for example
+// StepPreProcessor::preprocess) lands at the filesystem root. Restored on scope exit
+// even when an assertion throws, so it cannot leak into later tests.
+class ScopedSlic3rTemporaryDir : public ScopedTemporaryDir
+{
+public:
+    explicit ScopedSlic3rTemporaryDir(const std::string &prefix = "orca")
+        : ScopedTemporaryDir(prefix), m_previous(Slic3r::temporary_dir())
+    { Slic3r::set_temporary_dir(string()); }
+    // Runs before ~ScopedTemporaryDir, so the setting goes back while the directory
+    // it names still exists.
+    ~ScopedSlic3rTemporaryDir() { Slic3r::set_temporary_dir(m_previous); }
+
+private:
+    const std::string m_previous;
+};
+
 // ---------------------------------------------------------------------------
 // Debug-only test artifacts
 //