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

Approximates chatmessage's height and width value #6788

Closed
wants to merge 25 commits into from

Conversation

Archanial
Copy link
Member

@Archanial Archanial commented May 1, 2022

About The Pull Request

This PR removes call to MeasureText(), effectively making creating chatmessages synchronous.
This fixes a few possible runtimes and weird undefined behaviors.

CHAR_WIDTH is (width / 1000) * font_size
Where width is width as calculated by FontForge. width / 1000 is the conversion between FontForge's em-size value and 1000 em-size is just 1 em.
Check the FontForge's docs for better explanation. https://fontforge.org/docs/
APPROX_HEIGHT is font_size * 1.7
Where 1.7 is approximation, based on comparing results of the MeasureText() and used font.

The values from FontForge are saved in static list. If something isn't on the list we assume the worst and give it width of 1000.

The values from font forge were inacurate. The approx width compared to measured one differed by about 20%.
The more naive solution has been implemented - measure cache of some sort. Every client should have the same viewport so the measurements between clients should be the same. Thus, once, we calculate values for every character and keep using them.
After testing it is about ~1% inacurate (which is huge improvement over 20%).

Additionally the values are cached now per client plus every uncached character will be calculated for all the clients.

I changed my mind. I precalculated values for first 1500 characters for unicode. That should cover all the characters you would ever need. Anything that isn't included gets replaced with "?".
If you ever need a new character you can modify json with the value.

Why It's Good For The Game

Less chatmessage bugs and better performance.

Testing Photographs and Procedure

Screenshots&Videos

obraz

obraz
obraz

obraz

obraz

obraz

obraz

NEW CONTENT BELLOW
(old is still relevant though)
image
sunglasses emoji was replaced with "?"

Changelog

🆑
refactor: refactored how chatmessage width and height is calculated
/:cl:

@Archanial Archanial marked this pull request as draft May 3, 2022 19:06
@Archanial
Copy link
Member Author

Issue with this is that it's not very accurate to display pixels without knowing the DPI of someone's screen.
When assuming the DPI of 75 (easiest to assume) we're losing 1 px on average on ~96 DPI screens.
This needs a way to calculate either per client dpi multiplier or actual dpi.

@ike709
Copy link
Member

ike709 commented May 6, 2022

Could simply give people a game setting to set their DPI, but there's probably a lot of players who don't know what DPI is.

@Archanial
Copy link
Member Author

I have 2 ideas for that,
you could either use TGUI to check someone's dpi
or do some MeasureText for clients on login

@Archanial Archanial marked this pull request as ready for review May 10, 2022 16:02
@Archanial Archanial removed the Test Merged This PR is currently in rotation label May 19, 2022
@LemonInTheDark
Copy link
Contributor

Every client should have the same viewport

what does this mean?

@github-actions
Copy link

github-actions bot commented Jul 3, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@ike709
Copy link
Member

ike709 commented Sep 5, 2022

Wasn't this more or less done?

@Archanial
Copy link
Member Author

we so back

@LemonInTheDark
Copy link
Contributor

what?

@Archanial
Copy link
Member Author

I can't reopen so #11947 is new pr for this

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.

4 participants