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 change breaks rendering and map interaction needed to update map view #395

Closed
pathmapper opened this issue Sep 23, 2018 · 6 comments
Closed

Comments

@pathmapper
Copy link
Contributor

Style: https://maputnik.github.io/editor/?style=https://rawgit.com/maputnik/osm-liberty/gh-pages/style.json

Glyph URL: https://orangemug.github.io/font-glyphs/glyphs/{fontstack}/{range}.pbf
(same behaviour for: https://fonts.openmaptiles.org/{fontstack}/{range}.pbf)

change_font

For comparison:

Style: https://maputnik.github.io/editor/?style=https://raw.githubusercontent.com/openmaptiles/klokantech-basic-gl-style/master/style.json

Glyph URL: https://free.tilehosting.com/fonts/{fontstack}/{range}.pbf?key={key}
Looks like a fallback font is served even if there is no fallback font referenced in the style for this layer.

change_font_tilehosting

@orangemug orangemug added the bug label Sep 23, 2018
@orangemug
Copy link
Collaborator

So looking at the web inspector in chrome, it looks like https://free.tilehosting.com/fonts/ always returns a http 200 status, regardless of it the font name is valid or not. Whereas https://orangemug.github.io/font-glyphs/glyphs/ 404's

I think we need to

  1. Update to mapbox-gl to 0.49.0 (latest) see if the problem is still occurs
  2. If the issue still occurs, raise a bug in mapbox-gl

@pathmapper
Copy link
Contributor Author

After update mapbox-gl to 0.49.0 and mapbox-gl-style-spec to 13.2.1, I get the following errors
on npm install :

ERROR in ./node_modules/mapbox-gl/src/geo/coordinate.js
Module parse failed: Unexpected token (13:10)
You may need an appropriate loader to handle this file type.
|  */
| class Coordinate {
|     column: number;
|     row: number;
|     zoom: number;
 @ ./node_modules/mapbox-gl/src/util/util.js 51:18-46
 @ ./node_modules/mapbox-gl/src/util/mapbox.js
 @ ./src/components/App.jsx
 @ ./src/index.jsx
 @ multi (webpack)-dev-server/client?http://127.0.0.1:8888 webpack/hot/dev-server webpack-dev-server/client?http://127.0.0.1:8888 webpack/hot/only-dev-server ./src/index.jsx

ERROR in ./node_modules/mapbox-gl/src/style-spec/util/deep_equal.js
Module parse failed: Unexpected token (8:20)
You may need an appropriate loader to handle this file type.
|  * @private
|  */
| function deepEqual(a: ?mixed, b: ?mixed): boolean {
|     if (Array.isArray(a)) {
|         if (!Array.isArray(b) || a.length !== b.length) return false;
 @ ./node_modules/mapbox-gl/src/util/util.js 63:18-58
 @ ./node_modules/mapbox-gl/src/util/mapbox.js
 @ ./src/components/App.jsx
 @ ./src/index.jsx
 @ multi (webpack)-dev-server/client?http://127.0.0.1:8888 webpack/hot/dev-server webpack-dev-server/client?http://127.0.0.1:8888 webpack/hot/only-dev-server ./src/index.jsx

@pathmapper
Copy link
Contributor Author

@orangemug npm install works now after #408.

https://maps.tilehosting.com/fonts/ also returns a 404 status if there is no font name at all, otherwise it delivers the requested font or a fallback font with 200:

404_tilehosting

So the issue is the 404 status which breaks the map, and looking at the demonstration posted in this issue mapbox/mapbox-gl-js#7387 it seems like this is currently expected behaviour.

@orangemug
Copy link
Collaborator

Ok so I guess, we should just close this issue and wait for it to be fixed upstream?

@pathmapper
Copy link
Contributor Author

pathmapper commented Oct 11, 2018

I'm pretty sure that upstream this (map breaks on font 404) is not considered as a bug, so likely this behaviour won't change.

I think we have to deal with it on our side and here are two suggestions how to solve it:

  1. Fix the current behavior that you have to interact with the map to update it after an available font is used after a font 404. So the map would render or not depending on the availability of the font.

or

  1. When modifying the text-font property, only replace the style in the mapbox-gl renderer if the font is available (to prevent that the map breaks on font 404).

In either way there should be a notification for the user if an invalid font name is used. Such a notification could be also done as outlined in #353.

@pathmapper pathmapper changed the title Serving glyphs from static directory: Font change breaks rendering and map interaction needed to update map view Font change breaks rendering and map interaction needed to update map view Oct 21, 2018
@pathmapper
Copy link
Contributor Author

Fixed in #531

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

No branches or pull requests

3 participants