#4807 Wrong glissando slopes with accidental in target chord

[Simon Albrecht]

http://codereview.appspot.com/342100043

As Harm reported, glissando slopes may be wrongly calculated, if there is an accidental on the target chord. This worked fine in v2.16.2.

:::TeX
\version "2.18.2"
%% up to 2.19.39

{ <d f>1\glissando <fis'' a''>1 }

%% this one is ok:
{ <d f>1\glissando <f'' a''>1 }

[David Kastrup]

The responsible commit is
b47350d1efb1128b2a8ea13f340dd5e3a53f0735 is the first bad commit
commit b47350d1efb1128b2a8ea13f340dd5e3a53f0735
Author: Keith OHara k-ohara5a5a@oco.net
Date: Sat Aug 24 14:06:57 2013 -0700

Draw glissando to accidental, not accidental_placement; issue 40

:040000 040000 6d0810fb614f48d24011889d305bdc62782aee7f 593b92fc536ba2491ab749d937fefa70c6344b29 M lily

This was committed in version 2.17.25

I'd have marked Keith as the owner (for apparent lack of a CC option in the tracker, expecting him to disown the issue if he is not seeing fit to look at it) but it appears like he's not in the SourceForge user base anyway.

[Malte Meyn]

After looking at that commit (b47350d1) and at issue [#40] it looks to me as if one could simply revert this commit and get correct behaviour for both cases (issue 40 and this issue), make check shows no other problems. But of course there was a reason fo that commit so I’m confused …

--- a/lily/line-spanner.cc
+++ b/lily/line-spanner.cc
@@ -114,7 +114,7 @@ Line_spanner::calc_bound_info (SCM smob, Direction dir)
                       ? Axis_group_interface::generic_bound_extent (bound_grob, commonx, X_AXIS)
                       : robust_relative_extent (bound_grob, commonx, X_AXIS)).linear_combination (attach);

-      Grob *acc = unsmob_grob (bound_grob->get_object ("accidental-grob"));
+      Grob *acc = Note_column::accidentals (bound_grob->get_parent (X_AXIS));
       if (acc && to_boolean (ly_assoc_get (ly_symbol2scm ("end-on-accidental"), details, SCM_BOOL_F)))
         x_coord = robust_relative_extent (acc, commonx, X_AXIS).linear_combination (attach);

[Malte Meyn]

Issue 4807: fix glissando in case of accidentals

This reverts b47350d1 as both issue 40 and 4807 are solved then.

Contains additions to regtest.

http://codereview.appspot.com/342100043

[Anonymous]

Pasees make, make check and a full make doc.

Reg test diff attached

[Malte Meyn]

This is a regression, isn’t it? I don’t know where the label had gone but I’m sure I found this issue in the “Open (Critical)” category so I’ll add the label.

[Malte Meyn]

Hm … Why did this change the type? I’m very confused …

[Anonymous]

It's a 'feature' (limitation) of git-cl, it sets the label back to 'nothing', I didn';t notice that it was a regression, else I'd have set it back to that. 'Enhancement' is my go-to label (unless it is obviously something like Ugly or Doc.

[Anonymous]

Patch on countdown for May 20th

[Anonymous]

Patch counted down - please push.

[Malte Meyn]

commit 4d958aa0ac05f7bc4c1b294e8697274a015d38fb
Author: Malte Meyn lilypond@maltemeyn.de
Date: Sat May 12 10:53:45 2018 +0200

Issue 4807/2: add chord glissandi to regtest

commit 13e34233a0bb42a481da1471f4e4acbb62ab5cff
Author: Malte Meyn lilypond@maltemeyn.de
Date: Wed May 9 15:49:09 2018 +0200

Issue 4807/1: fix glissando in case of accidentals

This reverts b47350d1 as both issue 40 and 4807 are solved then.