Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misaligned cursor on quintuplets #1527

Open
alexshenker opened this issue Mar 23, 2024 · 1 comment
Open

Misaligned cursor on quintuplets #1527

alexshenker opened this issue Mar 23, 2024 · 1 comment

Comments

@alexshenker
Copy link

alexshenker commented Mar 23, 2024

Cursor thinks the empty spaces are notes.
img 1 - result of clicking 'E'
img 2 - result of clicking 'F-nat'
The C, in the middle of the first measure is not clickable - the cursor thinks that is the location of the A.
Can be reproduced on the live osmd sponsored demo with the attached .mxl.

1

2

Sample MXL:
brahms_mini.mxl.zip

@sschmidTU
Copy link
Contributor

sschmidTU commented Mar 25, 2024

The problem is that the x-position for these first few notes is wrong.
image
We get these from Vexflow, which in some cases isn't precise.
The best bet currently is probably fixing the stavenote.getBoundingBox() method in Vexflow, which VexflowStaffEntry.calculateXPosition() uses.
I haven't found the time for that yet, maybe you can look into it? I would debug the method for this sample.

I tried using the SVG bounding box which is accurate, but I think the SVG doesn't exist at this point of the code yet.

public calculateXPosition(): void {
        const stave: VF.Stave = (this.parentMeasure as VexFlowMeasure).getVFStave();

        // sets the vexflow x positions back into the bounding boxes of the staff entries in the osmd object model.
        // The positions are needed for cursor placement and mouse/tap interactions
        let lastBorderLeft: number = 0;
        for (const gve of this.graphicalVoiceEntries as VexFlowVoiceEntry[]) {
            if (gve.vfStaveNote) {
                gve.vfStaveNote.setStave(stave);
                if (!gve.vfStaveNote.preFormatted) {
                    continue;
                }
                gve.applyBordersFromVexflow();
                let isSecondaryWholeRest: boolean = false;
                let bboxToAdjust: BoundingBox = this.PositionAndShape;
                if (gve.notes[0].sourceNote.isWholeRest() && !this.hasOnlyRests()) {
                    isSecondaryWholeRest = true;
                    // continue; // also an option (simpler), but makes the voice entry bounding boxes very wrong (shifted)
                    bboxToAdjust = gve.PositionAndShape;
                    // don't use a whole rest's position for the staffentry.x if we also have a normal note in another voice (#1267)
                    //   a more ideal solution would probably be to give a secondary whole note its own staffentry and staffentry position,
                    //   since it's so different from a normal note which is also the first note of the measure.
                    //   But we probably have some code that assumes there's only one staffentry per staff per timestamp.
                    //   "A [[SourceStaffEntry]] is a container spanning all the [[VoiceEntry]]s at one timestamp for one [[StaffLine]]"
                }
                if (this.parentMeasure.ParentStaff.isTab) {
                    // the x-position could be finetuned for the cursor.
                    // somehow, gve.vfStaveNote.getBoundingBox() is null for a TabNote (which is a StemmableNote).
                    bboxToAdjust.RelativePosition.x = (gve.vfStaveNote.getAbsoluteX() + (<any>gve.vfStaveNote).glyph.getWidth()) / unitInPixels;
                } else {
                    let svgXPositionFound: boolean = false;
                    for (const note of gve.notes) {
                        const bboxX: number = (note as VexFlowGraphicalNote).getSVGGElement()?.getBBox()?.x;
                        // SVGGElement usually doesn't exist here yet, unfortunately.
                        if (bboxX !== undefined && bboxX !== 0) {
                            bboxToAdjust.RelativePosition.x = bboxX / unitInPixels;
                            svgXPositionFound = true;
                            break;
                        }
                    }
                    if (!svgXPositionFound) {
                        bboxToAdjust.RelativePosition.x = gve.vfStaveNote.getBoundingBox().getX() / unitInPixels;
                    }
                    if (isSecondaryWholeRest) {
                        bboxToAdjust.RelativePosition.x -= stave.getNoteStartX() / unitInPixels;
                        bboxToAdjust.RelativePosition.x -= 1.3;
                        // fix whole rest bounding box for these cases, slightly hacky admittedly, probably depends on WholeRestXShiftVexflow
                    }
                }
                const sourceNote: Note = gve.notes[0].sourceNote;
                if (sourceNote.isRest() && sourceNote.Length.RealValue === this.parentMeasure.parentSourceMeasure.ActiveTimeSignature.RealValue) {
                    // whole rest: length = measure length. (4/4 in a 4/4 time signature, 3/4 in a 3/4 time signature, 1/4 in a 1/4 time signature, etc.)
                    // see Note.isWholeRest(), which is currently not safe
                    bboxToAdjust.RelativePosition.x +=
                        this.parentMeasure.parentSourceMeasure.Rules.WholeRestXShiftVexflow - 0.1; // xShift from VexFlowConverter
                    gve.PositionAndShape.BorderLeft = -0.7;
                    gve.PositionAndShape.BorderRight = 0.7;
                }
                if (gve.PositionAndShape.BorderLeft < lastBorderLeft) {
                    lastBorderLeft = gve.PositionAndShape.BorderLeft;
                }
            }
        }
        // this.PositionAndShape.RelativePosition.x -= lastBorderLeft;
        // TODO sometimes subtracting lastBorderLeft fixes the x-position for lyrics spacing, sometimes it makes it wrong
        //   e.g. wrong for Beethoven Geliebte measure 1 ("auf - dem", distance < width of "auf"), correct for measure 3 ("spä - hend")
        //   this leads to a (lyrics) measure elongation of ~1.3 for measure 1, though it doesn't need any elongation (should be factor 1)
        this.PositionAndShape.calculateBoundingBox();
    }

(you need to add the import VexFlowGraphicalNote for this)

A workaround would be to check and prefer the SVG bounding boxes instead of their OSMD bounding box when checking which note was clicked, though that wouldn't fix the OSMD bounding box (GraphicalNote/VoiceEntry/StaffEntry position).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants