Re: Fixes slope errors from incorrect X extents in Beam::print. (issue 5293060)

[address@hidden]

On Oct 26, 2011, at 9:41 PM, address@hidden wrote:

> On Wed, 26 Oct 2011 11:40:27 -0700, address@hidden
> <address@hidden> wrote:
> 
>> What needs comments?
> 
> Description of the different setup of the three instances of
> Beam_scoring_problem, and their goals.
> 
Done

>> -) the x-span of the Beam are stored in X-positions
> (calc_x_positions).  these are used everywhere that the span of the beam
> used to be calculated.  the slopes of broken beams were off before
> because x_spans were calculated 3 different ways in 3 different
> functions, and this standardizes it.
> 
> What is the standard ?

From the left end of the visible beam (including all overhang) to the right 
end.  It is gotten by taking a union between the extents of all the segments 
under the assumption (which I believe to be correct) that the union of the 
X-extent of all of the beam segments is the only true measure of a beam's 
extent.  Measuring between normal stems or left and right bounds or mixtures of 
the two (which were other ways of doing it in the old code) lead to correct 
results where the first-normal-stem = bound = overhang, but for broken beams, 
this is rarely the case.  So, the standard is a union between the extents of 
all the segments.

> What is the span of the beam r8[ g' \beam a' r] ?

Thanks for the idea - I've put it as a regtest (see the attached PDF for the 
output).

\paper { ragged-right = ##t }
{
 r2.
 \override Beam #'breakable = ##t
 r8[ g' \break a' r]
}
{
 r2.
 \override Beam #'broken-beam-style = #'strict-prolong'ation
 \override Beam #'breakable = ##t
 r8[ g' \break a' r]
}
{
 r2.
 \override Beam #'broken-beam-style = #'peters-prolong'ation
 \override Beam #'breakable = ##t
 r8[ g' \break a' r]
}

I think all three options do what they're supposed to.  In the first, the two 
slopes do not know of each other's existence and thus give flat slopes (with 
the new warning message that slopes cannot be calculated from a stem of 1).  In 
the second, as expected, the slope on the second line is a prolongation of that 
on the first.  And in the third example, the beam on the second line is 
slightly higher to represent the higher change in note.

Granted, it's not perfect.  Change it to r8[g \break a'' r] and you'll see same 
ugliness in current master and in the present patchset.  But I'd rather save 
this fix for a different patchset given the size of the current patchset.

> What about the x_span_ of the Beam_scoring_problem ?
> 

It represents two things at two different stages of Beam_scoring_problem.
In least_squares_positions (), slope_damping (), and shift_region_to_valid () 
it represents the distance between the first and last normal stem in the 
Beam_scoring_problem space, which always starts at 0 for X and extends until 
the end of the beam (either an individual beam or all the parts of a broken 
beam if we're calculating consistent slopes).

account_for_extremal_hangover () doesn't touch x_span_.

Starting at update_x_span_after_extremal_hangover_compensation (), the x_span_ 
becomes the real span of the beam, tacking on left and right overhang that may 
come from stemlets or broken beams.  This is necessary for the quanting (we 
always want edges quanted) but not possible for the slope calculations, which 
are predicated on the idea that unquanted_y_ represents the beginning and end 
of the beam (otherwise, we'd have to compensate for extremal hangover in each 
function).


> Consistent slopes help a bit, but they way there is done here, the risks
> of code complexity seem to outweigh the benefit.

I do not see where the code is complex save perhaps Beam::quanting, which now 
has lots of comments as per your suggestion.

Otherwise, the bulwark of changes in beam.cc come from the transformation of 
two internal calculations (the beam segments and the x positions) into 
properties with callbacks.  I think that this simplifies and standardizes the 
code in several areas.  In beam-quanting.cc, the only major changes come from 
the addition of account_for_extremal_hangover () and 
update_x_span_after_extremal_hangover_compensation (), which are necessary to 
use the correct x_span_.

Of course, my goal is to make the change with the least complexity possible.  
If you have any suggestions on how to cut down complexity, I definitely want to 
hear them.

> The difference between
> consistent_broken_slope_ and consistent_broken_slope is dangerous all by
> itself.
> 

I'm not sure what you mean - how is this dangerous?

>> Take it for a spin!
> 
> Sorry. I have lost interest.
> 
> 
> http://codereview.appspot.com/5293060/diff/2004/lily/beam-quanting.cc
> File lily/beam-quanting.cc (right):
> 
> http://codereview.appspot.com/5293060/diff/2004/lily/beam-quanting.cc#newcode383
> lily/beam-quanting.cc:383: x_span_ -= extremal_hangover_[d];
> so x_span_ is the span from first normal stem to last normal stem.

In least_squares_positions (), slope_damping (), and shift_region_to_valid ().  
Starting from update_x_span_after_extremal_hangover_compensation (), this is 
tacked back on.

foo.pdf
Description: Adobe PDF document