Fix bugs in mismatched spelling of chord name and notes

When the notes of a chord were re-spelled to match the degrees of the
matching signature, the root note was not being updated, so the
chord-name symbol still reflected the name of the un-respelled root
note. This was a bug introduced in the recent shakeup to do analysis
more entirely in the scherzo layer without using score_note_t.

Fixing this required a fair amount of refactoring. Hopefully the
result is cleaner and more maintainable.

diff --git a/scherzo.c b/scherzo.c
index 361301de0d57e58677c3d6281b2fd6d65d72c790..6a5f2b3176c7f441fdf097714746705a74a91a2c 100644 (file)
--- a/scherzo.c
+++ b/scherzo.c
@@ -24,7 +24,7 @@
 #include "score.h"
 #include "mnemon.h"
 
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
+#define ARRAY_SIZE(arr) ((int) (sizeof(arr) / sizeof(arr[0])))
 
 #define unused(foo) foo __attribute__((unused))
 
@@ -411,6 +411,17 @@ _midi_to_pitch (unsigned char midi_note)
     }
 }
 
+/* Return true if 'a' and 'b' sound identical, (even if spelled differently)
+ *
+ * This comparison considers octaves as significant. So Bb and A# in
+ * the same octave are considered enharmonic, but Bb and A# in
+ * different octaves are not. */
+static int
+_pitches_are_enharmonic (pitch_t a, pitch_t b)
+{
+    return _pitch_to_midi (a) == _pitch_to_midi (b);
+}
+
 /* Determine a chord name (if any) from the current notes pressed */
 
 typedef struct analyzed_pitch {
@@ -835,7 +846,6 @@ scherzo_adjust_pitch_to_match_modified_degree (pitch_t *pitch,
 
 static void
 _spell_chord_by_signature (pitch_group_t *chord,
-                          int num_notes,
                           chord_signature_t *signature,
                           pitch_t root)
 {
@@ -847,7 +857,7 @@ _spell_chord_by_signature (pitch_group_t *chord,
     root_midi = _pitch_to_midi (root);
     root = _midi_to_pitch (root_midi);
     
-    for (i = 0; i < num_notes; i++) {
+    for (i = 0; i < chord->num_pitches; i++) {
        note_half_steps = _pitch_from_root_in_half_steps (chord->pitches[i],
                                                          root);
        for (degree = 0; degree < signature->num_degrees; degree++) {
@@ -867,30 +877,24 @@ _spell_chord_by_signature (pitch_group_t *chord,
 }
 
 static void
-scherzo_analyze_chord (scherzo_t *scherzo)
+_analyze_chord (pitch_group_t *chord,
+               chord_signature_t **signature_ret,
+               pitch_t **root_ret)
 {
     void *local = talloc_new (NULL);
     analyzed_pitch_t *pitches;
-    pitch_group_t *pitch_group;
-    unsigned i, j, num_pitches;
+    int i, j, num_pitches;
     int bass_pitch, inversion;
     pitch_t root;
     chord_signature_t *match = NULL;
-    char *chord_name;
 
-    if (scherzo->pedal_pressed)
-       pitch_group = &scherzo->notes_pedaled;
-    else
-       pitch_group = &scherzo->notes_pressed;
+    /* Default to no-match for any early return */
+    *signature_ret = NULL;
+    *root_ret = NULL;
 
-    num_pitches = pitch_group->num_pitches;
+    num_pitches = chord->num_pitches;
 
-    if (scherzo->chord) {
-       score_remove_chord (scherzo->chord);
-       scherzo->chord = NULL;
-    }
-
-    if (num_pitches <= 1)
+    if (chord->num_pitches <= 1)
        goto DONE;
 
     pitches = talloc_array (local, analyzed_pitch_t, num_pitches);
@@ -898,7 +902,7 @@ scherzo_analyze_chord (scherzo_t *scherzo)
        goto DONE;
 
     for (i = 0; i < num_pitches; i++) {
-       pitch_t pitch = pitch_group->pitches[i];
+       pitch_t pitch = chord->pitches[i];
        pitches[i].pitch = pitch;
        pitches[i].midi_pitch = _pitch_to_midi (pitch);
        /* Relative pitch will be filled in after sorting. */
@@ -960,27 +964,16 @@ scherzo_analyze_chord (scherzo_t *scherzo)
     }
 HAVE_MATCH:
 
-    chord_name = (char *) signatures[i].name;
-
     if (match) {
-       /* Don't print root pitch for octaves and inversions,
-        * (since a pitch name alone looks like a major triad) */
-       if (num_pitches < 3) {
-           chord_name = talloc_strdup (local, chord_name);
-       } else {
-           chord_name = talloc_asprintf (local, "%s%s",
-                                         _pitch_str (root),
-                                         chord_name);
+       *signature_ret = match;
+       for (i = 0; i < chord->num_pitches; i++) {
+           if (chord->pitches[i] == root) {
+               *root_ret = &chord->pitches[i];
+               break;
+           }
        }
-
-       _spell_chord_by_signature (pitch_group, num_pitches,
-                                  match, root);
-    } else {
-       chord_name = talloc_strdup (local, "?");
     }
 
-    scherzo->chord = score_add_chord (scherzo->treble, chord_name);
-
 DONE:
     talloc_free (local);
 }
@@ -1038,6 +1031,22 @@ pitch_group_remove_pitch_at (pitch_group_t *group, int i)
     group->num_pitches--;
 }
 
+/* Remove a 'pitch' from 'group', (including any enharmonic pitch)
+ * See also: pitch_group_remove_pitch
+ */
+static void
+pitch_group_remove_pitch_enharmonic (pitch_group_t *group, pitch_t pitch)
+{
+    int i;
+
+    for (i = 0; i < group->num_pitches; i++)
+       if (_pitches_are_enharmonic (group->pitches[i], pitch))
+           pitch_group_remove_pitch_at (group, i);
+}
+
+/* Remove an exactly-matching pitch from 'group'
+ * See also: pitch_group_remove_pitch_enharmonic
+ *
 static void
 pitch_group_remove_pitch (pitch_group_t *group, pitch_t pitch)
 {
@@ -1047,6 +1056,7 @@ pitch_group_remove_pitch (pitch_group_t *group, pitch_t pitch)
        if (group->pitches[i] == pitch)
            pitch_group_remove_pitch_at (group, i);
 }
+*/
 
 static void
 scherzo_update_notes_and_chord (scherzo_t *scherzo)
@@ -1056,8 +1066,7 @@ scherzo_update_notes_and_chord (scherzo_t *scherzo)
     {
        score_remove_notes (scherzo->score);
 
-       if (scherzo->chord)
-           score_remove_chord (scherzo->chord);
+       score_remove_chords (scherzo->score);
 
        gtk_widget_queue_draw (scherzo->window);
     }
@@ -1066,6 +1075,11 @@ scherzo_update_notes_and_chord (scherzo_t *scherzo)
 static void
 scherzo_press_note (scherzo_t *scherzo, pitch_t pitch)
 {
+    void *local = talloc_new (NULL);
+    pitch_group_t *chord;
+    chord_signature_t *signature;
+    char *chord_name = NULL;
+    pitch_t *root;
     int i;
 
     /* Do nothing if this note is already pressed. */
@@ -1078,8 +1092,38 @@ scherzo_press_note (scherzo_t *scherzo, pitch_t pitch)
     if (scherzo->pedal_pressed)
        pitch_group_add_pitch (&scherzo->notes_pedaled, pitch);
 
-    /* Remove all notes from score, then add current notes. */
+    /* Remove all notes/chords from score, then add current notes. */
     score_remove_notes (scherzo->score);
+    score_remove_chords (scherzo->score);
+
+    if (scherzo->pedal_pressed)
+       chord = &scherzo->notes_pedaled;
+    else
+       chord = &scherzo->notes_pressed;
+
+    _analyze_chord (chord, &signature, &root);
+
+    if (signature) {
+       /* Don't print root pitch for octaves and inversions,
+        * (since a pitch name alone looks like a major triad) */
+       if (signature->num_degrees < 3) {
+           chord_name = talloc_strdup (local, signature->name);
+       } else {
+           _spell_chord_by_signature (&scherzo->notes_pressed,
+                                      signature, *root);
+           _spell_chord_by_signature (&scherzo->notes_pedaled,
+                                      signature, *root);
+
+           chord_name = talloc_asprintf (local, "%s%s",
+                                         _pitch_str (*root),
+                                         signature->name);
+       }
+    } else if (chord->num_pitches > 2) {
+       chord_name = talloc_strdup (local, "?");
+    }
+
+    if (chord_name)
+       scherzo->chord = score_add_chord (scherzo->treble, chord_name);
 
     for (i = 0; i < scherzo->notes_pressed.num_pitches; i++)
     {
@@ -1093,13 +1137,13 @@ scherzo_press_note (scherzo_t *scherzo, pitch_t pitch)
                        SCORE_DURATION_WHOLE);
     }
 
-    scherzo_analyze_chord (scherzo);
+    talloc_free (local);
 }
 
 static void
 scherzo_release_note (scherzo_t *scherzo, pitch_t pitch)
 {
-    pitch_group_remove_pitch (&scherzo->notes_pressed, pitch);
+    pitch_group_remove_pitch_enharmonic (&scherzo->notes_pressed, pitch);
 
     if (scherzo->notes_pressed.num_pitches == 0)
        scherzo_update_notes_and_chord (scherzo);
@@ -1126,6 +1170,9 @@ scherzo_press_pedal (scherzo_t *scherzo)
 {
     int i;
 
+    if (scherzo->pedal_pressed)
+       return;
+
     scherzo->pedal_pressed = 1;
 
     /* Copy all pressed notes to pedaled notes */
@@ -1138,6 +1185,9 @@ scherzo_release_pedal (scherzo_t *scherzo)
 {
     int i;
 
+    if (! scherzo->pedal_pressed)
+       return;
+
     /* Remove all previously pedaled notes. */
     for (i = scherzo->notes_pedaled.num_pitches - 1; i >=0; i--)
        pitch_group_remove_pitch_at (&scherzo->notes_pedaled, i);

diff --git a/score.c b/score.c
index 5c0e0b023545b15c1a5f48c69c94fd6ca5aaa026..ecfc2352662194790ec40b39577146fca64c08ee 100644 (file)
--- a/score.c
+++ b/score.c
@@ -596,26 +596,21 @@ score_add_chord (score_staff_t *staff,
 }
 
 void
-score_remove_chord (score_chord_t *chord)
+score_staff_remove_chords (score_staff_t *staff)
 {
-    score_staff_t *staff = chord->staff;
-    int i;
-
-    for (i = 0; i < staff->num_chords; i++)
-       if (staff->chords[i] == chord)
-           break;
+    talloc_free (staff->chords);
+    staff->chords = NULL;
 
-    if (i == staff->num_chords)
-       return;
+    staff->num_chords = 0;
+}
 
-    if (i < staff->num_chords - 1)
-    {
-       memmove (staff->chords + i,
-                staff->chords + i + 1, 
-                (staff->num_chords - 1 - i) * sizeof (score_chord_t *));
-    }
+void
+score_remove_chords (score_t *score)
+{
+    int i;
 
-    staff->num_chords -= 1;
+    for (i = 0; i < score->num_staves; i++)
+       score_staff_remove_chords (score->staves[i]);
 }
 
 void

diff --git a/score.h b/score.h
index c5d275de0bc4e7614f1effed1ff6ea5d5f42308a..e71b8ab2c98b6b6754ffc551a8f26e7c4ab7fe04 100644 (file)
--- a/score.h
+++ b/score.h
@@ -122,10 +122,13 @@ score_chord_t *
 score_add_chord (score_staff_t *staff,
                 const char * name);
 
-/* Remove the given chord from its staff. */
+/* Remove all chords from the given staff. */
 void
-score_remove_chord (score_chord_t *chord);
+score_staff_remove_chords (score_staff_t *staff);
 
+/* Remove all chords from the score. */
+void
+score_remove_chords (score_t *score);
 
 /* Remove all notes from the given staff. */
 void