search

mapbox / mapnik-vector-tile

Mapnik implemention of Mapbox Vector Tile specification
BSD 3-Clause "New" or "Revised" License
554 stars 117 forks source link

Unimplemented type: 4 while decoding tiles with japanese characters #295

Closed rouen-sk closed 5 years ago

rouen-sk commented 5 years ago

Hi, I am using mapbox GL JS (including newest 1.0.0. and some older versions) with my own vector tiles, generated with mapnik-vector-tile from OSM data. Works fine with vast majority of tiles, but I have noticed that some tiles in Japan are not rendered and console saying only Unimplemented type: 4. I tried to isolate the issue, and it seems to me, that is happens when feature property (name) contains japanese characters. I have no idea, what to do next about this - if this is bug in encoder (mapnik-vector-tile) or decoder (mapbox GL JS). But I can decode the tile without issues with protobuf-net and I can see japanese names in values just fine in debugger.

See attached sample tile, layer road osm_13_7179_3254_broken.zip

mourner commented 5 years ago

Does it throw an error when decoding the vector tiles themselves, or maybe the fontstack requests? Could you show us a live minimal test case (e.g. on JSFiddle)?

rouen-sk commented 5 years ago

@mourner Thanks for reply. I am not sure, I can show you console output right now, if it helps. It can take some time to prepare live test (I am not sure about intellectual property stuff in this case). image

mourner commented 5 years ago

That looks like the problem with the tile itself so this might be a bug in mapnik-vector-tile, not sure... @flippmoke have you seen anything similar?

flippmoke commented 5 years ago

Very strange, this definitely might be a bug in mapnik-vector-tile. If you could file a reproducible test case of this in node-mapnik it would be helpful.

rouen-sk commented 5 years ago

Thanks guys, but if it is bug in mapnik-vector-tile, can you identify (with your decoder, from attached tile), what is actually wrong? So I can give guys at mapnik reason, why I think the tile is incorrect? (I suspect they will not take "mapbox GL JS says Unimplemented type: 4" as valid reason). What does this error actually means? Protobuf serialization is wrong? Geometries are wrong? Dictrionary of properties is wrong?

mourner commented 5 years ago

@rouen-sk it could be anything — hard to say without a reproducible test case of the code that encodes this invalid tile. Don't worry about Mapnik & mapnik-vector-tile maintainers not taking GL JS seriously — they work at Mapbox and @flippmoke above is one of them. :)

I'll transfer the issue to the mvt repo since this doesn't seem anything to do with the decoding logic, it's the encoding that's problematic.

rouen-sk commented 5 years ago

@mourner Thanks. I was thinking about the issue this way: I have no idea, if the issue is on encoding or decoding side, so I supply the sample tile to you "decoder guys" (since you have error to work with), and you say something like "your tile is not valid to MVT specification, because feature 17 on layer road has wrong this and this" (I imagined, you have testing tools to "run" mapbox GL JS processing over this single tile, and identify exactly, where is the error coming from). Then I would go to "encoder guys" with this diagnosis, and they would be happy with exact bug description :) Now, since we are moving to mapnik, let me give you more info on this side. As I said, it will be problematic to give reproducible test case for encoding, because the infrastructure is large. I am using .NET wrapper for Mapnik (3.0.17, last version able to build on Windows), mapnik-vector-tile 1.5.0. (yep, I know its pretty old, but again, this is the last one I got working with mapnik 3.0.17 in NET-Mapnik), with "enriched" OSM database, used by mapnik postgis datasource, using custom views and functions. So I really hoped sample "broken" tiles would be enough for you to identify the problem.

mourner commented 5 years ago

OK, let me try to pinpoint the exact layer / feature that fails...

mourner commented 5 years ago

OK, it looks like the tile is perfectly fine and decodes properly by vector-tile-js (the library used by GL JS). So it might be that the error is not in the tile itself, but in the fonstack pbfs. I don't think we'll be able to diagnose this further without a live test case.

rouen-sk commented 5 years ago

OK, but let me double check - we are back to testing mapbox GL JS with these tiles, so you want test case of visualization - that means vector style with sources (pointing to already generated tiles), glyphs etc. Right? That is much easier than test case of tile generation, I am on it...

rouen-sk commented 5 years ago

Here you go: https://mapgen.blob.core.windows.net/issue295/demo1.html (I copied only few tiles needed for this zoom and view, so expect 404s if you move/zoom the map)

mourner commented 5 years ago

@rouen-sk OK, traced this down to bad font requests — specifically the following URL from the demo page above actually returns HTML instead of PBF (you can inspect this in the Network tab), with "You can’t perform that action at this time." from the Klokantech GL server. So definitely not on our side.

https://mapgen.blob.core.windows.net/issue295/glyphs/Open%20Sans%20Regular/65280-65535.pbf

rouen-sk commented 5 years ago

EDIT: I see what you mean, the content of some PBFs is clearly wrong. No idea how this happened... let me try to fix those real quick...

rouen-sk commented 5 years ago

Yep, whole issue fixed by replacing few invalid glyphs with correct ones. I am facepalming really hard right now, sorry for wasting your time @mourner (still, more helpfull error message would be nice :)

mourner commented 5 years ago

@rouen-sk you're welcome! We should definitely look into making more descriptive error messages, although it's tricky in this case because invalid requests still return 200 with PBF mime type — should be something to handle more gracefully on the Klokantech server side.

rouen-sk commented 5 years ago

I agree. I received those files as prerequisites, never crossed my mind to doubt their integrity.. I guess you really learn something new every day. Thanks again.

flippmoke commented 5 years ago

@rouen-sk

(3.0.17, last version able to build on Windows)

Newer versions of node-mapnik and this code should work on windows and happy to try to address any issues with windows builds not compiling etc, we just didn't have a good way to continue to build binaries for windows here internally so we stop making the prebuilt ones (the ones that automatically download with npm).

rouen-sk commented 5 years ago

@flippmoke that is good to hear. But I am not using node-mapnik. I was using https://github.com/mapbox/windows-builds to build mapnik SDK, and then mapnik SDK with mapnik-vector-tiles to build .NET mapnik library (with custom fork of dead project https://github.com/jbrwn/NET-Mapnik, which I managed to update to support mapnik up to 3.0.17). I tried to adapt windows-builds for newer mapnik some time ago (that was maybe 3.0.20) and it was like "one error fixed, two new appeared" so I pretty much gave up (didn't have much time for it at the time). If I ever find courage to try again, I will definitely reach out to you, thanks! :)