[changeset] - improve clf() compatibility

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: [changeset] - improve clf() compatibility

David Bateman-3
John W. Eaton wrote:

> I still see some strange behavior.
>
>
> * Try
>
>     demo plotyy
>     demo quiver3
>
>   Both of these demos seem to work OK if run separately in fresh Octave
>   sessions, but plotyy seems to be leaving some state that causes
>   quiver3 to fail with
>
>   octave:2> demo quiver3
>   quiver3 example 1:
>    [x,y]=meshgrid (-1:0.1:1);
>    z=sin(2*pi*sqrt(x.^2+y.^2));
>    theta=2*pi*sqrt(x.^2+y.^2)+pi/2;
>    quiver3(x,y,z,sin(theta),cos(theta),ones(size(z)));
>    hold on;
>    mesh(x,y,z);
>    hold off;
>
>   quiver3 example 1: failed
>   get: invalid handle (= -1.22961)Press <enter> to continue:
>   quiver3 example 2:
>    [x, y, z] = peaks (25);
>    surf (x, y, z);
>    hold on;
>    [u, v, w] = surfnorm (x, y, z / 10);
>    h = quiver3 (x, y, z, u, v, w);
>    set (h, "maxheadsize", 0.33);
>
>   quiver3 example 2: failed
>   get: invalid handle (= -1.22961)error: get: invalid handle (= -14.6238)
>   error: called from:
>   error:   /home/jwe/src/octave/scripts/plot/__go_draw_axes__.m at line 60, column 9
>   error:   /home/jwe/src/octave/scripts/plot/__go_draw_figure__.m at line 58, column 8
>   error:   /home/jwe/src/octave/scripts/plot/gnuplot_drawnow.m at line 66, column 5
>
>  
Ok, this one is subtle.. The problem is that the callback function that
is associated with the plotyy axes is not removed by the newplot call in
quiver3 -> __quiver__ -> plot3. Therefore, as one of the axes of the
plotyy object remains and the other is destroyed, the update_position
callback in plotyy.m tries to update a non existent axis with the change
of view.

There are two solutions to this

1) The simple and wrong solution is to just test if the axis to update
in the plotyy callback still exists before trying to update it. A patch
as simple as

diff --git a/scripts/plot/plotyy.m b/scripts/plot/plotyy.m
--- a/scripts/plot/plotyy.m
+++ b/scripts/plot/plotyy.m
@@ -203,7 +203,7 @@
   persistent recursion = false;
 
   ## Don't allow recursion
-  if (! recursion)
+  if (! recursion && ishandle (ax2) && strcmp (get (ax2, "type"), "axes"))
     unwind_protect
       recursion = true;
       position = get (h, "position");

will do that and will prevent the error. However, the callback is
continually called while the axis exists..

2) The correct solution is to have a means of removing callback
functions in the deletefcn of plotyy. So we need a dellistener function
that corresponds to the existing addlistener function.. I'm working on a
patch along these lines.

D.

--
David Bateman                                [hidden email]
Motorola Labs - Paris                        +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 6 72 01 06 33 (Mob)
91193 Gif-Sur-Yvette FRANCE                  +33 1 69 35 77 01 (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

Reply | Threaded
Open this post in threaded view
|

Re: [changeset] - improve clf() compatibility

John W. Eaton-6
On 22-Oct-2008, David Bateman wrote:

| 2) The correct solution is to have a means of removing callback
| functions in the deletefcn of plotyy. So we need a dellistener function
| that corresponds to the existing addlistener function.. I'm working on a
| patch along these lines.

The Matlab docs indicate that calling delete on the handle returned by
addlistener should delete the object remove the listener function.  It
looks like Matlab creates a "handle class object" to represent the
listener function.  I know, we don't have "handle class objects".
Maybe we need to fake something for now, but it seems we should at
least use the same interface if possible instead of introducing a new
dellistener function.

jwe
Reply | Threaded
Open this post in threaded view
|

Re: [changeset] - improve clf() compatibility

David Bateman-3
John W. Eaton wrote:

> On 22-Oct-2008, David Bateman wrote:
>
> | 2) The correct solution is to have a means of removing callback
> | functions in the deletefcn of plotyy. So we need a dellistener function
> | that corresponds to the existing addlistener function.. I'm working on a
> | patch along these lines.
>
> The Matlab docs indicate that calling delete on the handle returned by
> addlistener should delete the object remove the listener function.  It
> looks like Matlab creates a "handle class object" to represent the
> listener function.  I know, we don't have "handle class objects".
> Maybe we need to fake something for now, but it seems we should at
> least use the same interface if possible instead of introducing a new
> dellistener function.
>
> jwe
>
>  
In fact the problem is that "newplot" calls __go_axis__init__ which
reinitializes the axis but keeps the listeners. Deleting the axis does
correctly delete the listener functions. From that point there are two
choices, make __go_axis_init__ (ca, "replace") search all of the axis
properties for listeners are remove them.. Or alternatively, delete by
hand the callbacks elsewhere.. It probably makes sense to do both..

D.


--
David Bateman                                [hidden email]
Motorola Labs - Paris                        +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 6 72 01 06 33 (Mob)
91193 Gif-Sur-Yvette FRANCE                  +33 1 69 35 77 01 (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary

Reply | Threaded
Open this post in threaded view
|

Re: [changeset] - improve clf() compatibility

David Bateman-3
David Bateman wrote:

> John W. Eaton wrote:
>> On 22-Oct-2008, David Bateman wrote:
>>
>> | 2) The correct solution is to have a means of removing callback |
>> functions in the deletefcn of plotyy. So we need a dellistener
>> function | that corresponds to the existing addlistener function..
>> I'm working on a | patch along these lines.
>>
>> The Matlab docs indicate that calling delete on the handle returned by
>> addlistener should delete the object remove the listener function.  It
>> looks like Matlab creates a "handle class object" to represent the
>> listener function.  I know, we don't have "handle class objects".
>> Maybe we need to fake something for now, but it seems we should at
>> least use the same interface if possible instead of introducing a new
>> dellistener function.
>>
>> jwe
>>
>>  
> In fact the problem is that "newplot" calls __go_axis__init__ which
> reinitializes the axis but keeps the listeners. Deleting the axis does
> correctly delete the listener functions. From that point there are two
> choices, make __go_axis_init__ (ca, "replace") search all of the axis
> properties for listeners are remove them.. Or alternatively, delete by
> hand the callbacks elsewhere.. It probably makes sense to do both..
>
Coming back to this issue that was never resolved, I now think the best
thing to do here is a change to graphics.cc
(axes::properties::set_default) such that it removes all listeners from
all properties of the axis. The attached patch does this but keeps the
idea of a dellistener function that complements the addlistener
function. Finally, there is a couple of other corrections that allow all
of the tests of rundemos("plot") to complete successfully.

Regards
David



--
David Bateman                                [hidden email]
Motorola Labs - Paris                        +33 1 69 35 48 04 (Ph)
Parc Les Algorithmes, Commune de St Aubin    +33 6 72 01 06 33 (Mob)
91193 Gif-Sur-Yvette FRANCE                  +33 1 69 35 77 01 (Fax)

The information contained in this communication has been classified as:

[x] General Business Information
[ ] Motorola Internal Use Only
[ ] Motorola Confidential Proprietary


# HG changeset patch
# User David Bateman <[hidden email]>
# Date 1225452620 -3600
# Node ID 1043c783d8187197cacb412c69ffbbd5efab2321
# Parent  6d6d60e970a998dd03d8d4d99f7322e20ad8ed4c
Add code to remove listeners from properties and use it with newplot

diff --git a/scripts/ChangeLog b/scripts/ChangeLog
--- a/scripts/ChangeLog
+++ b/scripts/ChangeLog
@@ -1,3 +1,11 @@
+2008-10-31  David Bateman  <[hidden email]>
+
+ * plot/__contour__.m: Exclude infinite values when calculating contour
+ levels.
+ * plot/clabel.m: Close previous plots in demos to avoid pollution
+ between other plot demos.
+ * plot/plotyy.m: Ditto.
+
 2008-10-30  David Bateman  <[hidden email]>
 
  * plot/legend.m: Add support for the "left" and "right" options.
diff --git a/scripts/plot/__contour__.m b/scripts/plot/__contour__.m
--- a/scripts/plot/__contour__.m
+++ b/scripts/plot/__contour__.m
@@ -78,6 +78,13 @@
     vn = 10;
   endif
 
+  if (isscalar (vn))
+    lvl = linspace (min (z1(!isinf(z1))), max (z1(!isinf(z1))),
+    vn + 2)(1:end-1);
+  else
+    lvl = vn;
+  endif
+
   if (strcmpi (filled, "on"))
     if (isvector (x1) || isvector (y1))
       [x1, y1] = meshgrid (x1, y1);
@@ -91,9 +98,9 @@
     y0 = [y0(:, 1), y0, y0(:, 1)];
     z0 = -Inf(nr+2, nc+2);
     z0(2:nr+1, 2:nc+1) = z1;
-    [c, lev] = contourc (x0, y0, z0, vn);
+    [c, lev] = contourc (x0, y0, z0, lvl);
   else
-    [c, lev] = contourc (x1, y1, z1, vn);
+    [c, lev] = contourc (x1, y1, z1, lvl);
   endif
 
   hg = hggroup ();
@@ -128,11 +135,7 @@
     endif
   endif
 
-  if (isscalar (vn))
-    lvlstep = (max(z1(:)) - min(z1(:))) / vn;
-  else
-    lvlstep = (max(z1(:)) - min(z1(:))) / 10;
-  endif
+  lvlstep = sum (abs (diff (lvl))) / (length (lvl) - 1);
 
   addproperty ("levellist", hg, "data", lev);
   addproperty ("levelstep", hg, "double", lvlstep);
diff --git a/scripts/plot/clabel.m b/scripts/plot/clabel.m
--- a/scripts/plot/clabel.m
+++ b/scripts/plot/clabel.m
@@ -129,9 +129,11 @@
 endfunction
 
 %!demo
+%! close all
 %! [c, h] = contour (peaks(), -4 : 6);
 %! clabel (c, h, -4 : 2 : 6, 'fontsize', 12);
 
 %!demo
+%! close all
 %! [c, h] = contourf (peaks(), -7 : 6);
 %! clabel (c, h, -6 : 2 : 6, 'fontsize', 12);
diff --git a/scripts/plot/plotyy.m b/scripts/plot/plotyy.m
--- a/scripts/plot/plotyy.m
+++ b/scripts/plot/plotyy.m
@@ -182,6 +182,7 @@
 endfunction
 
 %!demo
+%! close all;
 %! x = 0:0.1:2*pi;
 %! y1 = sin (x);
 %! y2 = exp (x - 1);
diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,7 +1,21 @@
+2008-10-31  David Bateman  <[hidden email]>
+
+ * graphics.h.in (base_property::delete_listener): New method.
+ (property::delete_listener): New method.
+ (base_graphics_object::delete_listener): New method.
+ (base_graphics_object::delete_property_listener): New method.
+ (base_graphics_object::remove_all_listeners): New method.
+ (graphics_object::delete_property_listener): New method.
+ (axes::set_defaults): Call remove_all_listeners.
+ * graphics.cc (void base_properties::delete_listener): New method
+ (void base_graphics_object::remove_all_listeners (void)): New method
+ (Fdellistener): New command to remove listener functions associated
+ with a property.
+
 2008-10-30  David Bateman  <[hidden email]>
 
- * graphic.h.in (axes::properties): Add keyreverse property.
- * graphic.cc (axes::properties::set_defaults): Initialize
+ * graphics.h.in (axes::properties): Add keyreverse property.
+ * graphics.cc (axes::properties::set_defaults): Initialize
  keyreverse property.
 
 2008-10-28  Jaroslav Hajek <[hidden email]>
diff --git a/src/graphics.cc b/src/graphics.cc
--- a/src/graphics.cc
+++ b/src/graphics.cc
@@ -1808,6 +1808,16 @@
     p.add_listener (v, mode);
 }
 
+void
+base_properties::delete_listener (const caseless_str& nm,
+  const octave_value& v, listener_mode mode)
+{
+  property p = get_property (nm);
+
+  if (! error_state && p.ok ())
+    p.delete_listener (v, mode);
+}
+
 // ---------------------------------------------------------------------
 
 class gnuplot_backend : public base_graphics_backend
@@ -1938,6 +1948,23 @@
     }
   else
     error ("base_graphics_object::update_axis_limits: invalid graphics object");
+}
+
+void
+base_graphics_object::remove_all_listeners (void)
+{
+  Octave_map m = get (true).map_value ();
+
+  for (Octave_map::const_iterator pa = m.begin (); pa != m.end (); pa++)
+    {
+      if (get_properties().has_property (pa->first))
+ {
+  property p = get_properties ().get_property (pa->first);
+
+  if (! error_state && p.ok ())
+    p.delete_listener ();
+ }
+    }
 }
 
 // ---------------------------------------------------------------------
@@ -4954,6 +4981,72 @@
  }
       else
  error ("addlistener: invalid handle");
+    }
+  else
+    print_usage ();
+
+  return retval;
+}
+
+DEFUN (dellistener, args, ,
+   "-*- texinfo -*-\n\
+@deftypefn {Built-in Function} {} dellistener (@var{h}, @var{prop}, @var{fcn})\n\
+Remove the registration of @var{fcn} as a listener for the property\n\
+@var{prop} of the graphics object @var{h}. The function @var{fcn} must\n\
+be the same variable (not just the same value), as was passed to the\n\
+original call to @code{addlistener}.\n\
+\n\
+If @var{fcn} is not defined then all listener functions of @var{prop}\n\
+are removed.\n\
+\n\
+Example:\n\
+\n\
+@example\n\
+function my_listener (h, dummy, p1)\n\
+  fprintf (\"my_listener called with p1=%s\\n\", p1);\n\
+endfunction\n\
+\n\
+c = @{@@my_listener, \"my string\"@};\n\
+addlistener (gcf, \"position\", c);\n\
+dellistener (gcf, \"position\", c);\n\
+@end example\n\
+\n\
+@end deftypefn")
+{
+  gh_manager::autolock guard;
+
+  octave_value retval;
+
+  if (args.length () == 3 || args.length () == 2)
+    {
+      double h = args(0).double_value ();
+
+      if (! error_state)
+ {
+  std::string pname = args(1).string_value ();
+
+  if (! error_state)
+    {
+      graphics_handle gh = gh_manager::lookup (h);
+
+      if (gh.ok ())
+ {
+  graphics_object go = gh_manager::get_object (gh);
+
+  if (args.length () == 2)
+    go.delete_property_listener (pname, octave_value (), POSTSET);
+  else
+    go.delete_property_listener (pname, args(2), POSTSET);
+ }
+      else
+ error ("dellistener: invalid graphics object (= %g)",
+       h);
+    }
+  else
+    error ("dellistener: invalid property name, expected a string value");
+ }
+      else
+ error ("dellistener: invalid handle");
     }
   else
     print_usage ();
diff --git a/src/graphics.h.in b/src/graphics.h.in
--- a/src/graphics.h.in
+++ b/src/graphics.h.in
@@ -396,6 +396,37 @@
     {
       octave_value_list& l = listeners[mode];
       l.resize (l.length () + 1, v);
+    }
+
+  void delete_listener (const octave_value& v = octave_value (),
+ listener_mode mode = POSTSET)
+    {
+      octave_value_list& l = listeners[mode];
+
+      if (v.is_defined ())
+ {
+  bool found = false;
+  int i;
+
+  for (i = 0; i < l.length (); i++)
+    {
+      if (v.internal_rep () == l(i).internal_rep ())
+ {
+  found = true;
+  break;
+ }
+    }
+  if (found)
+    {
+      for (int j = i; j < l.length() - 1; j++)
+ l(j) = l (j + 1);
+
+      l.resize (l.length () - 1);
+    }
+ }
+      else
+ l.resize (0);
+
     }
 
   OCTINTERP_API void run_listeners (listener_mode mode = POSTSET);
@@ -1287,6 +1318,10 @@
   void add_listener (const octave_value& v, listener_mode mode = POSTSET)
     { rep->add_listener (v, mode); }
 
+  void delete_listener (const octave_value& v = octave_value (),
+ listener_mode mode = POSTSET)
+  { rep->delete_listener (v, mode); }
+
   void run_listeners (listener_mode mode = POSTSET)
     { rep->run_listeners (mode); }
 
@@ -1632,6 +1667,9 @@
   virtual void add_listener (const caseless_str&, const octave_value&,
      listener_mode = POSTSET);
 
+  virtual void delete_listener (const caseless_str&, const octave_value&,
+ listener_mode = POSTSET);
+
   void set_tag (const octave_value& val) { tag = val; }
 
   void set_parent (const octave_value& val);
@@ -1927,6 +1965,16 @@
  get_properties ().add_listener (nm, v, mode);
     }
 
+  virtual void delete_property_listener (const std::string& nm,
+ const octave_value& v,
+ listener_mode mode = POSTSET)
+    {
+      if (valid_object ())
+ get_properties ().delete_listener (nm, v, mode);
+    }
+
+  virtual void remove_all_listeners (void);
+
 protected:
   // A reference count.
   int count;
@@ -2086,6 +2134,10 @@
   void add_property_listener (const std::string& nm, const octave_value& v,
       listener_mode mode = POSTSET)
     { rep->add_property_listener (nm, v, mode); }
+
+  void delete_property_listener (const std::string& nm, const octave_value& v,
+ listener_mode mode = POSTSET)
+    { rep->delete_property_listener (nm, v, mode); }
 
 private:
   base_graphics_object *rep;
@@ -2773,6 +2825,7 @@
 
   void set_defaults (const std::string& mode)
   {
+    remove_all_listeners ();
     xproperties.set_defaults (*this, mode);
   }
 

12