Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
I gather this is connected to issue 684, so it should probably be marked closed, Jan. Thanks, Colin http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/08/30 07:41:55, janneke-list_xs4all.nl wrote: Thanks! There seems to be no difference in the metronome-marking-break-align regression test file, you'll want to amend it so that it shows what is fixed. Yes, I found that slightly puzzling, since your patch produced verifiable changes in the existing tests for metronome marks. Cheers, Neil http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op zondag 29-08-2010 om 23:27 uur [tijdzone +], schreef n.putt...@gmail.com: Neil, I've posted a fix for all these issues here: http://codereview.appspot.com/2042043/ Thanks! There seems to be no difference in the metronome-marking-break-align regression test file, you'll want to amend it so that it shows what is fixed. Greetings, Jan http://codereview.appspot.com/1579041/ -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyOfSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
http://codereview.appspot.com/1579041/diff/42001/43004 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/42001/43004#newcode109 lily/metronome-engraver.cc:109: } this is out of style with the rest of lilypond code base. The normal pattern is to use XXx_interface::has_interface (for hard-coded interfaces), or grob-[internal_]has_interface(), to check for softcoded interfaces. The reason for this is that Grob names are only used as keys at creation time, and for providing context in debugging (grep for -name() ). The rest of the code uses interface names rather grob name, so we can easily swap in and out implementations of formatting behavior. Sorry for coming with this after the fact, but could you fix the implementation to be in accordance with this? This engraver should call grob-has_interface() for each of the symbols from non-break-align-symbols. thanks http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On Sun, Aug 29, 2010 at 3:54 PM, Jan Nieuwenhuizen janneke-l...@xs4all.nl wrote: Op zondag 29-08-2010 om 18:04 uur [tijdzone +], schreef hanw...@gmail.com: http://codereview.appspot.com/1579041/diff/42001/43004#newcode109 lily/metronome-engraver.cc:109: } this is out of style with the rest of lilypond code base. The normal pattern is to use XXx_interface::has_interface (for hard-coded interfaces), or grob-[internal_]has_interface(), to check for softcoded interfaces. Thanks for catching this. I've applied the patch below. LGTM -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op zondag 29-08-2010 om 18:04 uur [tijdzone +], schreef
hanw...@gmail.com:
http://codereview.appspot.com/1579041/diff/42001/43004#newcode109
lily/metronome-engraver.cc:109: }
this is out of style with the rest of lilypond code base.
The normal pattern is to use XXx_interface::has_interface (for
hard-coded interfaces), or grob-[internal_]has_interface(), to check
for softcoded interfaces.
Thanks for catching this. I've applied the patch below.
Jan.
From 19b37df119ff6ca84421d984fe1d33112ad08299 Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen jann...@gnu.org
Date: Sun, 29 Aug 2010 20:51:49 +0200
Subject: [PATCH] Metronome mark: check for interface rather than grob name in
non-break-aligned.
This remove the usage of grob_name so so we can easily swap in and out
implementations of formatting behavior.
---
lily/metronome-engraver.cc | 19 ++-
scm/define-grob-properties.scm |2 +-
scm/define-grobs.scm |2 +-
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/lily/metronome-engraver.cc b/lily/metronome-engraver.cc
index 7eaf314..aed0275 100644
--- a/lily/metronome-engraver.cc
+++ b/lily/metronome-engraver.cc
@@ -99,24 +99,17 @@ Metronome_mark_engraver::acknowledge_break_aligned
(Grob_info info)
}
}
-SCM
-grob_name_scm (Grob *g)
-{
- SCM name_pair = scm_assq (ly_symbol2scm (name), g-get_property (meta));
- return (scm_is_pair (name_pair)
- ? ly_camel_case_2_lisp_identifier (scm_cdr (name_pair))
- : SCM_EOL);
-}
-
void
Metronome_mark_engraver::acknowledge_grob (Grob_info info)
{
Grob *g = info.grob ();
- if (text_
- safe_is_member (grob_name_scm (g),
-text_-get_property (non-break-align-symbols)))
-text_-set_parent (g, X_AXIS);
+ if (text_)
+for (SCM s = text_-get_property (non-break-align-symbols);
+scm_is_pair (s);
+s = scm_cdr (s))
+ if (g-internal_has_interface (scm_car (s)))
+ text_-set_parent (g, X_AXIS);
}
void
diff --git a/scm/define-grob-properties.scm b/scm/define-grob-properties.scm
index d4ee62b..4202b26 100644
--- a/scm/define-grob-properties.scm
+++ b/scm/define-grob-properties.scm
@@ -608,7 +608,7 @@ staff is crucial for @var{padding}).
@code{VerticalAlignment}; rather, place it using its own
@code{Y-offset} callback.)
(non-break-align-symbols ,list? A list of symbols that determine
-which NON-break-aligned grobs to align this to.)
+which NON-break-aligned interfaces to align this to.)
(no-ledgers ,boolean? If set, don't draw ledger lines on this
object.)
(no-stem-extend ,boolean? If set, notes with ledger lines do not
diff --git a/scm/define-grobs.scm b/scm/define-grobs.scm
index 38bd6eb..71a167d 100644
--- a/scm/define-grobs.scm
+++ b/scm/define-grobs.scm
@@ -1171,7 +1171,7 @@
(list
ly:self-alignment-interface::x-aligned-on-self)
(self-alignment-X . ,LEFT)
(break-align-symbols . (key-signature time-signature))
- (non-break-align-symbols . (multi-measure-rest))
+ (non-break-align-symbols . (multi-measure-rest-interface))
(non-musical . #t)
(meta . ((class . Item)
(interfaces . (break-alignable-interface
--
1.7.0.4
1
--
Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org
Freelance IT http://JoyOfSource.com | Avatar® http://AvatarAcademy.nl
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
First, great to see that this feature has finally been implemented and pushed. Unfortunately, it seems that it needs some small tweaking, still. The problem is that the metronome mark is now placed directly above the key signature, while Gardner Read says that it is aligned over the meter signature, or -- if none is present -- over the first notational element of the measure, such as note-heads, accidentals, repeat signs, and so on. (Gardner Read, p.278) So, it should never be aligned with the key signature, but rather with the time signature or the first element after that... Actually, Gardner Read shows an example with a key signature and a metronome mark, where the metronome mark is really aligned with the time signature and not with the key signature... http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
http://codereview.appspot.com/1579041/diff/42001/43004 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/42001/43004#newcode81 lily/metronome-engraver.cc:81: } is there a reason you are worried about cyclical data structures? I don't think we check for them anywhere else. http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op zondag 29-08-2010 om 20:09 uur [tijdzone +], schreef hanw...@gmail.com: http://codereview.appspot.com/1579041/diff/42001/43004 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/42001/43004#newcode81 lily/metronome-engraver.cc:81: } is there a reason you are worried about cyclical data structures? I don't think we check for them anywhere else. It's not cyclic, the idea was about not crashing when the data structure is not a list at all. I think Neil had a remark about this. Jan. -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyOfSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On Sun, Aug 29, 2010 at 5:21 PM, Jan Nieuwenhuizen janneke-l...@xs4all.nl wrote: It's not cyclic, the idea was about not crashing when the data structure is not a list at all. I think Neil had a remark about this. Most of the similar code uses scm_c_memq instead, which skips the checking. On closer inspection, scm_ilength (which does the actual checking) also catches other errors besides circular lists. We could make more strict checking a habit, but it's not really a scalable, since many routines require more complex data structures as input parameters. -- Han-Wen Nienhuys - han...@xs4all.nl - http://www.xs4all.nl/~hanwen ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/08/29 19:54:28, Reinhold wrote:
First, great to see that this feature has finally been implemented and
pushed.
Unfortunately, it seems that it needs some small tweaking, still. The
problem is
that the metronome mark is now placed directly above the key
signature, while
Gardner Read says that it is aligned over the meter signature, or --
if none is
present -- over the first notational element of the measure, such as
note-heads,
accidentals, repeat signs, and so on. (Gardner Read, p.278)
So, it should never be aligned with the key signature, but rather with
the time
signature or the first element after that... Actually, Gardner Read
shows an
example with a key signature and a metronome mark, where the metronome
mark is
really aligned with the time signature and not with the key
signature...
Hmm, I took that to mean key signature as first notational element, but
checking a few scores at random suggests it to be uncommon.
Unfortunately, I didn't do any tests with key signatures present, but
it's actually worse than you've found: the Metronome_engraver
acknowledges *all* key signatures, including those which will be
suicided later due to break-visibility. This means that if there's a
key signature present, it hijacks the metronome mark's positioning for a
note at the start of a bar, resulting in its being aligned on the
barline:
\relative c' {
\key g \major
% \override Score.MetronomeMark #'break-align-symbols =
#'(time-signature)
c1
\tempo Allegro
c1
}
On a related note, I see now why the Metronome_engraver ignores the
ordering of break-align-symbols: it doesn't acknowledge a BreakAlignment
(since it needs to acknowledge the break-aligned directly).
http://codereview.appspot.com/1579041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/08/29 22:02:18, Neil Puttock wrote:
On 2010/08/29 19:54:28, Reinhold wrote:
First, great to see that this feature has finally been implemented
and pushed.
Unfortunately, it seems that it needs some small tweaking, still.
The problem
is
that the metronome mark is now placed directly above the key
signature, while
Gardner Read says that it is aligned over the meter signature, or
-- if none
is
present -- over the first notational element of the measure, such as
note-heads,
accidentals, repeat signs, and so on. (Gardner Read, p.278)
So, it should never be aligned with the key signature, but rather
with the
time
signature or the first element after that... Actually, Gardner Read
shows an
example with a key signature and a metronome mark, where the
metronome mark is
really aligned with the time signature and not with the key
signature...
Hmm, I took that to mean key signature as first notational element,
but checking
a few scores at random suggests it to be uncommon.
Unfortunately, I didn't do any tests with key signatures present, but
it's
actually worse than you've found: the Metronome_engraver acknowledges
*all* key
signatures, including those which will be suicided later due to
break-visibility. This means that if there's a key signature present,
it
hijacks the metronome mark's positioning for a note at the start of a
bar,
resulting in its being aligned on the barline:
\relative c' {
\key g \major
% \override Score.MetronomeMark #'break-align-symbols =
#'(time-signature)
c1
\tempo Allegro
c1
}
On a related note, I see now why the Metronome_engraver ignores the
ordering of
break-align-symbols: it doesn't acknowledge a BreakAlignment (since it
needs to
acknowledge the break-aligned directly).
I've posted a fix for all these issues here:
http://codereview.appspot.com/2042043/
http://codereview.appspot.com/1579041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Thanks. Applied and set to Fixed. http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan,
I've tested the latest patch thoroughly, and it seems fine for the most
part.
The only niggle I've come across is with full-bar rests at the start of
a system:
\relative c' {
c1 \break
\tempo 4 = 60
R1
}
\paper { ragged-right = ##t }
Ideally, the tempo mark would be positioned after the clef in this case,
but I can't see how we can automate this without breaking the alignment
in other situations.
Cheers,
Neil
http://codereview.appspot.com/1579041/
___
lilypond-devel mailing list
lilypond-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/08/24 13:26:14, jan.nieuwenhuizen wrote: Obviously, I missed your last set of comments. I removed metronome-mark from the break-aligned lists and removed the break-aligned-interface. The self-alignment-interface was already added in a previous version of the patch, I think. OK, so it's unlikely, but a user might use a callback to set 'break-align-symbols instead of a simple list. lily/metronome-engraver.cc:88: text_-get_property_data (break-align-symbols)) get_property () Ah, I see. All changed. Cheers, looks fine. I'm testing at the moment; will report back shortly. I'm curious about other get_property_data () usages though, in beam, bar-number-engraver, etc? I think in the case of beam-engraver.cc it's used to check whether there's been an explicit setting for staff-position (rather than relying on a callback): if so, it comes from a manually positioned rest (e.g., c4\rest) so doesn't need to be chained for beam avoidance. http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
From whom? Obviously, I missed your last set of comments. I removed metronome-mark from the break-aligned lists and removed the break-aligned-interface. The self-alignment-interface was already added in a previous version of the patch, I think. OK, so it's unlikely, but a user might use a callback to set 'break-align-symbols instead of a simple list. lily/metronome-engraver.cc:88: text_-get_property_data (break-align-symbols)) get_property () Ah, I see. All changed. I'm curious about other get_property_data () usages though, in beam, bar-number-engraver, etc? I also added list_p checks. + (non-musical . #t) Added. http://codereview.appspot.com/1579041/ ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/07/27 14:14:12, janneke-list_xs4all.nl wrote: I'm waiting for an ack. From whom? http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Am Sonntag, 20. Juni 2010, 23:57:20 schrieb n.putt...@gmail.com: Here are some more comments for you. What happened to this patch? AFAICS, it has not been pushed to master, right? I'm just afraid that it might be forgotten, which would be very bad, since I need this in LilyPond, too... Cheers, Reinhold -- -- Reinhold Kainhofer, reinh...@kainhofer.com, http://reinhold.kainhofer.com/ * Financial Actuarial Math., Vienna Univ. of Technology, Austria * http://www.fam.tuwien.ac.at/, DVR: 0005886 * LilyPond, Music typesetting, http://www.lilypond.org ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op dinsdag 27-07-2010 om 15:45 uur [tijdzone +0200], schreef Reinhold Kainhofer: What happened to this patch? AFAICS, it has not been pushed to master, right? I'm waiting for an ack. Greetings, Jan. -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyOfSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan, On 2010/06/22 19:01:58, jan.nieuwenhuizen wrote: Alas, it can't. When adding staff-bar to that list, the broken marks appear next to the bar numbers, right at the start of the staves. That's a shame. It would be great if the symbol order in 'break-align-symbols determined the priority (like it does for RehearsalMark/BarNumber). http://codereview.appspot.com/1579041/diff/19001/20007#newcode418 scm/define-grobs.scm:418: metronome-mark Is this necessary? IIUC, only break-aligned grobs will be acknowledged by the Break_align_engraver, so a MetronomeMark will never appear in the list of elements for ordering. Okay...so I've added the break-aligned-interface. It makes sense to have this position added - this is probably not the only symbol that we want positioned like this? I'm still confused as to why it's necessary, since it's only useful for ordering break-aligned grobs inside the stave; ditto for the defaults for 'break-align-symbol and 'break-align-symbols. http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op zondag 04-07-2010 om 19:54 uur [tijdzone +], schreef n.putt...@gmail.com: Alas, it can't. When adding staff-bar to that list, the broken marks appear next to the bar numbers, right at the start of the staves. That's a shame. It would be great if the symbol order in 'break-align-symbols determined the priority (like it does for RehearsalMark/BarNumber). It does. The symbol order does determine the priority. Problem is, at start of a stave the *only* symbol that is present is the staff bar. Okay...so I've added the break-aligned-interface. It makes sense to have this position added - this is probably not the only symbol that we want positioned like this? I'm still confused as to why it's necessary, since it's only useful for ordering break-aligned grobs inside the stave; ditto for the defaults for 'break-align-symbol and 'break-align-symbols. It is not necessary, we could scrap both of these. I figure however, that when adding break-aligned-ness to other engravers (text-spanner etc), it could would be nice if they could make use of mm-rest's alignment position. Possibly it's better to remove both and add when needed -- I don't know. -- Jan Nieuwenhuizen jann...@gnu.org | GNU LilyPond http://lilypond.org Freelance IT http://JoyOfSource.com | Avatar® http://AvatarAcademy.nl ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/07/04 20:36:28, janneke-list_xs4all.nl wrote: It does. The symbol order does determine the priority. Hmm, this doesn't appear to be the case, since key signatures are preferred over time signatures for aligning. Problem is, at start of a stave the *only* symbol that is present is the staff bar. Ah, good point. :) It would prevent aligning on notes. Okay...so I've added the break-aligned-interface. It makes sense to have this position added - this is probably not the only symbol that we want positioned like this? I'm still confused as to why it's necessary, since it's only useful for ordering break-aligned grobs inside the stave; ditto for the defaults for 'break-align-symbol and 'break-align-symbols. It is not necessary, we could scrap both of these. I figure however, that when adding break-aligned-ness to other engravers (text-spanner etc), it could would be nice if they could make use of mm-rest's alignment position. Possibly it's better to remove both and add when needed -- I don't know. I'd rather remove them, since it's likely to cause confusion among users. Below, I've suggested adding 'non-musical, since it improves spacing for full-bar rests: there's currently a bit of extra space added between the non-musical and musical paper columns. http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
http://codereview.appspot.com/1579041/diff/30001/31004 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/30001/31004#newcode82 lily/metronome-engraver.cc:82: g-get_property_data (break-align-symbol) text_-get_property (break-align-symbol) OK, so it's unlikely, but a user might use a callback to set 'break-align-symbols instead of a simple list. http://codereview.appspot.com/1579041/diff/30001/31004#newcode88 lily/metronome-engraver.cc:88: text_-get_property_data (break-align-symbols)) get_property () http://codereview.appspot.com/1579041/diff/30001/31004#newcode112 lily/metronome-engraver.cc:112: text_-get_property_data (non-break-align-symbols)) get_property () http://codereview.appspot.com/1579041/diff/30001/31008 File scm/define-grobs.scm (right): http://codereview.appspot.com/1579041/diff/30001/31008#newcode1159 scm/define-grobs.scm:1159: (non-break-align-symbols . (multi-measure-rest)) + (non-musical . #t) http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
So, how are we doing here? Ready to commit close http://code.google.com/p/lilypond/issues/detail?id=684 http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan, On 2010/07/01 08:08:36, jan.nieuwenhuizen wrote: So, how are we doing here? Ready to commit close I'm testing the latest set at the moment; will report back with more comments. Cheers, Neil http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
On 2010/06/20 21:57:20, Neil Puttock wrote: AARGH. Again this web interface eats my email. Is anyone using this, it is so frustrating. I'm going to try using plain email response now. http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Neil, Not sure how to find the email address to bind this to http://codereview.appspot.com/1579041 ? http://codereview.appspot.com/1579041/diff/19001/20003#newcode82 lily/metronome-engraver.cc:82: == ly_symbol2scm (staff-bar)) can't this be incorporated into 'break-align-symbols for MetronomeMark? Alas, it can't. When adding staff-bar to that list, the broken marks appear next to the bar numbers, right at the start of the staves. though I'd prefer more lisp-like syntax for this using camel_case_to_lisp_identifier () Done. http://codereview.appspot.com/1579041/diff/19001/20006#newcode610 scm/define-grob-properties.scm:610: (non-break-align-symbols ,list? A list of symbols that determine needs adding to an interface Added to the break-aligned-interface. http://codereview.appspot.com/1579041/diff/19001/20007 File scm/define-grobs.scm (right): http://codereview.appspot.com/1579041/diff/19001/20007#newcode418 scm/define-grobs.scm:418: metronome-mark Is this necessary? IIUC, only break-aligned grobs will be acknowledged by the Break_align_engraver, so a MetronomeMark will never appear in the list of elements for ordering. Okay...so I've added the break-aligned-interface. It makes sense to have this position added - this is probably not the only symbol that we want positioned like this? http://codereview.appspot.com/1579041/diff/19001/20007#newcode1161 scm/define-grobs.scm:1161: break-alignable-interface + self-alignment-interface otherwise regtests spit out loads of warnings for missing interface (from self-alignment-X setting) Added. http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan, Here are some more comments for you. Cheers, Neil http://codereview.appspot.com/1579041/diff/19001/20003 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/19001/20003#newcode81 lily/metronome-engraver.cc:81: g-get_property_data (break-align-symbol) get_property () http://codereview.appspot.com/1579041/diff/19001/20003#newcode82 lily/metronome-engraver.cc:82: == ly_symbol2scm (staff-bar)) can't this be incorporated into 'break-align-symbols for MetronomeMark? http://codereview.appspot.com/1579041/diff/19001/20003#newcode86 lily/metronome-engraver.cc:86: scm_member (g-get_property_data (break-align-symbol), get_property () http://codereview.appspot.com/1579041/diff/19001/20003#newcode87 lily/metronome-engraver.cc:87: text_-get_property_data (break-align-symbols)) get_property () http://codereview.appspot.com/1579041/diff/19001/20003#newcode96 lily/metronome-engraver.cc:96: grob_name_scm (Grob *g) ly_symbol2scm (g-name ().c_str ()); though I'd prefer more lisp-like syntax for this using camel_case_to_lisp_identifier () http://codereview.appspot.com/1579041/diff/19001/20003#newcode109 lily/metronome-engraver.cc:109: text_-get_property_data (non-break-align-symbols)) get_property () http://codereview.appspot.com/1579041/diff/19001/20006 File scm/define-grob-properties.scm (right): http://codereview.appspot.com/1579041/diff/19001/20006#newcode610 scm/define-grob-properties.scm:610: (non-break-align-symbols ,list? A list of symbols that determine needs adding to an interface http://codereview.appspot.com/1579041/diff/19001/20007 File scm/define-grobs.scm (right): http://codereview.appspot.com/1579041/diff/19001/20007#newcode418 scm/define-grobs.scm:418: metronome-mark Is this necessary? IIUC, only break-aligned grobs will be acknowledged by the Break_align_engraver, so a MetronomeMark will never appear in the list of elements for ordering. http://codereview.appspot.com/1579041/diff/19001/20007#newcode1161 scm/define-grobs.scm:1161: break-alignable-interface + self-alignment-interface otherwise regtests spit out loads of warnings for missing interface (from self-alignment-X setting) http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Reviewers: Neil Puttock, Message: On 2010/06/08 22:22:43, Neil Puttock wrote: Comments processed in patch 4 5. Here are a few thoughts on positioning: -) A metronome mark at a full-bar rest should be aligned with the barline instead of the paper column to the left of the rest. This is not in Read or #684's description... -) If there's a tempo change at a key signature, the metronome mark shouldn't be aligned with the following note. and neither is this. I added both to the latest patch, although this may be incorrect and was unexpected work for 684... Sorry for the terse answer, I tried replying twice only to find the replies never made it. I'm probably not trusting web interfaces enough ;-) Jan. Description: Align metronome mark at time signature or first musical element. Fixes #684. Only if no TimeSignature is present, align on musical column. Break-alignable metronome marks. Please review this at http://codereview.appspot.com/1579041/show Affected files: A input/regression/metronome-marking-break-align.ly M lily/default-bar-line-engraver.cc M lily/metronome-engraver.cc M lily/part-combine-engraver.cc M lily/timing-translator.cc M scm/define-grob-properties.scm M scm/define-grobs.scm ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan, On 2010/06/16 12:08:06, jan.nieuwenhuizen wrote: On 2010/06/08 22:22:43, Neil Puttock wrote: -) A metronome mark at a full-bar rest should be aligned with the barline instead of the paper column to the left of the rest. This is not in Read or #684's description... See http://code.google.com/p/lilypond/issues/detail?id=712 -) If there's a tempo change at a key signature, the metronome mark shouldn't be aligned with the following note. and neither is this. I added both to the latest patch, although this may be incorrect and was unexpected work for 684... The key signature counts as the first notational element in such cases. Cheers, Neil http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Hi Jan, I've tested the latest patch, and it looks pretty good so far. Here are a few thoughts on positioning: -) A metronome mark at a full-bar rest should be aligned with the barline instead of the paper column to the left of the rest. -) If there's a tempo change at a key signature, the metronome mark shouldn't be aligned with the following note. Cheers, Neil http://codereview.appspot.com/1579041/diff/8001/9001 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/8001/9001#newcode88 lily/metronome-engraver.cc:88: Metronome_mark_engraver::acknowledge_grob (Grob_info info) Since this always gets called before acknowledge_break_aligned (), you could fold the code above into this with a check for break-aligned-interface. http://codereview.appspot.com/1579041/diff/8001/9001#newcode98 lily/metronome-engraver.cc:98: text_-set_parent (g, X_AXIS); I find this really weird: do you know why this is necessary for the tempo mark to appear on the full-bar rest even though stop_translation_timestep () resets the X-parent to currentMusicalColumn? http://codereview.appspot.com/1579041/diff/8001/9002 File scm/define-grobs.scm (right): http://codereview.appspot.com/1579041/diff/8001/9002#newcode1150 scm/define-grobs.scm:1150: (self-alignment-X . -1) (self-alignment-X . ,LEFT) http://codereview.appspot.com/1579041/diff/8001/9002#newcode1157 scm/define-grobs.scm:1157: break-alignable-interface + self-alignment-interface http://codereview.appspot.com/1579041/diff/8001/9002#newcode1162 scm/define-grobs.scm:1162: (break-align-symbol . multi-measure-rest) This doesn't really work, since a MultiMeasureRest isn't a break-aligned grob (same for MetronomeMark). The tempo mark regtests spit out loads of warnings due to the missing interface for this property, but you can't add break-aligned-interface to silence them since the rest then gets picked up in the break-alignment-interface, causing a segfault. http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: Align metronome mark at time signature or first musical element. Fixes #684. (issue1579041)
Op maandag 07-06-2010 om 16:59 uur [tijdzone +], schreef n.putt...@gmail.com: Hi Neil, Have you checked what happens with full-bar rests? Uhuh, I have now ;-) I haven't tested your patch, but it's similar to the one I posted, which suffers from invisible tempo marks at full-bar rests. Good catch, that should also be fixed now. Thanks! Greetings, Jan. Cheers, Neil http://codereview.appspot.com/1579041/diff/2001/3001 File lily/metronome-engraver.cc (right): http://codereview.appspot.com/1579041/diff/2001/3001#newcode78 lily/metronome-engraver.cc:78: == text_-get_property_data (break-align-symbol))) should be break-align-symbols (which complicates it a bit) http://codereview.appspot.com/1579041/show ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel ___ lilypond-devel mailing list lilypond-devel@gnu.org http://lists.gnu.org/mailman/listinfo/lilypond-devel
