Re: [PATCH] Clusters

[Juergen Reuter]

On Fri, 15 Nov 2002, Han-Wen Nienhuys wrote:

> ...
> Your patch looks like a kludge.
>
> The real problem is that it does
>
>      typeset_grob (cluster);
>      cluster = 0;
>
> from process_music(), which happens before the last notes are caught
> in acknowledge_grob().  Move the typeset_grob() to
> stop_translation_timestep(), and you should be fine.

I do not understand.  This is exactly, what my patch *does*: it moves
typeset_grob() to stop_translation_timestep().

> ...
> Some other nitpicks:
>
>   * member naming is inconsistent with the standards. Please use
>     "member_" (note the final underscore.) instead of "member"
>

Ok, will eventually be done.

>   * The engraver duplicates the mistake of the ambitus engraver,
>     namely that it puts musical info (pitches) into the backend, all
>     the more since all that is needed is the vertical difference in
>     position between two note heads. Try
>     Staff_symbol_referencer::staff_position() for getting the position
>     of a head.
>

Ok, sounds reasonable.

>     Better yet, simply store a list of note heads in the cluster grob,
>     and let the Cluster grob sort out X and Y-positions. [*]
>

This may turn out to produce unexpected behaviour, e.g. if a user removes
the noteheads engraver from a voice rather than just setting noteheads
transparent.

>   * The contents of columns_scm are not protected from Garbage
>     collection. Your code will make mysterious ugly crashs on larger
>     scores. Use Protected_scm, or better yet, use [*]
>

Ok, will be fixed (btw. what did you mean with "[*]"?).

> (oh, btw, I'm probably sounding terribly grumpy. Sorry.  Clusters are
> cool.)
>

Thanks for the compliment. ;-)

> ...
> i.e. something like
>
>   startCluster = \notes {
>     #(ly:export (make-request-chord (list
>          (make-span-event 'ClusterEvent START))))
>   }
>

Ok, the following seems (almost) to work:

startCluster = \notes {
  #(ly:export (make-event-chord (list
                                 (make-span-event 'ClusterEvent START))))
  \property Thread.NoteHead \override #'transparent = ##t
  \property Voice.Stem \override #'transparent = ##t
  \property Voice.Beam \override #'transparent = ##t
  \property Staff.Accidental \override #'transparent = ##t
}

stopCluster = \notes {
  #(ly:export (make-event-chord (list
                                 (make-span-event 'ClusterEvent STOP))))
  \property Thread.NoteHead \revert #'transparent
  \property Voice.Stem \revert #'transparent
  \property Voice.Beam \revert #'transparent
  \property Staff.Accidental \revert #'transparent
}

with a little problem:

> then you can do
>
>      \notes { c4 \startCluster d4 e4 }
>
> but the \startCluster will happen after the C, simultaneously with the
> start of the D.
>
> If startCluster is an event identifier, you can do
>
>   \notes { c4-\startCluster d4 e4 }
>
> and the startCluster happens simultaneously with the C.

Right, \stopCluster as defined above now stops the cluster and makes the
note heads visible simultaneously, which is not what you expect.  So,
looking at the current definition

stopCluster = #(make-span-event 'ClusterEvent STOP)

(which are both accepted by the parser when appearing alone) I would like
to combine these two approaches to a single definition, as in:

stopCluster = #(make-span-event 'ClusterEvent STOP) \notes {
  \property Thread.NoteHead \revert #'transparent
  \property Voice.Stem \revert #'transparent
  \property Voice.Beam \revert #'transparent
  \property Staff.Accidental \revert #'transparent
}

which again gives me a parse error.

> Jan insists
> that without the dash it is easier to use, so we also allow
>
>   \notes { c4 \startCluster d4 e4 }
>
> to mean the same. As you can see, this is a little ambiguous, since it
> means something different when \startCluster is a music identifier.
> (I'm still uncertain whether this is an improvement)
>

Ah!  That is, the above combination gives me a parser error, since it is a
mixture of a music identifier and an event.  Mmh, well, this may lead to
unexpected behaviour of the parser.

> ...
> You can download the PFA fonts assuming that the latest tarball built
> correctly with the following script
> ...

Ok, thanks, I'll try it next time.

> what I really mean, is that you should also build the internal
> documentation. That's also done if you do
>
>    make -C  Documentation/user/
>

Ok.

Greetings,
Juergen