#4654 Wrong fret label values in predefined custom fretboards containing text strings

[Palmer Ralph]

pls pls@philomelos.ne wrote :

looks like fret label values are not computed correctly when
predefined fretboards contain text strings instead of fingering indications.
This small issue only shows when alternate fretboards are defined and
used. That's why my minimal example is not really tiny!

The fret label value of the custom fretboard "e-shape-with-movable-do" is "vii" for both the F and the G diagram instead
of "i" and "iii" repectively. If I'm not mistaken this is because
LilyPond adds up the offset-fret values in the \storePredefinedDiagram
commands of this custom fretboard (offset-fret (1) + 2 + 4
= 7). The fret label value is "iii" (offset-fret (1) + 2 = 3) if you comment out the definitions
for the A diagram for this custom fretboard.

I couldn't produce the effect with custom fretboards containing only
fingering indications.

[Thomas Morley]

Changing two lines to
\override FretBoard #'size = #'1.5
\override FretBoard #'fret-diagram-details #'orientation = #'landscape
will make it compile with 2.16.2 and even 2.14.2

The bug is still there.
At least one can say it's not one of the recent changes to blame.

[Thomas Morley]

After further experiments I think it has nothing to do whether text or numbers are used for fingering-indications.
But I observed the bug if verbose-syntax is used in addChordShape and more than one chord uses this chord-shape.
This does not happen if terse-syntax is used:

A smaller example:

\version "2.19.31"

%%%%% buggy:

\addChordShape #'f,:1.8.10.12_fingering-1
               #guitar-tuning
               #'((mute 1)
                 (place-fret 2 1 1)
                 ;(barre 6 2 1)
                 (place-fret 3 2 2)
                 (place-fret 4 3 3)
                 (mute 5)
                 (place-fret 6 1 1)
                 (barre 6 2 1))

\storePredefinedDiagram #default-fret-table
                       \chordmode { f':1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 0 (
                         chord-shape 'f,:1.8.10.12_fingering-1
                         guitar-tuning))

%% with this second setting the bug appears
%% comment to see the difference
%%{           
\storePredefinedDiagram #default-fret-table
                       \chordmode { g':1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 2 (
                         chord-shape 'f,:1.8.10.12_fingering-1
                         guitar-tuning))
%}

\new FretBoards 
  \chordmode {
    f':1.8.10.12
  }    
%}

%%%% works nicely:
%%{
\addChordShape #'f,:1.8.10.12_fingering-2
               #guitar-tuning
               #"1-1-(;x;3-3;2-2;1-1-);x;"

\storePredefinedDiagram #default-fret-table
                       \chordmode { f,:1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 0 (
                         chord-shape 'f,:1.8.10.12_fingering-2
                         guitar-tuning))

\storePredefinedDiagram #default-fret-table
                       \chordmode { g,:1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 2 (
                         chord-shape 'f,:1.8.10.12_fingering-2
                         guitar-tuning))

\new FretBoards 
  \chordmode {
    f,:1.8.10.12
  }
%}

[Patrick Schmidt]

Yes, you were right: the bug was restricted to verbose-syntax. terse-syntax always worked fine for both fret-diagrams and tablature. (I can tell because I used it in roughly 500 to 1000 chord shape definitions.) Unfortunately now fret diagrams and tablature don't seem to be in sync anymore. (Until now it was possible to also use the predefined diagrams (in terse-syntax) in tablature. Now diagrams differ from tablature regardless of syntax:

\version "2.19.31"

%%%% fret diagrams and tablature are not synchronized:
\markup { "verbose-syntax" }
\addChordShape #'f,:1.8.10.12_fingering-1
               #guitar-tuning
               #'((mute 1)
                 (place-fret 2 1 1)
                 (place-fret 3 2 2)
                 (place-fret 4 3 3)
                 (mute 5)
                 (place-fret 6 1 1)
                 (barre 6 2 1))
%%{
\storePredefinedDiagram #default-fret-table
                       \chordmode { a,,:1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 4 (
                         chord-shape 'f,:1.8.10.12_fingering-1
                         guitar-tuning))
<<
  \new FretBoards \chordmode {
    a,,:1.8.10.12
  }
  \new TabStaff \chordmode {
    a,,:1.8.10.12
  }
>>
%}

%%%% fret diagrams and tablature are not synchronized *anymore*:
\markup { "terse-syntax" }
%%{
\addChordShape #'f,:1.8.10.12_fingering-2
               #guitar-tuning
               #"1-1-(;x;3-3;2-2;1-1-);x;"

\storePredefinedDiagram #default-fret-table
                       \chordmode { a,,:1.8.10.12 }
                       #guitar-tuning
                       #(offset-fret 4 (
                         chord-shape 'f,:1.8.10.12_fingering-2
                         guitar-tuning))
<<
  \new FretBoards \chordmode {
    a,,:1.8.10.12
  }
  \new TabStaff \chordmode {
    a,,:1.8.10.12
  }
>>
%}

[David Kastrup]

Patrick, the output here appears the same to me both before and after the patch. How do you reckon the patch to be involved here?

[Patrick Schmidt]

Sorry, my bad again! In my last example I simply forgot to include a \set predefinedDiagramTable... in the TabStaff. Sorry for the noise. Your patch is just fine! Thanks for your work and special thanks for killing the fret label value bug so quickly!

[David Kastrup]

Issue 4654: offset-fret modifies its definition

http://codereview.appspot.com/276810043

[David Kastrup]

Just opencode the offsetting

http://codereview.appspot.com/276810043

[Anonymous]

passes make, make check and a full make doc.

[Anonymous]

Patch on countdown for November 13th

[Anonymous]

Patch counted down - please push

[David Kastrup]

Pushed to staging as
commit df9fc0bd0d50943cdc48d4226009bd30c3e99fe4
Author: David Kastrup dak@gnu.org
Date: Sat Nov 7 00:22:58 2015 +0100

Issue 4654: offset-fret modifies its definition