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

Use get_char_size(' ') to calculate space width. #45922

Merged
merged 1 commit into from
Feb 12, 2021

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Feb 12, 2021

Fixes regressing form Text Server implementation (get_char_size('m') was accidentally used.)

@bruvzg bruvzg added this to the 4.0 milestone Feb 12, 2021
@akien-mga akien-mga merged commit 5e528f3 into godotengine:master Feb 12, 2021
@akien-mga
Copy link
Member

Thanks!

@reduz
Copy link
Member

reduz commented Feb 12, 2021

Ah, I attempted a different fix here in #45923 which I believe is more correct. Spaces are generally inconsistent between fonts and not a good indication of size, which is why sometimes I used 'x' instead. This is specially a problem with mono-spaced fonts which, when using space, will have a huge minimum size.

This PR also does not fix the scale problem, which I fixed in mine.

@bruvzg
Copy link
Member Author

bruvzg commented Feb 12, 2021

Spaces are generally inconsistent between fonts and not a good indication of size, which is why sometimes I used 'x' instead.

Feel free to revert in, spaces were used before Text Server (and still used in 3.2) and change was not intentional, but they are quite inconsistent indeed.

I would use capital 'N' or 'M' (En/Em space which are standard typographic units) instead of 'm' or 'x'.

@reduz
Copy link
Member

reduz commented Feb 12, 2021

@bruvzg Yeah I know, this was an issue before too. I will change it to M then, thanks!

@reduz
Copy link
Member

reduz commented Feb 12, 2021

@bruvzg your changes for tab size are good though, so keeping those

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.

3 participants