Cleanup beam scoring code.

Cleanup beam scoring code.

Introduce Beam_scoring_problem to encapsulate parameters and methods
for beam scoring.

This is a preparation for using a priority queue on the to-be-computed
scores.

Document Interval::delta().


Don't crash on a pending spanner in Staff_symbol_engraver dtor.

[hanwenn, 2011-01-31 15:41:46]

Hi,

this is a first step in making the beam scoring a bit saner and more efficient.

Please review; i've been brainwashed by google's coding standards so there are
likely some style issues.

[hanwenn, 2011-01-31 15:44:01]

Note - there are some beam related regressions.  I'll fix those tonight. 
cheers.

[hanwenn, 2011-02-01 01:26:27]

On 2011/01/31 15:44:01, hanwenn wrote:
> Note - there are some beam related regressions.  I'll fix those tonight. 
> cheers.

Fixed.  

Please take another look.

[MikeSol, 2011-02-01 01:44:30]

LGTM.  Reads a lot cleaner and is much more extensible.

This may be for another patch, but I think one problem that remains with the
algorithm is that the "reasonable_score" parameter may inadvertently exclude
certain quants from receiving a demerit they deserve because they are not part
of a for loop.  More qualified quants then get hit with a demerit and become
less reasonable than the previously "unreasonable" quant.  I know that taking
away the "if" statement that checks for reasonability would increase computation
time, but is there a better way to assure that non-reasonable scores will not
rise to the top of the pile because they have been excluded from stages of the
penalizing process?

[hanwenn, 2011-02-01 02:25:55]

The reasonable score is to go in a subsequent patch. Stay tuned.

On Mon, Jan 31, 2011 at 11:44 PM,  <mtsolo@gmail.com> wrote:
> LGTM.  Reads a lot cleaner and is much more extensible.
>
> This may be for another patch, but I think one problem that remains with
> the algorithm is that the "reasonable_score" parameter may inadvertently
> exclude certain quants from receiving a demerit they deserve because
> they are not part of a for loop.  More qualified quants then get hit
> with a demerit and become less reasonable than the previously
> "unreasonable" quant.  I know that taking away the "if" statement that
> checks for reasonability would increase computation time, but is there a
> better way to assure that non-reasonable scores will not rise to the top
> of the pile because they have been excluded from stages of the
> penalizing process?
>
> http://codereview.appspot.com/4001046/
>



-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[Graham Percival (old account), 2011-02-01 22:12:13]

I see some fairly crazy beams in
  input/regression/spacing-to-grace.ly
  input/regression/accidental-contemporary.ly
... etc...
there's some really steep slopes.

I know you said there was another patch coming, so all well and good.

[hanwenn, 2011-02-02 03:30:12]

On 2011/02/01 22:12:13, Graham Percival wrote:
> I see some fairly crazy beams in
>   input/regression/spacing-to-grace.ly
>   input/regression/accidental-contemporary.ly
> ... etc...
> there's some really steep slopes.
0

Fixed for real now.

[hanwenn, 2011-02-02 03:34:32]

There is one diff remaining: see the attached file.   I introduced
some streamlining of an inner loop, which affects the way we handle
beams that start/end with invisible stems.

The compare image is the new one.  I have no examples of tremolo beams
handy, but the new layout actually looks better to me.

What do you guys think?


On Wed, Feb 2, 2011 at 1:30 AM,  <hanwenn@gmail.com> wrote:
> On 2011/02/01 22:12:13, Graham Percival wrote:
>>
>> I see some fairly crazy beams in
>>   input/regression/spacing-to-grace.ly
>>   input/regression/accidental-contemporary.ly
>> ... etc...
>> there's some really steep slopes.
>
> 0
>
> Fixed for real now.
>
>
> http://codereview.appspot.com/4001046/
>



-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[c_sorensen, 2011-02-02 03:50:25]

On 2/1/11 8:34 PM, "Han-Wen Nienhuys" <hanwenn@gmail.com> wrote:

> There is one diff remaining: see the attached file.   I introduced
> some streamlining of an inner loop, which affects the way we handle
> beams that start/end with invisible stems.
> 
> The compare image is the new one.  I have no examples of tremolo beams
> handy, but the new layout actually looks better to me.
> 
> What do you guys think?

I much prefer the new one.


Thanks,

Carl

[wl_gnu.org, 2011-02-02 05:49:26]

> There is one diff remaining: see the attached file.  I introduced
> some streamlining of an inner loop, which affects the way we handle
> beams that start/end with invisible stems.
> 
> The compare image is the new one.  I have no examples of tremolo beams
> handy, but the new layout actually looks better to me.

For me, it doesn't.  While the old behaviour is not optimal, the new
one looks really worse to me, especially in bar 3 (which is admittedly
a quite artificial example).  The most important issue for me: Why is
there no slant of the beams?  I currently have not enough time to
closely follow the new code, but I suggest that `stemless tremolos'
are handled similar to the tremolos within the 1/4 bars, but with
shorter stems (this means that for computing the positions, the
routine should pretend that there *are* stems).

IMHO, the ideal vertical position is somewhere in the middle between
the two images, *with* slant.


    Werner

[reinhold_kainhofer.com, 2011-02-02 11:34:38]

Am Mittwoch, 2. Februar 2011, um 06:49:14 schrieb Werner LEMBERG:
> > There is one diff remaining: see the attached file.  I introduced
> > some streamlining of an inner loop, which affects the way we handle
> > beams that start/end with invisible stems.
> > 
> > The compare image is the new one.  I have no examples of tremolo beams
> > handy, but the new layout actually looks better to me.
> 
> For me, it doesn't.  

The beams are better in that they are not so far away from the whole notes any 
more, but it's still not ideal.

> While the old behaviour is not optimal, the new
> one looks really worse to me, especially in bar 3 (which is admittedly
> a quite artificial example).  The most important issue for me: Why is
> there no slant of the beams?  

I wanted to ask the same: According to Gardner Read (p. 236-237) and Kurt 
Stone (p. 150-151), the beams of whole-note tremolos are slanted like with 
shorter durations with stems, but the beams are placed centered between the 
whole notes (i.e. not where an imaginary stem would end!). The only situation 
where an imaginary stem is used to determine the position/slant of the beams 
is in cross-staff tremolos.

If you want, I can take images of those pages.

Cheers,
Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

[hanwenn, 2011-02-02 14:54:39]

On Wed, Feb 2, 2011 at 3:49 AM, Werner LEMBERG <wl@gnu.org> wrote:
>> There is one diff remaining: see the attached file.  I introduced
>> some streamlining of an inner loop, which affects the way we handle
>> beams that start/end with invisible stems.
>>
>> The compare image is the new one.  I have no examples of tremolo beams
>> handy, but the new layout actually looks better to me.
>
> For me, it doesn't.  While the old behaviour is not optimal, the new
> one looks really worse to me, especially in bar 3 (which is admittedly
> a quite artificial example).  The most important issue for me: Why is
> there no slant of the beams?  I currently have not enough time to

But the old version also had no slant?  I'm inclined to take interpret
all these answers as "tremolo on whole notes was broken, and it still
is, just differently," and commit.

My concern is about breaking something that was working as intended before.

-- 
Han-Wen Nienhuys - hanwen@xs4all.nl - http://www.xs4all.nl/~hanwen

[mike_apollinemike.com, 2011-02-03 02:16:33]

On Feb 2, 2011, at 9:54 AM, Han-Wen Nienhuys wrote:

> On Wed, Feb 2, 2011 at 3:49 AM, Werner LEMBERG <wl@gnu.org> wrote:
>>> There is one diff remaining: see the attached file.  I introduced
>>> some streamlining of an inner loop, which affects the way we handle
>>> beams that start/end with invisible stems.
>>> 
>>> The compare image is the new one.  I have no examples of tremolo beams
>>> handy, but the new layout actually looks better to me.
>> 
>> For me, it doesn't.  While the old behaviour is not optimal, the new
>> one looks really worse to me, especially in bar 3 (which is admittedly
>> a quite artificial example).  The most important issue for me: Why is
>> there no slant of the beams?  I currently have not enough time to
> 
> But the old version also had no slant?  I'm inclined to take interpret
> all these answers as "tremolo on whole notes was broken, and it still
> is, just differently," and commit.
> 
> My concern is about breaking something that was working as intended before.

Hey all,

Han-Wen: I know that you had mentioned earlier revisiting the reasonable_score
problem.  A thought just occurred to me.  In a loop like:

 for (vsize i = configs.size (); i--;)
   if (configs[i]->demerits < reasonable_score)
     {
       score_count ++;
       score_stem_lengths (configs[i]);
     }

If the function score_stem_lengths returned the value of the score, then you
could do something like:

vector<Beam_configuration*> rejects;
Real high_score = 0.0;
 for (vsize i = configs.size (); i--;)
   if (configs[i]->demerits < reasonable_score)
     {
       score_count ++;
       high_sore = max (high_score, score_stem_lengths (configs[i]));
     }
  else
    rejects.push_back (configs[i]);

 for (vsize i = rejects.size(); i--)
   rejects[i]->add (high_score, "F");

 rejects.clear ();

I don't have a good sense of how much compiling time this would add on, but it
is a way to pad the unreasonable scores with a value that preserves their
unreasonableness.

Cheers,
Mike