Re: Tied accidentals in chords still taking up space (was: Issue 415 in lilypond: Hidden accidental of tied note still takes space)

[Han-Wen Nienhuys]

On Thu, Mar 26, 2009 at 3:21 PM, Joe Neeman <address@hidden> wrote:
>> > One solution might be the following, which involves modifying only
>> > accidental-placement: make 2 copies of every accidental, one for the
>> > start-of-a-line case and one for the middle-of-a-line case (so the
>> > start-of-a-line accidentals would be, in the terminology of
>> > Accidental_placement::split_accidentals, the break_reminder accidentals
>> > and the real_accidentals while the middle-of-a-line accidentals would
>> > have copies of only the real_accidentals). Run the accidental layout
>> > algorithm (currently in Accidental_placement::calc_positioning_done, but
>> > you'll want to move it to a different function and make
>> > calc_positioning_done call it twice) twice, once for each set of
>> > accidentals. Then modify Accidental_placement::get_relevant_accidentals
>> > to return one of the sets of accidentals.
>>
>> I've looked into this approach, and have a few questions:
>>
>> I tried cloning the accidentals in Accidental_placement::add_accidental,
>> but that led to a segfault later on while trying to calculate the
>> X-extent of the cloned accidental, since its layout_ was null. From what
>> I can tell, this is because the cloned grob was never announced. Perhaps
>> I should modify accidental-engraver as well, and have it create two
>> copies there, passing them both to Accidental_placement::add_accidental?
>
> It's not necessary to announce every grob (for example, of the three
> copies of every breakable grob, only the middle-of-line copy gets
> announced). The code for creating these non-announced grobs is in
> Item::copy_breakable_items. In particular, you need the
> get_root_system(original_grob)->typeset_grob(cloned_grob) line to give
> your clone a layout_.

This is not applicable here: the accidentals themselves are not on
linebreaks, spacing wise, so there should be only one copy of them.
Also, I seem to recall that we already have support for this in the
form of  conditional elements (grep for conditional) in skyline and
separation items.  Why aren't we using or extending that?

The last time I looked, I had to conclude the wenot can ever
completely get correct behavior for tie-accidental reminder; I forgot
the details though.  Can you point me to the discussion that led to
this patch?  It seems that the conversation is fragmented over several
threads.


>> Which copy of the accidental should be put into the accidental-grob
>> property of the note head?
>
> This is a bit tricky. To do this properly, you might need to add a
> Note_head::get_accidental function that has the logic for finding the
> right accidental (you could create a new property,
> "break-reminder-accidental-grob", for the new copy). Then you'd need to
> change every place that does get_object("accidental-grob") to use the
> new function; there aren't very many such places. It should be safe to
> just pick one copy if you want to put this off until later.

I've glossed over the patch, and I am less than enthused: it seems to
add a lot of special casing and duplicate code for the reminder and
normal accidentals, and I have seen very dubious constructs like
get_property("accidental-grob") rather than get_object.



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