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

Issue 342060043: issue 5319: Make skylines reflect grob rotation

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 2 weeks ago by Be-3
Modified:
4 months, 1 week ago
Reviewers:
dak, lemzwerg
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

issue 5319: Make skylines reflect grob rotation modified: ../lily/include/stencil.hh Add parameter to skylines_from_stencil for rot (in analogy to pad) modified: ../lily/stencil-integral.cc Stencil::skylines_from_stencil If new rotation parameter rot is specified, use a rotated copy of the original stencil to build the skyline transformation matrix. As the rotation is rarely used, the general (non-rotated) case will still use the original stencil as before. Grob::vertical_sklyines_from_grob and Grob::horizontal_skylines_from_grob Read "rotation" property an pass it (SCM) to skylines_from_stencil. modified: ../lily/accidental.cc Accidental_interface::horizontal_skylines Read "rotation" property and pass it (SCM) to skylines_from_stencil. Announced in ../Documentation/changes.tely Documentation example slightly adapted (20° to 15°) to new skyline behaviour (padding will now be respected). In all affected languages: modified: ../Documentation/de/notation/changing-defaults.itely modified: ../Documentation/es/notation/changing-defaults.itely modified: ../Documentation/fr/notation/changing-defaults.itely modified: ../Documentation/it/notation/changing-defaults.itely modified: ../Documentation/ja/notation/changing-defaults.itely modified: ../Documentation/notation/changing-defaults.itely modified: ../input/regression/skyline-color-rotation.ly (20° to 15°, cf. documentation) new: ../input/regression/skyline-grob-rotation.ly New regression test.

Patch Set 1 #

Total comments: 1

Patch Set 2 : New upload due to inconsistency #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+89 lines, -17 lines) Patch
M Documentation/changes.tely View 1 1 chunk +24 lines, -0 lines 0 comments Download
M Documentation/de/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
M Documentation/es/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
M Documentation/fr/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
M Documentation/it/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
M Documentation/ja/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
M Documentation/notation/changing-defaults.itely View 1 chunk +1 line, -1 line 0 comments Download
A input/regression/skyline-grob-rotation.ly View 1 chunk +28 lines, -0 lines 0 comments Download
M input/regression/stencil-color-rotation.ly View 1 chunk +1 line, -1 line 0 comments Download
M lily/accidental.cc View 1 chunk +2 lines, -1 line 0 comments Download
M lily/include/stencil.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/stencil-integral.cc View 3 chunks +27 lines, -8 lines 1 comment Download

Messages

Total messages: 5
Be-3
Please review... Inadvertently, file ../lily/key-signature-interface.cc slipped into the patch (from issue 5312 currently in staging). ...
5 months, 2 weeks ago (2018-05-03 12:51:11 UTC) #1
Be-3
New upload due to inconsistency
5 months, 2 weeks ago (2018-05-03 17:41:33 UTC) #2
lemzwerg
LGTM https://codereview.appspot.com/342060043/diff/1/lily/key-signature-interface.cc File lily/key-signature-interface.cc (right): https://codereview.appspot.com/342060043/diff/1/lily/key-signature-interface.cc#newcode114 lily/key-signature-interface.cc:114: padding += (intersection (ht_right, last_ht_left).length () The outermost ...
5 months, 2 weeks ago (2018-05-03 17:42:22 UTC) #3
Be-3
On 2018/05/03 17:42:22, lemzwerg wrote: > LGTM > > https://codereview.appspot.com/342060043/diff/1/lily/key-signature-interface.cc > File lily/key-signature-interface.cc (right): > ...
5 months, 2 weeks ago (2018-05-03 17:58:00 UTC) #4
dak
4 months, 1 week ago (2018-06-09 20:43:22 UTC) #5
https://codereview.appspot.com/342060043/diff/20001/lily/stencil-integral.cc
File lily/stencil-integral.cc (right):

https://codereview.appspot.com/342060043/diff/20001/lily/stencil-integral.cc#...
lily/stencil-integral.cc:1132: Stencil *q = unsmob<Stencil> (s->smobbed_copy
());
Once you create a smobbed copy, it must be protected from garbage collection. 
You don't do this here at all.  What you need to do is to retain the SCM value
on the stack, not its unsmobbification.  If you don't want to keep unsmobbing
it, you can keep both and do an scm_remember_upto_here (scm_q);
at the point up to which it must not be collected.  However, it seems that uses
here are few enough that you can just do
SCM q = s->smobbed_copy ();
and use unsmob<Stencil> (q) where you now use q.
Sign in to reply to this message.

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