Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4164)

Issue 5060: Fix ly:stencil-rotate; add function to rotate about absolute coordinate (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
15 years, 6 months ago by Reinhold
Modified:
14 years, 8 months ago
Reviewers:
hanwenn
Visibility:
Public.

Description

ly:stencil-rotate appeared to rotate the stencil's contents correctly around some given point, but the rotation of the bounding box was so broken that I had a hard time understanding what was happening at all and what the code was supposed to do. In the end, I simply reimplemented that calculation from scratch, also adding a function ly:stencil-rotate-absolute-coordinates, which takes the point of rotation in absolute coordinates. I will need this for always rotating a stencil around the point (0,0), no matter what the stencil's extents are.

Patch Set 1 #

Patch Set 2 : Re-add some debug boxes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -22 lines) Patch
M input/regression/flags-in-scheme.ly View 1 2 chunks +37 lines, -4 lines 0 comments Download
M lily/include/stencil.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/stencil.cc View 2 chunks +37 lines, -17 lines 3 comments Download
M lily/stencil-scheme.cc View 2 chunks +22 lines, -1 line 1 comment Download

Messages

Total messages: 2
Reinhold
Here is a patch to fix ly:stencil-rotate (rotating bounding box of the stencil was completely ...
15 years, 6 months ago (2008-09-10 09:47:33 UTC) #1
hanwenn
15 years, 6 months ago (2008-09-10 14:38:56 UTC) #2
Looks good to me.

http://codereview.appspot.com/5060/diff/201/204
File lily/stencil-scheme.cc (right):

http://codereview.appspot.com/5060/diff/201/204#newcode309
Line 309: LY_DEFINE (ly_stencil_rotate_absolute_coordinates,
"ly:stencil-rotate-absolute-coordinates",
in the interest of brevity, you could drop 'coordinates' from the name.

http://codereview.appspot.com/5060/diff/201/205
File lily/stencil.cc (right):

http://codereview.appspot.com/5060/diff/201/205#newcode98
Line 98: expr_, SCM_UNDEFINED);
in effect, this copies the underlying expression.  It might be a little bit
nicer to mirror this in the api, ie. make a

  Stencil::rotated()

and have Stencil::rotate be an abbrev of 

  *this = rotated()

http://codereview.appspot.com/5060/diff/201/205#newcode127
Line 127: //   const Offset cen = Offset (extent (X_AXIS).center (), extent
(Y_AXIS).center ());
can you remove debug code?

http://codereview.appspot.com/5060/diff/201/205#newcode128
Line 128: const Real x = extent (X_AXIS).linear_combination
(relative_off[X_AXIS]);
probably not worth the effort, but you could loop through the axes here.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b