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

Issue 5908043: Brings beamed rest closer to cross staff beam. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 3 months ago by MikeSol
Modified:
6 years, 10 months ago
Reviewers:
Keith, Milimetr88, mike7
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Brings beamed rest closer to cross staff beam.

Patch Set 1 #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+28 lines, -2 lines) Patch
A input/regression/beam-cross-staff-rest.ly View 1 chunk +19 lines, -0 lines 0 comments Download
M lily/beam.cc View 3 chunks +9 lines, -2 lines 3 comments Download

Messages

Total messages: 4
Keith
http://codereview.appspot.com/5908043/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/5908043/diff/1/lily/beam.cc#newcode1326 lily/beam.cc:1326: || beam->get_property_data ("direction") == ly_symbol2scm ("calculation-in-progress")) Mike, any idea ...
7 years, 2 months ago (2012-04-25 06:21:53 UTC) #1
mike7
http://codereview.appspot.com/5908043/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/5908043/diff/1/lily/beam.cc#newcode1326 lily/beam.cc:1326: || beam->get_property_data ("direction") == ly_symbol2scm ("calculation-in-progress")) On 2012/04/25 06:21:54, ...
7 years, 2 months ago (2012-04-25 06:29:14 UTC) #2
Milimetr88
http://codereview.appspot.com/5908043/diff/1/lily/beam.cc File lily/beam.cc (right): http://codereview.appspot.com/5908043/diff/1/lily/beam.cc#newcode1326 lily/beam.cc:1326: || beam->get_property_data ("direction") == ly_symbol2scm ("calculation-in-progress")) On 2012/04/25 06:29:14, ...
7 years, 2 months ago (2012-04-25 11:10:40 UTC) #3
mike7
7 years, 2 months ago (2012-04-25 11:14:41 UTC) #4
On 2012/04/25 11:10:40, Milimetr88 wrote:
> http://codereview.appspot.com/5908043/diff/1/lily/beam.cc
> File lily/beam.cc (right):
> 
> http://codereview.appspot.com/5908043/diff/1/lily/beam.cc#newcode1326
> lily/beam.cc:1326: || beam->get_property_data ("direction") == ly_symbol2scm
> ("calculation-in-progress"))
> On 2012/04/25 06:29:14, mike7 wrote:
> > On 2012/04/25 06:21:54, Keith wrote:
> > > Mike, any idea why you added this?
> > > It just adds to the cases where we lie and say the beam is on the
> centerline. 
> > > It causes this to go wrongly
> > > << {b''8[ r16. g''32] } \\ {r8 <f' g' a' b'>16. r } >>
> > > 
> > > Take a look at today's patchset at 
> > > http://codereview.appspot.com/6035053/
> > 
> > If this bit isn't there, a circular dependency could be called up later on
in
> > the function while looking for the direction.  I forget the exact reason for
> the
> > circular dependency, but it crept up in several regtests.  Like several
other
> > pure functions, I have it cowardly fail if a pure estimation is not possible
> > because of circular dependencies, which usually has a minimal impact on the
> > typeset result.
> 

I'm not well-placed to say.  I try to let the code act as its own commentary as
much as possible.  Here, I think that a person who knows the code base knows
that if the function exits because a property's calculation is in progress, it
is to avoid triggering that calculation.  However, if anyone feels that this is
not pick-upable upon reading, please add the comment!
Sign in to reply to this message.

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