[Top][All Lists]
[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
- Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/09
- Re: Potential fix for issue 1504. (issue4237057), mtsolo, 2011/03/10
- Re: Potential fix for issue 1504. (issue4237057), mtsolo, 2011/03/10
- Re: Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/11
- Re: Potential fix for issue 1504. (issue4237057), mtsolo, 2011/03/12
- Re: Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/12
- Re: Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/13
Re: Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/15
Re: Potential fix for issue 1504. (issue4237057), hanwenn, 2011/03/15
Re: Potential fix for issue 1504. (issue4237057), mtsolo, 2011/03/15