From c017629d11268eb07871a6eab7fe7c8e7f007b16 Mon Sep 17 00:00:00 2001 From: ExPikaPaka Date: Thu, 1 Oct 2026 09:15:48 +0200 Subject: [PATCH] Release GL textures, menus and handlers the GUI was leaking The filament legend held its texture id in a function local static while the loader it calls generates a new texture every time, and BitmapCache deletes none, so a dual nozzle printer leaked one texture per filament per frame while the preview was open, along with an SVG read and a rasterize per frame. The same pattern sat in menu_item_with_icon. Textures are now cached on what actually varies and released with the rest of the ImGui resources. The canvas reuses frames by hashing the draw commands, which include the texture id, so a fresh id every frame also kept frame skipping from ever triggering. TriangleSelectorPatch did not release its VAO and buffers in the destructor, so every gizmo close leaked them. The base class already issues GL deletes on that same path, so this adds no new context requirement. bind_event_handlers() installed three lambdas that unbind_event_handlers() could not remove, because wx matches handlers by functor address. They are member functions now, so bind and unbind are symmetric, and the view switch stops stacking handlers. The five selection dependent menus were allocated with new on every popup and never owned. PopupMenu is synchronous and none of them is used as a submenu, so a single owner is enough. remove_notification_of_type stopped at the first match, so clear_all() left the later instances of multi-instance types alive with object ids from a project that is gone. The measure gizmo cleared its raycaster map but not the map holding one full mesh decomposition per volume. --- src/slic3r/GUI/BitmapCache.hpp | 2 + src/slic3r/GUI/GLCanvas3D.cpp | 39 ++++++++++---- src/slic3r/GUI/GLCanvas3D.hpp | 3 ++ src/slic3r/GUI/GUI_Factories.cpp | 18 ++++--- src/slic3r/GUI/GUI_Factories.hpp | 10 ++++ src/slic3r/GUI/Gizmos/GLGizmoMeasure.cpp | 4 ++ src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp | 7 +-- src/slic3r/GUI/ImGuiWrapper.cpp | 54 ++++++++++++++++---- src/slic3r/GUI/ImGuiWrapper.hpp | 8 +++ src/slic3r/GUI/NotificationManager.cpp | 3 +- 10 files changed, 118 insertions(+), 30 deletions(-) diff --git a/src/slic3r/GUI/BitmapCache.hpp b/src/slic3r/GUI/BitmapCache.hpp index e2ebe75314..d733ec0366 100644 --- a/src/slic3r/GUI/BitmapCache.hpp +++ b/src/slic3r/GUI/BitmapCache.hpp @@ -55,6 +55,8 @@ public: static bool parse_color(const std::string& scolor, unsigned char* rgb_out); static bool parse_color4(const std::string& scolor, unsigned char* rgba_out); + // Rasterizes the SVG into a freshly generated GL texture; the caller owns it and has to delete + // it (ImGuiWrapper::svg_texture() caches the result for the whole session). static bool load_from_svg_file_change_color(const std::string &filename, unsigned width, unsigned height, ImTextureID &texture_id, const char *hexColor); diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index 01ff6de714..224cd57275 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -3245,7 +3245,8 @@ void GLCanvas3D::load_sla_preview() void GLCanvas3D::bind_event_handlers() { - if (m_canvas != nullptr) { + // Every view switch binds, so binding twice would run each handler twice per event. + if (m_canvas != nullptr && !m_event_handlers_bound) { m_canvas->Bind(wxEVT_SIZE, &GLCanvas3D::on_size, this); m_canvas->Bind(wxEVT_IDLE, &GLCanvas3D::on_idle, this); m_canvas->Bind(wxEVT_CHAR, &GLCanvas3D::on_char, this); @@ -3255,9 +3256,9 @@ void GLCanvas3D::bind_event_handlers() m_canvas->Bind(wxEVT_TIMER, &GLCanvas3D::on_timer, this); m_canvas->Bind(EVT_GLCANVAS_RENDER_TIMER, &GLCanvas3D::on_render_timer, this); m_toolbar_highlighter.set_timer_owner(m_canvas, 0); - m_canvas->Bind(EVT_GLCANVAS_TOOLBAR_HIGHLIGHTER_TIMER, [this](wxTimerEvent&) { m_toolbar_highlighter.blink(); }); + m_canvas->Bind(EVT_GLCANVAS_TOOLBAR_HIGHLIGHTER_TIMER, &GLCanvas3D::on_toolbar_highlighter_timer, this); m_gizmo_highlighter.set_timer_owner(m_canvas, 0); - m_canvas->Bind(EVT_GLCANVAS_GIZMO_HIGHLIGHTER_TIMER, [this](wxTimerEvent&) { m_gizmo_highlighter.blink(); }); + m_canvas->Bind(EVT_GLCANVAS_GIZMO_HIGHLIGHTER_TIMER, &GLCanvas3D::on_gizmo_highlighter_timer, this); m_canvas->Bind(wxEVT_LEFT_DOWN, &GLCanvas3D::on_mouse, this); m_canvas->Bind(wxEVT_LEFT_UP, &GLCanvas3D::on_mouse, this); m_canvas->Bind(wxEVT_MIDDLE_DOWN, &GLCanvas3D::on_mouse, this); @@ -3272,14 +3273,7 @@ void GLCanvas3D::bind_event_handlers() m_canvas->Bind(wxEVT_RIGHT_DCLICK, &GLCanvas3D::on_mouse, this); m_canvas->Bind(wxEVT_PAINT, &GLCanvas3D::on_paint, this); m_canvas->Bind(wxEVT_SET_FOCUS, &GLCanvas3D::on_set_focus, this); - m_canvas->Bind(wxEVT_KILL_FOCUS, [this](wxFocusEvent& evt) { - // The key-up that would commit a keyboard edit goes to whatever took the focus. - if (m_selection_edit.kind != SelectionEdit::None) - finish_selection_edit(); - ImGui::SetWindowFocus(nullptr); - render(); - evt.Skip(); - }); + m_canvas->Bind(wxEVT_KILL_FOCUS, &GLCanvas3D::on_kill_focus, this); m_event_handlers_bound = true; m_canvas->Bind(wxEVT_GESTURE_PAN, &GLCanvas3D::on_gesture, this); @@ -3317,6 +3311,9 @@ void GLCanvas3D::unbind_event_handlers() m_canvas->Unbind(wxEVT_RIGHT_DCLICK, &GLCanvas3D::on_mouse, this); m_canvas->Unbind(wxEVT_PAINT, &GLCanvas3D::on_paint, this); m_canvas->Unbind(wxEVT_SET_FOCUS, &GLCanvas3D::on_set_focus, this); + m_canvas->Unbind(wxEVT_KILL_FOCUS, &GLCanvas3D::on_kill_focus, this); + m_canvas->Unbind(EVT_GLCANVAS_TOOLBAR_HIGHLIGHTER_TIMER, &GLCanvas3D::on_toolbar_highlighter_timer, this); + m_canvas->Unbind(EVT_GLCANVAS_GIZMO_HIGHLIGHTER_TIMER, &GLCanvas3D::on_gizmo_highlighter_timer, this); m_event_handlers_bound = false; m_canvas->Unbind(wxEVT_GESTURE_PAN, &GLCanvas3D::on_gesture, this); @@ -4893,6 +4890,26 @@ void GLCanvas3D::on_set_focus(wxFocusEvent& evt) m_is_touchpad_navigation = wxGetApp().app_config->get_bool("camera_navigation_style"); } +void GLCanvas3D::on_kill_focus(wxFocusEvent& evt) +{ + // The key-up that would commit a keyboard edit goes to whatever took the focus. + if (m_selection_edit.kind != SelectionEdit::None) + finish_selection_edit(); + ImGui::SetWindowFocus(nullptr); + render(); + evt.Skip(); +} + +void GLCanvas3D::on_toolbar_highlighter_timer(wxTimerEvent& evt) +{ + m_toolbar_highlighter.blink(); +} + +void GLCanvas3D::on_gizmo_highlighter_timer(wxTimerEvent& evt) +{ + m_gizmo_highlighter.blink(); +} + bool GLCanvas3D::clicked_button_matches_action(const wxMouseEvent& evt, const MouseAction action, const std::map& mappings) const { MouseButton clicked = MouseButton::None; diff --git a/src/slic3r/GUI/GLCanvas3D.hpp b/src/slic3r/GUI/GLCanvas3D.hpp index 36a6c7b944..ecb411e0c4 100644 --- a/src/slic3r/GUI/GLCanvas3D.hpp +++ b/src/slic3r/GUI/GLCanvas3D.hpp @@ -1133,6 +1133,9 @@ public: void on_gesture(wxGestureEvent& evt); void on_paint(wxPaintEvent& evt); void on_set_focus(wxFocusEvent& evt); + void on_kill_focus(wxFocusEvent& evt); + void on_toolbar_highlighter_timer(wxTimerEvent& evt); + void on_gizmo_highlighter_timer(wxTimerEvent& evt); void force_set_focus(); enum class MouseButton { None, Left, Middle, Right }; diff --git a/src/slic3r/GUI/GUI_Factories.cpp b/src/slic3r/GUI/GUI_Factories.cpp index 8bc899bb3d..966ef02327 100644 --- a/src/slic3r/GUI/GUI_Factories.cpp +++ b/src/slic3r/GUI/GUI_Factories.cpp @@ -2057,9 +2057,15 @@ wxMenu* MenuFactory::instance_menu() return &m_instance_menu; } +MenuWithSeparators* MenuFactory::new_transient_menu() +{ + m_transient_menu = std::make_unique(); + return m_transient_menu.get(); +} + wxMenu* MenuFactory::layer_menu() { - MenuWithSeparators* menu = new MenuWithSeparators(); + MenuWithSeparators* menu = new_transient_menu(); append_menu_item_settings(menu); return menu; @@ -2085,13 +2091,13 @@ wxMenu* MenuFactory::multi_selection_menu() } if (all_plates) { - wxMenu* menu = new MenuWithSeparators(); + wxMenu* menu = new_transient_menu(); append_menu_item_replace_all_with_stl(menu); return menu; } if (undefined_type) return nullptr; - wxMenu* menu = new MenuWithSeparators(); + wxMenu* menu = new_transient_menu(); if (!multi_volume) { int index = 0; if (obj_list()->can_merge_to_multipart_object()) { @@ -2165,7 +2171,7 @@ wxMenu* MenuFactory::assemble_multi_selection_menu() // show this menu only for Objects(Instances mixed with Objects)/Volumes selection return nullptr; - wxMenu* menu = new MenuWithSeparators(); + wxMenu* menu = new_transient_menu(); append_menu_item_set_visible(menu); //append_menu_item_fix_through_cgal(menu); //append_menu_item_simplify(menu); @@ -2211,7 +2217,7 @@ wxMenu* MenuFactory::plate_menu() wxMenu* MenuFactory::assemble_object_menu() { - wxMenu* menu = new MenuWithSeparators(); + wxMenu* menu = new_transient_menu(); // Set Visible append_menu_item_set_visible(menu); // Delete @@ -2231,7 +2237,7 @@ wxMenu* MenuFactory::assemble_object_menu() wxMenu* MenuFactory::assemble_part_menu() { - wxMenu* menu = new MenuWithSeparators(); + wxMenu* menu = new_transient_menu(); append_menu_item_set_visible(menu); append_menu_item_delete(menu); diff --git a/src/slic3r/GUI/GUI_Factories.hpp b/src/slic3r/GUI/GUI_Factories.hpp index ad694f15de..aed912eef5 100644 --- a/src/slic3r/GUI/GUI_Factories.hpp +++ b/src/slic3r/GUI/GUI_Factories.hpp @@ -2,6 +2,7 @@ #define slic3r_GUI_Factories_hpp_ #include +#include #include #include #include @@ -120,6 +121,12 @@ private: MenuWithSeparators m_assemble_part_menu; wxMenu m_filament_action_menu; + + // The selection dependent menus are rebuilt for every popup, so they cannot be members that + // outlive a build like the ones above; this owns the current one and destroys the previous. + // One slot is enough because PopupMenu() is synchronous: the menu a caller was handed is gone + // from the screen before anything can ask for the next one. + std::unique_ptr m_transient_menu; // Removed/Prepended Items according to the view mode @@ -127,6 +134,9 @@ private: std::array items_decrease; std::array items_set_number_of_copies; + // Replaces m_transient_menu with an empty menu and returns it. + MenuWithSeparators* new_transient_menu(); + void create_default_menu(); void create_common_object_menu(wxMenu *menu); void create_object_menu(); diff --git a/src/slic3r/GUI/Gizmos/GLGizmoMeasure.cpp b/src/slic3r/GUI/Gizmos/GLGizmoMeasure.cpp index 8ec564e5b4..701f858906 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoMeasure.cpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoMeasure.cpp @@ -2272,6 +2272,10 @@ void GLGizmoMeasure::update_measurement_result() void GLGizmoMeasure::reset_all_pick() { std::map>().swap(m_mesh_raycaster_map); + // register_single_mesh_pick() fills both maps in lockstep, so the measurings have to go with + // the raycasters; otherwise the entries keyed on the GLVolumes of the previous selection stay + // behind for the rest of the session. + std::map>().swap(m_mesh_measure_map); reset_gripper_pick(GripperType::UNDEFINE,true); } diff --git a/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp b/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp index 19856603fa..9b40b45f99 100644 --- a/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp +++ b/src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp @@ -27,8 +27,7 @@ enum class PainterGizmoType { FDM_SUPPORTS, SEAM, MM_SEGMENTATION, - FUZZY_SKIN, - TEXTURE_DISPLACEMENT + FUZZY_SKIN }; class TriangleSelectorGUI : public TriangleSelector { @@ -102,7 +101,9 @@ class TriangleSelectorPatch : public TriangleSelectorGUI { public: explicit TriangleSelectorPatch(const TriangleMesh& mesh, const std::vector ebt_colors, float edge_limit = 0.6f) : TriangleSelectorGUI(mesh, edge_limit), m_ebt_colors(ebt_colors) {} - virtual ~TriangleSelectorPatch() = default; + // Releases the VAO and the per-patch VBOs built by finalize_triangle_indices(). The base class + // already deletes GL buffers from its GLModel members here, so this needs no context of its own. + virtual ~TriangleSelectorPatch() { release_geometry(); } // Render current selection. Transformation matrices are supposed // to be already set. diff --git a/src/slic3r/GUI/ImGuiWrapper.cpp b/src/slic3r/GUI/ImGuiWrapper.cpp index fe5ddc9fa0..8bf0beeacb 100644 --- a/src/slic3r/GUI/ImGuiWrapper.cpp +++ b/src/slic3r/GUI/ImGuiWrapper.cpp @@ -361,6 +361,7 @@ ImGuiWrapper::~ImGuiWrapper() { //destroy_fonts_texture(); destroy_font(); + destroy_svg_textures(); ImGui::DestroyContext(); } @@ -542,6 +543,12 @@ bool ImGuiWrapper::update_key_data(wxKeyEvent &evt) return ret; } +// SVG icons rasterized into GL textures, keyed on file name, size and recolor. Cleared as a whole +// from new_frame() once it grows past MAX_SVG_TEXTURES, which is safe there: the previous frame has +// been rendered and the frame about to be recorded asks for every icon it draws again. +static std::map s_svg_textures; +static const size_t MAX_SVG_TEXTURES = 256; + void ImGuiWrapper::new_frame() { if (m_new_frame_open) { @@ -552,6 +559,11 @@ void ImGuiWrapper::new_frame() init_font(true); } + // Recolored icons accumulate one texture per color the session has shown; drop them before + // anything references them again. This frame recreates the handful it actually draws. + if (s_svg_textures.size() > MAX_SVG_TEXTURES) + destroy_svg_textures(); + ImGuiIO& io = ImGui::GetIO(); ImGui::NewFrame(); @@ -1804,8 +1816,7 @@ bool menu_item_with_icon(const char *label, const char *shortcut, ImVec2 icon_si if (icon_color != 0) ImGui::RenderFrame(icon_pos, icon_pos + icon_size, icon_color); else { - static ImTextureID transparent; - IMTexture::load_from_svg_file(Slic3r::resources_dir() + "/images/transparent.svg", icon_size.x, icon_size.y, transparent); + ImTextureID transparent = ImGuiWrapper::svg_texture(Slic3r::resources_dir() + "/images/transparent.svg", icon_size.x, icon_size.y); window->DrawList->AddImage(transparent, icon_pos, icon_pos + icon_size, { 0,0 }, { 1,1 }, ImGui::GetColorU32(ImVec4(1.f, 1.f, 1.f, 1.f))); } } @@ -2631,11 +2642,7 @@ void ImGuiWrapper::push_toolbar_style(const float scale) ImGui::PushStyleColor(ImGuiCol_FrameBgActive, ImVec4(238 / 255.0f, 238 / 255.0f, 238 / 255.0f, 1.00f)); // 10 ImGui::PushStyleColor(ImGuiCol_FrameBg, ImVec4(238 / 255.0f, 238 / 255.0f, 238 / 255.0f, 0.00f)); // 11 ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, COL_GREEN_LIGHT); // 12 - // The checkbox/radio frame behind this is drawn fully transparent (see FrameBg above, - // alpha 0), showing the light window background through it - a white check mark there is - // invisible. Dark mode doesn't have this problem (its window background is dark), so only - // this branch needs a check mark color with real contrast against a light background. - ImGui::PushStyleColor(ImGuiCol_CheckMark, ImVec4(0.f, 156 / 255.f, 136 / 255.f, 1.00f));//13 + ImGui::PushStyleColor(ImGuiCol_CheckMark, ImVec4(1.00f, 1.00f, 1.00f, 1.00f));//13 ImGui::PushStyleColor(ImGuiCol_ScrollbarGrab, ImVec4(0.42f, 0.42f, 0.42f, 1.00f)); ImGui::PushStyleColor(ImGuiCol_ScrollbarGrabHovered, ImVec4(0.93f, 0.93f, 0.93f, 1.00f)); ImGui::PushStyleColor(ImGuiCol_ScrollbarGrabActive, ImVec4(0.93f, 0.93f, 0.93f, 1.00f)); @@ -3389,6 +3396,36 @@ bool ImGuiWrapper::display_initialized() const return io.DisplaySize.x >= 0.0f && io.DisplaySize.y >= 0.0f; } +ImTextureID ImGuiWrapper::svg_texture(const std::string& filename, unsigned width, unsigned height, const char* hex_color) +{ + std::string key = filename + "|" + std::to_string(width) + "x" + std::to_string(height); + if (hex_color != nullptr) + key += std::string("|") + hex_color; + + const auto it = s_svg_textures.find(key); + if (it != s_svg_textures.end()) + return it->second; + + ImTextureID texture_id = nullptr; + const bool loaded = (hex_color != nullptr) ? + BitmapCache::load_from_svg_file_change_color(filename, width, height, texture_id, hex_color) : + IMTexture::load_from_svg_file(filename, width, height, texture_id); + if (!loaded) + return nullptr; + + s_svg_textures.emplace(std::move(key), texture_id); + return texture_id; +} + +void ImGuiWrapper::destroy_svg_textures() +{ + for (const auto& texture : s_svg_textures) { + GLuint texture_id = (GLuint)(intptr_t)texture.second; + glsafe(::glDeleteTextures(1, &texture_id)); + } + s_svg_textures.clear(); +} + void ImGuiWrapper::destroy_font() { if (m_font_texture != 0) { @@ -3468,7 +3505,6 @@ void ImGuiWrapper::filament_group(const std::string& filament_type, const char* //ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(0, 0)); std::string id = std::to_string(static_cast (filament_id + 1)); ImDrawList* draw_list = ImGui::GetWindowDrawList(); - static ImTextureID transparent; ImVec2 text_size = ImGui::CalcTextSize(filament_type.c_str()); // BBS image sizing based on text width (DPI scaling) float img_width = ImGui::CalcTextSize("ABC").x; @@ -3481,7 +3517,7 @@ void ImGuiWrapper::filament_group(const std::string& filament_type, const char* if (rgba[3] == 0x00) { svg_path = "/images/outlined_rect_transparent.svg"; } - BitmapCache::load_from_svg_file_change_color(Slic3r::resources_dir() + svg_path, img_size.x, img_size.y, transparent, hex_color); + ImTextureID transparent = svg_texture(Slic3r::resources_dir() + svg_path, img_size.x, img_size.y, hex_color); ImGui::BeginGroup(); { ImVec2 cursor_pos = ImGui::GetCursorScreenPos(); diff --git a/src/slic3r/GUI/ImGuiWrapper.hpp b/src/slic3r/GUI/ImGuiWrapper.hpp index ad83d50d2d..3e2056ddd2 100644 --- a/src/slic3r/GUI/ImGuiWrapper.hpp +++ b/src/slic3r/GUI/ImGuiWrapper.hpp @@ -104,6 +104,14 @@ public: // Hash of every draw list's vertices, indices and commands. static ImGuiID draw_data_signature(const ImDrawData* draw_data); + // A GL texture holding an SVG icon rasterized at width x height, optionally recolored. + // Rasterizing an SVG is far too expensive to redo for every frame that draws the icon, and the + // texture the previous frame generated would leak, so the result is kept until the frame that + // finds the cache overgrown drops it (and rebuilds only what it still draws). + static ImTextureID svg_texture(const std::string& filename, unsigned width, unsigned height, const char* hex_color = nullptr); + // Deletes every texture svg_texture() handed out. Requires a current GL context. + static void destroy_svg_textures(); + float scaled(float x) const { return x * m_font_size; } ImVec2 scaled(float x, float y) const { return ImVec2(x * m_font_size, y * m_font_size); } /// diff --git a/src/slic3r/GUI/NotificationManager.cpp b/src/slic3r/GUI/NotificationManager.cpp index e6172135a6..26b1d548e2 100644 --- a/src/slic3r/GUI/NotificationManager.cpp +++ b/src/slic3r/GUI/NotificationManager.cpp @@ -2225,11 +2225,12 @@ void NotificationManager::close_and_delete_self(PopNotification * self) } void NotificationManager::remove_notification_of_type(const NotificationType type) { + // Seven notification types may have several instances alive at once, so erase every match: + // stopping at the first one leaves the rest (and the ObjectIDs they hold) behind. for (auto it = m_pop_notifications.begin(); it != m_pop_notifications.end();) { std::unique_ptr ¬ification = *it; if (notification->get_type() == type) { it = m_pop_notifications.erase(it); - break; } else ++it; }