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

Font issue when multiple fonts defined #1367

Closed
acalcutt opened this issue Aug 24, 2024 · 7 comments
Closed

Font issue when multiple fonts defined #1367

acalcutt opened this issue Aug 24, 2024 · 7 comments

Comments

@acalcutt
Copy link
Collaborator

acalcutt commented Aug 24, 2024

I have started to notice in one of my styles at https://tiles.wifidb.net/styles/WDB_DARK_MATTER/#2/0/0 that there seems to be some kind of font issue that prevents the map from loading. It is especially noticeable zooming into the USA, since it does not zoom in

You can see in the network tab of chrome that the font is failing to load. for example, this link
https://tiles.wifidb.net/fonts/Metropolis%20Regular,Noto%20Sans%20Regular/5120-5375.pbf

In my font directory I have these fonts and have both
fonts/Metropolis Regular/5120-5375.pbf
fonts/Noto Sans Regular/5120-5375.pbf
(Though I will note both pbf files are 1kb have little in them)

I am still trying to pinpoint where this broke. it seems like it was recent, maybe something that started with #1294

I have seen this error when loading the raster map

 mlgl: {
  class: 'Style',
   severity: 'ERROR',
   text: 'Failed to load glyph range 5120-5375 for font stack Metropolis Regular,Noto Sans Regular: start must be between 0 and 255; given Infinit>
 }
@acalcutt
Copy link
Collaborator Author

acalcutt commented Aug 24, 2024

I did some more testing and it does seem like it broke in 4.12.0

I locally tested 4.11.1 at commit 68d2f71 and it gives me back this pbf file for the mentioned url
5120-5375.pbf.txt

But when I moved up to 4.12.0 at commit 416d783, it doesn't give me back a file, just and empty "{}"
image

@prashis do you have any suggestions for this? @akx I know you also touched fonts recently with #1320 but from my testing this seemed to be broken before that PR.

Note that I used node 20.14.0 in my tests since that was the version that still worked with the old @mapbox/glyph-pbf-composite

@prashis
Copy link
Contributor

prashis commented Aug 24, 2024

@acalcutt It seems to be an issue at https://github.com/jessekrubin/pbfont/blob/main/src/index.ts#L80

Screenshot 2024-08-24 at 11 15 50 PM

After adding following changes, this issue appears to have been resolved

Screenshot 2024-08-24 at 11 20 50 PM

@jessekrubin Will be needing your help resolving this 😅

@akx
Copy link
Contributor

akx commented Aug 27, 2024

So apparently fixed via #1369?

@acalcutt
Copy link
Collaborator Author

acalcutt commented Aug 27, 2024

There is a more complete fix in pbfont v0.2.2 that should be merged soon by dependbot, but v0.2.1 at least allows the page to load.

After working with jessekrubin on a test for this issue and comparing the previous pbf I noticed there was also a range issue. 0.2.1 gives back a pbf, but the range was wrong (0 - 255) instead of (5120-5375) for the example I gave. In 0.2.2 a fix was added for that issue.

It seems like it occurs when both files being combined have no glyphs. before the minimum id was based on the lowest glyph id, whcih wasn't working since there were none. The workaround fix from above was also setting this min id to 0 if it did not exist.

@jessekrubin
Copy link

Howdy team (who's map server I do not personally use). I did a bit of rejiggering of things within the library to fix this problem. As bro dude above said it happens with effectively empty bufs (no glyphs). Lmk if you all have any problems.

Grepping around different map libs it looks like some toss out font stacks ranges that have no glyphs while others do not (tho they should).

@acalcutt
Copy link
Collaborator Author

This should be fixed in v4.13.0

@acalcutt
Copy link
Collaborator Author

acalcutt commented Aug 27, 2024

@jessekrubin Thank you for your help getting this working

@prashis thanks for troubleshooting this with me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants