From 845b1ed2d760b53d0b0bb79c7f6011f3f9ad42d8 Mon Sep 17 00:00:00 2001 From: PBS Date: Sat, 9 Nov 2024 15:20:25 +0000 Subject: [PATCH] Clean up junk in Inkscape::Application * Only call INKSCAPE.{add|remove}_document() once when needed. * Remove unused and undefined functions: - application_init() - load_config() - active_event_context() * Remove unused functions/members: - sole_desktop_for_document() - signal_shut_down - InkscapeApplication::document_window_count() - InkscapeApplication::windows_update() * Restore comment for exit() after context lost in 84188567. * Remove unused refcounts from document_set, making it a set not a map. --- src/actions/actions-file.cpp | 6 +-- src/desktop.cpp | 6 --- src/inkscape-application.cpp | 48 +----------------------- src/inkscape-application.h | 5 --- src/inkscape-window.cpp | 1 - src/inkscape.cpp | 73 ++++-------------------------------- src/inkscape.h | 22 +++-------- 7 files changed, 17 insertions(+), 144 deletions(-) diff --git a/src/actions/actions-file.cpp b/src/actions/actions-file.cpp index 6b78dd1162..6dc623c01d 100644 --- a/src/actions/actions-file.cpp +++ b/src/actions/actions-file.cpp @@ -34,8 +34,7 @@ file_open(const Glib::VariantBase& value, InkscapeApplication *app) show_output(Glib::ustring("file_open: file '") + s.get().raw() + "' does not exist."); return; } - SPDocument *document = app->document_open(file).first; - INKSCAPE.add_document(document); + auto document = app->document_open(file).first; app->set_active_document(document); app->set_active_selection(document->getSelection()); @@ -68,8 +67,7 @@ file_new(const Glib::VariantBase& value, InkscapeApplication *app) { Glib::Variant s = Glib::VariantBase::cast_dynamic >(value); - SPDocument *document = app->document_new(s.get()); - INKSCAPE.add_document(document); + auto document = app->document_new(s.get()); app->set_active_document(document); app->set_active_selection(document->getSelection()); diff --git a/src/desktop.cpp b/src/desktop.cpp index 40ebdc6bbe..e23bfcb4e6 100644 --- a/src/desktop.cpp +++ b/src/desktop.cpp @@ -237,10 +237,6 @@ SPDesktop::~SPDesktop() } _guides_message_context = nullptr; - - if (document) { - INKSCAPE.remove_document(document); - } } //-------------------------------------------------------------------- @@ -1278,9 +1274,7 @@ SPDesktop::setDocument (SPDocument *doc) // Formerly in View::View VVVVVVVVVVVVVVVVVVV if (document) { _document_uri_set_connection.disconnect(); - INKSCAPE.remove_document(document); } - INKSCAPE.add_document(doc); document = doc; _document_uri_set_connection = document->connectFilenameSet([this](auto const filename) { diff --git a/src/inkscape-application.cpp b/src/inkscape-application.cpp index fc7d3a41e7..f89f73ee7c 100644 --- a/src/inkscape-application.cpp +++ b/src/inkscape-application.cpp @@ -129,6 +129,7 @@ SPDocument *InkscapeApplication::document_add(std::unique_ptr docume assert(document); auto [it, inserted] = _documents.try_emplace(std::move(document)); assert(inserted); + INKSCAPE.add_document(it->first.get()); return it->first.get(); } @@ -261,10 +262,6 @@ bool InkscapeApplication::document_swap(InkscapeWindow *window, SPDocument *docu doc_it->second.push_back(std::move(win_uniq)); - // To be removed (add/delete once per window)! - INKSCAPE.add_document(document); - INKSCAPE.remove_document(old_document); - _active_document = document; _active_selection = desktop->getSelection(); _active_desktop = desktop; @@ -346,23 +343,10 @@ void InkscapeApplication::document_close(SPDocument *document) std::cerr << "InkscapeApplication::close_document: Window vector not empty!" << std::endl; } + INKSCAPE.remove_document(it->first.get()); _documents.erase(it); } -/** - * Return number of windows with document. - */ -unsigned InkscapeApplication::document_window_count(SPDocument *document) -{ - auto it = _documents.find(document); - if (it == _documents.end()) { - std::cerr << "InkscapeApplication::document_window_count: Document not in map!" << std::endl; - return 0; - } - - return it->second.size(); -} - /** Fix up a document if necessary (Only fixes that require GUI). MOVE TO ANOTHER FILE! */ void @@ -424,9 +408,6 @@ InkscapeWindow *InkscapeApplication::window_open(SPDocument *document) auto win_uniq = std::make_unique(document); // TODO Add window to application. (Instead of in InkscapeWindow constructor.) - // To be removed (add once per window)! - INKSCAPE.add_document(document); - _active_window = win_uniq.get(); _active_desktop = win_uniq->get_desktop(); _active_selection = win_uniq->get_desktop()->getSelection(); @@ -459,9 +440,6 @@ void InkscapeApplication::window_close(InkscapeWindow *window) auto document = window->get_document(); assert(document); - // To be removed (remove once per window)! - INKSCAPE.remove_document(document); - // Leave active document alone (maybe should find new active window and reset variables). _active_selection = nullptr; _active_desktop = nullptr; @@ -500,25 +478,6 @@ void InkscapeApplication::window_close_active() window_close(_active_window); } -/** Update windows in response to: - * - New active window - * - Document change - * - Selection change - */ -void InkscapeApplication::windows_update(SPDocument *document) -{ - // Find windows: - // auto it = _documents.find( document ); - // if (it != _documents.end()) { - // std::vector windows = it->second; - // std::cout << "InkscapeApplication::update_windows: windows size: " << windows.size() << std::endl; - // Loop over InkscapeWindows. - // Loop over DialogWindows. TBD - // } else { - // std::cout << "InkscapeApplication::update_windows: no windows found" << std::endl; - // } -} - /** Debug function */ void InkscapeApplication::dump() @@ -954,9 +913,6 @@ InkscapeApplication::destroy_all() void InkscapeApplication::process_document(SPDocument* document, std::string output_path) { - // Add to Inkscape::Application... - INKSCAPE.add_document(document); - // Are we doing one file at a time? In that case, we don't recreate new windows for each file. bool replace = _use_pipe || _batch_process; diff --git a/src/inkscape-application.h b/src/inkscape-application.h index 7b9b318ec5..d11335e8dc 100644 --- a/src/inkscape-application.h +++ b/src/inkscape-application.h @@ -102,7 +102,6 @@ class InkscapeApplication bool document_swap(InkscapeWindow* window, SPDocument* document); bool document_revert(SPDocument* document); void document_close(SPDocument* document); - unsigned document_window_count(SPDocument* document); /* These require a GUI! */ void document_fix(InkscapeWindow* window); // MOVE TO ANOTHER FILE @@ -115,10 +114,6 @@ class InkscapeApplication void window_close_active(); void startup_close(); - // Update all windows connected to a document. - void windows_update(SPDocument* document); - - /****** Actions *******/ InkActionExtraData& get_action_extra_data() { return _action_extra_data; } InkActionEffectData& get_action_effect_data() { return _action_effect_data; } diff --git a/src/inkscape-window.cpp b/src/inkscape-window.cpp index 0dbc7900a4..87565683ec 100644 --- a/src/inkscape-window.cpp +++ b/src/inkscape-window.cpp @@ -301,7 +301,6 @@ InkscapeWindow::on_is_active_changed() _app->set_active_document(_document); _app->set_active_desktop(_desktop); _app->set_active_selection(_desktop->getSelection()); - _app->windows_update(_document); update_dialogs(); retransientize_dialogs(*this); } diff --git a/src/inkscape.cpp b/src/inkscape.cpp index 6b174f9ac9..d55d303e0a 100644 --- a/src/inkscape.cpp +++ b/src/inkscape.cpp @@ -374,10 +374,7 @@ Application::crash_handler (int /*signum*/) gchar *curdir = g_get_current_dir(); // This one needs to be freed explicitly std::vector savednames; std::vector failednames; - for (std::map::iterator iter = INKSCAPE._document_set.begin(), e = INKSCAPE._document_set.end(); - iter != e; - ++iter) { - SPDocument *doc = iter->first; + for (auto doc : INKSCAPE._document_set) { Inkscape::XML::Node *repr; repr = doc->getReprRoot(); if (doc->isModifiedSinceSave()) { @@ -778,51 +775,14 @@ Application::external_change() signal_external_change.emit(); } -/** - * fixme: These need probably signals too - */ -void -Application::add_document (SPDocument *document) +void Application::add_document(SPDocument *document) { - g_return_if_fail (document != nullptr); - - // try to insert the pair into the list - if (!(_document_set.insert(std::make_pair(document, 1)).second)) { - //insert failed, this key (document) is already in the list - for (auto & iter : _document_set) { - if (iter.first == document) { - // found this document in list, increase its count - iter.second ++; - } - } - } + _document_set.emplace(document); } - -// returns true if this was last reference to this document, so you can delete it -bool -Application::remove_document (SPDocument *document) +void Application::remove_document(SPDocument *document) { - g_return_val_if_fail (document != nullptr, false); - - for (std::map::iterator iter = _document_set.begin(); - iter != _document_set.end(); - ++iter) { - if (iter->first == document) { - // found this document in list, decrease its count - iter->second --; - if (iter->second < 1) { - // this was the last one, remove the pair from list - _document_set.erase (iter); - - return true; - } else { - return false; - } - } - } - - return false; + _document_set.erase(document); } SPDesktop * @@ -843,27 +803,12 @@ Application::active_document() } else if (!_document_set.empty()) { // If called from the command line there will be no desktop // So 'fall back' to take the first listed document in the Inkscape instance - return _document_set.begin()->first; + return *_document_set.begin(); } return nullptr; } -bool -Application::sole_desktop_for_document(SPDesktop const &desktop) { - SPDocument const* document = desktop.doc(); - if (!document) { - return false; - } - for (auto other_desktop : *_desktops) { - SPDocument *other_document = other_desktop->doc(); - if ( other_document == document && other_desktop != &desktop ) { - return false; - } - } - return true; -} - /*##################### # HELPERS #####################*/ @@ -872,12 +817,8 @@ Application::sole_desktop_for_document(SPDesktop const &desktop) { * Handler for Inkscape's Exit verb. This emits the shutdown signal, * saves the preferences if appropriate, and quits. */ -void -Application::exit () +void Application::exit() { - //emit shutdown signal so that dialogs could remember layout - signal_shut_down.emit(); - Inkscape::Preferences::unload(); } diff --git a/src/inkscape.h b/src/inkscape.h index a5f0cf0698..c47eb27034 100644 --- a/src/inkscape.h +++ b/src/inkscape.h @@ -14,7 +14,7 @@ * Released under GNU GPL v2+, read the file 'COPYING' for more information. */ -#include +#include #include #include @@ -78,16 +78,8 @@ class Application { // no setter for this -- only we can control this variable static bool isCrashing() { return _crashIsHappening; } - // useful functions - void application_init (gboolean use_gui); - void load_config (const gchar *filename, Inkscape::XML::Document *config, const gchar *skeleton, - unsigned int skel_size, const gchar *e_notreg, const gchar *e_notxml, - const gchar *e_notsp, const gchar *warn); - - Inkscape::UI::Tools::ToolBase * active_event_context(); SPDocument * active_document(); SPDesktop * active_desktop(); - bool sole_desktop_for_document(SPDesktop const &desktop); Inkscape::UI::ThemeContext *themecontext = nullptr; @@ -109,11 +101,11 @@ class Application { // Moved document add/remove functions into public inkscape.h as they are used // (rightly or wrongly) by console-mode functions - void add_document (SPDocument *document); - bool remove_document (SPDocument *document); + void add_document(SPDocument *document); + void remove_document(SPDocument *document); - // fixme: This also - void exit (); + // Fixme: This has to be rethought + void exit(); static void crash_handler(int signum); @@ -139,8 +131,6 @@ class Application { // these are orphaned signals (nothing emits them and nothing connects to them) sigc::signal signal_destroy_document; - // inkscape is quitting - sigc::signal signal_shut_down; // a document was changed by some external means (undo or XML editor); this // may not be reflected by a selection change and thus needs a separate signal sigc::signal signal_external_change; @@ -173,7 +163,7 @@ class Application { Application(Application const&); // no copy Application& operator=(Application const&); // no assign Application* operator&() const; // no pointer access - std::map _document_set; + std::set _document_set; std::vector *_desktops = nullptr; std::string _pages;