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 8418856. * Remove unused refcounts from document_set, making it a set not a map.
facsimiles · Nov 11, 2024 · 845b1ed · 845b1ed
1 parent 724773e
commit 845b1ed
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 144 deletions.
diff --git 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<Glib::ustring> s = Glib::VariantBase::cast_dynamic<Glib::Variant<Glib::ustring> >(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
@@ -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
@@ -129,6 +129,7 @@ SPDocument *InkscapeApplication::document_add(std::unique_ptr<SPDocument> 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<InkscapeWindow>(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<InkscapeWindow*> 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
@@ -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
@@ -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
@@ -374,10 +374,7 @@ Application::crash_handler (int /*signum*/)
     gchar *curdir = g_get_current_dir(); // This one needs to be freed explicitly
     std::vector<gchar *> savednames;
     std::vector<gchar *> failednames;
-    for (std::map<SPDocument*,int>::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<SPDocument *,int>::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
@@ -14,7 +14,7 @@
  * Released under GNU GPL v2+, read the file 'COPYING' for more information.
  */
 
-#include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -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<void (SPDocument *)> signal_destroy_document;
 
-    // inkscape is quitting
-    sigc::signal<void ()> 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<void ()> 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<SPDocument *, int> _document_set;
+    std::set<SPDocument *> _document_set;
     std::vector<SPDesktop *> *_desktops = nullptr;
     std::string _pages;