Re: [changeset] - improve clf() compatibility

[John W. Eaton]

[With the changeset this time...]

On 21-Oct-2008, David Bateman wrote:

| John W. Eaton wrote:
| > Before we apply this change, I think we need to fix the title, xlabel,
| > ylabel, and zlabel properties to be hidden children of the axes
| > object.  My best guess at how to do that is below, but it does not
| > quite work.  I don't have any more time to work on this problem, but I
| > thought I would post the diffs anyway to give anyone else who wants to
| > work on this a starting point.
| >
| > jwe
| >   
| There are also other issues related to hidden children such as the 
| "BaseLine" of stem series, etc that cause issues with the "legend" 
| function as they aren't currently invisible.. So yes, I'd also like to 
| see this issue addressed. Could you give more information about what 
| exactly doesn't work with your patch?

This version seems to work better.

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



* Try

    demo quiver3
    demo ribbon

  Both of these seem to work OK if run separately in fresh Octave
  sessions, but quiver3 seems to be leaving some state that causes
  ribbon to do the wrong thing.  Maybe this is a hold problem?  I
  haven't looked at it yet.



* The refreshdata demo does not seem to be working correctly for me.
  I'm seeing

    octave:1> demo refreshdata
    refreshdata example 1:
     x = 0:0.1:10;
     y = sin (x);
     plot (x, y, "ydatasource", "y");
     for i = 1 : 100
       pause(0.1)
       y = sin (x + 0.1 * i);
       refreshdata();
     endfor

    refreshdata example 1: failed
    `y' undefined near line 0 column 1octave:2> 

jwe


# HG changeset patch
# User John W. Eaton <address@hidden>
# Date 1224612385 14400
# Node ID 1f429086565cd3e38e0db01a3be97126cfed4c63
# Parent  08bb643966858d025204676799c2f8d7d75f1f84
[mq]: hidden-children

diff --git a/scripts/ChangeLog b/scripts/ChangeLog
--- a/scripts/ChangeLog
+++ b/scripts/ChangeLog
@@ -1,3 +1,12 @@
+2008-10-21  John W. Eaton  <address@hidden>
+
+       * plot/gnuplot_drawnow.m: Pass handle to __go_draw_figure__, not
+       struct.  Use get instead of examining struct fields directly.
+       * plot/__go_draw_figure__.m: First arg is now handle, not figure
+       object struct.  Use get instead of examining struct elements
+       directly.  Use allhild instead of looking at children field of
+       figure object struct.
+
 2008-10-20  Ben Abbott <address@hidden>
 
        * plot/orient.m: Fix syntax error.
diff --git a/scripts/plot/__go_draw_figure__.m 
b/scripts/plot/__go_draw_figure__.m
--- a/scripts/plot/__go_draw_figure__.m
+++ b/scripts/plot/__go_draw_figure__.m
@@ -20,14 +20,16 @@
 
 ## Author: jwe
 
-function __go_draw_figure__ (f, plot_stream, enhanced, mono)
+function __go_draw_figure__ (h, plot_stream, enhanced, mono)
 
   if (nargin == 4)
-    if (strcmp (f.type, "figure"))
+    htype = get (h, "type");
+    if (strcmp (htype, "figure"))
 
       ## Set figure properties here?
 
-      kids = f.children;
+      ## Get complete list of children.
+      kids = allchild (h);
       nkids = length (kids);
 
       if (nkids > 0)
@@ -64,7 +66,7 @@
       endif
     else
       error ("__go_draw_figure__: expecting figure object, found `%s'",
-            f.type);
+            htype);
     endif
   else
     print_usage ();
diff --git a/scripts/plot/gnuplot_drawnow.m b/scripts/plot/gnuplot_drawnow.m
--- a/scripts/plot/gnuplot_drawnow.m
+++ b/scripts/plot/gnuplot_drawnow.m
@@ -34,16 +34,15 @@
   endif
 
   if (nargin >= 3 && nargin <= 5)
-    f = __get__ (h);
     plot_stream = [];
     fid = [];
     unwind_protect
       [plot_stream, enhanced] = open_gnuplot_stream (1, [], term, file);
-      __go_draw_figure__ (f, plot_stream, enhanced, mono);
+      __go_draw_figure__ (h, plot_stream, enhanced, mono);
       if (nargin == 5)
         fid = fopen (debug_file, "wb");
         enhanced = init_plot_stream (fid, [], term, file);
-        __go_draw_figure__ (f, fid, enhanced, mono);
+        __go_draw_figure__ (h, fid, enhanced, mono);
       endif
     unwind_protect_cleanup
       if (! isempty (plot_stream))
@@ -54,15 +53,14 @@
       endif
     end_unwind_protect
   elseif (nargin == 1)
-    f = __get__ (h);
-    plot_stream = f.__plot_stream__;
+    plot_stream = get (h, "__plot_stream__");
     if (isempty (plot_stream))
       [plot_stream, enhanced] = open_gnuplot_stream (2, h);
       set (h, "__enhanced__", enhanced);
     else
-      enhanced = f.__enhanced__;
+      enhanced = get (h, "__enhanced__");
     endif
-    __go_draw_figure__ (f, plot_stream (1), enhanced, mono);
+    __go_draw_figure__ (h, plot_stream (1), enhanced, mono);
     fflush (plot_stream (1));
   else
     print_usage ();
diff --git a/src/ChangeLog b/src/ChangeLog
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,4 +1,27 @@
-2008-10-20  John W. Eaton  <address@hidden>
+2008-10-21  John W. Eaton  <address@hidden>
+
+       * graphics.h.in (gh_manager::is_handle_visible): New function.
+       (axes::properites): Move title property after label properties.
+       (graphics_object::is_handle_visible): New function.
+       (base_properties::is_handle_visible): New function.
+       (base_properties): Mark children property with G.
+       * graphics.cc (axes::properties::set_text_child): New function.
+       (axes::properties::set_title, axes::properties::set_xlabel,
+       axes::properties::set_ylabel, axes::properties::set_zlabel): Use it.
+       (base_property::get_children): New function.  Only
+       return handles that are visible.
+       (axes::properties::delete_text_child): Set handlevisibility to
+       "off" for newly created object and add it to the list of children.
+       Call base_properties::remove_child on previous handle value.
+       (axes::properties::set_defaults): Don't explicitly delete xlabel,
+       ylabel, zlabel, and title handles.  Reinitialize them after
+       deleting children.
+       (axes::properties::remove_child): Don't explicitly delete xlabel,
+       ylabel, zlabel, and title handles.
+       (axes::properties::init): Move here from graphics.h.in.  Set
+       handlevisibility to "off" for xlabel, ylabel, zlabel, and title
+       and add them to the list of children.
+       (axes::properties::delete_children): Delete.
 
        * genprops.awk: Allow whitespace between BEGIN_PROPERTIES and
        opening paren.  Accept optional second argument for graphics
diff --git a/src/graphics.cc b/src/graphics.cc
--- a/src/graphics.cc
+++ b/src/graphics.cc
@@ -2133,6 +2133,54 @@
 
 // ---------------------------------------------------------------------
 
+void
+axes::properties::init (void)
+{
+  position.add_constraint (dim_vector (1, 4));
+  position.add_constraint (dim_vector (0, 0));
+  outerposition.add_constraint (dim_vector (1, 4));
+  colororder.add_constraint (dim_vector (-1, 3));
+  dataaspectratio.add_constraint (dim_vector (1, 3));
+  plotboxaspectratio.add_constraint (dim_vector (1, 3));
+  xlim.add_constraint (2);
+  ylim.add_constraint (2);
+  zlim.add_constraint (2);
+  clim.add_constraint (2);
+  alim.add_constraint (2);
+  xtick.add_constraint (dim_vector (1, -1));
+  ytick.add_constraint (dim_vector (1, -1));
+  ztick.add_constraint (dim_vector (1, -1));
+  Matrix vw (1, 2, 0);
+  vw(1) = 90;
+  view = vw;
+  view.add_constraint (dim_vector (1, 2));
+  cameraposition.add_constraint (dim_vector (1, 3));
+  Matrix upv (1, 3, 0.0);
+  upv(2) = 1.0;
+  cameraupvector = upv;
+  cameraupvector.add_constraint (dim_vector (1, 3));
+  currentpoint.add_constraint (dim_vector (2, 3));
+  ticklength.add_constraint (dim_vector (1, 2));
+  tightinset.add_constraint (dim_vector (1, 4));
+
+  x_zlim.resize (1, 2);
+  sx = "linear";
+  sy = "linear";
+  sz = "linear";
+
+  xset (xlabel.handle_value (), "handlevisibility", "off");
+  xset (ylabel.handle_value (), "handlevisibility", "off");
+  xset (zlabel.handle_value (), "handlevisibility", "off");
+
+  xset (title.handle_value (), "handlevisibility", "off");
+
+  adopt (xlabel.handle_value ());
+  adopt (ylabel.handle_value ());
+  adopt (zlabel.handle_value ());
+
+  adopt (title.handle_value ());
+}
+
 void 
 axes::properties::sync_positions (void)
 {
@@ -2168,59 +2216,54 @@
 }
 
 void
+axes::properties::set_text_child (handle_property& hp,
+                                 const std::string& who,
+                                 const octave_value& v)
+{
+  graphics_handle val = ::reparent (v, "set", who, __myhandle__, false);
+
+  if (! error_state)
+    {
+      xset (val, "handlevisibility", "off");
+
+      gh_manager::free (hp.handle_value ());
+
+      base_properties::remove_child (hp.handle_value ());
+
+      hp = val;
+
+      adopt (hp.handle_value ());
+    }
+}
+
+void
+axes::properties::set_xlabel (const octave_value& v)
+{
+  set_text_child (xlabel, "xlabel", v);
+}
+
+void
+axes::properties::set_ylabel (const octave_value& v)
+{
+  set_text_child (ylabel, "ylabel", v);
+}
+
+void
+axes::properties::set_zlabel (const octave_value& v)
+{
+  set_text_child (zlabel, "zlabel", v);
+}
+
+void
 axes::properties::set_title (const octave_value& v)
 {
-  graphics_handle val = ::reparent (v, "set", "title", __myhandle__, false);
-
-  if (! error_state)
-    {
-      gh_manager::free (title.handle_value ());
-      title = val;
-    }
-}
-
-void
-axes::properties::set_xlabel (const octave_value& v)
-{
-  graphics_handle val = ::reparent (v, "set", "xlabel", __myhandle__, false);
-
-  if (! error_state)
-    {
-      gh_manager::free (xlabel.handle_value ());
-      xlabel = val;
-    }
-}
-
-void
-axes::properties::set_ylabel (const octave_value& v)
-{
-  graphics_handle val = ::reparent (v, "set", "ylabel", __myhandle__, false);
-
-  if (! error_state)
-    {
-      gh_manager::free (ylabel.handle_value ());
-      ylabel = val;
-    }
-}
-
-void
-axes::properties::set_zlabel (const octave_value& v)
-{
-  graphics_handle val = ::reparent (v, "set", "zlabel", __myhandle__, false);
-
-  if (! error_state)
-    {
-      gh_manager::free (zlabel.handle_value ());
-      zlabel = val;
-    }
+  set_text_child (title, "title", v);
 }
 
 void
 axes::properties::set_defaults (base_graphics_object& obj,
                                const std::string& mode)
 {
-  delete_text_child (title);
-
   box = "on";
   key = "off";
   keybox = "off";
@@ -2244,10 +2287,6 @@
   ylimmode = "auto";
   zlimmode = "auto";
   climmode = "auto";
-
-  delete_text_child (xlabel);
-  delete_text_child (ylabel);
-  delete_text_child (zlabel);
 
   xgrid = "off";
   ygrid = "off";
@@ -2328,10 +2367,27 @@
       activepositionproperty = "outerposition";
     }
 
-
   delete_children ();
 
   children = Matrix ();
+
+  xlabel = gh_manager::make_graphics_handle ("text", __myhandle__, false);
+  ylabel = gh_manager::make_graphics_handle ("text", __myhandle__, false);
+  zlabel = gh_manager::make_graphics_handle ("text", __myhandle__, false);
+
+  title = gh_manager::make_graphics_handle ("text", __myhandle__, false);
+
+  xset (xlabel.handle_value (), "handlevisibility", "off");
+  xset (ylabel.handle_value (), "handlevisibility", "off");
+  xset (zlabel.handle_value (), "handlevisibility", "off");
+
+  xset (title.handle_value (), "handlevisibility", "off");
+
+  adopt (xlabel.handle_value ());
+  adopt (ylabel.handle_value ());
+  adopt (zlabel.handle_value ());
+
+  adopt (title.handle_value ());
 
   update_transform ();
 
@@ -2349,37 +2405,66 @@
 
       if (go.valid_object ())
        gh_manager::free (h);
-    }
+
+      base_properties::remove_child (h);
+    }
+
+  // FIXME -- is it necessary to check whether the axes object is
+  // being deleted now?  I think this function is only called when an
+  // individual child object is delete and not when the parent axes
+  // object is deleted.
 
   if (! is_beingdeleted ())
-    hp = gh_manager::make_graphics_handle ("text", __myhandle__);
+    {
+      hp = gh_manager::make_graphics_handle ("text", __myhandle__, false);
+
+      xset (hp.handle_value (), "handlevisibility", "off");
+
+      adopt (hp.handle_value ());
+    }
 }
 
 void
 axes::properties::remove_child (const graphics_handle& h)
 {
-  if (title.handle_value ().ok () && h == title.handle_value ())
-    delete_text_child (title);
-  else if (xlabel.handle_value ().ok () && h == xlabel.handle_value ())
+  if (xlabel.handle_value ().ok () && h == xlabel.handle_value ())
     delete_text_child (xlabel);
   else if (ylabel.handle_value ().ok () && h == ylabel.handle_value ())
     delete_text_child (ylabel);
   else if (zlabel.handle_value ().ok () && h == zlabel.handle_value ())
     delete_text_child (zlabel);
+  else if (title.handle_value ().ok () && h == title.handle_value ())
+    delete_text_child (title);
   else
     base_properties::remove_child (h);
 }
 
-void
-axes::properties::delete_children (void)
-{
-  base_properties::delete_children ();
-
-  delete_text_child (title);
-
-  delete_text_child (xlabel);
-  delete_text_child (ylabel);
-  delete_text_child (zlabel);
+Matrix
+base_properties::get_children (void) const
+{
+  Matrix retval = children;
+  
+  graphics_object go = gh_manager::get_object (0);
+
+  root_figure::properties& props =
+      dynamic_cast<root_figure::properties&> (go.get_properties ());
+
+  if (! props.is_showhiddenhandles ())
+    {
+      octave_idx_type k = 0;
+
+      for (octave_idx_type i = 0; i < children.numel (); i++)
+       {
+         graphics_handle kid = children (i);
+
+         if (gh_manager::is_handle_visible (kid))
+           retval(k++) = children(i);
+       }
+
+      retval.resize (k, 1);
+    }
+
+  return retval;;
 }
 
 inline Matrix
diff --git a/src/graphics.h.in b/src/graphics.h.in
--- a/src/graphics.h.in
+++ b/src/graphics.h.in
@@ -1669,6 +1669,11 @@
   virtual bool is_zliminclude (void) const { return false; }
   virtual bool is_climinclude (void) const { return false; }
   virtual bool is_aliminclude (void) const { return false; }
+
+  bool is_handle_visible (void) const
+  {
+    return ! handlevisibility.is ("off");
+  }
  
 protected:
   void set_dynamic (const caseless_str&, const octave_value&);
@@ -1685,7 +1690,7 @@
     radio_property busyaction , "{queue}|cancel"
     callback_property buttondownfcn , Matrix ()
     // FIXME: use a property class for children
-    Matrix children fs , Matrix ()
+    Matrix children Gfs , Matrix ()
     bool_property clipping , "on"
     callback_property createfcn , Matrix ()
     callback_property deletefcn , Matrix ()
@@ -2070,6 +2075,9 @@
   
   bool is_aliminclude (void) const
   { return get_properties ().is_aliminclude (); }
+
+  bool is_handle_visible (void) const
+  { return get_properties ().is_handle_visible (); }
   
   graphics_backend get_backend (void) const { return rep->get_backend (); }
 
@@ -2534,6 +2542,9 @@
     Matrix x_zlim;
     std::list<octave_value> zoom_stack;
 
+    void set_text_child (handle_property& h, const std::string& who,
+                        const octave_value& v);
+
     void delete_text_child (handle_property& h);
 
     // See the genprops.awk script for an explanation of the
@@ -2543,7 +2554,6 @@
 
     BEGIN_PROPERTIES (axes)
       array_property position u , default_axes_position ()
-      handle_property title SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__)
       bool_property box , "on"
       bool_property key , "off"
       bool_property keybox , "off"
@@ -2562,9 +2572,10 @@
       radio_property zlimmode al , "{auto}|manual"
       radio_property climmode al , "{auto}|manual"
       radio_property alimmode    , "{auto}|manual"
-      handle_property xlabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__)
-      handle_property ylabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__)
-      handle_property zlabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__)
+      handle_property xlabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__, false)
+      handle_property ylabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__, false)
+      handle_property zlabel SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__, false)
+      handle_property title SOf , gh_manager::make_graphics_handle ("text", 
__myhandle__, false)
       bool_property xgrid , "off"
       bool_property ygrid , "off"
       bool_property zgrid , "off"
@@ -2643,40 +2654,7 @@
    END_PROPERTIES
 
   protected:
-    void init (void)
-      {
-        position.add_constraint (dim_vector (1, 4));
-       position.add_constraint (dim_vector (0, 0));
-        outerposition.add_constraint (dim_vector (1, 4));
-        colororder.add_constraint (dim_vector (-1, 3));
-        dataaspectratio.add_constraint (dim_vector (1, 3));
-        plotboxaspectratio.add_constraint (dim_vector (1, 3));
-       xlim.add_constraint (2);
-       ylim.add_constraint (2);
-       zlim.add_constraint (2);
-       clim.add_constraint (2);
-       alim.add_constraint (2);
-        xtick.add_constraint (dim_vector (1, -1));
-        ytick.add_constraint (dim_vector (1, -1));
-        ztick.add_constraint (dim_vector (1, -1));
-        Matrix vw (1, 2, 0);
-        vw(1) = 90;
-        view = vw;
-        view.add_constraint (dim_vector (1, 2));
-       cameraposition.add_constraint (dim_vector (1, 3));
-       Matrix upv (1, 3, 0.0);
-       upv(2) = 1.0;
-       cameraupvector = upv;
-       cameraupvector.add_constraint (dim_vector (1, 3));
-       currentpoint.add_constraint (dim_vector (2, 3));
-       ticklength.add_constraint (dim_vector (1, 2));
-       tightinset.add_constraint (dim_vector (1, 4));
-
-       x_zlim.resize (1, 2);
-       sx = "linear";
-       sy = "linear";
-       sz = "linear";
-      }
+    void init (void);
 
   private:
     void update_xscale (void) { sx = get_xscale (); }
@@ -3653,6 +3631,18 @@
     return (instance_ok () ?  instance->do_process_events (true) : 0);
   }
 
+  static bool is_handle_visible (const graphics_handle& h)
+  {
+    bool retval = false;
+
+    graphics_object go = get_object (h);
+
+    if (go.valid_object ())
+      retval = go.is_handle_visible ();
+
+    return retval;
+  }
+
 public:
   class autolock
   {