changeset 8063:41bc700ff642

Trigger actions/listeners only for actual property change
author Michael Goffioul
date Tue, 26 Aug 2008 13:39:24 -0400
parents e04a4beeb283
children 4f1ebb704545
files src/ChangeLog src/genprops.awk src/graphics.cc src/graphics.h.in
diffstat 4 files changed, 231 insertions(+), 61 deletions(-) [+]
line wrap: on
line diff
--- a/src/ChangeLog	Tue Aug 26 13:38:14 2008 -0400
+++ b/src/ChangeLog	Tue Aug 26 13:39:24 2008 -0400
@@ -1,5 +1,25 @@
 2008-08-26  Michael Goffioul  <michael.goffioul@gmail.com>
 
+	* genprops.awk (emit_declarations): Call updaters/listeners only when
+	'set' method returned true.
+	* graphics.h.in (base_property::set, base_property::do_set):
+	Return bool value.
+	(property::set): Likewise.
+	(string_property::do_set): Check value change and return bool value.
+	(radio_property::do_set, double_property::do_set,
+	array_property::do_set): Likewise.
+	(color_property::do_set, double_radio_property::do_set,
+	row_vector_property::do_set, bool_property::do_set,
+	handle_property::do_set): Return bool value.
+	(any_property::do_set, callback_property::do_set): Return always true.
+	(color_values::operator==, color_values::operator!=): Add.
+	(array_property::is_equal): Add.
+	* graphics.cc (base_property::set): Executes listeners/notifiers only
+	when do_set returns true. Return bool value.
+	(color_property::do_set, double_radio_property::do_set): Check value
+	change and return bool value.
+	(array_property::is_equal): Add.
+
 	* genprops.awk (emit_declarations, emit_source): Change code emission
 	when emitting base_properties code (base is 1).
 	(BEGIN): Initialize pcount to 0.
--- a/src/genprops.awk	Tue Aug 26 13:38:14 2008 -0400
+++ b/src/genprops.awk	Tue Aug 26 13:39:24 2008 -0400
@@ -332,17 +332,20 @@
 	else
 	  has_builtin_listeners = 0;
 
-        printf ("\n  {\n    if (! error_state)\n      {\n        %s.set (val, %s);\n",
+        printf ("\n  {\n    if (! error_state)\n      {\n        if (%s.set (val, %s))\n          {\n",
           name[i], (has_builtin_listeners ? "false" : "true"));
         if (updater[i])
-          printf ("        update_%s ();\n", name[i]);
+          printf ("            update_%s ();\n", name[i]);
         if (limits[i])
-          printf ("        update_axis_limits (\"%s\");\n", name[i]);
+          printf ("            update_axis_limits (\"%s\");\n", name[i]);
         if (mode[i])
-          printf ("        set_%smode (\"manual\");\n", name[i]);
+          printf ("            set_%smode (\"manual\");\n", name[i]);
 	if (has_builtin_listeners)
-	  printf ("        %s.run_listeners (POSTSET);\n", name[i]);
-        printf ("        mark_modified ();\n");
+	  printf ("            %s.run_listeners (POSTSET);\n", name[i]);
+        printf ("            mark_modified ();\n");
+	printf ("          }\n");
+	if (mode[i])
+	  printf ("        else\n          set_%smode (\"manual\");\n", name[i]);
         printf ("      }\n  }\n\n");
       }
       else
--- a/src/graphics.cc	Tue Aug 26 13:38:14 2008 -0400
+++ b/src/graphics.cc	Tue Aug 26 13:39:24 2008 -0400
@@ -459,26 +459,32 @@
 
 // ---------------------------------------------------------------------
 
-void
+bool
 base_property::set (const octave_value& v, bool do_run )
 {
-  do_set (v);
-
-  // notify backend
-  if (id >= 0)
+  if (do_set (v))
     {
-      graphics_object go = gh_manager::get_object (parent);
-      if (go)
+
+      // notify backend
+      if (id >= 0)
 	{
-	  graphics_backend backend = go.get_backend();
-	  if (backend)
-	    backend.property_changed (go, id);
+	  graphics_object go = gh_manager::get_object (parent);
+	  if (go)
+	    {
+	      graphics_backend backend = go.get_backend();
+	      if (backend)
+		backend.property_changed (go, id);
+	    }
 	}
+
+      // run listeners
+      if (do_run && ! error_state)
+	run_listeners (POSTSET);
+
+      return true;
     }
-  
-  // run listeners
-  if (do_run && ! error_state)
-    run_listeners (POSTSET);
+
+  return false;
 }
 
 
@@ -568,7 +574,7 @@
   return retval;
 }
 
-void
+bool
 color_property::do_set (const octave_value& val)
 {
   if (val.is_string ())
@@ -579,20 +585,28 @@
 	{
 	  if (radio_val.contains (s))
 	    {
-	      current_val = s;
-	      current_type = radio_t;
+	      if (current_type != radio_t || current_val != s)
+		{
+		  current_val = s;
+		  current_type = radio_t;
+		  return true;
+		}
 	    }
           else
 	    {
 	      color_values col (s);
 	      if (! error_state)
 		{
-		  color_val = col;
-		  current_type = color_t;
+		  if (current_type != color_t || col != color_val)
+		    {
+		      color_val = col;
+		      current_type = color_t;
+		      return true;
+		    }
 		}
 	      else
 		error ("invalid value for color property \"%s\" (value = %s)",
-               get_name ().c_str (), s.c_str ());
+		       get_name ().c_str (), s.c_str ());
 	    }	
 	}
       else
@@ -608,8 +622,12 @@
 	  color_values col (m (0), m (1), m(2));
 	  if (! error_state)
 	    {
-	      color_val = col;
-	      current_type = color_t;
+	      if (current_type != color_t || col != color_val)
+		{
+		  color_val = col;
+		  current_type = color_t;
+		  return true;
+		}
 	    }
 	}
       else
@@ -619,9 +637,11 @@
   else 
     error ("invalid value for color property \"%s\"",
            get_name ().c_str ());
+
+  return false;
 }
 
-void
+bool
 double_radio_property::do_set (const octave_value& val)
 {
   if (val.is_string ())
@@ -630,8 +650,12 @@
 
       if (! s.empty () && radio_val.contains (s))
 	{
-	  current_val = s;
-	  current_type = radio_t;
+	  if (current_type != radio_t || s != current_val)
+	    {
+	      current_val = s;
+	      current_type = radio_t;
+	      return true;
+	    }
 	}
       else
 	error ("invalid value for double_radio property \"%s\"",
@@ -639,12 +663,20 @@
     }
   else if (val.is_scalar_type () && val.is_real_type ())
     {
-      dval = val.double_value ();
-      current_type = double_t;
+      double new_dval = val.double_value ();
+
+      if (current_type != double_t || new_dval != dval)
+	{
+	  dval = new_dval;
+	  current_type = double_t;
+	  return true;
+	}
     }
   else 
     error ("invalid value for double_radio property \"%s\"",
 	   get_name ().c_str ());
+
+  return false;
 }
 
 bool
@@ -697,6 +729,53 @@
   return xok;
 }
 
+bool
+array_property::is_equal (const octave_value& v) const
+{
+  if (data.type_name () == v.type_name ())
+    {
+      if (data.dims () == v.dims ())
+	{
+#define CHECK_ARRAY_EQUAL(T,F) \
+	    { \
+	      const T* d1 = data.F ().data (); \
+	      const T* d2 = v.F ().data (); \
+	      \
+	      bool flag = true; \
+	      \
+	      for (int i = 0; flag && i < data.numel (); i++) \
+		if (d1[i] != d2[i]) \
+		  flag = false; \
+	      \
+	      return flag; \
+	    }
+
+	  if (data.is_double_type())
+	    CHECK_ARRAY_EQUAL (double, array_value)
+	  else if (data.is_single_type ())
+	    CHECK_ARRAY_EQUAL (float, float_array_value)
+	  else if (data.is_int8_type ())
+	    CHECK_ARRAY_EQUAL (octave_int8, int8_array_value)
+	  else if (data.is_int16_type ())
+	    CHECK_ARRAY_EQUAL (octave_int16, int16_array_value)
+	  else if (data.is_int32_type ())
+	    CHECK_ARRAY_EQUAL (octave_int32, int32_array_value)
+	  else if (data.is_int64_type ())
+	    CHECK_ARRAY_EQUAL (octave_int64, int64_array_value)
+	  else if (data.is_uint8_type ())
+	    CHECK_ARRAY_EQUAL (octave_uint8, uint8_array_value)
+	  else if (data.is_uint16_type ())
+	    CHECK_ARRAY_EQUAL (octave_uint16, uint16_array_value)
+	  else if (data.is_uint32_type ())
+	    CHECK_ARRAY_EQUAL (octave_uint32, uint32_array_value)
+	  else if (data.is_uint64_type ())
+	    CHECK_ARRAY_EQUAL (octave_uint64, uint64_array_value)
+	}
+    }
+
+  return false;
+}
+
 void
 array_property::get_data_limits (void)
 {
@@ -729,7 +808,7 @@
     }
 }
 
-void
+bool
 handle_property::do_set (const octave_value& v)
 {
   double dv = v.double_value ();
@@ -739,7 +818,13 @@
       graphics_handle gh = gh_manager::lookup (dv);
 
       if (xisnan (gh.value ()) || gh.ok ())
-        current_val = gh;
+	{
+	  if (current_val != gh)
+	    {
+	      current_val = gh;
+	      return true;
+	    }
+	}
       else
         error ("set: invalid graphics handle (= %g) for property \"%s\"",
 	       dv, get_name ().c_str ());
@@ -747,6 +832,8 @@
   else
     error ("set: invalid graphics handle for property \"%s\"",
 	   get_name ().c_str ());
+
+  return false;
 }
 
 bool
--- a/src/graphics.h.in	Tue Aug 26 13:38:14 2008 -0400
+++ b/src/graphics.h.in	Tue Aug 26 13:39:24 2008 -0400
@@ -378,7 +378,7 @@
 
   // Sets property value, notifies backend.
   // If do_run is true, runs associated listeners.
-  void set (const octave_value& v, bool do_run = true);
+  bool set (const octave_value& v, bool do_run = true);
   
   virtual octave_value get (void) const
     {
@@ -404,8 +404,11 @@
     { return new base_property (*this); }
 
 protected:
-  virtual void do_set (const octave_value&)
-    { error ("set: invalid property \"%s\"", name.c_str ()); }
+  virtual bool do_set (const octave_value&)
+    {
+      error ("set: invalid property \"%s\"", name.c_str ());
+      return false;
+    }
 
 private:
   typedef std::map<listener_mode, octave_value_list> listener_map;
@@ -447,13 +450,22 @@
   base_property* clone (void) const { return new string_property (*this); }
 
 protected:
-  void do_set (const octave_value& val)
+  bool do_set (const octave_value& val)
     {
       if (val.is_string ())
-        str = val.string_value ();
+	{
+	  std::string new_str = val.string_value ();
+
+	  if (new_str != str)
+	    {
+	      str = new_str;
+	      return true;
+	    }
+	}
       else
         error ("set: invalid string property value for \"%s\"",
                get_name ().c_str ());
+      return false;
     }
 
 private:
@@ -544,13 +556,19 @@
   base_property* clone (void) const { return new radio_property (*this); }
 
 protected:
-  void do_set (const octave_value& newval) 
+  bool do_set (const octave_value& newval) 
   {
     if (newval.is_string ())
       {
         std::string s = newval.string_value ();
         if (vals.validate (s))
-          current_val = s;
+	  {
+	    if (s != current_val)
+	      {
+		current_val = s;
+		return true;
+	      }
+	  }
         else
           error ("set: invalid value for radio property \"%s\" (value = %s)",
               get_name ().c_str (), s.c_str ());
@@ -558,6 +576,7 @@
     else	
       error ("set: invalid value for radio property \"%s\"",
           get_name ().c_str ());
+    return false;
   }
 
 private:
@@ -599,6 +618,16 @@
     return *this;
   }
 
+  bool operator == (const color_values& c) const
+    {
+      return (xrgb(0) == c.xrgb(0)
+	      && xrgb(1) == c.xrgb(1)
+	      && xrgb(2) == c.xrgb(2));
+    }
+
+  bool operator != (const color_values& c) const
+    { return ! (*this == c); }
+
   Matrix rgb (void) const { return xrgb; }
 
   operator octave_value (void) const { return xrgb; }
@@ -706,7 +735,7 @@
   base_property* clone (void) const { return new color_property (*this); }
 
 protected:
-  OCTINTERP_API void do_set (const octave_value& newval);
+  OCTINTERP_API bool do_set (const octave_value& newval);
 
 private:
   enum current_enum { color_t, radio_t } current_type;
@@ -741,13 +770,22 @@
   base_property* clone (void) const { return new double_property (*this); }
 
 protected:
-  void do_set (const octave_value& v)
+  bool do_set (const octave_value& v)
     {
       if (v.is_scalar_type () && v.is_real_type ())
-        current_val = v.double_value ();
+	{
+	  double new_val = v.double_value ();
+
+	  if (new_val != current_val)
+	    {
+	      current_val = new_val;
+	      return true;
+	    }
+	}
       else
         error ("set: invalid value for double property \"%s\"",
                get_name ().c_str ());
+      return false;
     }
 
 private:
@@ -827,7 +865,7 @@
     { return new double_radio_property (*this); }
 
 protected:
-  OCTINTERP_API void do_set (const octave_value& v);
+  OCTINTERP_API bool do_set (const octave_value& v);
 
 private:
   enum current_enum { double_t, radio_t } current_type;
@@ -901,22 +939,32 @@
     }
 
 protected:
-  void do_set (const octave_value& v)
+  bool do_set (const octave_value& v)
     {
       if (validate (v))
 	{
-	  data = v;
-
-	  get_data_limits ();
+	  // FIXME: should we check for actual data change?
+	  if (! is_equal (v))
+	    {
+	      data = v;
+
+	      get_data_limits ();
+
+	      return true;
+	    }
 	}
       else
         error ("invalid value for array property \"%s\"",
                get_name ().c_str ());
+      
+      return false;
     }
 
 private:
   OCTINTERP_API bool validate (const octave_value& v);
 
+  OCTINTERP_API bool is_equal (const octave_value& v) const;
+
   OCTINTERP_API void get_data_limits (void);
 
 protected:
@@ -982,9 +1030,9 @@
     }
 
 protected:
-  void do_set (const octave_value& v)
+  bool do_set (const octave_value& v)
   {
-    array_property::do_set (v);
+    bool retval = array_property::do_set (v);
 
     if (! error_state)
       {
@@ -998,7 +1046,11 @@
 
 	    data = data.reshape (dv);
 	  }
+
+	return retval;
       }
+
+    return false;
   }
 
 private:
@@ -1034,12 +1086,12 @@
   base_property* clone (void) const { return new bool_property (*this); }
 
 protected:
-  void do_set (const octave_value& val)
+  bool do_set (const octave_value& val)
     {
       if (val.is_bool_scalar ())
-        radio_property::do_set (val.bool_value () ? "on" : "off");
+        return radio_property::do_set (val.bool_value () ? "on" : "off");
       else
-        radio_property::do_set (val);
+        return radio_property::do_set (val);
     }
 };
 
@@ -1075,7 +1127,7 @@
   base_property* clone (void) const { return new handle_property (*this); }
 
 protected:
-  OCTINTERP_API void do_set (const octave_value& v);
+  OCTINTERP_API bool do_set (const octave_value& v);
 
 private:
   graphics_handle current_val;
@@ -1104,7 +1156,11 @@
   base_property* clone (void) const { return new any_property (*this); }
 
 protected:
-  void do_set (const octave_value& v) { data = v; }
+  bool do_set (const octave_value& v)
+    {
+      data = v;
+      return true;
+    }
 
 private:
   octave_value data;
@@ -1135,13 +1191,17 @@
   base_property* clone (void) const { return new callback_property (*this); }
 
 protected:
-  void do_set (const octave_value& v)
+  bool do_set (const octave_value& v)
     {
       if (validate (v))
-        callback = v;
+	{
+	  callback = v;
+	  return true;
+	}
       else
         error ("invalid value for callback property \"%s\"",
                get_name ().c_str ());
+      return false;
     }
 
 private:
@@ -1204,8 +1264,8 @@
   octave_value get (void) const
     { return rep->get (); }
 
-  void set (const octave_value& val)
-    { rep->set (val); }
+  bool set (const octave_value& val)
+    { return rep->set (val); }
 
   property& operator = (const octave_value& val)
     {