Re: Stop smobifying Transform (issue 567580043 by address@hidden)

[hanwenn]

Reviewers: dak,

Message:
On 2020/05/09 11:03:12, dak wrote:
> Rationale?  This negates work done as part of issue 5347 with the
long-term goal
> of making transforms a better integrated and accessible part of
stencils.  This
> will be increasingly important when we move to Cairo since Cairo
cannot
> represent rotations by multiples of 90 degrees cleanly other than by
using an
> explicit transform matrix since (in defiance of all other graphical
systems like
> Postscript, PDF, METAPOST) it refuses to represent angles in any other
form than
> as radians (an actual patch implementing angles as degrees was
proferred and
> rejected on what can hardly be called anything but philosophical
grounds), and
> PI does not have an exact representation in those.

Could you sketch how SCM-accessible Transform help with implementing
Cairo support? 

As you can see from the current state, it is not necessary to export
them to Scheme, and 
I actually think the state from before
https://codereview.appspot.com/344970043 is preferable
to SCM encapsulation. With SCM encapsulation, we create GC overhead, and
lose the type checking that C++ gives us.  

It is extremely hard to  deprecate or change APIs once they gain use in
Scheme, so everything being equal, the safe option is to not make them
accessible IMO.



Description:
Stop smobifying Transform

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

Affected files (+1, -36 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 
aa0f01f4cde2c19ed21719b4b8035326ceef11bb..6834e9fe43f43defa72a2ba14b0a6a43aba28b62
 100644
--- a/lily/include/transform.hh
+++ b/lily/include/transform.hh
@@ -24,7 +24,7 @@
 #include "offset.hh"
 #include "smobs.hh"
 
-class Transform : public Simple_smob<Transform>
+class Transform
 {
   PangoMatrix m_;
 
@@ -77,7 +77,6 @@ public:
   Offset operator () (Offset point) const;
   Transform operator () (const Transform &t) const;
   std::string to_string () const;
-  int print_smob (SCM p, scm_print_state *) const;
 
   Real get_xx () const { return m_.xx; }
   Real get_xy () const { return m_.xy; }
@@ -87,15 +86,4 @@ public:
   Real get_y0 () const { return m_.y0; }
 };
 
-Offset scm_transform (SCM trans, Offset p);
-Transform scm_transform (SCM trans, const Transform &t);
-
-inline Transform
-robust_scm2transform (SCM trans, const Transform &fallback = 
Transform::identity)
-{
-  if (Transform *tp = unsmob<Transform> (trans))
-    return *tp;
-  return fallback;
-}
-
 #endif
Index: lily/transform.cc
diff --git a/lily/transform.cc b/lily/transform.cc
index 
768390956c72017647a807aaf4dbdb95264cb1b5..5aaf62af4528edf9edaad438003a382a151c3971
 100644
--- a/lily/transform.cc
+++ b/lily/transform.cc
@@ -94,15 +94,6 @@ Transform::to_string () const
                                       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)
 {
@@ -126,17 +117,3 @@ Transform::operator () (const Transform &t) const
   // prove an issue.
   return Transform (*this).concat (t);
 }
-
-Offset scm_transform (SCM trans, Offset p)
-{
-  if (Transform *tp = unsmob<Transform> (trans))
-    return (*tp) (p);
-  return p;
-}
-
-Transform scm_transform (SCM trans, const Transform &t)
-{
-  if (Transform *tp = unsmob<Transform> (trans))
-    return (*tp) (t);
-  return t;
-}