Re: Transform: add print_smob to aid debugging (issue 561680043 by address@hidden)

[hanwenn]

Reviewers: lemzwerg, Dan Eble,


https://codereview.appspot.com/561680043/diff/577820044/lily/include/transform.hh
File lily/include/transform.hh (right):

https://codereview.appspot.com/561680043/diff/577820044/lily/include/transform.hh#newcode33
lily/include/transform.hh:33: int print_smob (SCM p, scm_print_state *)
const;
On 2020/04/25 22:32:55, Dan Eble wrote:
> This function looks out of place in a group of static constants.  How
about
> moving it below with the rest of the functions?

Done.

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc
File lily/transform.cc (left):

https://codereview.appspot.com/561680043/diff/577820044/lily/transform.cc#oldcode41
lily/transform.cc:41: #include "offset.hh"
On 2020/04/25 22:32:55, Dan Eble wrote:
> The include order was better before.  If transform.hh is the first
header that
> transform.cc includes, then if transform.hh lacks any includes, it
will be
> detected.
> 
> Standard headers should follow the rest to make it more likely to that
missing
> includes in other headers will be detected.  (I think I'm less of a
sticker than
> Jonas on this point.  The possibility of false negatives is high for
commonly
> used headers, but it's still a good idea.)

Done.

Description:
Transform: add print_smob to aid debugging

Please review this at https://codereview.appspot.com/561680043/

Affected files (+23, -1 lines):
  M lily/include/transform.hh
  M lily/transform.cc


Index: lily/include/transform.hh
diff --git a/lily/include/transform.hh b/lily/include/transform.hh
index 
781f79d904c99ea20608e25d16695ae3d4f0fe62..ba563106f4f005109cb27879e2b3baee88b14452
 100644
--- a/lily/include/transform.hh
+++ b/lily/include/transform.hh
@@ -27,7 +27,10 @@
 class Transform : public Simple_smob<Transform>
 {
   PangoMatrix m_;
+
+
 public:
+  int print_smob (SCM p, scm_print_state *) const;
   static const char *const type_p_name_;
 
   static const Transform identity;
@@ -75,6 +78,7 @@ public:
 
   Offset operator () (Offset point) const;
   Transform operator () (const Transform &t) const;
+  std::string to_string () const;
 
   Real get_xx () const { return m_.xx; }
   Real get_xy () const { return m_.xy; }
Index: lily/transform.cc
diff --git a/lily/transform.cc b/lily/transform.cc
index 
c13a61ee73802cd27a2639c1392cf20ec9d4811c..7a289fc11e195f5f37d7d9287539f82b925b60fe
 100644
--- a/lily/transform.cc
+++ b/lily/transform.cc
@@ -36,9 +36,11 @@
  * The last row of both transform matrix and coordinates is not stored
  * but merely implied.
  */
+#include <string>
 
-#include "transform.hh"
 #include "offset.hh"
+#include "string-convert.hh"
+#include "transform.hh"
 
 const char *const
 Transform::type_p_name_ = "ly:transform?";
@@ -83,6 +85,22 @@ Transform::rotate (Real angle, Offset center)
   return concat (tmp);
 }
 
+std::string
+Transform::to_string () const
+{
+  return String_convert::form_string ("[[%f %f %f] [%f %f %f]]", m_.xx, m_.xy,
+                                      m_.x0, m_.yx, m_.yy, m_.y0);
+}
+
+int
+Transform::print_smob (SCM p, scm_print_state *) const
+{
+  scm_puts ("#<Transform ", p);
+  scm_puts (to_string ().c_str (), p);
+  scm_puts (">", p);
+  return 1;
+}
+
 Transform &
 Transform::scale (Real xscale, Real yscale)
 {