raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
157 stars 57 forks source link

Some TTF fonts contain glyphs with extra large advance values #122

Closed pjbroad closed 3 years ago

pjbroad commented 3 years ago

@bendoughty pointed out that the buddy window was unnecessarily wide for some TTF fonts. Looking into the reason, it appears that some fonts contain glyphs with extra large advance values for the last 36 glyphs beyond "z". For these fonts, when the max_width_spacing() function is used, the return value is much larger than it would appear needed for the actual text. This in turn causes calculations in the maximum horizontal space need, to be to large. You can see this for the buddy window, stats bars, buttons etc when such a font is used for the UI_FONT. The calculations could be changed to size using the actual text but that would increase the calculation time and these calculations are done each frame. Maybe these are just poor fonts, may be we can exclude these glyphs, but perhaps @gvissers you have a view?

gvissers commented 3 years ago

Maybe I could add a maximum width for characters that can occur in a name. Though I'm not sure how much that'll help, typically characters like 'W' are pretty wide as well. Do you have an example of a font where this is really problematic?

We could also try to store the maximum width and resize the buddy window when names are added or removed? Not a solution I like all that much, resizing windows are not easy on the eyes.

Or use a typical width, and clip names that are too wide, perhaps with a tooltip?

pjbroad commented 3 years ago

I was considering some of those options. Changing just the buddy list would be OK but the same problem shows other places in the UI for these fonts. There are plenty of fonts that work great so its not that big a problem IMHO. I'm on Ubuntu 20.10 and I did my testing using "fonts-gujr-extra/padmaa-Bold.1.1.ttf". This one is a poor font, for example the "x" is missing; I checked using the gnome-desktop fonts application and the "x" is missing there too. It also has capitals that are much wider than lower case.

I'm curious what the expected glyphs would be for those beyond "z". When I tried to used them, they did not render anything. Perhaps we could add a function that returned the maximum advance for "A-Z", "a-z", "0-9" and the standard special characters. That would work in most cases.

gvissers commented 3 years ago

I was considering some of those options. Changing just the buddy list would be OK but the same problem shows other places in the UI for these fonts.

I'm aware. Exactly this problem is why I made the input lines scrollable.

There are plenty of fonts that work great so its not that big a problem IMHO. I'm on Ubuntu 20.10 and I did my testing using "fonts-gujr-extra/padmaa-Bold.1.1.ttf". This one is a poor font, for example the "x" is missing; I checked using the gnome-desktop fonts application and the "x" is missing there too. It also has capitals that are much wider than lower case.

I'm curious what the expected glyphs would be for those beyond "z". When I tried to used them, they did not render anything.

Accented characters mostly. If even 'x' is missing, I'm not surprised these don't show up. Though it looks like sdl-ttf seems to think the glyphs exist, otherwise the font should fall back on the default font. iirc of course.

Perhaps we could add a function that returned the maximum advance for "A-Z", "a-z", "0-9" and the standard special characters. That would work in most cases.

Yeah I guess I'll add yet another width to the font struct 😄

gvissers commented 3 years ago

Okay, I have added a get_max_name_width_zoom() function that return the maximum character width for characters that can occur in a name, and used it in the buddy window. This indeed reduces the window width by 1/3 for padmaa-Bold.1.1.ttf. Speaking of which, that particular font is indeed awful. Not only the 'x' is missing, all accented characters are wrong.

Would it be a good idea to bundle a few good looking free fonts (e.g. from Google fonts) with he game? We'd have to update the code a bit to also look for TTF fonts in the game data directory, but the change is probably minimal, and it might be worthwhile.

bendoughty commented 3 years ago

Would it be a good idea to bundle a few good looking free fonts (e.g. from Google fonts) with he game? We'd have to update the code a bit to also look for TTF fonts in the game data directory, but the change is probably minimal, and it might be worthwhile.

I for one would be in favour of this. I was thinking that it might be nice to have a unified default font for all new installs, regardless of platform. I previously mentioned this to @pjbroad and he mentioned he uses the Ubuntu font for the Android client, which I can confirm looks lovely on the mac too.

pjbroad commented 3 years ago

Thanks for the update, that's a lot better. There's probably more we could do elsewhere in the UI but it's diminishing returns. I think some standard good fonts would be great; I've been thinking about the first impression people get when they install the client and nice fonts out of the box would be great. As Ben said, I've added the Ubuntu fonts to the Android client assets. For the packages I build, I was considering use the same fonts by default for new installs where they are present on the system. That would be even easier if we could add the chosen font set to the data pack and enable them by default. Or, perhaps would could use font family information to set some defaults.

gvissers commented 3 years ago

I have added some code that allows us to load TTF fonts from the game's data directory, in addition to the user specified TTF directory. Fonts placed in the fonts/ directory in the data dir should be picked up by the client, so we could easily bundle fonts with it if we want.

pjbroad commented 3 years ago

This is a good addition. I tried it out by copying my usual font set into a fonts/ directory under data and setting my TTF_DIRECTORY somewhere invalid. That worked fine but now, as the font numbers were different I had to reselect the fonts; I guess that's to be expected. When I restored TTF_DIRECTORY, I now have duplicate fonts listed which is a bit of a pain.

The other thing I now realise is as the font number is used I wonder what happens if fonts are added or removed from TTF_DIRECTORY. Does the number change? I'm a bit confused as it looks like find_multi_option_by_value() should handle this. Doh, its the path that's changed.

More edits sorry: Was there a reason to not use the same "one deep" search for the data_dir fonts? Does the font name need the /fonts/ prefix included in the ini file?

gvissers commented 3 years ago

That worked fine but now, as the font numbers were different I had to reselect the fonts;

Hmm...yeah. It would have picked that up, except that the directory path of the file name is different too. I guess we should only match the filename itself. I'll see if I can whip up a patch.

I now have duplicate fonts listed which is a bit of a pain.

On the one hand, deduplication sounds good. On the other, we don't know if two files with the same (file, font) name really contain the same font. I'm running out of hands, but on the third hand there is no way to distinguish between two fonts with the same name in the font selector, especially so if we only match the file name and not the path.

I guess I'll have a look at removing duplicates.

Was there a reason to not use the same "one deep" search for the data_dir fonts?

Not really, we can add it back in. I just did not envision us adding a whole font directory structure to the client.

Does the font name need the /fonts/ prefix included in the ini file?

I'll make it so it matches base name only. If a user then has two different fonts with the same file name they both want to use, they can complain then.

pjbroad commented 3 years ago

Closing as this is all done.