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; -}
