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

avoid skipping ways if mixing encoders in OSMRoadClassParser #1845

Merged
merged 3 commits into from
Jan 2, 2020

Conversation

karussell
Copy link
Member

@karussell karussell commented Dec 31, 2019

fixes #1843

The reason why there is a difference for bike vs. mtb: in AcceptWay we use HashMap and getAccess uses the first entry which changes for different String keys.

  • remove missleading AcceptWay.getAccess

@karussell karussell added the bug label Dec 31, 2019
@karussell karussell added this to the 1.0 milestone Dec 31, 2019
public IntsRef handleWayTags(IntsRef edgeFlags, ReaderWay readerWay, EncodingManager.Access access, IntsRef relationFlags) {
if (!access.isWay())
public IntsRef handleWayTags(IntsRef edgeFlags, ReaderWay readerWay, boolean ferry, IntsRef relationFlags) {
if (ferry)
Copy link
Member Author

@karussell karussell Dec 31, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the actual fix. I.e. it skips only for ferries (not really necessary but also does not hurt - at least now ;))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, that explains the issue 👍.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW: do we need to pass the ferry boolean? We pass the that should contain the ferry information? Or is this for some ferry relations or something like this?

Copy link
Member

@boldtrn boldtrn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this looks good. I gave it a quick test with #1844 and it seems to work fine now 👍. One thing though, should we remove the boolean ferry completely from the method? We already pass the ReaderWay which should contain this information anyway. On the other hand, the import could get slightly slower if we have to perform the ferry check multiple times.

@karussell
Copy link
Member Author

We already pass the ReaderWay which should contain this information anyway

Yes, true. I would do this later when we remove the FlagEncoders completely. E.g. I'm unsure if we need the separate encodingManager.acceptWay method. Like you said: it might be slightly faster with it, and if it is not e.g. at least 10% faster it is certainly not necessary. And when we remove that method we could remove the ferry parameter too.

Another solution would be to create artificial tags like we already do with estimated_distance or set a ferry EncodedValue and ensure that it is available in every TagParser.

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

Successfully merging this pull request may close these issues.

Issue RoadClassEncoder via EncodedValueLookup when using mtb and motorway
2 participants