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 invalid MonoStyle text dimensions calculation #648

Merged
merged 2 commits into from
Feb 11, 2022

Conversation

derchr
Copy link
Contributor

@derchr derchr commented Feb 2, 2022

The method len() on &str does return the number of bytes of a character.
Using characters such as '°' (which are two bytes long) caused an
incorrect calculation of the text bounding box.

@rfuest
Copy link
Member

rfuest commented Feb 2, 2022

Good catch, thanks for fixing this. There is another use of text.len() in the draw_string method, which also needs to be fixed.

Could you also add a unit test for the invalid bounding box? You can simply copy transparent_text_dimensions_one_line and modify it to use a string with multi byte characters and e.g. a latin1 font.

@derchr
Copy link
Contributor Author

derchr commented Feb 2, 2022

I'll look into it this weekend!

@derchr derchr force-pushed the work/text-fix branch 2 times, most recently from c303e2b to 0f31213 Compare February 5, 2022 13:46
@derchr
Copy link
Contributor Author

derchr commented Feb 9, 2022

I also changed the calculation in the draw_stringmethod and added a test case. Are my changes sufficient? :)

Copy link
Member

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix! I'll let @rfuest merge this in case there's anything else he'd like to see.

@rfuest
Copy link
Member

rfuest commented Feb 10, 2022

Looks good! The only thing that is missing is an entry in CHANGELOG.md.

The method len() on &str does return the number of bytes of a character,
not the number of characters.
Using characters such as '°' (which are multiple bytes long) caused an
incorrect calculation of the text bounding box.

This commit fixes this by calling .chars().count() on &str.
Also, a test was added to prevent false dimension calculations of
multi-byte character strings in the future.
@rfuest rfuest merged commit c0e050a into embedded-graphics:master Feb 11, 2022
@rfuest
Copy link
Member

rfuest commented Feb 11, 2022

Merged. Thanks again.

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.

None yet

3 participants