search

a-b-street / osm2streets

Convert OSM to street networks with detailed geometry
https://a-b-street.github.io/osm2streets
Apache License 2.0
101 stars 9 forks source link

Handle pbf input too #225

Closed dabreegster closed 11 months ago

dabreegster commented 1 year ago

Today we support XML input: https://github.com/a-b-street/osm2streets/blob/main/streets_reader/src/osm_reader/reader.rs

I've been using the osmpbf crate in other projects, like https://github.com/dabreegster/od2net/blob/main/od2net/src/network/create_from_osm.rs. We should support this in osm2streets too -- pbf files are so much smaller than xml, and can be faster to parse.

sqrtM commented 1 year ago

hey Dustin!

Been having a little look at this over the weekend and have a (mostly..) working quick and dirty version of this up and running over on my fork.

Used this pbf for testing and have gotten some nice results on the outputs so far.

image (geojson data visualized)

image (the dot file is exremely large so I wasn't able to get it visualized, but it seems correct).

I'll continue testing and working on this when I get a moment to do so. Hopefully I'll have something nice and clean and fully functional before too terribly long.

Thanks and have a great rest of your weekend :) If you need anything just let me know.

dabreegster commented 1 year ago

Wow, awesome to see this! Let me know if you have any questions or hit weird problems with the codebase.

(the dot file is exremely large so I wasn't able to get it visualized, but it seems correct)

I wouldn't worry about testing this much; I've never looked at any beyond a super tiny test area

sqrtM commented 1 year ago

Hey!

Just a little follow-up because I've hit a little bit of a snag on the WASM side of things.

When I run the tests native, everything runs well and gives great output (at least from what I can tell ; I'm new to the codebase so there could be something obvious I'm missing!). But when I run the StreetExplorer web app, I can't seem to get the .osm.pbf file to read correctly. The file gets found, it attempts to read it, but the best output that I've been able to get out of it is that the Blob header has an "unexpected EOF", which is weird, since this doesn't come up at all on the native test suite, leading me to think there's some kind of data corruption/incorrect parsing between the JS and Rust ends, but after fiddling with it for a few hours, I just can't for the life of me figure out where the issue is!

I've passed the data as Uint8Array and raw string (read by calling as_bytes() in rust) and both produce the same result. I don't want to prepare for a pull request with such a big feature missing so I was wondering if you (or anyone else!) had any ideas as to what was causing this. I've never worked with .pbf before, so maybe there's something obvious going on here! I'm attaching some links to the relevant files (and the whole fork) in their current (all tests passing) state in case you had an idea as to where the issue might stem from.

I'll keep playing with this as I get the time to do so and I'll let you know if I figure something out. Thanks a bunch and have a nice evening!

Whole Fork Main .pbf reading function (i don't think this would be it, since it works native) Where the .pbf is actually fetched in the svelte app (a little ugly, I was testing a lot of different stuff) The JsStreetNetwork binding (also test-ugly, but you can see what I was going for)

dabreegster commented 1 year ago

I can look into it in detail in a few days maybe, but it sounds suspiciously similar to a problem I hit in https://github.com/a-b-street/abstreet/pull/1014 ages ago. Something about passing a vector of bytes breaks.

I would vote for moving on with the PR and not worrying about parsing pbf from WASM. I don't know of a great use case for it anyway; Overpass doesn't output as pbf. The goal of handling pbf at all in osm2streets is for native use cases, I think. The A/B Street import pipeline could get much faster and require less disk to store pbf than xml. But the web importer for osm2streets will probably keep handling xml, unless there's a nice API somewhere that can extract a bbox or polygon and output as pbf.

sqrtM commented 1 year ago

Okay, great, good stuff. In that case, I'll remove all the JS/WASM side changes and keep this PR focused on native. We'll consider the WASM/StreetExplorer stuff as a seperate issue and keep this PR nice and thin.

Be back when the fork is ready 👍 thanks a ton. have a nice day.