Issue 567580043: Stop smobifying Transform
DescriptionStop smobifying Transform
Patch Set 1 #
MessagesTotal messages: 5
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.
Sign in to reply to this 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.
Sign in to reply to this message.
On 2020/05/09 12:00:01, hanwenn wrote: > 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? I have not said that SCM-accessible transforms help with implementing Cairo support. The problem with Cairo is that it cannot properly represent rotations by multiple of 90 degrees (a common use case) except by using transform matrices, and so the in-stencil (and thus Scheme-accessible) support of transformations, currently mostly constrained to translations and rotations, would have to be moved over to whole transform matrices. > 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. That statement is not borne out by facts since a Simple_smob does not incur _any_ GC or Scheme overhead or loses any of the type checking of C++ unless you actually call smobbed_copy on it. Overhead occurs once you do significant manipulation with it using Scheme, and when doing so, there would be a reason for it offsetting the overhead. The Simple_smob base class itself contains no virtual functions and no non-static data members and thus comes without any runtime overhead until you actually use it from within Scheme. > 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. They aren't available from Scheme as of yet. This is just done in preparation of changes that are sensible to make in future. And stencil expressions, albeit expressed in terms of Scheme data structures, don't really are manipulated to any significant degree from the user. Which is good since it would make a whole lot of sense to implement them in terms of, say, Cairo data structures in the long run. And the refactoring of the transform matrix and its use (at the current point in terms of a Pango transform matrix, but changing that detail would be comparatively trivial even though annoyingly the respective structures don't have the same internal layout) lends itself to that purpose very well in that regard. As I stated: performance cannot possibly be an issue before smobbed_copy () is even called since the memory layout and management and access is completely unchanged until one does. So undoing work that has been done for the sake of future extensibility would warrant an actually valid reason. In the long run, I'd be in preference of moving the SCM reflection of data types now represented by class derivation from Simple_smob (namely expressions without an SCM identity that sensibly can be eq?-compared and a straightforward storage layout) to a templated mechanism that allows confining the Scheme reflection to files actually concerned with it. As long as we are not there, however, I don't see the point in making life harder for those who would want to use transforms from the Scheme side. We have enough graphical entities represented by Scheme lists already, an appallingly expensive storage representation for material of fixed size. Having a way to represent transforms with a single SCM cell rather than 6 cons cells and 6 float cells can avert exactly the situation you are concerned with: too much use of SCM data management for the sake of getting a job done.
Sign in to reply to this message.
On Sat, May 9, 2020 at 4:56 PM <dak@gnu.org> wrote: > > On 2020/05/09 12:00:01, hanwenn wrote: > > 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? > > I have not said that SCM-accessible transforms help with implementing > Cairo support. The problem with Cairo is that it cannot properly > represent rotations by multiple of 90 degrees (a common use case) except > by using transform matrices, and so the in-stencil (and thus > Scheme-accessible) support of transformations, currently mostly > constrained to translations and rotations, would have to be moved over > to whole transform matrices. > > > 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. > > That statement is not borne out by facts since a Simple_smob does not > incur _any_ GC or Scheme overhead or loses any of the type checking of > C++ unless you actually call smobbed_copy on it. Overhead occurs once Well, the change I we are discussing, you are actually calling smobbed_copy, for example in line 412 here: https://codereview.appspot.com/344970043/diff/60001/lily/stencil-integral.cc > > 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. > > They aren't available from Scheme as of yet. This is just done in > preparation of changes that are sensible to make in future. And stencil > expressions, albeit expressed in terms of Scheme data structures, don't > really are manipulated to any significant degree from the user. > > Which is good since it would make a whole lot of sense to implement them > in terms of, say, Cairo data structures in the long run. And the > refactoring of the transform matrix and its use (at the current point in > terms of a Pango transform matrix, but changing that detail would be > comparatively trivial even though annoyingly the respective structures > don't have the same internal layout) lends itself to that purpose very > well in that regard. > > As I stated: performance cannot possibly be an issue before smobbed_copy > () is even called since the memory layout and management and access is > completely unchanged until one does. > > So undoing work that has been done for the sake of future extensibility > would warrant an actually valid reason. In the long run, I'd be in > preference of moving the SCM reflection of data types now represented by > class derivation from Simple_smob (namely expressions without an SCM > identity that sensibly can be eq?-compared and a straightforward storage > layout) to a templated mechanism that allows confining the Scheme > reflection to files actually concerned with it. > > As long as we are not there, however, I don't see the point in making > life harder for those who would want to use transforms from the Scheme > side. We have enough graphical entities represented by Scheme lists > already, an appallingly expensive storage representation for material of > fixed size. I sitll don't understand. You want to have Transform as a smob in support of some future functionality. Can you explain what that functionality is, and how it would use a smobbed transform? This is currently dead code, and the normal treatment of dead code is to delete it. > Having a way to represent transforms with a single SCM cell rather than > 6 cons cells and 6 float cells can avert exactly the situation you are > concerned with: too much use of SCM data management for the sake of > getting a job done. > > https://codereview.appspot.com/567580043/ -- Han-Wen Nienhuys - hanwenn@gmail.com - http://www.xs4all.nl/~hanwen
Sign in to reply to this message.
On 2020/05/09 16:19:59, hanwenn wrote: > On Sat, May 9, 2020 at 4:56 PM <mailto:dak@gnu.org> wrote: > > > > On 2020/05/09 12:00:01, hanwenn wrote: > > > 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? > > > > I have not said that SCM-accessible transforms help with implementing > > Cairo support. The problem with Cairo is that it cannot properly > > represent rotations by multiple of 90 degrees (a common use case) except > > by using transform matrices, and so the in-stencil (and thus > > Scheme-accessible) support of transformations, currently mostly > > constrained to translations and rotations, would have to be moved over > > to whole transform matrices. > > > > > 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. > > > > That statement is not borne out by facts since a Simple_smob does not > > incur _any_ GC or Scheme overhead or loses any of the type checking of > > C++ unless you actually call smobbed_copy on it. Overhead occurs once > > Well, the change I we are discussing, you are actually calling > smobbed_copy, for example in line 412 here: > https://codereview.appspot.com/344970043/diff/60001/lily/stencil-integral.cc Which actually is a pretty good use of it since passing around an SCM expression (and the stencil expression being passed in parallel also is an SCM expression) is cheaper than passing a whole transform matrix by value as done before. And there is only rather occasional change needed (necessitating the creation of a new object) since most stencil expressions do not change the current transform. > > > > 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. > > > > They aren't available from Scheme as of yet. This is just done in > > preparation of changes that are sensible to make in future. And stencil > > expressions, albeit expressed in terms of Scheme data structures, don't > > really are manipulated to any significant degree from the user. > > > > Which is good since it would make a whole lot of sense to implement them > > in terms of, say, Cairo data structures in the long run. And the > > refactoring of the transform matrix and its use (at the current point in > > terms of a Pango transform matrix, but changing that detail would be > > comparatively trivial even though annoyingly the respective structures > > don't have the same internal layout) lends itself to that purpose very > > well in that regard. > > > > As I stated: performance cannot possibly be an issue before smobbed_copy > > () is even called since the memory layout and management and access is > > completely unchanged until one does. > > > > So undoing work that has been done for the sake of future extensibility > > would warrant an actually valid reason. In the long run, I'd be in > > preference of moving the SCM reflection of data types now represented by > > class derivation from Simple_smob (namely expressions without an SCM > > identity that sensibly can be eq?-compared and a straightforward storage > > layout) to a templated mechanism that allows confining the Scheme > > reflection to files actually concerned with it. > > > > As long as we are not there, however, I don't see the point in making > > life harder for those who would want to use transforms from the Scheme > > side. We have enough graphical entities represented by Scheme lists > > already, an appallingly expensive storage representation for material of > > fixed size. > > I sitll don't understand. You want to have Transform as a smob in > support of some future functionality. Can you explain what that > functionality is, and how it would use a smobbed transform? Stencil expressions during interpretation currently need to recreate transforms on the fly. It would be easier to just store them along with the expression. > This is currently dead code, and the normal treatment of dead code is > to delete it. Han-Wen, you don't accept long-term plans of other developers without proof of concept. And you throw out intermediate steps working towards that goal and declare them dead code. The right point of time for making objections to other people's contributions is when they were put up for review and discussed. You did not participate in those discussions. The next thing you are doing (when git blame clearly points to available information about the contributions, their time, their issues, their rationale and likely point of time) is deleting them without offering a rationale for that, indicating that you have no respect of previous decisions, previous discussions, and previous work. The last decade of LilyPond development has largely been marked by the goal to make much of LilyPond's internals workable and accessible to power users at the Scheme level. This has been a significant driver of LilyPond-based solutions and of an active and helpful user base. It has enabled significant contributions by community members that are not core C++ programmers. I don't see the point in reversing this course and locking access to LilyPond's internals to anybody but C++ programmers. Rewinding every effort in that direction throws away the potential of a quite larger community of contributors than the C++ programmers, and in this particular case the C++ programmers do not even get anything in return.
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||
