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

Fix vertical anchor on font symbols to the baseline #60044

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

signedav
Copy link
Contributor

@signedav signedav commented Jan 3, 2025

With Font Marker Symbols an offset ("off1" in the sketch) is appended to the coordinates to know where to start drawing the symbol (at "point"). This offset is half of the scaled defined size (what later defines the size from baseline to top of a sign without accents). On the other hand, an offset ("off2") is appended to this point concerning the vertical anchor (top, bottom, center). This second offset ("off2") was not half of the scaled defined size, but the half of the distance between baseline and the FontMetric Ascent - and these two measurements differed! What leaded to the wrong placement (alpha and beta)...

With this fix, the two offsets are always calculated the same, which leads to a correct position of the FontMarkers on Bottom.

IMG_20250103_091221

Means before on bottom (example with Font DejaVu Sans and CadastraSymbol):

Screenshot from 2025-01-03 09-25-44

Screenshot from 2025-01-03 09-26-13

Now:

Screenshot from 2025-01-03 09-25-38

Screenshot from 2025-01-03 09-26-28

Fixes #59732

Since this fix should not affect positions of markers in existing projects, we had to append a new setting instead of replacing the old. With the implementation of calculating the positions with bounds (like mentioned in this issue #59732) the existing anchor types should become legacy. This is prepared in the following implementation, having a verticalAnchorMode in the FontMarker. As well it's provided that when I configure a Font Marker with a Bottom on Baseline, then change the marker to an SVG Marker, the anchor is Bottom, and when I go back to Font Marker it's Bottom on Baseline again.

What about "Top on Baseline" (and Center)?
With this fix as well the Top and the Center Anchors would differ on the verticalAnchorMode:Baseline. But since it's not really recognizable and - I think - rarely used, I did not add it to the settings. But the backend with the verticalAnchorMode, would be prepared to add them on request.

The implementation of the bounds might follow. I see what I can do...

…lculated with the same offset like the offset of the font. this differed before what leaded to issues. still the old way is available as legacy
… setting for this mode making a differene between legacy and baseline (and future bounds) on the positioning of the characters
@github-actions github-actions bot added this to the 3.42.0 milestone Jan 3, 2025
@nyalldawson
Copy link
Collaborator

I'd like to review this but can't for the next couple of weeks. please do not merge before then!

@signedav
Copy link
Contributor Author

signedav commented Jan 3, 2025

@nyalldawson Cool. I hoped for your feedback, since you already brought up your thoughts in the issue.

Since it's definitely a bug (although a side case) it would be great to have it in the upcoming LTR. Hope this can still be possible.

Copy link

github-actions bot commented Jan 3, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 475bbc4)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 475bbc4)

This comment was marked as outdated.

This comment was marked as outdated.

@m-kuhn
Copy link
Member

m-kuhn commented Jan 9, 2025

Test comments are outdated and no longer valid (now hidden)
It would be great if these could be backported to 3.40, @nyalldawson do you think you'll be able to review before 3.42?

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

Successfully merging this pull request may close these issues.

Allow using baseline for font marker placement
3 participants