Re: reorganize self_alignment_interface (issue 7768043)

[address@hidden]

On 19 mars 2013, at 22:04, address@hidden wrote:

> Hi Mike & all,
> 
> On 2013/03/19 15:21:37, MikeSol wrote:
>> I like where you're going with this.
> 
> cool! :)
> 
>> I would recommend creating a function algined_on_grobs that accepts a
> grob, an
>> vector of grobs to align to, and an axis.  Then algined_on_parent
> becomes a
>> subset of this where the vector is of length one and is the parent.
>> Then, you can create a function
>> Self_alignment_interface::lyrics_algined_on_note_columns that invokes
>> aligned_on_grobs with a vector of note columns returned using the
> search method
>> in this function.
> 
> On 2013/03/19 15:28:45, MikeSol wrote:
>> With respect to my last review, this function should accept a pointer
> to a
>> vector of grob pointers:
> 
>> vector<Grob *> &hims
> 
>> so that you can do the note column stuff for lyrics.
> 
> I don't think this (defining special lyrics_algined_on_note_columns
> callback) is optimal approach.
> 
> With your suggestion, here's how i expect alignment stuff would look
> like:
> - there'd be a method Self_alignment_interface::aligned_on_grobs that
> can align a grob on another grob(s)
> - we'd have two callbacks:
>  Self_alignment_interface::lyrics_aligned_on_note_columns
>  and
>  Self_alignment_interface::lyrics_aligned_on_parent_notehead
> - we'd have to decide when which callback should be used.  How should
> LyricText definition in define-grobs.scm look like?  We should use
> lyrics_aligned_on_note_columns when lyrics are unassociated, and
> lyrics_aligned_on_parent_notehead when there is an associatedVoice - how
> to write that in define-grobs?
> 
> And the same thing will be repeated for many other grobs, for example
> DynamicTexts: we'll have to specify different callback for DynamicTexts
> attached to NoteHeads, and different one for standalone Dynamics (e.g.
> in Dynamic Context).
> 
> How alignment stuff would look like with my approach:
> - aligned_on_grobs would be more complicated, because it'd have to grab
> NoteColumn extent if 'him' turns out to be a PaperColumn
> - we'd have one callback for everything
> - we won't have to think about anything when defining grobs.  We'd just
> write X-offset = ly:self-alignment-interface::align-grob, and it would
> be aligned_on_grobs' job to align on appropriate grob if the default one
> (parent) is inappropriate.
> 
> The difference is conceptual: aligning on NoteColumn is an exception, an
> emergency exit needed only when grob doesn't have an ordinary parent.
> As far as i know, if grob's parent is a NoteHead or some other "normal"
> grob, we always want to align on that; and when grob's parent is a
> PaperColumn, we always want to align grob on an appropriate NoteColumn.
> 
> As for using a vector 'hims' instead of single 'him', maybe it's a good
> idea, but i don't see any uses for it (besides your above suggestion,
> which seems to be unnecessary).

The main reason that I suggest this is that exceptions in the code base lead to 
harder maintenance.  I am guilty of adding some from time to time (i.e. in 
stencil integral) but I try to get rid of them as fast as possible.  One 
example in the code base that I don't like: check out line 136.  Granted, 
whoever created it doesn't like it either from the looks of the comment.  It 
makes the code harder to read, maintain and understand.  The worst offender by 
far is the metronome mark - there are so many exceptions concerning who its 
parent is that it leads to actual problems with layout in certain cases.

I definitely like your idea of coming up with a single function, though.  
Perhaps try the following:

-) always decompose axis-groups into elements (recursively if need be)
-) create a non_musical flag that, if set, weeds out all grobs where 
non-musical is set to #t

This may achieve the same effect but it is a lot more general, as the 
distinction is between musical and non-musical rather than between note column 
and everything else.

Cheers,
MS