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

Allow specifying scale_factor #29

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cboone
Copy link
Contributor

@cboone cboone commented Dec 16, 2023

I added a scale-factor option to the CLI, with the existing value of 1.0 as the default. Playing with @wipfli's idea https://github.com/wipfli/double-resolution-font.

I also improved the CLI help text a little. Happy to break that out into a separate PR if that's better.

Let me know what you think and if I can make any changes or if this is not needed / wanted. No problems here, just thought I might as well contribute it back upstream.

Thanks for all the awesome work!

cc: @bdon @wipfli

@cboone
Copy link
Contributor Author

cboone commented Dec 17, 2023

Hmm, having played with this a little now, I'd say that, at least for my purposes, adjusting the scale_factor isn't worth it. It increases the total PBF file sizes by about 1/3 (from 1.2 mb to 1.4-1.6 mb, usually on the higher end of that range). And the sharpness improvements are subtle, at best.

So perhaps I should pull out the help text related code and re-PR that?

@wipfli
Copy link

wipfli commented Dec 17, 2023

It helps with fonts that have corners like Roboto. But the effect is only visible at large font sizes

@bdon
Copy link
Collaborator

bdon commented Dec 18, 2023

@cboone @wipfli did you also increase the internal GPU texture size to use 48pt glyphs?

https://github.com/maplibre/maplibre-gl-js/blob/36dc43bc5cbe5e3de6899d7b371ad9cee9fc7a78/src/symbol/quads.ts#L277

That should have the most dramatic effect on font sharpness, but it may just be less effective for Latin text at typical <30pt sizes.

@wipfli
Copy link

wipfli commented Dec 18, 2023

I did not, so the effect is that with the same text-size in MapLibre, the font will be twice as big...

@cboone
Copy link
Contributor Author

cboone commented Dec 19, 2023

Nope, but I'll give it a try. Thanks @bdon!

@cboone
Copy link
Contributor Author

cboone commented Dec 19, 2023

Whew, that was an interesting challenge. I couldn't figure out the really correct place to add the isDoubleResolution flag to the glyph metrics (would I have to edit the original OTFs? fork https://github.com/mapbox/sdf-glyph-foundry?), so I wound up just hardcoding const textureScale = 2 on https://github.com/maplibre/maplibre-gl-js/blob/36dc43bc5cbe5e3de6899d7b371ad9cee9fc7a78/src/symbol/quads.ts#L277.

That worked, ish. The other metrics are all wrong (doubled, I assume), so the rendering is all kinds of wonky. But at least I could compare characters to characters.

At smaller sizes, the characters look weird. They're "sharper" in that some of the straight lines are less fuzzy, but the width of the straight lines is inconsistent. Curved lines are inconsistent as well. Take a look at these samples:

burlington-1xburlington-2x

At larger sizes, rendering is better. But I'd have a hard time saying the overall effect is better, just looks different (in a way that might be achievable just through changes in paint styles).

atlantic-1xatlantic-2x

Additional note, just for completeness: At @wipfli's suggestion, I compared the sizes of the double resolution PBFs after gzipping. The 1x resolution PBFs were all 1.2M uncompressed (checked using du -hs) and compressed to 1.1M (using gzip -r to compress each PBF individually, then calculate total directory size using du -hs). The 2x resolution PBS were 1.4-1.6M uncompressed and 1.2M compressed. So only a ≈10% increase.

@cboone cboone mentioned this pull request Dec 19, 2023
@wipfli
Copy link

wipfli commented Dec 19, 2023

Interesting, thanks @cboone. If you don't change MapLibre GL JS, but just use half the font size in your style.json file, what happens? To you get identical size fonts?

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.

3 participants