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

add draw_align for graphics::Text #344

Closed
wants to merge 1 commit into from
Closed

add draw_align for graphics::Text #344

wants to merge 1 commit into from

Conversation

Chen1Plus
Copy link

@Chen1Plus Chen1Plus commented Jan 27, 2024

resolve #343

@17cupsofcoffee
Copy link
Owner

Thanks for picking this up!

I've tested this locally and it seems to work - my only concern is that this approach (aligning as part of drawing, rather than as part of layout) gives you some surprising behaviour with multi-line text:

image

The Text as a whole is centered, but each line is still left-aligned.

If possible, I think it would be better to do the centering as part of the layout (which lives in src/graphics/text/cache.rs). The way I've seen this done in other libraries is:

  • At the start of laying out the text, calculate the width of the first line up-front.
    • The existing measure_word function will most of this for you, I think - you'd just need to also include spaces in the total.
  • If the text is centered, set cursor.x to -line_width / 2.0; if the text is right-aligned, set cursor.x to -line_width.
  • The rest of the existing layout logic should then place the characters in the right place.
  • Repeat the line width calculation and offsetting of the cursor every time a new line starts.

This should make multi-line text work as you'd expect, and it also has the nice side effect of making it so Text::get_bounds gives you bounds that take into account alignment.

The only slightly annoying thing about this approach is that you can't re-use the same Text for multiple alignments (at least not without repeatng the layout, like we do when you change the content/font).

Let me know what you think - maybe I'm overthinking this 😄

@Chen1Plus
Copy link
Author

I agree with you. It would be a better solution. However, I've been a bit busy lately, so I might need some more time to open next PR for this.

@Chen1Plus Chen1Plus closed this Jan 30, 2024
@17cupsofcoffee
Copy link
Owner

I agree with you. It would be a better solution. However, I've been a bit busy lately, so I might need some more time to open next PR for this.

That's no problem, appreciate the work you've done up to this point!

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.

add alignment option for text
2 participants