Skip to content

add text height calculation for multiline text area #5404

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

Closed
outofambit opened this issue Sep 11, 2021 · 7 comments
Closed

add text height calculation for multiline text area #5404

outofambit opened this issue Sep 11, 2021 · 7 comments

Comments

@outofambit
Copy link
Contributor

(As previously noted in #5366)

We support vertical alignment of text with textAlign(), but right now the vertical height for multiline text is not computed. This means that a single line text is vertically aligned properly, but not multiline text.

It might be tricky to calculate the total height of multiline text because the text wrap style will change how many lines of text are laid out.

https://editor.p5js.org/outofambit/sketches/eqIUrqXH6 would demonstrate this issue, but won't render until we release a version of p5.js with the bug fix in #5366.

@dhowe
Copy link
Contributor

dhowe commented Sep 12, 2021

image

@dhowe dhowe self-assigned this Sep 13, 2021
@dhowe
Copy link
Contributor

dhowe commented Sep 16, 2021

can someone help double-check these alignments?

image

@dhowe
Copy link
Contributor

dhowe commented Sep 16, 2021

NOTES:

  1. seems vertical 'baseline' and 'top' are identical when a width is specified, which appears to be the case in existing reference tests as well.

image

  1. I have changed the algorithm so that spaces are not included on the right edge of right-aligned text, which I believe is correct, but is a change from previous behavior.

  2. When comparing with images generated from Processing (in test/manual-test-examples/p5.font), one will notice different leading. This is due to how leading is computed when textSize but not textLeading is called:

      Processing (top): (textAscent() + textDescent()) * 1.275f;
      p5.js (bottom):    textSize * 1.25

image

@outofambit
Copy link
Contributor Author

hey @dhowe thanks so much for doing this! i was going to say all the examples look right but i wasn't sure about baseline vs center but it seems you've figured that out. :) For the leading question, i think we might want to fix that, but that doesn't need to happen as part of this fix. (i just checked and the behavior your're showing is the same on the main branch.) so i think this is good as-is!

dhowe added a commit that referenced this issue Oct 1, 2021
@dhowe
Copy link
Contributor

dhowe commented Oct 1, 2021

Cool. If I remember correctly the different manner of calculating leading was a group decision made at the time as it yielded better results, but certainly open to re-discussing.

@dhowe
Copy link
Contributor

dhowe commented Oct 1, 2021

also @outofambit perhaps you have a preference, one way or the other, regarding #5408 ...

@outofambit
Copy link
Contributor Author

fixed with #5432!

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

No branches or pull requests

2 participants