Skip to content

Conversation

@davepagurek
Copy link
Contributor

@davepagurek davepagurek commented Dec 3, 2025

Resolves #8319

Changes:

  • Adds a vertical shift to text when vertically centering to make it look more centered in its bounding box.

Screenshots of the change:

BeforeAfter
image image
image image
image image

PR Checklist

@davepagurek davepagurek requested a review from dhowe December 3, 2025 21:41
Copy link
Contributor

@dhowe dhowe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to work for some fonts, but not others.

For example consider the following manual test, from test/manual-test-examples/type/index.html

pr:
image

dev-2.0:
image

@davepagurek
Copy link
Contributor Author

That's the browser default font, right? If I draw a single line of text, and draw a straight line at the y value I want my text centered at, on this PR it looks like this:

image

Whereas previously it looked like this, which seems less centered:
image

Live version to test it out: https://editor.p5js.org/davepagurek/sketches/DJt2049Qn

To me, the above looks much more like the text is centered at the line I draw.

For multi lines, it does look less centered with these changes:

image

compared to the previous:
image

So maybe this is telling me that there's something happening in the single line case that's becoming a problem? Single line is a tad different because textLeading shouldn't play a part, I'll take a closer look to see what's going on there.

@dhowe
Copy link
Contributor

dhowe commented Dec 3, 2025

I guess it depends on the mix of upper-case and lower-case letters, but I was thinking of it in terms of the bounding box:

new-pr:
image

existing:
image

@davepagurek
Copy link
Contributor Author

I think either of those for the default font is good for me, but for other fonts it gets quite offset. Ideally I'd want it halfway between the alphabetic baseline and the caps height. This PR doesnt do that perfectly, definitely open to more suggestions there.

@davepagurek
Copy link
Contributor Author

With the above goal in mind, I found that shifting by fontBoundingBoxAscent universally puts fonts at the baseline, so the question is how to shift them down halfway through the cap height. For some fonts fontBoundingBoxAscent/2 works well, but others have a better estimate as sCapHeight in the font data, so this now uses:

p5.Renderer.prototype._middleAlignOffset = function() {
  const { textFont, textSize } = this.states;
  const font = textFont?.font
  const ctx = this.textDrawingContext();
  const metrics = ctx.measureText('X');
  let sCapHeight = (font?.data || {})['OS/2']?.sCapHeight
  if (sCapHeight) {
    const unitsPerEm = font.data.head.unitsPerEm;
    sCapHeight *= textSize / unitsPerEm;
  } else {
    sCapHeight = metrics.fontBoundingBoxAscent;
  }
  return metrics.alphabeticBaseline + sCapHeight / 2;
}

Here's another before/after comparison, with HTML flexbox centering shown below as a comparison (the HTML one is the same in the new + old code). The new code (left) more closely matches HTML + what I expect for HennyPenny onwards, half the fonts I'm testing:

FontNewPrevious
sans-serif image image
Inter image image
Raleway image image
HennyPenny image image
Bree Serif image image
Poppins image image
Inknut Antiqua image image
Oswald image image

With this change, the centering in the manual test example looks more like before:

NewLast iterationdev-2.0
image image image

@davepagurek
Copy link
Contributor Author

as a bonus, the existing screenshots on dev-2.0 for text alignment seem to work, because this new iteration seems to leave more fonts aligned how they were before, while updating the problematic ones.

Copy link
Member

@ksen0 ksen0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the helpful overview table! Very minor/optional: is there a visual test that can be added to prevent regression? Because the arithmetic of yOff is more complex now / includes a helper

@davepagurek
Copy link
Contributor Author

good idea! I added some visual tests going through some of the fonts from the comparison above, showing a line down the middle.
000
000

@davepagurek davepagurek merged commit d287796 into dev-2.0 Dec 5, 2025
5 checks passed
@davepagurek davepagurek deleted the fix/font-align branch December 5, 2025 13:55
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 this pull request may close these issues.

4 participants