fix(core): GPIF imports keep tapping and hammer-on origin semantics

core/src/gp.rs

@@ -43,7 +43,10 @@ use crate::{
     },
     slice::TickRange,
 };
-use guitarpro::io::gpif::{Gpif, Property as GpifProperty, Track as GpifTrack};
+use guitarpro::io::gpif::{
+    Bar as GpifBar, Beat as GpifBeat, Gpif, Note as GpifNote, Property as GpifProperty,
+    Track as GpifTrack, Voice as GpifVoice,
+};
 use guitarpro::io::gpx::{read_gp, read_gpx};
 use guitarpro::model::legacy::key_signature::Duration as GpDuration;
 use guitarpro::model::legacy::note::NoteEffect as GpNoteEffect;
@@ -158,30 +161,145 @@ pub enum GpImportError {
 /// Conversion losses are carried on the returned [`Score`] as a [`LossReport`].
 pub fn import_gp_score(data: &[u8]) -> Result<Score, GpImportError> {
     let mut song = guitarpro::Song::default();
-    match detect_gp_version(data) {
-        Some(3) => song.read_gp3(data)?,
-        Some(4) => song.read_gp4(data)?,
-        Some(5) => song.read_gp5(data)?,
+    let unrestored_voices = match detect_gp_version(data) {
+        Some(3) => {
+            song.read_gp3(data)?;
+            0
+        }
+        Some(4) => {
+            song.read_gp4(data)?;
+            0
+        }
+        Some(5) => {
+            song.read_gp5(data)?;
+            0
+        }
         Some(6) => read_gpif_song(&mut song, &read_gpx(data)?, 6),
         // GP7/8 decode to the same GPIF the GP6 path uses; `read_gp` unzips
         // `Content/score.gpif`. The Song's version.number.0 becomes 7, so
         // `gp_song_to_score` tags "GP7" and takes the same `>= 6` behaviour
         // (raw repeat counts).
         Some(7) => read_gpif_song(&mut song, &read_gp(data)?, 7),
         _ => return Err(GpImportError::UnsupportedFormat),
-    }
-    Ok(gp_song_to_score(&song))
+    };
+    let mut score = gp_song_to_score(&song);
+    if unrestored_voices > 0 {
+        score.loss.add(ImportWarning::Other(format!(
+            "GPIF tapping and hammer-on origins not restored in {unrestored_voices} voice(s): \
+             the converted beat/note layout differs from the GPIF document"
+        )));
+    }
+    Ok(score)
 }
 
 // ── GPIF string orientation ───────────────────────────────────────────────────
 
 /// Reads a parsed GPIF document (GP6 `.gpx`, GP7/8 `.gp`) into `song` — the
-/// crate's own `read_gpx` / `read_gp` steps — then renumbers its strings to the
-/// convention the rest of this adapter reads ([`normalise_gpif_strings`]).
-fn read_gpif_song(song: &mut guitarpro::Song, gpif: &Gpif, major: u8) {
+/// crate's own `read_gpx` / `read_gp` steps — then restores the note
+/// techniques the conversion loses ([`restore_gpif_note_techniques`]) and
+/// renumbers its strings to the convention the rest of this adapter reads
+/// ([`normalise_gpif_strings`]). Returns how many voices could not be restored.
+fn read_gpif_song(song: &mut guitarpro::Song, gpif: &Gpif, major: u8) -> usize {
     song.version.number = (major, 0, 0);
     song.read_gpif(gpif);
+    let unrestored = restore_gpif_note_techniques(song, gpif);
     normalise_gpif_strings(song, gpif);
+    unrestored
+}
+
+/// Whitespace-separated GPIF ids (`-1` marks an empty voice slot).
+fn gpif_ids(ids: &str) -> Vec<i32> {
+    ids.split_whitespace()
+        .filter_map(|id| id.parse().ok())
+        .collect()
+}
+
+/// Restores the GPIF note techniques the `guitarpro` 0.4.2 conversion drops
+/// or merges:
+///
+/// - **`Tapped`** is never read there (the beat's tap effect stays a
+///   placeholder). A beat with a tapped note gets the legacy beat-level
+///   `SlapEffect::Tapping`, exactly how GP3/4/5 store tapping, so the rest of
+///   this adapter marks its notes `NoteMark::Tap`;
+/// - **`HopoOrigin` / `HopoDestination`** both set the one legacy `hammer`
+///   flag there, while GP3/4/5 set it on the origin note only (the flag means
+///   "legato to the next note on this string"). The flag is reset to
+///   `HopoOrigin` alone.
+///
+/// The GPIF document is walked the way the crate walks it — each track's bar
+/// per master bar, voice slots skipping `-1`, then existing beats and notes in
+/// order — and zipped with the converted song. A voice whose converted beats
+/// or notes do not line up with the document is left as converted and
+/// counted, never re-labelled out of step. Returns that count.
+fn restore_gpif_note_techniques(song: &mut guitarpro::Song, gpif: &Gpif) -> usize {
+    let bars: HashMap<i32, &GpifBar> = gpif.bars.bars.iter().map(|b| (b.id, b)).collect();
+    let voices: HashMap<i32, &GpifVoice> = gpif.voices.voices.iter().map(|v| (v.id, v)).collect();
+    let beats: HashMap<i32, &GpifBeat> = gpif.beats.beats.iter().map(|b| (b.id, b)).collect();
+    let notes: HashMap<i32, &GpifNote> = gpif.notes.notes.iter().map(|n| (n.id, n)).collect();
+    let enabled = |note: &GpifNote, name: &str| {
+        note.properties
+            .properties
+            .iter()
+            .any(|p| p.name == name && p.enable.is_some())
+    };
+
+    let mut unrestored = 0_usize;
+    for (track_index, track) in song.tracks.iter_mut().enumerate() {
+        for (measure, master_bar) in track.measures.iter_mut().zip(&gpif.master_bars.master_bars) {
+            let Some(bar) = gpif_ids(&master_bar.bars)
+                .get(track_index)
+                .and_then(|id| bars.get(id))
+            else {
+                continue;
+            };
+            let voice_ids: Vec<i32> = gpif_ids(&bar.voices)
+                .into_iter()
+                .filter(|&id| id >= 0)
+                .collect();
+            for (voice, voice_id) in measure.voices.iter_mut().zip(voice_ids) {
+                let Some(g_voice) = voices.get(&voice_id) else {
+                    continue;
+                };
+                let g_beats: Vec<&GpifBeat> = gpif_ids(&g_voice.beats)
+                    .iter()
+                    .filter_map(|id| beats.get(id).copied())
+                    .collect();
+                let g_notes: Vec<Vec<&GpifNote>> = g_beats
+                    .iter()
+                    .map(|b| {
+                        b.notes
+                            .as_deref()
+                            .map(gpif_ids)
+                            .unwrap_or_default()
+                            .iter()
+                            .filter_map(|id| notes.get(id).copied())
+                            .collect()
+                    })
+                    .collect();
+                let aligned = voice.beats.len() == g_notes.len()
+                    && voice
+                        .beats
+                        .iter()
+                        .zip(&g_notes)
+                        .all(|(beat, g)| beat.notes.len() == g.len());
+                if !aligned {
+                    unrestored = unrestored.saturating_add(1);
+                    continue;
+                }
+                for (beat, g_beat_notes) in voice.beats.iter_mut().zip(&g_notes) {
+                    let mut tapped = false;
+                    for (note, g_note) in beat.notes.iter_mut().zip(g_beat_notes) {
+                        note.effect.hammer = enabled(g_note, "HopoOrigin");
+                        tapped |= enabled(g_note, "Tapped");
+                    }
+                    if tapped {
+                        beat.effect.slap_effect = guitarpro::SlapEffect::Tapping;
+                    }
+                }
+            }
+        }
+    }
+    unrestored
 }
 
 /// A GPIF track's tuning as stored: open-string pitches, **lowest string

core/tests/gp_gpif_techniques.rs

@@ -0,0 +1,195 @@
+//! Red → GPIF (GP6 `.gpx` / GP7 `.gp`) imports keep tapping and read
+//! hammer-on/pull-off with the GP3/4/5 semantics.
+//!
+//! Two losses in the `guitarpro` 0.4.2 GPIF conversion, measured on a 410-file
+//! corpus (31 of 145 GPIF files carry tapping):
+//!
+//! - the `Tapped` note property is never read (the beat's tap effect is left a
+//!   placeholder), so no GP6/GP7 tapping reached griff;
+//! - `HopoOrigin` and `HopoDestination` both set the one legacy `hammer` flag.
+//!   GP3/4/5 set it on the origin note only, and griff's `HammerOn` span
+//!   follows that note. On GPIF input the destination got a span too, so two
+//!   adjacent independent hammer-on pairs were indistinguishable from one chain.
+//!
+//! The fixtures are authored GPIF text packed with the crate's public writer;
+//! no copyrighted tab.
+
+#![allow(
+    clippy::expect_used,
+    clippy::unwrap_used,
+    clippy::missing_assert_message,
+    clippy::indexing_slicing
+)]
+
+use griff_core::{
+    event::{NoteMark, SpanTechnique},
+    gp::import_gp_score,
+    score::{AtomEvent, Score},
+};
+use guitarpro::io::gpx::{compress_bcfz, pack_bcfs};
+use std::fmt::Write as _;
+
+/// One note: GPIF raw string (0 = lowest), fret, and enabled note properties.
+struct GpifNote {
+    string: u8,
+    fret: u8,
+    enabled: &'static [&'static str],
+}
+
+const fn n(string: u8, fret: u8, enabled: &'static [&'static str]) -> GpifNote {
+    GpifNote {
+        string,
+        fret,
+        enabled,
+    }
+}
+
+/// A one-track Standard-E GPIF, one quarter-note beat per entry of `beats`.
+fn gpif(beats: &[Vec<GpifNote>]) -> String {
+    let flat: Vec<(usize, &GpifNote)> = beats
+        .iter()
+        .enumerate()
+        .flat_map(|(b, notes)| notes.iter().map(move |note| (b, note)))
+        .collect();
+    let mut note_xml = String::new();
+    for (id, (_, note)) in flat.iter().enumerate() {
+        write!(
+            note_xml,
+            r#"<Note id="{id}"><Properties><Property name="Fret"><Fret>{}</Fret></Property><Property name="String"><String>{}</String></Property>"#,
+            note.fret, note.string
+        )
+        .expect("writing to a String");
+        for p in note.enabled {
+            write!(note_xml, r#"<Property name="{p}"><Enable /></Property>"#)
+                .expect("writing to a String");
+        }
+        note_xml.push_str("</Properties></Note>");
+    }
+    let mut beat_xml = String::new();
+    for b in 0..beats.len() {
+        let ids: Vec<String> = flat
+            .iter()
+            .enumerate()
+            .filter(|(_, (beat, _))| *beat == b)
+            .map(|(id, _)| id.to_string())
+            .collect();
+        write!(
+            beat_xml,
+            r#"<Beat id="{b}"><Rhythm ref="0"/><Notes>{}</Notes></Beat>"#,
+            ids.join(" ")
+        )
+        .expect("writing to a String");
+    }
+    let beat_ids: Vec<String> = (0..beats.len()).map(|b| b.to_string()).collect();
+    format!(
+        r#"<?xml version="1.0" encoding="utf-8"?>
+<GPIF>
+<GPVersion>7</GPVersion>
+<Score><Title><![CDATA[synthetic]]></Title></Score>
+<MasterTrack><Tracks>0</Tracks></MasterTrack>
+<Tracks>
+<Track id="0">
+<Name><![CDATA[Guitar]]></Name>
+<ShortName><![CDATA[gtr]]></ShortName>
+<Properties><Property name="Tuning"><Pitches>40 45 50 55 59 64</Pitches></Property></Properties>
+</Track>
+</Tracks>
+<MasterBars>
+<MasterBar><Time>4/4</Time><Bars>0</Bars></MasterBar>
+</MasterBars>
+<Bars><Bar id="0"><Voices>0 -1 -1 -1</Voices></Bar></Bars>
+<Voices><Voice id="0"><Beats>{}</Beats></Voice></Voices>
+<Beats>{beat_xml}</Beats>
+<Notes>{note_xml}</Notes>
+<Rhythms><Rhythm id="0"><NoteValue>Quarter</NoteValue></Rhythm></Rhythms>
+</GPIF>"#,
+        beat_ids.join(" ")
+    )
+}
+
+fn import(beats: &[Vec<GpifNote>]) -> Score {
+    let xml = gpif(beats);
+    import_gp_score(&compress_bcfz(&pack_bcfs("score.gpif", xml.as_bytes())))
+        .expect("synthetic GPIF imports")
+}
+
+/// Per event group: whether it carries a hammer-on span, and each note's tap mark.
+fn groups(score: &Score) -> Vec<(bool, Vec<bool>)> {
+    score.tracks[0].voices[0]
+        .event_groups
+        .iter()
+        .map(|g| {
+            let hammer = g
+                .technique_spans
+                .iter()
+                .any(|s| s.technique == SpanTechnique::HammerOn);
+            let taps = g
+                .atoms
+                .iter()
+                .filter_map(|a| match a {
+                    AtomEvent::Note(note) => Some(note.marks.contains(NoteMark::Tap)),
+                    AtomEvent::Rest(_) => None,
+                })
+                .collect();
+            (hammer, taps)
+        })
+        .collect()
+}
+
+#[test]
+fn only_the_hopo_origin_carries_the_hammer_span() {
+    // D string: 5 hammered to 7, then an unrelated 5.
+    let score = import(&[
+        vec![n(2, 5, &["HopoOrigin"])],
+        vec![n(2, 7, &["HopoDestination"])],
+        vec![n(2, 5, &[])],
+    ]);
+    let hammers: Vec<bool> = groups(&score).iter().map(|g| g.0).collect();
+    assert_eq!(hammers, vec![true, false, false]);
+}
+
+#[test]
+fn a_hopo_chain_marks_every_origin_and_not_the_last_destination() {
+    // 5 → 7 → 5: the middle note is destination and origin.
+    let score = import(&[
+        vec![n(2, 5, &["HopoOrigin"])],
+        vec![n(2, 7, &["HopoDestination", "HopoOrigin"])],
+        vec![n(2, 5, &["HopoDestination"])],
+    ]);
+    let hammers: Vec<bool> = groups(&score).iter().map(|g| g.0).collect();
+    assert_eq!(hammers, vec![true, true, false]);
+}
+
+#[test]
+fn two_adjacent_hopo_pairs_stay_two_pairs() {
+    // 5 → 7, then 5 → 7: no hammer span may join the first pair to the second.
+    let score = import(&[
+        vec![n(2, 5, &["HopoOrigin"])],
+        vec![n(2, 7, &["HopoDestination"])],
+        vec![n(2, 5, &["HopoOrigin"])],
+        vec![n(2, 7, &["HopoDestination"])],
+    ]);
+    let hammers: Vec<bool> = groups(&score).iter().map(|g| g.0).collect();
+    assert_eq!(hammers, vec![true, false, true, false]);
+}
+
+#[test]
+fn tapped_notes_are_imported_as_taps() {
+    // 5, tap 12, 5 on the D string.
+    let score = import(&[
+        vec![n(2, 5, &[])],
+        vec![n(2, 12, &["Tapped"])],
+        vec![n(2, 5, &[])],
+    ]);
+    let taps: Vec<Vec<bool>> = groups(&score).into_iter().map(|g| g.1).collect();
+    assert_eq!(taps, vec![vec![false], vec![true], vec![false]]);
+}
+
+#[test]
+fn a_tapped_note_in_a_chord_taps_its_beat() {
+    // Guitar Pro's legacy model holds tapping per beat, as GP3/4/5 do: a chord
+    // beat with one tapped note marks the beat's notes. Other beats stay untouched.
+    let score = import(&[vec![n(2, 5, &[]), n(3, 7, &["Tapped"])], vec![n(2, 5, &[])]]);
+    let taps: Vec<Vec<bool>> = groups(&score).into_iter().map(|g| g.1).collect();
+    assert_eq!(taps, vec![vec![true, true], vec![false]]);
+}

docs/decisions.log.md

@@ -2834,3 +2834,20 @@ Architectural decisions go to [`adr/`](adr/) instead.
   fingering audit (PhysShell/griff#197), whose line normalization masked
   the GP6 half. The crate defect is upstream's to fix; this adapter no
   longer depends on it.
+
+- 2026-09-17 — In the context of GPIF imports losing note techniques in
+  `guitarpro` 0.4.2 (the `Tapped` property never read; `HopoOrigin` and
+  `HopoDestination` merged into one hammer flag), we decided to **restore
+  them at the import boundary in griff, walking the GPIF document as the
+  crate walks it, rather than depend on a fork of the crate**, to achieve
+  GP6/7 technique labels with the same semantics as GP3/4/5 now, accepting
+  a second workaround next to the tuning one (#198) until upstream ships a
+  fix. A git dependency on a fork is ruled out by `deny.toml`
+  (`unknown-git = "deny"`) and would need vendoring hashes in the nix and
+  wasm builds; the fix is instead offered upstream (Codeberg
+  `slundi/scorelib`), and griff drops the workaround once a release carries
+  it, keeping its regression tests. Measured on the corpus's 145 GPIF files:
+  tapped notes 0 → 2,013, destination-only hammer spans removed (18,284 →
+  10,906), hammer edges on the same string 76.8% → 99.7%; GP3/4/5 unchanged.
+  Deriving hammer-on versus pull-off direction for all formats is a separate
+  decision (it changes corpus technique tags everywhere).