contract edges with differing non-conditional access restrictions #4613
Conversation
nilsnolde
force-pushed
the
nn-shortcuts-access-restrictions
branch
from
0c6fc9e
to
ad2fa07
Compare
|
Updating the table of #4608 (comment):
Note, between #4608 and this PR there's not much change for OSM data, but still a few more edges are superseded of course. I'd expect that to look different for commercial data. |
|
I think there's a problem with opposing edges of shortcuts. If I read the current code correctly, then we use the last base edge's |
| if (new_ar.type() == AccessType::kTimedAllowed || new_ar.type() == AccessType::kTimedDenied || | ||
| new_ar.type() == AccessType::kDestinationAllowed) { | ||
| continue; | ||
| } | ||
| modes |= new_ar.modes(); |
There was a problem hiding this comment.
did this small change to not record modes of restrictions we don't care about
|
no rush here, I wanted to understand better how shorcut's opposites are generated |
|
got a minute to talk with @kevinkreiser about this. and yeah, there's some problems with how we build shortcuts today (and same in this PR), see #4656 . that's for another PR though, as we're not breaking anything here, so all good for final review. |
nilsnolde
force-pushed
the
nn-shortcuts-access-restrictions
branch
2 times, most recently
from
a3d1de5
to
4a61b00
Compare
…even remove most of edgeinfo stuff for shortcuts, no need to store name, pronunciations etc; adapt all tests which were relying on the fact that names/speed break up shortcuts
…es: seems like it's randomly selecting one of the 2, no matter what the costing says..
… access restrictions
| uint32_t forward_restrictions_count = 0; | ||
| uint32_t reverse_restrictions_count = 0; |
There was a problem hiding this comment.
need to initialize it, further down we only add to it. resulted in bogus restriction counts for the first level
based on #4608
if 2 edges have differing conditional access restrictions, this PR will not contract the node. this is done because kTimeAllowed would be a bit more work to consolidate onto shortcuts, we'd have to only include overlapping time periods of the base edges. certainly possible, I left a TODO, just wanted to keep this PR lean. also conditionals are (at least in OSM) sufficiently rare to hardly have an impact.
if 2 edges have other differing access restrictions (maxheight/weight/width etc), we'll contract that node and keep the most restrictive attribute of all base edges for the entire shortcut.
I think this should work fine, but I do owe some tests. also some RAD tests. personally I'd still be happy in this case to only RAD test
de_benchmark_routes.txt, which together with gurka tests, should prove that it works the way I'm thinking it does.