Mercurial > octave

changeset 26930:ead8f91c5556

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Improve performance when closing figures (bug #55908). * graphics.cc (gh_manager::do_free): Don't execute remove_child when deleting from groot. * graphics.[cc,in.h] (delete_children, free): Add optional argument to indicate that an object is deleted from graphics root.
author Markus Mützel <markus.muetzel@gmx.de>
date Sat, 16 Mar 2019 22:21:26 +0100
parents c8055304ad21
children ef922c0631e7
files libinterp/corefcn/graphics.cc libinterp/corefcn/graphics.in.h
diffstat 2 files changed, 65 insertions(+), 55 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/graphics.cc	Sat Mar 16 09:56:00 2019 -0700
+++ b/libinterp/corefcn/graphics.cc	Sat Mar 16 22:21:26 2019 +0100
@@ -1798,10 +1798,16 @@
 }
 
 void
-children_property::do_delete_children (bool clear)
-{
-  while (! children_list.empty ())
-    gh_manager::free (children_list.front ());
+children_property::do_delete_children (bool clear, bool from_root)
+{
+  for (graphics_handle hchild : children_list)
+    {
+      graphics_object go = gh_manager::get_object (hchild);
+
+      if (hchild.value () > 0 && go.valid_object ()
+          && ! go.get_properties ().is_beingdeleted ())
+        gh_manager::free (hchild, from_root);
+    }
 
   if (clear)
     children_list.clear ();
@@ -2746,7 +2752,7 @@
 }
 
 void
-gh_manager::do_free (const graphics_handle& h)
+gh_manager::do_free (const graphics_handle& h, bool from_root)
 {
   if (h.ok ())
     {
@@ -2764,7 +2770,9 @@
         return;
 
       graphics_handle parent_h = p->second.get_parent ();
-      graphics_object parent_go = gh_manager::get_object (parent_h);
+      graphics_object parent_go = nullptr;
+      if (! from_root)
+        parent_go = gh_manager::get_object (parent_h);
 
       bp.set_beingdeleted (true);
 
@@ -2781,9 +2789,11 @@
       // Notify graphics toolkit.
       p->second.finalize ();
 
-      // NOTE: Call remove_child before erasing the go from the map.
+
+      // NOTE: Call remove_child before erasing the go from the map if not
+      // removing from groot.
       // A callback function might have already deleted the parent
-      if (parent_go.valid_object () && h.ok ())
+      if (! from_root && parent_go.valid_object () && h.ok ())
         parent_go.remove_child (h);
 
       // Note: this will be valid only for first explicitly deleted
@@ -2910,7 +2920,7 @@
 }
 
 static void
-delete_graphics_object (const graphics_handle& h)
+delete_graphics_object (const graphics_handle& h, bool from_root = false)
 {
   if (h.ok ())
     {
@@ -2922,7 +2932,7 @@
           // NOTE: Freeing the handle also calls any deletefcn.  It also calls
           //       the parent's delete_child function.
 
-          gh_manager::free (h);
+          gh_manager::free (h, from_root);
 
           Vdrawnow_requested = true;
         }
@@ -2930,16 +2940,16 @@
 }
 
 static void
-delete_graphics_object (double val)
-{
-  delete_graphics_object (gh_manager::lookup (val));
+delete_graphics_object (double val, bool from_root = false)
+{
+  delete_graphics_object (gh_manager::lookup (val), from_root);
 }
 
 // Flag to stop redraws due to callbacks while deletion is in progress.
 static bool delete_executing = false;
 
 static void
-delete_graphics_objects (const NDArray vals)
+delete_graphics_objects (const NDArray vals, bool from_root = false)
 {
   // Prevent redraw of partially deleted objects.
   octave::unwind_protect frame;
@@ -2947,7 +2957,7 @@
   delete_executing = true;
 
   for (octave_idx_type i = 0; i < vals.numel (); i++)
-    delete_graphics_object (vals.elem (i));
+    delete_graphics_object (vals.elem (i), from_root);
 }
 
 static void
@@ -2967,7 +2977,7 @@
   xset (h, "deletefcn", Matrix ());
   xset (h, "closerequestfcn", Matrix ());
 
-  delete_graphics_object (h);
+  delete_graphics_object (h, true);
 }
 
 void
@@ -3926,7 +3936,7 @@
 */
 
 void
-root_figure::properties::remove_child (const graphics_handle& h)
+root_figure::properties::remove_child (const graphics_handle& h, bool)
 {
   gh_manager::pop_figure (h);
 
@@ -3934,7 +3944,7 @@
 
   xset (0, "currentfigure", cf.value ());
 
-  base_properties::remove_child (h);
+  base_properties::remove_child (h, true);
 }
 
 void
@@ -3962,9 +3972,9 @@
 }
 
 void
-figure::properties::remove_child (const graphics_handle& h)
-{
-  base_properties::remove_child (h);
+figure::properties::remove_child (const graphics_handle& h, bool from_root)
+{
+  base_properties::remove_child (h, from_root);
 
   if (h == currentaxes.handle_value ())
     {
@@ -5483,7 +5493,7 @@
 }
 
 void
-axes::properties::delete_text_child (handle_property& hp)
+axes::properties::delete_text_child (handle_property& hp, bool from_root)
 {
   graphics_handle h = hp.handle_value ();
 
@@ -5492,7 +5502,7 @@
       graphics_object go = gh_manager::get_object (h);
 
       if (go.valid_object ())
-        gh_manager::free (h);
+        gh_manager::free (h, from_root);
     }
 
   // FIXME: is it necessary to check whether the axes object is
@@ -5512,36 +5522,36 @@
 }
 
 void
-axes::properties::remove_child (const graphics_handle& h)
+axes::properties::remove_child (const graphics_handle& h, bool from_root)
 {
   graphics_object go = gh_manager::get_object (h);
 
-  if (go.isa ("light") && go.get_properties ().is_visible ())
-    decrease_num_lights ();
-
   if (xlabel.handle_value ().ok () && h == xlabel.handle_value ())
     {
-      delete_text_child (xlabel);
+      delete_text_child (xlabel, from_root);
       update_xlabel_position ();
     }
   else if (ylabel.handle_value ().ok () && h == ylabel.handle_value ())
     {
-      delete_text_child (ylabel);
+      delete_text_child (ylabel, from_root);
       update_ylabel_position ();
     }
   else if (zlabel.handle_value ().ok () && h == zlabel.handle_value ())
     {
-      delete_text_child (zlabel);
+      delete_text_child (zlabel, from_root);
       update_zlabel_position ();
     }
   else if (title.handle_value ().ok () && h == title.handle_value ())
     {
-      delete_text_child (title);
+      delete_text_child (title, from_root);
       update_title_position ();
     }
+  else if (get_num_lights () > 0 && go.isa ("light")
+           && go.get_properties ().is_visible ())
+    decrease_num_lights ();
 
   if (go.valid_object ())
-      base_properties::remove_child (h);
+    base_properties::remove_child (h, from_root);
 
 }
 
@@ -10198,17 +10208,17 @@
 // ---------------------------------------------------------------------
 
 void
-hggroup::properties::remove_child (const graphics_handle& h)
+hggroup::properties::remove_child (const graphics_handle& h, bool from_root)
 {
   graphics_object go = gh_manager::get_object (h);
-  if (go.isa ("light") && go.get_properties ().is_visible ())
+  if (! from_root && go.isa ("light") && go.get_properties ().is_visible ())
     {
       axes::properties& ax_props =
         dynamic_cast<axes::properties&>
         (go.get_ancestor ("axes").get_properties ());
       ax_props.decrease_num_lights ();
     }
-  base_properties::remove_child (h);
+  base_properties::remove_child (h, from_root);
   update_limits ();
 }
 
--- a/libinterp/corefcn/graphics.in.h	Sat Mar 16 09:56:00 2019 -0700
+++ b/libinterp/corefcn/graphics.in.h	Sat Mar 16 22:21:26 2019 +0100
@@ -1759,9 +1759,9 @@
     return octave_value (get_children ());
   }
 
-  void delete_children (bool clear = false)
-  {
-    do_delete_children (clear);
+  void delete_children (bool clear = false, bool from_root = false)
+  {
+    do_delete_children (clear, from_root);
   }
 
   void renumber (graphics_handle old_gh, graphics_handle new_gh)
@@ -1885,7 +1885,7 @@
     children_list.push_front (val);
   }
 
-  void do_delete_children (bool clear);
+  void do_delete_children (bool clear, bool from_root);
 };
 
 // ---------------------------------------------------------------------
@@ -2234,7 +2234,7 @@
 
   bool is_modified (void) const { return is___modified__ (); }
 
-  virtual void remove_child (const graphics_handle& h)
+  virtual void remove_child (const graphics_handle& h, bool = false)
   {
     if (children.remove_child (h.value ()))
       {
@@ -2306,9 +2306,9 @@
 
   virtual void update_uicontextmenu (void) const;
 
-  virtual void delete_children (bool clear = false)
-  {
-    children.delete_children (clear);
+  virtual void delete_children (bool clear = false, bool from_root = false)
+  {
+    children.delete_children (clear, from_root);
   }
 
   void renumber_child (graphics_handle old_gh, graphics_handle new_gh)
@@ -2541,12 +2541,12 @@
     return get_properties ().get___myhandle__ ();
   }
 
-  virtual void remove_child (const graphics_handle& h)
+  virtual void remove_child (const graphics_handle& h, bool from_root = false)
   {
     if (! valid_object ())
       error ("base_graphics_object::remove_child: invalid graphics object");
 
-    get_properties ().remove_child (h);
+    get_properties ().remove_child (h, from_root);
   }
 
   virtual void adopt (const graphics_handle& h)
@@ -2895,7 +2895,7 @@
   class OCTINTERP_API properties : public base_properties
   {
   public:
-    void remove_child (const graphics_handle& h);
+    void remove_child (const graphics_handle& h, bool from_root = false);
 
     Matrix get_boundingbox (bool internal = false,
                             const Matrix& parent_pix_size = Matrix ()) const;
@@ -3076,7 +3076,7 @@
       integerhandle = val;
     }
 
-    void remove_child (const graphics_handle& h);
+    void remove_child (const graphics_handle& h, bool from_root = false);
 
     void set_visible (const octave_value& val);
 
@@ -3401,7 +3401,7 @@
   public:
     void set_defaults (base_graphics_object& obj, const std::string& mode);
 
-    void remove_child (const graphics_handle& h);
+    void remove_child (const graphics_handle& h, bool from_root = false);
 
     void adopt (const graphics_handle& h);
 
@@ -3599,7 +3599,7 @@
     void set_text_child (handle_property& h, const std::string& who,
                          const octave_value& v);
 
-    void delete_text_child (handle_property& h);
+    void delete_text_child (handle_property& h, bool from_root = false);
 
     // See the genprops.awk script for an explanation of the
     // properties declarations.
@@ -5264,7 +5264,7 @@
   class OCTINTERP_API properties : public base_properties
   {
   public:
-    void remove_child (const graphics_handle& h);
+    void remove_child (const graphics_handle& h, bool from_root = false);
 
     void adopt (const graphics_handle& h);
 
@@ -5338,9 +5338,9 @@
   class OCTINTERP_API properties : public base_properties
   {
   public:
-    void remove_child (const graphics_handle& h)
+    void remove_child (const graphics_handle& h, bool from_root = false)
     {
-      base_properties::remove_child (h);
+      base_properties::remove_child (h, from_root);
     }
 
     void adopt (const graphics_handle& h)
@@ -6224,10 +6224,10 @@
            : graphics_handle ();
   }
 
-  static void free (const graphics_handle& h)
+  static void free (const graphics_handle& h, bool from_root = false)
   {
     if (instance_ok ())
-      instance->do_free (h);
+      instance->do_free (h, from_root);
   }
 
   static void renumber_figure (const graphics_handle& old_gh,
@@ -6479,7 +6479,7 @@
 
   graphics_handle do_get_handle (bool integer_figure_handle);
 
-  void do_free (const graphics_handle& h);
+  void do_free (const graphics_handle& h, bool from_root);
 
   void do_renumber_figure (const graphics_handle& old_gh,
                            const graphics_handle& new_gh);