lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Potential fix for issue 1504. (issue4237057)


From: address@hidden
Subject: Re: Potential fix for issue 1504. (issue4237057)
Date: Sun, 13 Mar 2011 17:15:56 -0400

On Mar 12, 2011, at 8:49 AM, address@hidden wrote:

> 
> http://codereview.appspot.com/4237057/diff/1/lily/spanner.cc
> File lily/spanner.cc (right):
> 
> http://codereview.appspot.com/4237057/diff/1/lily/spanner.cc#newcode405
> lily/spanner.cc:405: Spanner::broken_spanner_index (Spanner const *sp)
> On 2011/03/12 10:18:06, MikeSol wrote:
>> On 2011/03/09 23:03:44, hanwenn wrote:
>> > why not make it a real member funtcion?
> 
>> Actually - sorry, I spoke too soon.  I see that there's a function
>> get_break_index.  Could these two functions be merged?  Is there a
> reason that
>> the two functions exist separately?  If not, I'll merge them together.
> 
> good point. Can you replace broken_spanner_index by get_break_index
> everywhere?  should be separate commit to go in before this one.  If it
> passes the regtest cleanly, does not need to be reviewed.
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc
> File lily/beam.cc (right):
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode144
> lily/beam.cc:144: Beam::get_beam_span (Spanner* me)
> Why can't you use spanner::spanner_length()?  these numbers don't need
> to be that exact, do they?
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode160
> lily/beam.cc:160: extract_grob_set (me, "stems", stems);
> you already have them.  line 145. In this case you might as well use
> get_bound() rather than the stems, btw.
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode169
> lily/beam.cc:169: Beam::get_span_data (Spanner *me)
> _data -> _widths ?
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode171
> lily/beam.cc:171:
> drop
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode634
> lily/beam.cc:634: if (Spanner::broken_spanner_index (me) == (int)i)
> use newstyle casting int(i)
> 
> http://codereview.appspot.com/4237057/diff/8001/lily/beam.cc#newcode646
> lily/beam.cc:646: }
> how about you store this in a property? You could compute the property
> in a separate function, say
> 
>  feather-fractions pair?  "how much of feathering for this beam"
> 
> and the computation would only need to be done once.
> 
> 1st beam gets:  (0.0 . 0.25)
> 2nd: (0.25 . 0.75)
> 3rd: (0.75 . 1.0)
> 
> (assuming total beam length of 2x linesize).
> 
> then print() only needs to do the part for its own system.
> 
> unfeathered beams get (1.0 . 1.0) and shrinking beams (1.0 . 0.0)
> 

All issues addressed.  New patch set up at 
http://codereview.appspot.com/4237057/

Cheers,
MS


reply via email to

[Prev in Thread] Current Thread [Next in Thread]