Re: Avoid repeats of 'staff-affinity' warning; change text. (issue4278058)

[Joe Neeman]

On Mon, Mar 21, 2011 at 8:11 PM, Keith OHara <address@hidden> wrote:

On Sat, 19 Mar 2011 22:46:51 -0700, Joe Neeman <address@hidden> wrote:

It might cause problems if "pure" is true. When the method is called with
"pure," it shouldn't cause any side effects. For a concrete example, this
will mess up if you have

Staff
Lyrics with affinity down
Staff that sometimes disappears
Lyrics with affinity up
Staff

Unfortunately, if I protect the assignment to the property with an  if (!pure), I am letting the page-breaking planning rely on the user-requested affinities, and then changing them for the page-layout phase.  The boolean 'pure' asks explicitly that we keep the state unchanged, but there was always an implicit expectation that certain properties are unchanging.

I don't quite understand this comment. The only effect of set_property is to prevent the warning from being triggered more than once per system. In fact, the layout would be completely unchanged even if you removed the whole if(after_affinity > before_affinity) block. So why does it matter if we condition set_property on something?

On Mon, 21 Mar 2011 19:22:49 -0700, <address@hidden> wrote:

Wouldn't a comment be better then?

Yep. I'll add a comment next time I have Linux up.


The only effect of the original code was to give warning, not to change the effective affinity, demonstrating that nothing explodes if 'staff-affinities increase'.  Do you remember what the warning intended to protect against?

I think it was just to let the user know not to expect a sane layout.

The serious problem (coming up again in issue 1569) occurs when staff-affinity of the first or last non-staff points to a spaceable staff that is not present in the system.  It seems this needs to be caught one or two layers up, in distribute_loose_lines() or maybe better in Align_interface::internal_get_minimum_translations()

So I'll tidy up this small issue, but let's start looking at the problem of unrequited affinity of loose lines at the top/bottom of the system.

I think the problem is that not enough space is being reserved in the first pass (ie. the non-loose lines part) of the layout and so the loose lines don't fit. Align_interface::internal_get_minimum_translations may be responsible, but it isn't actually used for distances between systems, so it's also worth investigating bottom_skyline_.

Unfortunately, I'm going backpacking starting tomorrow, but I'm happy to answer questions once I get back (Sunday).

Cheers,
Joe