Fixes error for tuplet bracket direction calculation when tuplets contain rests.

Fixes error for tuplet bracket direction calculation when tuplets contain rests.

[MikeSol, 2011-06-29 11:25:18]

Cheers,
MS

[James.Lowe_datacore.com, 2011-06-29 12:35:55]

Mike,
________________________________________
From: lilypond-devel-bounces+james.lowe=datacore.com@gnu.org
[lilypond-devel-bounces+james.lowe=datacore.com@gnu.org] on behalf of
mtsolo@gmail.com [mtsolo@gmail.com]
Sent: 29 June 2011 12:25
To: mtsolo@gmail.com
Cc: reply@codereview.appspotmail.com; lilypond-devel@gnu.org
Subject: Fixes error for tuplet bracket direction calculation when tuplets     
contain rests. (issue4668045)

Reviewers: ,

Message:
Cheers,
MS

Description:
Fixes error for tuplet bracket direction calculation when tuplets
contain rests.

Please review this at http://codereview.appspot.com/4668045/


-----------

Is there a tracker issue for this?

James

[hanwenn, 2011-06-29 12:50:43]

test missing.

On Wed, Jun 29, 2011 at 8:25 AM,  <mtsolo@gmail.com> wrote:
> Reviewers: ,
>
> Message:
> Cheers,
> MS
>
> Description:
> Fixes error for tuplet bracket direction calculation when tuplets
> contain rests.
>
> Please review this at http://codereview.appspot.com/4668045/
>
> Affected files:
>  M lily/tuplet-bracket.cc
>
>
> Index: lily/tuplet-bracket.cc
> diff --git a/lily/tuplet-bracket.cc b/lily/tuplet-bracket.cc
> index
>
3bf2d1b49e8487815b79d1ec8af0b89a526803b2..f09a3a8e093eebb3f7435abbe99a859c561689cc
> 100644
> --- a/lily/tuplet-bracket.cc
> +++ b/lily/tuplet-bracket.cc
> @@ -730,6 +730,8 @@ Tuplet_bracket::get_default_dir (Grob *me)
>   for (vsize i = 0; i < columns.size (); i++)
>     {
>       Grob *nc = columns[i];
> +      if (Note_column::has_rests (nc))
> +        continue;
>       Direction d = Note_column::dir (nc);
>       if (d)
>        dirs[d]++;
>
>
>
> _______________________________________________
> lilypond-devel mailing list
> lilypond-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/lilypond-devel
>



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

[Graham Percival, 2011-06-29 13:20:57]

On Wed, Jun 29, 2011 at 12:35:35PM +0000, James Lowe wrote:
> > Fixes error for tuplet bracket direction calculation when tuplets
> > contain rests.
> > 
> > Please review this at http://codereview.appspot.com/4668045/

> Is there a tracker issue for this?

No, so James please make one.
http://lilypond.org/doc/v2.15/Documentation/contributor/patch-handling
Helpers: adding patches.  First bullet point.

Cheers,
- Graham

[pkx166h, 2011-06-29 13:25:41]

Pass all reg tests. 

However, I note that in the tuplet_rest.ly file 

i) the tuplet is above the staff for the 3 x crotchet/quarter rest measure where
it was below previously (i.e r4 r r)

ii) the last 2 measures where the last note is a stemUp instead of stemDown but
has a rest in either first quarter note or second quarter note (i.e. r c g and c
r g) the tuplet bracket is also above instead of below the staff as it was
previously

Now this may be right but I have no tracker to check against and cannot remember
a discussion on whether tuplets should be up or down in this case.

So it may be fine, but technically this reg test is significantly different -
although it fixes the 'bad' case and puts the tuplet aobve the staff instead of
below.

Mike is it worth doing the same as the first measure but th eother way round?
i.e make a test where the note is very high and the tuplet sits below? Seems odd
to only have one case and not the other.

James

[MikeSol, 2011-06-29 13:37:40]

Ah, the old coup de regtest.  Gets me every time...
New patch set uploaded.

Cheers,
MS

[mike_apollinemike.com, 2011-06-29 16:32:33]

On Jun 29, 2011, at 3:25 PM, pkx166h@gmail.com wrote:

> Pass all reg tests.
> 
> However, I note that in the tuplet_rest.ly file
> 
> i) the tuplet is above the staff for the 3 x crotchet/quarter rest
> measure where it was below previously (i.e r4 r r)
> 
> ii) the last 2 measures where the last note is a stemUp instead of
> stemDown but has a rest in either first quarter note or second quarter
> note (i.e. r c g and c r g) the tuplet bracket is also above instead of
> below the staff as it was previously
> 
> Now this may be right but I have no tracker to check against and cannot
> remember a discussion on whether tuplets should be up or down in this
> case.
> 
> So it may be fine, but technically this reg test is significantly
> different - although it fixes the 'bad' case and puts the tuplet aobve
> the staff instead of below.
> 
> Mike is it worth doing the same as the first measure but th eother way
> round? i.e make a test where the note is very high and the tuplet sits
> below? Seems odd to only have one case and not the other.
> 

Hey James,

Currently, Lilypond does not factor in note position w/ respect to the staff
symbol in order to place the tuplet bracket.  Nor did my patch...until I read
your e-mail.  What you're saying makes sense.  I've made a new patch so that
LilyPond, as a default, chooses the direction that is closest to the StaffSymbol
when there is an equal number of up and down notes.  I've updated the regtest to
show this.

The only issue with this may be one of sight reading.  If I am reading at
quaver=200 and I see \relative c''' { \times 2/3 { c4 b,, b } } , I'm likely
going to want to see the tuplet above the staff so that I don't accidentally
play the first note as a quarter.  Is there anything in Behind Bars on this?

Cheers,
MS

[pkx166h, 2011-06-29 23:59:37]

On 2011/06/29 13:20:57, graham_percival-music.ca wrote:
> On Wed, Jun 29, 2011 at 12:35:35PM +0000, James Lowe wrote:
> > Is there a tracker issue for this?
> 
> No, so James please make one.

http://code.google.com/p/lilypond/issues/detail?id=1720

Hope this is correct :)

James

[pkx166h, 2011-06-30 00:13:20]

On 2011/06/29 16:32:33, mike_apollinemike.com wrote:
> On Jun 29, 2011, at 3:25 PM, mailto:pkx166h@gmail.com wrote:

> 
> Currently, Lilypond does not factor in note position w/ respect to the staff
> symbol in order to place the tuplet bracket.  Nor did my patch...until I read
> your e-mail.  What you're saying makes sense.  I've made a new patch so that
> LilyPond, as a default, chooses the direction that is closest to the
StaffSymbol
> when there is an equal number of up and down notes.  I've updated the regtest
to
> show this.
> 
> The only issue with this may be one of sight reading.  If I am reading at
> quaver=200 and I see \relative c''' { \times 2/3 { c4 b,, b } } , I'm likely
> going to want to see the tuplet above the staff so that I don't accidentally
> play the first note as a quarter.  Is there anything in Behind Bars on this?

Mike I don't own that book, my Read shows examples (admittedly not that extreme
as these) where the bracket can be either/or up/down regardless of stem
direction.

Stone says " The numerals and the brackets should be placed at the stem-side of
respective groups so that the space at the note-heads will be free for slurs and
other articulations marks" then goes on to say "If a group contains both upstems
and downstems, it is often best to let the majority of the stems determine the
position of the numeral and the bracket, but much depends on the amount of
articulation. The less interference with articulation and phrasing the better.
... Phrasing and articulation marks are always placed closer to the note-heads
than the numerals and the brackets."

Also consider that the numeral is supposed to go under/over the note head midway
in the tupelet in terms of beat not in terms of middle by distance. So this may
also affect where the bracket lies if the extreme notes are not the middle note.

James

[MikeSol, 2011-07-13 05:49:53]

On 2011/06/30 00:13:20, J_lowe wrote:
> On 2011/06/29 16:32:33, http://mike_apollinemike.com wrote:
> > On Jun 29, 2011, at 3:25 PM, mailto:pkx166h@gmail.com wrote:
> 
> > 
> > Currently, Lilypond does not factor in note position w/ respect to the staff
> > symbol in order to place the tuplet bracket.  Nor did my patch...until I
read
> > your e-mail.  What you're saying makes sense.  I've made a new patch so that
> > LilyPond, as a default, chooses the direction that is closest to the
> StaffSymbol
> > when there is an equal number of up and down notes.  I've updated the
regtest
> to
> > show this.
> > 
> > The only issue with this may be one of sight reading.  If I am reading at
> > quaver=200 and I see \relative c''' { \times 2/3 { c4 b,, b } } , I'm likely
> > going to want to see the tuplet above the staff so that I don't accidentally
> > play the first note as a quarter.  Is there anything in Behind Bars on this?
> 
> Mike I don't own that book, my Read shows examples (admittedly not that
extreme
> as these) where the bracket can be either/or up/down regardless of stem
> direction.
> 
> Stone says " The numerals and the brackets should be placed at the stem-side
of
> respective groups so that the space at the note-heads will be free for slurs
and
> other articulations marks" then goes on to say "If a group contains both
upstems
> and downstems, it is often best to let the majority of the stems determine the
> position of the numeral and the bracket, but much depends on the amount of
> articulation. The less interference with articulation and phrasing the better.
> ... Phrasing and articulation marks are always placed closer to the note-heads
> than the numerals and the brackets."
> 
> Also consider that the numeral is supposed to go under/over the note head
midway
> in the tupelet in terms of beat not in terms of middle by distance. So this
may
> also affect where the bracket lies if the extreme notes are not the middle
note.
> 
> James

Just touching base on this patch as well.  As a reminder, it calculates the
placement of TupletBrackets based on the number of up/down stems, moving the
tuplet bracket closer to the staff in cases where there is a tie between the
number of up/down stems.

Cheers,
MS

[Keith, 2011-07-16 03:41:25]

Looks good and works good for me.

On 2011/06/29 16:32:33, mike_apollinemike.com wrote:
> 
> The only issue with this may be one of sight reading.  If I am reading at
> quaver=200 and I see \relative c''' { \times 2/3 { c4 b,, b } } , I'm likely
> going to want to see the tuplet above the staff so that I don't accidentally
> play the first note as a quarter.  

People read ahead of where they are playing (by quite a lot; remember how early
you turn pages for a piano player) and keeping the tuplet bracket near the staff
makes it easier to see what *group* of notes it applies to.  I like it.

[Neil Puttock, 2011-07-18 21:45:16]

Hi Mike,

This looks fine apart from the following change:

\relative c'' {
  \voiceTwo
 \times 2/3 { r r r }
}

-> bracket still above stave

The simplest fix is probably to add TupletBracket to direction-polyphonic-grobs.

Cheers,
Neil

[mike_apollinemike.com, 2011-07-19 06:15:23]

On Jul 18, 2011, at 11:45 PM, n.puttock@gmail.com wrote:

> Hi Mike,
> 
> This looks fine apart from the following change:
> 
> \relative c'' {
>  \voiceTwo
> \times 2/3 { r r r }
> }
> 
> -> bracket still above stave
> 
> The simplest fix is probably to add TupletBracket to
> direction-polyphonic-grobs.
> 
> Cheers,
> Neil
> 

Works like a charm!  New patchset uploaded.

Cheers,
MS

[MikeSol, 2011-07-28 07:59:06]

Pushed as eed6f105c0b478572ec2b8fa93e1f96c0c5869ba.

Cheers,
MS