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

Crossify adjustments #3 #7

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

3vivekb
Copy link

@3vivekb 3vivekb commented Nov 19, 2017

Still needs commenting, input variables are handled poorly.

@3vivekb
Copy link
Author

3vivekb commented Nov 19, 2017

Needs default parameters, I suggest 2 for the distance and .5 for the angle

@nbolten
Copy link
Member

nbolten commented Nov 19, 2017

Hey! Thanks for the pull request/idea. A couple things, though:

  1. Make sure to base / rebase all pull requests (including this one, if we follow up on it) on the latest master branch. There's been a few changes since you initially began work, including a reorganization of the filters and the structure of the cost function. I'm pushing one in a few minutes as well, just a heads up.

  2. I haven't run your code yet, but I'm not sure if this solves issue Crossify adjustments #3. I think you may have gotten better candidates due to decreasing the increment size and (relatively) decreasing the importance of 'closeness to the intersection' by introducing a third cost term.

The reason the angles generated are sometimes not orthogonal to the street is part of the candidate generation step. The algorithm goes like this:

A. Increment the distance along the street, find the point. e.g. the point 5 meters down the street.

B. Find the closest point on the sidewalk to the left, and closest on the right.

C. Draw a line between them.

D. Go to A until a set maximum distance down the street has been met.

The reason this strategy is used is that it's compatible with many different shapes / situations of sidewalks, and won't accidentally miss any so long as they're within a set distance of the street.

But there are some tweaks that could increase the chance of finding an orthogonal crossing, either by messing with the candidates generated or by making adjustments after a pair have been found.

Changing how candidates are generated

Rather than selecting left/right points and just drawing a line between them, do a search with each. Example:

  1. Find closest 'left' sidewalk point to the street point.

  2. Find the closest 'right' sidewalk point to the 'left' sidewalk point, draw a line between: candidate generated.

  3. Do the same for the street point-selected 'right' sidewalk point.

This will more often create orthogonal-ish lines, as the sidewalks' shapes will usually track the street shape, so the nearest one will be nearly straight across. In addition, this would be a good time to incorporate the angle cost, if it's helpful.

This will also increase the run time of the algorithms (which is almost entirely spent on candidate generation), but if the cost function is always convex then we can stop generating candidates once a minimum has been found (the cost starts to increase).

Making adjustments later

If you want the crossings to be as perfect as possible, you could also attempt to tweak the chosen candidate: draw true orthogonal lines from the start/end and through the street, check to see if it still intersects the same sidewalk.

So, I'm totally open to this direction, but I think the focus should be on the candidates.

@3vivekb
Copy link
Author

3vivekb commented Nov 20, 2017

I missed the updates you made in the morning! Will rebase!

I agree the best direction moving forward is how the candidates are generated, but this was meant as an incremental step. It gives me slightly more acceptable results than just evaluating based on length of line and closeness to intersection.

@nbolten
Copy link
Member

nbolten commented Nov 20, 2017

Cool. I'm making some structural changes, so you may want to wait on that push I said would take "a few minutes".

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

Successfully merging this pull request may close these issues.

2 participants