Refactor window position & size persistence

in a way that is hopefully robust wrt. platform quirks

src/CMakeLists.txt

@@ -122,14 +122,14 @@ if (MSVC)
             add_custom_target("resources_symlink_${CONF}" ALL
                 DEPENDS slic3r
                 COMMAND if exist "${WIN_CONF_OUTPUT_DIR}" "("
-                        if not exist "${WIN_RESOURCES_SYMLINK}" "(" 
+                        if not exist "${WIN_RESOURCES_SYMLINK}" "("
-                            mklink /J "${WIN_RESOURCES_SYMLINK}" "${SLIC3R_RESOURCES_DIR_WIN}" 
+                            mklink /J "${WIN_RESOURCES_SYMLINK}" "${SLIC3R_RESOURCES_DIR_WIN}"
                         ")"
                     ")"
                 VERBATIM
             )
         endforeach ()
-    else () 
+    else ()
         file(TO_NATIVE_PATH "${CMAKE_CURRENT_BINARY_DIR}" OUTPUT_DIR)
         add_custom_target(resources_symlink ALL
             DEPENDS slic3r

src/slic3r/GUI/GUI_App.cpp

@@ -16,6 +16,7 @@
 
 #include "Utils.hpp"
 #include "GUI.hpp"
+#include "GUI_Utils.hpp"
 #include "AppConfig.hpp"
 #include "PresetBundle.hpp"
 #include "3DScene.hpp"
@@ -344,41 +345,43 @@ wxMenuItem* GUI_App::append_submenu(wxMenu* menu,
     return item;
 }
 
-void GUI_App::save_window_pos(wxTopLevelWindow* window, const std::string& name){
+void GUI_App::window_pos_save(wxTopLevelWindow* window, const std::string &name)
-    int x, y;
+{
-    window->GetScreenPosition(&x, &y);
+    if (name.empty()) { return; }
-    app_config->set(name + "_pos", wxString::Format("%d,%d", x, y).ToStdString());
+    const auto config_key = (boost::format("window_%1%") % name).str();
-
-    window->GetSize(&x, &y);
-    app_config->set(name + "_size", wxString::Format("%d,%d", x, y).ToStdString());
-
-    app_config->set(name + "_maximized", window->IsMaximized() ? "1" : "0");
 
+    WindowMetrics metrics = WindowMetrics::from_window(window);
+    app_config->set(config_key, metrics.serialize());
     app_config->save();
 }
 
-void GUI_App::restore_window_pos(wxTopLevelWindow* window, const std::string& name)
+void GUI_App::window_pos_restore(wxTopLevelWindow* window, const std::string &name)
 {
-    if (!app_config->has(name + "_pos"))
+    if (name.empty()) { return; }
-        return;
+    const auto config_key = (boost::format("window_%1%") % name).str();
 
-    std::string str = app_config->get(name + "_size");
+    if (! app_config->has(config_key)) { return; }
-    std::vector<std::string> values;
-    boost::split(values, str, boost::is_any_of(","));
-    wxSize size = wxSize(atoi(values[0].c_str()), atoi(values[1].c_str()));
-    window->SetSize(size);
 
-    auto display = (new wxDisplay())->GetClientArea();
+    auto metrics = WindowMetrics::deserialize(app_config->get(config_key));
-    str = app_config->get(name + "_pos");
+    if (! metrics) { return; }
-    values.resize(0);
-    boost::split(values, str, boost::is_any_of(","));
-    wxPoint pos = wxPoint(atoi(values[0].c_str()), atoi(values[1].c_str()));
-    if (pos.x + 0.5*size.GetWidth() < display.GetRight() &&
-        pos.y + 0.5*size.GetHeight() < display.GetBottom())
-        window->Move(pos);
 
-    if (app_config->get(name + "_maximized") == "1")
+    window->SetSize(metrics->get_rect());
-        window->Maximize();
+    window->Maximize(metrics->get_maximized());
+}
+
+void GUI_App::window_pos_sanitize(wxTopLevelWindow* window)
+{
+    const auto display_idx = wxDisplay::GetFromWindow(window);
+    if (display_idx == wxNOT_FOUND) { return; }
+
+    const auto display = wxDisplay(display_idx).GetClientArea();
+
+    auto metrics = WindowMetrics::from_window(window);
+
+    metrics.sanitize_for_display(display);
+    if (window->GetScreenRect() != metrics.get_rect()) {
+        window->SetSize(metrics.get_rect());
+    }
 }
 
 // select language from the list of installed languages

src/slic3r/GUI/GUI_App.hpp

@@ -113,8 +113,10 @@ public:
                                     const wxString& string,
                                     const wxString& description,
                                     const std::string& icon);
-    void            save_window_pos(wxTopLevelWindow* window, const std::string& name);
+
-    void            restore_window_pos(wxTopLevelWindow* window, const std::string& name);
+    void            window_pos_save(wxTopLevelWindow* window, const std::string &name);
+    void            window_pos_restore(wxTopLevelWindow* window, const std::string &name);
+    void            window_pos_sanitize(wxTopLevelWindow* window);
 
     bool            select_language(wxArrayString & names, wxArrayLong & identifiers);
     bool            load_language();

src/slic3r/GUI/GUI_Utils.cpp

@@ -1,16 +1,23 @@
 #include "GUI_Utils.hpp"
 
+#include <algorithm>
+#include <boost/lexical_cast.hpp>
+#include <boost/format.hpp>
+
+#include <wx/toplevel.h>
 #include <wx/sizer.h>
 #include <wx/panel.h>
 #include <wx/checkbox.h>
 
+#include "libslic3r/Config.hpp"
+
 
 namespace Slic3r {
 namespace GUI {
 
 
 CheckboxFileDialog::CheckboxFileDialog(wxWindow *parent,
-	const wxString &checkbox_label,
+    const wxString &checkbox_label,
     bool checkbox_value,
     const wxString &message,
     const wxString &default_dir,
@@ -24,31 +31,87 @@ CheckboxFileDialog::CheckboxFileDialog(wxWindow *parent,
     : wxFileDialog(parent, message, default_dir, default_file, wildcard, style, pos, size, name)
     , cbox(nullptr)
 {
-	if (checkbox_label.IsEmpty()) {
+    if (checkbox_label.IsEmpty()) {
-		return;
+        return;
-	}
+    }
 
-	extra_control_creator = [this, checkbox_label](wxWindow *parent) -> wxWindow* {
+    extra_control_creator = [this, checkbox_label](wxWindow *parent) -> wxWindow* {
-		wxPanel* panel = new wxPanel(parent, -1);
+        wxPanel* panel = new wxPanel(parent, -1);
-	    wxSizer* sizer = new wxBoxSizer(wxHORIZONTAL);
+        wxSizer* sizer = new wxBoxSizer(wxHORIZONTAL);
-	    this->cbox = new wxCheckBox(panel, wxID_HIGHEST + 1, checkbox_label);
+        this->cbox = new wxCheckBox(panel, wxID_HIGHEST + 1, checkbox_label);
-	    this->cbox->SetValue(true);
+        this->cbox->SetValue(true);
-	    sizer->AddSpacer(5);
+        sizer->AddSpacer(5);
-	    sizer->Add(this->cbox, 0, wxEXPAND | wxALL | wxALIGN_CENTER_VERTICAL, 5);
+        sizer->Add(this->cbox, 0, wxEXPAND | wxALL | wxALIGN_CENTER_VERTICAL, 5);
-	    panel->SetSizer(sizer);
+        panel->SetSizer(sizer);
-	    sizer->SetSizeHints(panel);
+        sizer->SetSizeHints(panel);
 
-	    return panel;
+        return panel;
-	};
+    };
 
     SetExtraControlCreator(*extra_control_creator.target<ExtraControlCreatorFunction>());
 }
 
 bool CheckboxFileDialog::get_checkbox_value() const
 {
-	return this->cbox != nullptr ? cbox->IsChecked() : false;
+    return this->cbox != nullptr ? cbox->IsChecked() : false;
 }
 
 
+
+WindowMetrics WindowMetrics::from_window(wxTopLevelWindow *window)
+{
+    WindowMetrics res;
+    res.rect = window->GetScreenRect();
+    res.maximized = window->IsMaximized();
+    return res;
+}
+
+boost::optional<WindowMetrics> WindowMetrics::deserialize(const std::string &str)
+{
+    std::vector<std::string> metrics_str;
+    metrics_str.reserve(5);
+
+    if (!unescape_strings_cstyle(str, metrics_str) || metrics_str.size() != 5) {
+        return boost::none;
+    }
+
+    int metrics[5];
+    try {
+        for (size_t i = 0; i < 5; i++) {
+            metrics[i] = boost::lexical_cast<int>(metrics_str[i]);
+        }
+    } catch(const boost::bad_lexical_cast &) {
+        return boost::none;
+    }
+
+    if ((metrics[4] & ~1) != 0) {    // Checks if the maximized flag is 1 or 0
+        metrics[4] = 0;
+    }
+
+    WindowMetrics res;
+    res.rect = wxRect(metrics[0], metrics[1], metrics[2], metrics[3]);
+    res.maximized = metrics[4];
+
+    return res;
+}
+
+void WindowMetrics::sanitize_for_display(const wxRect &screen_rect)
+{
+    rect = rect.Intersect(screen_rect);
+}
+
+std::string WindowMetrics::serialize()
+{
+    return (boost::format("%1%; %2%; %3%; %4%; %5%")
+        % rect.GetX()
+        % rect.GetY()
+        % rect.GetWidth()
+        % rect.GetHeight()
+        % static_cast<int>(maximized)
+    ).str();
+}
+
+
+
 }
 }

src/slic3r/GUI/GUI_Utils.hpp

@@ -2,10 +2,16 @@
 #define slic3r_GUI_Utils_hpp_
 
 #include <functional>
+#include <string>
+
+#include <boost/optional.hpp>
 
 #include <wx/filedlg.h>
+#include <wx/gdicmn.h>
 
 class wxCheckBox;
+class wxTopLevelWindow;
+class wxRect;
 
 
 namespace Slic3r {
@@ -36,6 +42,25 @@ private:
 };
 
 
+class WindowMetrics
+{
+private:
+    wxRect rect;
+    bool maximized;
+
+    WindowMetrics() : maximized(false) {}
+public:
+    static WindowMetrics from_window(wxTopLevelWindow *window);
+    static boost::optional<WindowMetrics> deserialize(const std::string &str);
+
+    wxRect get_rect() const { return rect; }
+    bool get_maximized() const { return maximized; }
+
+    void sanitize_for_display(const wxRect &screen_rect);
+    std::string serialize();
+};
+
+
 }}
 
 #endif

src/slic3r/GUI/MainFrame.cpp

@@ -78,8 +78,6 @@ wxFrame(NULL, wxID_ANY, SLIC3R_BUILD, wxDefaultPosition, wxDefaultSize, wxDEFAUL
     Fit();
     SetMinSize(wxSize(760, 490));
     SetSize(GetMinSize());
-    wxGetApp().restore_window_pos(this, "main_frame");
-    Show();
     Layout();
 
     // declare events
@@ -89,7 +87,7 @@ wxFrame(NULL, wxID_ANY, SLIC3R_BUILD, wxDefaultPosition, wxDefaultSize, wxDEFAUL
             return;
         }
         // save window size
-        wxGetApp().save_window_pos(this, "main_frame");
+        wxGetApp().window_pos_save(this, "mainframe");
         // Save the slic3r.ini.Usually the ini file is saved from "on idle" callback,
         // but in rare cases it may not have been called yet.
         wxGetApp().app_config->save();
@@ -101,6 +99,17 @@ wxFrame(NULL, wxID_ANY, SLIC3R_BUILD, wxDefaultPosition, wxDefaultSize, wxDEFAUL
         event.Skip();
     });
 
+    // NB: Restoring the window position is done in a two-phase manner here,
+    // first the saved position is restored as-is and validation is done after the window is shown
+    // and initial round of events is complete, because on some platforms that is the only way
+    // to get an accurate window position & size.
+    wxGetApp().window_pos_restore(this, "mainframe");
+    Bind(wxEVT_SHOW, [this](wxShowEvent&) {
+        CallAfter([this]() {
+            wxGetApp().window_pos_sanitize(this);
+        });
+    });
+
     update_ui_from_settings();
     return;
 }