# HG changeset patch
# User Pantxo Diribarne <pantxo.diribarne@gmail.com>
# Date 1534922084 -7200
#      Wed Aug 22 09:14:44 2018 +0200
# Node ID 235558cfebe37660feea648a2268671c7455c017
# Parent  849f1a7e9a30809a0f08d7e08598a943f55d5fbf
Refactor graphics event handling and fix interruption of callbacks (bug #54340)

* graphics.in.h (base_graphics_event::m_busyaction,
  base_graphics_event::busyaction): New data member and accessor method. Add an
  enum for allowed values of m_busyaction - INTERRUPT, QUEUE, CANCEL.
  (graphics_event::create_callback_event): Add new busyaction argument that
  defaults to base_graphics_event::QUEUE.
  (graphics_event::busyaction): Accessor for base_graphics_event::busyaction.

* graphics.cc (gh_manager::do_post_callback): Post all events tagged according
  to the "busyaction" of the callback object and the name of the callback.
  (gh_manager::do_process_events): Triage callback execution here only.
  (Fdrawnow): First call process_events and then redraw. Allow process_events
  to be called recursivelly.
  (figure::properties::set_position): Use gh_manager::post_callback to schedule
  the execution of the resizefcn.
  (F__go_post_callback__): new internal interpreter function for testing purpose
  Add BIST that test busyaction,interruptible and resizefcn properties

diff -r 849f1a7e9a30 -r 235558cfebe3 libinterp/corefcn/graphics.cc
--- a/libinterp/corefcn/graphics.cc	Sat Aug 18 13:50:45 2018 -0400
+++ b/libinterp/corefcn/graphics.cc	Wed Aug 22 09:14:44 2018 +0200
@@ -4055,7 +4055,7 @@
     {
       if (old_bb(2) != new_bb(2) || old_bb(3) != new_bb(3))
         {
-          execute_resizefcn ();
+          gh_manager::post_callback (__myhandle__, "resizefcn");
           update_boundingbox ();
         }
     }
@@ -10521,13 +10521,15 @@
 {
 public:
   callback_event (const graphics_handle& h, const std::string& name,
-                  const octave_value& data = Matrix ())
-    : base_graphics_event (), handle (h), callback_name (name),
+                  const octave_value& data = Matrix (),
+                  int busyaction = base_graphics_event::QUEUE)
+    : base_graphics_event (busyaction), handle (h), callback_name (name),
       callback (), callback_data (data) { }
 
   callback_event (const graphics_handle& h, const octave_value& cb,
-                  const octave_value& data = Matrix ())
-    : base_graphics_event (), handle (h), callback_name (),
+                  const octave_value& data = Matrix (),
+                  int busyaction = base_graphics_event::QUEUE)
+    : base_graphics_event (busyaction), handle (h), callback_name (),
       callback (cb), callback_data (data) { }
 
   void execute (void)
@@ -10620,17 +10622,19 @@
 graphics_event
 graphics_event::create_callback_event (const graphics_handle& h,
                                        const std::string& name,
-                                       const octave_value& data)
-{
-  return graphics_event (new callback_event (h, name, data));
+                                       const octave_value& data,
+                                       int busyaction)
+{
+  return graphics_event (new callback_event (h, name, data, busyaction));
 }
 
 graphics_event
 graphics_event::create_callback_event (const graphics_handle& h,
                                        const octave_value& cb,
-                                       const octave_value& data)
-{
-  return graphics_event (new callback_event (h, cb, data));
+                                       const octave_value& data,
+                                       int busyaction)
+{
+  return graphics_event (new callback_event (h, cb, data, busyaction));
 }
 
 graphics_event
@@ -10806,36 +10810,21 @@
 
   if (go.valid_object ())
     {
-      if (callback_objects.empty ())
-        do_post_event (graphics_event::create_callback_event (h, name, data));
-      else
-        {
-          const graphics_object& current = callback_objects.front ();
-
-          if (current.get_properties ().is_interruptible ())
-            do_post_event (graphics_event::create_callback_event (h, name,
-                                                                  data));
-          else
-            {
-              std::string busy_action (go.get_properties ().get_busyaction ());
-
-              if (busy_action == "queue")
-                do_post_event (graphics_event::create_callback_event (h, name,
-                                                                      data));
-              else
-                {
-                  caseless_str cname (name);
-
-                  if (cname.compare ("deletefcn")
-                      || cname.compare ("createfcn")
-                      || (go.isa ("figure")
-                          && (cname.compare ("closerequestfcn")
-                              || cname.compare ("resizefcn"))))
-                    do_post_event (
-                      graphics_event::create_callback_event (h, name, data));
-                }
-            }
-        }
+      caseless_str cname (name);
+      int busyaction = base_graphics_event::QUEUE;
+
+      if (cname.compare ("deletefcn")
+          || cname.compare ("createfcn")
+          || (go.isa ("figure")
+              && (cname.compare ("closerequestfcn")
+                  || cname.compare ("resizefcn"))))
+        busyaction = base_graphics_event::INTERRUPT;
+      else if (go.get_properties ().get_busyaction () == "cancel")
+        busyaction = base_graphics_event::CANCEL;
+
+
+      do_post_event (graphics_event::create_callback_event (h, name, data,
+                                                            busyaction));
     }
 }
 
@@ -10889,6 +10878,26 @@
 
                     event_queue.pop_front ();
                   }
+                else
+                  {
+                    std::list<graphics_event>::iterator p =
+                      event_queue.begin ();
+
+                    while (p != event_queue.end ())
+                      if (p->busyaction () == base_graphics_event::CANCEL)
+                        {
+                          p = event_queue.erase (p);
+                        }
+                      else if (p->busyaction ()
+                               == base_graphics_event::INTERRUPT)
+                        {
+                          e = (*p);
+                          event_queue.erase (p);
+                          break;
+                        }
+                      else
+                        p++;
+                  }
               }
           }
       }
@@ -10921,6 +10930,62 @@
   return 0;
 }
 
+
+/*
+## Test interruptible/busyaction properties
+%!function cb (h)
+%! setappdata (gcbf (), "cb_exec", [getappdata(gcbf (), "cb_exec") h]);
+%! drawnow ();
+%! setappdata (gcbf (), "cb_exec", [getappdata(gcbf (), "cb_exec") h]);
+%!endfunction
+%!
+%!test
+%! hf = figure ("visible", "off", "resizefcn", @cb);
+%! unwind_protect
+%!   cb2 = @(h) setappdata (gcbf (), "cb_exec",
+%!                          [getappdata(gcbf (), "cb_exec") h]);
+%!   ## Default
+%!   hui1 = uicontrol ("parent", hf, "interruptible", "on", "callback", @cb);
+%!   hui2 = uicontrol ("parent", hf, "busyaction", "queue", "callback", @cb);
+%!   hui3 = uicontrol ("parent", hf, "busyaction", "queue", "callback", @cb);
+%!   __go_post_callback__ (hui1, "callback");
+%!   __go_post_callback__ (hui2, "callback");
+%!   __go_post_callback__ (hui3, "callback");
+%!
+%!   assert (getappdata (hf, "cb_exec"), []);
+%!   drawnow ();
+%!   assert (getappdata (hf, "cb_exec"), [hui1 hui2 hui3 hui3 hui2 hui1]);
+%!
+%!   ## Interruptible off
+%!   setappdata (hf, "cb_exec", []);
+%!   set (hui1, "interruptible", "off");
+%!   __go_post_callback__ (hui1, "callback");
+%!   __go_post_callback__ (hui2, "callback");
+%!   __go_post_callback__ (hui3, "callback");
+%!   drawnow ();
+%!   assert (getappdata (hf, "cb_exec"), [hui1 hui1 hui2 hui3 hui3 hui2]);
+%!
+%!   ## "resizefcn" callback interrupts regardless of interruptible property
+%!   setappdata (hf, "cb_exec", []);
+%!   __go_post_callback__ (hui1, "callback");
+%!   __go_post_callback__ (hf, "resizefcn");
+%!   drawnow ();
+%!   assert (getappdata (hf, "cb_exec"), [hui1 hf hf hui1]);
+%!
+%!   ## test "busyaction" "cancel"
+%!   setappdata (hf, "cb_exec", []);
+%!   set (hui2, "busyaction", "cancel");
+%!   __go_post_callback__ (hui1, "callback");
+%!   __go_post_callback__ (hui2, "callback");
+%!   __go_post_callback__ (hui3, "callback");
+%!   __go_post_callback__ (hf, "resizefcn");
+%!   drawnow ();
+%!   assert (getappdata (hf, "cb_exec"), [hui1 hf hui3 hui3 hf hui1]);
+%! unwind_protect_cleanup
+%!   close (hf)
+%! end_unwind_protect
+*/
+
 void
 gh_manager::do_enable_event_processing (bool enable)
 {
@@ -12000,6 +12065,40 @@
   return ovl ();
 }
 
+DEFUN (__go_post_callback__, args, ,
+       doc: /* -*- texinfo -*-
+@deftypefn  {} {} __go_post_callback__ (@var{h}, @var{name})
+@deftypefnx {} {} __go_post_callback__ (@var{h}, @var{name}, @var{param})
+Undocumented internal function.
+@end deftypefn */)
+{
+  int nargin = args.length ();
+
+  if (nargin < 2 || nargin > 3)
+    print_usage ();
+
+  const NDArray vals = args(0).xarray_value ("__go_post_callback__: invalid graphics object");
+
+  std::string name = args(1).xstring_value ("__go_post_callback__: invalid callback name");
+
+  for (octave_idx_type i = 0; i < vals.numel (); i++)
+    {
+      double val = vals(i);
+
+      graphics_handle h = gh_manager::lookup (val);
+
+      if (! h.ok ())
+        error ("__go_execute_callback__: invalid graphics object (= %g)", val);
+
+      if (nargin == 2)
+        gh_manager::post_callback (h, name);
+      else
+        gh_manager::post_callback (h, name, args(2));
+    }
+
+  return ovl ();
+}
+
 DEFUN (__image_pixel_size__, args, ,
        doc: /* -*- texinfo -*-
 @deftypefn {} {@var{sz} =} __image_pixel_size__ (@var{h})
@@ -12116,25 +12215,46 @@
 @seealso{refresh}
 @end deftypefn */)
 {
-  static int drawnow_executing = 0;
-
   if (args.length () > 3)
     print_usage ();
 
   octave::unwind_protect frame;
 
   frame.protect_var (Vdrawnow_requested, false);
-  frame.protect_var (drawnow_executing);
-
-  // Redraw, unless we are in the middle of an existing redraw or deletion.
-  if (++drawnow_executing <= 1 && ! delete_executing)
+
+  // Redraw unless we are in the middle of a deletion.
+  if (! delete_executing)
     {
       gh_manager::auto_lock guard;
 
-      if (args.length () == 0 || args.length () == 1)
-        {
+      if (args.length () <= 1)
+        {
+          // First process events so that the redraw happens when all
+          // objects are in their definite state.
+          bool do_events = true;
+
+          if (args.length () == 1)
+            {
+              caseless_str val (args(0).xstring_value ("drawnow: first argument must be a string"));
+
+              if (val.compare ("expose"))
+                do_events = false;
+              else
+                error ("drawnow: invalid argument, 'expose' is only valid option");
+            }
+
+          if (do_events)
+            {
+              gh_manager::unlock ();
+
+              gh_manager::process_events ();
+
+              gh_manager::lock ();
+            }
+
           Matrix hlist = gh_manager::figure_handle_list (true);
 
+          // Redraw modified figures
           for (int i = 0; i < hlist.numel (); i++)
             {
               graphics_handle h = gh_manager::lookup (hlist(i));
@@ -12159,27 +12279,7 @@
                       fprops.set_modified (false);
                     }
                 }
-            }
-
-          bool do_events = true;
-
-          if (args.length () == 1)
-            {
-              caseless_str val (args(0).xstring_value ("drawnow: first argument must be a string"));
-
-              if (val.compare ("expose"))
-                do_events = false;
-              else
-                error ("drawnow: invalid argument, 'expose' is only valid option");
-            }
-
-          if (do_events)
-            {
-              gh_manager::unlock ();
-
-              gh_manager::process_events ();
-
-              gh_manager::lock ();
+
             }
         }
       else if (args.length () >= 2 && args.length () <= 3)
diff -r 849f1a7e9a30 -r 235558cfebe3 libinterp/corefcn/graphics.in.h
--- a/libinterp/corefcn/graphics.in.h	Sat Aug 18 13:50:45 2018 -0400
+++ b/libinterp/corefcn/graphics.in.h	Wed Aug 22 09:14:44 2018 +0200
@@ -5999,14 +5999,24 @@
 base_graphics_event
 {
 public:
+  enum priority { INTERRUPT, QUEUE, CANCEL };
 
   friend class graphics_event;
 
-  base_graphics_event (void) = default;
+  base_graphics_event (void): m_busyaction (QUEUE)
+  { };
+
+  base_graphics_event (int busyaction): m_busyaction (busyaction)
+  { };
 
   virtual ~base_graphics_event (void) = default;
 
+  int busyaction (void) { return m_busyaction; };
+
   virtual void execute (void) = 0;
+
+ private:
+  int m_busyaction;
 };
 
 class
@@ -6026,6 +6036,14 @@
 
   graphics_event& operator = (const graphics_event&) = default;
 
+  int busyaction (void)
+  {
+    if (ok ())
+      return rep->busyaction ();
+    else
+      error ("graphics_event::busyaction: invalid graphics_event");
+  }
+
   void execute (void)
   {
     if (ok ())
@@ -6037,12 +6055,14 @@
   static graphics_event
   create_callback_event (const graphics_handle& h,
                          const std::string& name,
-                         const octave_value& data = Matrix ());
+                         const octave_value& data = Matrix (),
+                         int busyaction = base_graphics_event::QUEUE);
 
   static graphics_event
   create_callback_event (const graphics_handle& h,
                          const octave_value& cb,
-                         const octave_value& data = Matrix ());
+                         const octave_value& data = Matrix (),
+                         int busyaction = base_graphics_event::QUEUE);
 
   static graphics_event
   create_function_event (event_fcn fcn, void *data = nullptr);
