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

Problems with Annotation placement #1265

Closed
AaronDavidNewman opened this issue Dec 20, 2021 · 5 comments · Fixed by #1270
Closed

Problems with Annotation placement #1265

AaronDavidNewman opened this issue Dec 20, 2021 · 5 comments · Fixed by #1270

Comments

@AaronDavidNewman
Copy link
Collaborator

I found this issue while investigating vertical formatting, #411. There are several problems in this issue so I am trying to address them one at a time. This is about the annotations, which do some crazy stuff. TL;DR: annotation Y placement is incorrect and will take some mildly breaking changes to fix.

Here is an example of some placements as the current code does them. The text 'topa' means 'top justification, above position' - see below for what that means.

image

  1. There are 2 competing parameters, one used during render and one used during formatting. For formatting, we only consider ModifierPosition, either ABOVE, or...not above, which is treated like below even though there are horizontal values. Based on above or below, it acts like other modifiers and calculates its line position and reports to the formatter how much line space it takes up. Aside from the issue of the parameter, the calculation seems to be almost correct.
  2. In the draw method, the position parameter is completely ignored in favor of the VerticalJustify parameter. It uses the line value calculated in the formatter based on position, but then applies it to a direction based on the vertical justify. This could lead to many combinations of things happening depending on how justification or placement are mixed.
  3. The factory Annotation constructor has default of BOTTOM for the vertical justification. The annotation constructor used by itself has a default of TOP, but a position default of 'LEFT', which is treated like BOTTOM! This is clearly not right.
  4. The rendered y position is calculated differently depending on whether vertical justification is bottom or top. I think this is to take into account the fact that text Y is aligned to the baseline, but it does not consider the correct text Y based on the font. You can see this issue below. It also does something different depending on the stem direction on the bottom, but not on the top, not quite sure what's up with that.
  5. The formatter calculates Y positions correctly, but for some reason adds different amounts of padding for above vs. below.

Based on all this, I think we can do the following:

  1. we should consider VerticalJustification and not ModifierPosition when calculating Y for both rendering and drawing. Only one parameter should specify Y placement, and since it could be a breaking change either way, I think it makes sense to keep the annotation-specific parameter. Even though it's not really 'justification' but placement (vertical justification would justify horizontal text on a vertical axis, which is not what we're doing here). I guess the intent is justifying relative to the note.
  2. If Y justification is set as CENTER or CENTER STEM, we shouldn't report any vertical line change in the note to the formatter.
  3. For 'above' or 'below', we should use the same text height value when rendering as we do when formatting, and the padding should be the same, just in a different direction.
@sschmidTU
Copy link
Contributor

To me it looks like VerticalJustify should just be removed as a paremeter, as it doesn't make much sense as you say, as it's positioning and not justification. And we have ModifierPosition as an established parameter for exactly this goal. But that's just my quick impression.

@AaronDavidNewman
Copy link
Collaborator Author

AaronDavidNewman commented Dec 21, 2021

@sschmidTU there is also a horizontalJustification in annotation, that is the downside. So then we'd have position and justification. So it's confusing either way, I will defer to the will of the community.

I am leaning towards using the justifications, and warn if someone calls setPosition since it has no effect. If someone calls setPosition with one of the horizontal parameters, we can set the horizontalJustification and likewise with the vertical.

I had the same issue with chordSymbol, and I went with using the justification parameters, and ignoring position altogether. So chord symbol and annotation would be somewhat consistent in this way.

In an ideal world, we'd have horizontal and vertical position in Modifier, maybe. A single position parameter makes more sense for modifiers that already have some rules for how they relate to the notehead, like ornaments and articulation. But since annotations can be almost anything, you need 2 parameters.

@mscuthbert
Copy link
Collaborator

This is a very hard problem (Finale made solving it their main release feature about 5 years ago, and Dorico made the fact that they had solved it one of their main selling points). I would definitely not try to get this into v4 before a release there.

@AaronDavidNewman
Copy link
Collaborator Author

This is a very hard problem

@mscuthbert which problem are you referring to? My mission for this issue is just to keep the vertically stacked modifiers from running into each other. This seems pretty do-able, since most modifiers handle this correctly already.

AaronDavidNewman added a commit to AaronDavidNewman/vexflow that referenced this issue Dec 23, 2021
Fix some issues with placement.  Also  fix some issues found in my last accidental PR with articulation placement.  Because of an error in sign, a code leg would never have been traversed.  I added a test case for that and fixed the error.
@AaronDavidNewman
Copy link
Collaborator Author

Modifiers treat top_text_line or text_line inconsistently. Usually it is treated with regard to the note, but Bend.format (for instance) assumes the text line is relative to the staff. This causes issues when modifiers are mixed.

I think all the modifiers that are vertically placed should treat the vertical state.text_line wrt to the closest note head (or string in the case of TabNote.

0xfe added a commit that referenced this issue Jan 15, 2022
fix #1265: Fix issues with vertical placement of annotations
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

Successfully merging a pull request may close this issue.

3 participants