[josm-dev] Validator patch for overlapping ways

[Roy Rankin]

Dave,

I am not confidant I can send the patch without it being altered by my 
mailer, so here as an attachment which will not be seen by the list.
Can you not get it from the TRAC ticket?

A quick look at your patch suggests to me that not all conflicting way 
segments will show in the validation layer.

Regards,
Roy Rankin

Dave Hansen wrote:
> On Wed, 2008-05-21 at 20:34 +1000, Roy Rankin wrote:
>> I have submitted a patch to TRAC, ticket 774. The following is from
>> the 
>> report:
> 
> Could you post here, too?
> 
> I've been doing some hacking on it as well, to the same end.  I also
> added the ability to fix a small class of these overlapping ways in an
> automated fashion.
> 
> If you post your patch, I'll give it a shot the next time I bring up
> JOSM.  We really need to get this stuff integrated.
> 
> --- OverlappingWays.java	(revision 7656)
> +++ OverlappingWays.java	(working copy)
> @@ -2,10 +2,11 @@
>  
>  import static org.openstreetmap.josm.tools.I18n.tr;
>  
> +import java.util.ArrayList;
> +import java.util.Collection;
>  import java.util.List;
> -import java.util.ArrayList;
>  
> -import org.openstreetmap.josm.data.coor.LatLon;
> +import org.openstreetmap.josm.command.*;
>  import org.openstreetmap.josm.data.osm.OsmPrimitive;
>  import org.openstreetmap.josm.data.osm.Way;
>  import org.openstreetmap.josm.data.osm.WaySegment;
> @@ -44,36 +45,124 @@
>  		nodePairs = new Bag<Pair<Node,Node>, WaySegment>(1000);
>  	}
>  
> +    @Override
> +    public Command fixError(TestError testError)
> +    {
> +		Way w1 = (Way)testError.getPrimitives().get(0); 
> +		Way w2 = (Way)testError.getPrimitives().get(1); 
> +        
> +		Collection<OsmPrimitive> primitives = new ArrayList<OsmPrimitive>();;
> +		// we want to delete the smaller one
> +		if (w1.nodes.size() >= w2.nodes.size())
> +			primitives.add(w2);
> +		else
> +			primitives.add(w1);
> +
> +        List<Command> cmds = new ArrayList<Command>();
> +        cmds.add(new DeleteCommand(primitives));
> +       	return new SequenceCommand(tr("Delete Ways"), cmds);
> +    }
> +
> +	public boolean isSubWay(Way little, Way big)
> +	{
> +		if (little.nodes.size() > big.nodes.size()) {
> +			//System.out.println("can not be subway, little>big");
> +			return false;
> +		}
> +		/*
> +		for ( Node n : little.nodes )
> +			System.out.println("little node: " + (n == little.nodes.get(0)));
> +		for ( Node n : big.nodes )
> +			System.out.println("big node: " + (n == big.nodes.get(0)));
> +	
> +		System.out.println("little.nodes.size(): " + little.nodes.size());
> +		System.out.println("big.nodes.size(): " + big.nodes.size());
> +		*/
> +		ArrayList<Pair<Node,Node>> little_pairs = little.getNodePairs(true);
> +		ArrayList<Pair<Node,Node>> big_pairs = big.getNodePairs(true);
> +
> +		Pair<Node,Node> first_little_pair = little_pairs.get(0);
> +		int start_at = big_pairs.indexOf(first_little_pair);
> +		if (start_at == -1) {
> +			System.out.println("can not be subway, no index for flp");
> +			return false;
> +		}
> +		Pair<Node,Node> last_little_pair = little_pairs.get(little_pairs.size()-1);
> +		int end_at = big_pairs.indexOf(last_little_pair);
> +		if (end_at == -1) {
> +			System.out.println("can not be subway, no index for llp");
> +			return false;
> +		}
> +		if (start_at > end_at) {
> +				int tmp = start_at;
> +				start_at = end_at;
> +				end_at = tmp;
> +		}
> +		// subList's second argument is exlcusive, so +1
> +		end_at++;
> +		//System.out.println("start_at: " + start_at);
> +		//System.out.println("end_at: " + end_at);
> +		List<Pair<Node,Node>> big_sub_list = big_pairs.subList(start_at, end_at);
> +		//System.out.println("big_sub_list size: " + big_sub_list.size());
> +		//System.out.println("little_pairs size: " + little_pairs.size());
> +
> +		int last_index = 0;
> +		for (Pair<Node,Node> pair : little_pairs) {
> +			if (big_sub_list.indexOf(pair) != last_index) {
> +				System.out.println("can not be subway, non-sequential pairs at: " +
> +								last_index + " but found at: " +
> +								big_sub_list.indexOf(pair));
> +				return false;
> +			}
> +			last_index++;
> +		}
> +		if (last_index < little_pairs.size()) {
> +				System.out.println("can not be subway, not right length: " + last_index + " vs. " + little_pairs.size());
> +				return false;
> +		}
> +		return true;
> +	}
> +
> +    @Override
> +    public boolean isFixable(TestError testError)
> +    {
> +		Way w1 = (Way)testError.getPrimitives().get(0);
> +		Way w2 = (Way)testError.getPrimitives().get(1);
> +		Way small;
> +		Way large;
> +
> +		if (w1.nodes.size() >= w2.nodes.size()) {
> +			small = w2;
> +			large = w1;
> +		} else {
> +			small = w1;
> +			large = w2;
> +		}
> +		return isSubWay(small, large);
> +    }
> +
>  	@Override
>  	public void endTest() 
>  	{
> -		for (List<WaySegment> duplicated : nodePairs.values())
> -		{
> -			if (duplicated.size() > 1)
> -			{
> -				List<OsmPrimitive> prims = new ArrayList<OsmPrimitive>();
> -				for (WaySegment ws : duplicated) prims.add(ws.way);
> -				errors.add(new TestError(this, Severity.OTHER,
> -					tr("Overlapping ways"), prims, duplicated));
> -			}
> +		for (Pair<Node,Node> pair : nodePairs.keySet()) {
> +			List<WaySegment> segments = nodePairs.get(pair);
> +			//in.debug("Segsize: " + segments.size());
> +			if (segments.size() == 1)
> +				continue;
> +			List<Way> ways = new ArrayList<Way>();
> +			for (WaySegment s : segments)
> +				ways.add(s.way);
> +			errors.add(new TestError(this, Severity.WARNING, tr("Overlapping ways a"), ways));
>  		}
> -		nodePairs = null;
>  	}
>  
>  	@Override
>  	public void visit(Way w) 
>  	{
> -		Node lastN = null;
> -		int i = -2;
> -		for (Node n : w.nodes) {
> -			i++;
> -			if (lastN == null) {
> -				lastN = n;
> -				continue;
> -			}
> -			nodePairs.add(Pair.sort(new Pair<Node,Node>(lastN, n)),
> -				new WaySegment(w, i));
> -			lastN = n;
> +		int i = 0;
> +		for (Pair<Node,Node> pair : w.getNodePairs(true)) {
> +			//Main.debug("node pair: " + pair.a.id + " " + pair.b.id);
> +			nodePairs.add(pair,	new WaySegment(w, i++));
>  		}
>  	}
>  }
> 
> 
> -- Dave
>