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

core:feature:multipolygon support #824

Merged
merged 38 commits into from
Sep 3, 2019
Merged

core:feature:multipolygon support #824

merged 38 commits into from
Sep 3, 2019

Conversation

metalstrolch
Copy link
Contributor

@metalstrolch metalstrolch commented Aug 1, 2019

This adds multi polygon support to navit binfile.

It uses the following attempt:

  • Multipolygon "outer" polygons are converted to polygons inside the binfile map.
  • Multipolygon "inner" polygons are added as attributes to the outer polygon.
  • New method is added to graphics to draw polygons with "holes" (currently qt5, gtk_drawing_area, and Windows (except CE) but at least sdl can do this as well)

Since the binfile format stays structural intact, new maps can be processed by old Navit versions, except they draw the multipolygons without their "holes",
New version of navit can still read old maps of course.

Limitations:

  • In the maps, there are multipolygons having other multipolygons as inner or outer. These are unsupported and fail the member check. According to https://wiki.openstreetmap.org/wiki/Relation:multipolygon these are invalid. And the current web display cannot display them as well. See https://www.openstreetmap.org/relation/5587300 . Don't know if this get a standard mapping feature. Hopefully not.
  • Multipolygon polygons tend to become huge. No algorithm to split them into smaller chunks is contained. But my personal opinion is to exclude some types of polygons from map altogether on constrained devices over not including or artificial cutting the multipolygon ones. Btw. there are already huge traditional polygons as well.
  • Currently I don't know if the resulting polygons are sorted to the correct map layer according to their size. However they "look" fine.
  • Processing the map required more disk space as one cannot early drop ways with unknown tags any more, as they could be part of multipolygon. Maptools -n flag works as expected for the final result though.

@metalstrolch metalstrolch added core WIP Work in progress. Do not merge as-is, but please contribute labels Aug 1, 2019
@metalstrolch
Copy link
Contributor Author

What's the state of this:

  • maptool successfully converts large parts of germany. Try run converting full europe is currently running. I don't have the disk space to try to convert world.
  • core support to deal with multipolygons with holes is added and operational.
  • qt5 graphics driver (linux / sailfish) is enhanced to draw polygons with holes.

Could be merged, if somebody can confirm the maptool is ordering the multipolygons right.

@metalstrolch
Copy link
Contributor Author

Fixes #398

@metalstrolch

This comment has been minimized.

@metalstrolch

This comment has been minimized.

metalstrolch and others added 8 commits August 2, 2019 17:17
Way faster because a) better cpu usage on multicores and b) multiple but
smaller hashes that scale O^2 on inserting. They are just O on reading,
so the multiple doesn't count that much. And besides that we save main
menory as we can process each hash independent.
This is baraindead. Lost days of computing time because getting invalid
zip files at the end because of being too big. Make 64 bit zips the
default. Neccessary for big maps anyway, and shouldn't hurt on small
files.
@metalstrolch
Copy link
Contributor Author

metalstrolch commented Aug 6, 2019

Finally Europe converts. ulimit -c unlimited is your friend. But the result was rubbish, as I forgot to give "-6" switch and the file was way over 4GiB (14GiB in fact). As I didn't give -k as well it threw away temp files. Bummer. Made -6 standard.
Next try in around 24 Hours. Btw: We could do the same multiprocessing improvement I did to multipolygons to turn restriction processing as well.
Added processing time by multipolygons: 4hours. Added map result size roughly: 2GiB. Better than I feared.

@metalstrolch
Copy link
Contributor Author

While Europe is processing, I processed a smaller map for the Alpes area. Here for comparisation two times the Ammersee Area near Munich (48° 0′ N, 11° 7′ O)
ammersee
ammersee_multipolygon

@jkoan
Copy link
Member

jkoan commented Aug 27, 2019

Any volunteer to do so?

the maptool version from this PR with the osm planet (pbf) is running since around 9 hours ;)
Currently (within phase 2) disk consumption is 210G as reported by du -sh

Bummer. Made -6 standard.

could you open a issue for that as we dont want to spam the PR with comments on this.

While Europe is processing, I processed a smaller map for the Alpes area. Here for comparisation two times the Ammersee Area near Munich (48° 0′ N, 11° 7′ O)

It looks amazing 😍

  • Multipolygon "outer" polygons are converted to polygons inside the binfile map.

  • Multipolygon "inner" polygons are added as attributes to the outer polygon.

Woulden't it in theory be enought to only have one of those? on the largest polygon inner as a list of references to the inner ones?

Could you explain how this approch is different from @jandegr's? He also showed Pictures which seem to be multipylogons, so whats the difference?

@jkoan jkoan added this to the 0.5.4 milestone Aug 27, 2019
@metalstrolch
Copy link
Contributor Author

metalstrolch commented Aug 27, 2019

Nice to see somebody is interested in that.

There is not so much conceptional difference to @jandegr's attempt shown in #398, in fact it was his work that brought me to the idea on how to do that without breaking anything. The maps are still upward and downward compatible the way I did it.

Main difference is in the way I implemented it. in #398, it was tried to just add the "holes" to the coordinate section of the binfile element. And then to detect the holes by finding "loops" in the graphics back end. However the Coordinates get heavily changed on zooming / moving / rotating and so on, so the "find a loop" attempt was doomed to fail. The only other place we can store something in the binfile is the attr section. So I invented an attr for a "hole" storing the coordinates there. This way we do not need to guess them, they can be given by looking at the attrs. This works quite flawlessly.

Next thing is, I completely moved the problem of drawing a polygon with holes to the graphics section. Qt for example can simply do that. So why take the hassle to try to triangulate them in core. If there is a graphics library that cannot do it, the we have the option to not support it for this library, or triangulate ourselves only for this library. Did not try for Windows or Android, but Qt and Gtk can easily do this.

For the references: In theory you're right. We only need to "reference" the hole polygons. That's what a relation is. Unfortunately, the navit binfile doesn't know any concept for referencing other map elements. And the inner "polygons" are in fact given as a lot of independent way snippets by the relations. So we couldn't just reference the "hole". In fact then we would be required to dereference the whole relations "on the fly". Barely doable. Remember how long the maptool takes to do the task. See https://navit.readthedocs.io/en/latest/maps.html#content for the binfile format.

@jkoan
Copy link
Member

jkoan commented Aug 28, 2019

PROGRESS4: Processed 0 nodes (0 out) 347200046 ways 0 relations 0 tiles 1881:
15 35 MB
error:maptool:write_item_way_subsection:/root/project/navit/maptool/osm.c:368
4 assertion failed:fwrite(attr, attr_len*4, 1, out)==1

Abgebrochen (Speicherabzug geschrieben)

aaand it crashed, didn't though that 300GB are not enough... Need to look for another machine to download and process the planet...

@metalstrolch
Copy link
Contributor Author

Yes that's my problem about this as well. Not so much processing power and ram, but disk space. Maybe I shell out some bucks to buy myself a 2 TB USB disk to do world conversion...

@jkoan
Copy link
Member

jkoan commented Aug 29, 2019

Processing planet again (local) already in phase 4

edit (Do 29. Aug 04:57:55 UTC 2019) now phase 6

@metalstrolch
Copy link
Contributor Author

Then you're machine is way faster than mine :) Mine is at at phase 4, 1375:22 minutes, currently using more than 300GiB disk storage.

* Add:graphics/windows: drawing polygons with holes

For all Windows except windows CE.
@metalstrolch
Copy link
Contributor Author

Added "draw polygon with holes" support to windows graphics as well as this was no big deal to do.

World progress:
PROGRESS6: Processed 0 nodes (0 out) 0 ways 421424 relations 0 tiles 1732:20 943 MB

@jkoan
Copy link
Member

jkoan commented Aug 31, 2019

Now on Phase 13 hope to finish soon :D

In the meantime I had one crash (no bug in maptool) so I needed a restart, but now I am optimistic to get it finished :)

@metalstrolch
Copy link
Contributor Author

Cool!
Mine is at 13 as well. No crashes so far. Peak Memory around 400 GiB on disk. Now at 303GiB again.
PROGRESS13: Processed 0 nodes (0 out) 456792987 ways 0 relations 0 tiles 4844:47 5189 MB

We definitely have some memory leaks in Maptool, but I think no new ones.

We definitely want https://github.com/navit-gps/navit/tree/maptool_turn_restriction_threading and some more multi threading enhancements. But lets stabilize this as is before.

@jkoan
Copy link
Member

jkoan commented Sep 1, 2019

My planet has finally succeeded to convert. Maps working and everything looks really nice. Only thing I am wondering, how do the multipylogons impact performance? If I understand correctly there are more objects to render on the same area as before. So this means rendering time rises and on embedded systems we will most likely have problems with those maps. Is that true?

@metalstrolch
Copy link
Contributor Author

Congrats. seems maptool is doing fine.

@jkoan: what graphics backind did you check the result with?

For the running navit, there is no difference if a element came from a multipolygon or from a traditional one. They are all treated the same, as maptool effectively converts multipolygons to polygons. But of course the number of elements present in binfile map affects performance on readin the elements from file as well as on rendering. You just need to compare the loading time of a rural area against a city like Munich and you'll get what I mean.
So for constrained devices we need to ask the performance question not wheather multipolygon support affects performance. Instead we need to ask how to limit the elements to be rendered for such devices.
I think using a layout (like car-simple) that just doesn't render many map elements like "meadows" or even "buildings" is the better solution than saying the "meadow" coming from the multipolygon is the one too much.

For example on a constrained device, somebody might switich off the rendering for most landuses, but he might want to keep the lakes and rivers, as those are usually the elements to orientate on map as one knows their shape and location.

Rivers are the best example. One just needs to look at the current map to see that filtering out only the multipolygons is not what one wants. You either want to have the complete river or none of it. But not only the pieces modelled as traditional polygons in OSM.

For really constrained devices, we could process a "minimal" world binfile throwing out anything one does not require for routing from the binfile, but keep only selected things like lakes and rivers. Again here, if a element comes from Polygon or not does not matter. This can be done quite easy. I was planning to give this a try anyways, as to learn how small a routable world map could be made throwing away all the candy.

My "world" completed as well. Resulting file 30GiB.
PROGRESS: Phase 14: done 5570:15 5189 MB
If somebody has space to upload this for others to download, just drop me how to upload it an I'll do.

@gefin
Copy link
Contributor

gefin commented Sep 1, 2019

jkoan wrote
"""If I understand correctly there are more objects to render on the same area as before. So this means rendering time rises and on embedded systems we will most likely have problems with those maps. Is that true?"""

It would be interesting to compare the object count of problematic regions like munich.
I tested the impact of Layout settings on devices with bad power/displayresolution relationship .
(Tomtom XL,TT730 and LG K9)
Just as sample munich B2R middle ring screen filling:
car simpĺe ~1.5 sec, car android ~2.5 sec, car day ~ 4 sec ,OSM-look ~ 4.5 sec ( all not measured).
So i think the main problem is the layout complexity.
May be its possible to extract sublayouts for just one display depth (order) and switch between while zooming. Hoping it helps to save time.

@metalstrolch
Copy link
Contributor Author

I think we should discuss rendering performance in a seperate issue, as I think it is not direct related to multipolygon support at all. If we compare e.g. the ancient Munich snapshot still used as sample map to the state of Munich today, then there is a highly increased object count over the years. Of course adding the long missing multipolygons gives a complexity jump for Navit, but that's still nothing compared to the complexity jump by just OSM getting more complex over the years.
And as @gefin proposes, there are plenty of improvements to rendering / layout handling possible to address constrained devices generally.
Interestingly, this doesn't seem to be "that" an issue, because nobody was really complaining about this over the last years. Map size increased drastically over the last 10 Years. Even without multipolygons.

@jkoan
Copy link
Member

jkoan commented Sep 2, 2019

@jkoan: what graphics backind did you check the result with?

I tested with sdl (i know it dont have the support) only had time to do a short test. Will test today (hopefully), can test wince and win10 64bit as well

I think we should discuss rendering performance in a seperate issue,

Yes, please open another issue for this

@metalstrolch
Copy link
Contributor Author

Still any objections against merging?

Will merge tomorrow Tuesday 3rd of September otherwise.

@metalstrolch metalstrolch merged commit b643162 into trunk Sep 3, 2019
@metalstrolch metalstrolch mentioned this pull request Sep 3, 2019
@metalstrolch metalstrolch deleted the multipolygon branch September 25, 2019 20:44
viktorgino pushed a commit that referenced this pull request Sep 22, 2020
This adds multi polygon support to navit binfile.

It uses the following attempt:

 -  Multipolygon "outer" polygons are converted to polygons inside the binfile map.
 -  Multipolygon "inner" polygons are added as attributes to the outer polygon.
 -  New method is added to graphics to draw polygons with "holes" (currently qt5,
    gtk_drawing_area, and Windows (except CE) but at least sdl can do this as well)

Since the binfile format stays structural intact, new maps can be processed by old Navit versions, except they draw the multipolygons without their "holes",
New version of Navit can still read old maps of course.

Limitations:

 -  In the maps, there are multipolygons having other multipolygons as inner or outer.
    These are unsupported and fail the member check. According to 
    https://wiki.openstreetmap.org/wiki/Relation:multipolygon these are invalid. And the
    current web display cannot display them as well. See
    https://www.openstreetmap.org/relation/5587300 . Don't know if this get a standard
    mapping feature. Hopefully not.
  - Multipolygon polygons tend to become huge. No algorithm to split them into smaller
    chunks is contained. But my personal opinion is to exclude some types of polygons
    from map altogether on constrained devices over not including or artificial cutting the
    multipolygon ones. Btw. there are already huge traditional polygons as well.
  - Processing the map required more disk space as one cannot early drop ways with
    unknown tags any more, as they could be part of multipolygon. Maptools -n flag
    works as expected for the final result though.

Additional changes:
  - Default to 64 bit zip files on maptool
viktorgino pushed a commit that referenced this pull request Sep 22, 2020
This adds multi polygon support to navit binfile.

It uses the following attempt:

 -  Multipolygon "outer" polygons are converted to polygons inside the binfile map.
 -  Multipolygon "inner" polygons are added as attributes to the outer polygon.
 -  New method is added to graphics to draw polygons with "holes" (currently qt5,
    gtk_drawing_area, and Windows (except CE) but at least sdl can do this as well)

Since the binfile format stays structural intact, new maps can be processed by old Navit versions, except they draw the multipolygons without their "holes",
New version of Navit can still read old maps of course.

Limitations:

 -  In the maps, there are multipolygons having other multipolygons as inner or outer.
    These are unsupported and fail the member check. According to 
    https://wiki.openstreetmap.org/wiki/Relation:multipolygon these are invalid. And the
    current web display cannot display them as well. See
    https://www.openstreetmap.org/relation/5587300 . Don't know if this get a standard
    mapping feature. Hopefully not.
  - Multipolygon polygons tend to become huge. No algorithm to split them into smaller
    chunks is contained. But my personal opinion is to exclude some types of polygons
    from map altogether on constrained devices over not including or artificial cutting the
    multipolygon ones. Btw. there are already huge traditional polygons as well.
  - Processing the map required more disk space as one cannot early drop ways with
    unknown tags any more, as they could be part of multipolygon. Maptools -n flag
    works as expected for the final result though.

Additional changes:
  - Default to 64 bit zip files on maptool
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement Improving an existing feature (but there is no actual bug)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants