pszufe / OpenStreetMapX.jl

OpenStreetMap (*.osm) support for Julia 1.0 and up
MIT License
119 stars 24 forks source link

several questions about the code #16

Open bsnyh opened 4 years ago

bsnyh commented 4 years ago

Hi, I like this package a lot and was working on to understand it more. Several questions pop up. I would really appreciate if you can help me clarify a bit about it.

First one: In the following piece of code, where exactly to add the new node? Moreover, what is the main purpose of the hash function? Based on the documentation, the hash function hash(x[, h::UInt]) is used to compute an integer hash code such that isequal(x,y) implies hash(x) == hash(y). So, are all the nodes dictionary have hash code as their keys?

function add_new_node!(nodes::Dict{Int,T},loc::T, start_id::Int = reinterpret((Int), hash(loc))) where T <: (Union{OpenStreetMapX.LLA,OpenStreetMapX.ENU})
    id = start_id
    while id <= typemax(Int)
        if !haskey(nodes, id)
            nodes[id] = loc
            return id
        end
        id += 1
    end

    msg = "Unable to add new node to map, $(typemax(Int)) nodes is the current limit."
    throw(error(msg))
end

Second one For the following filter_feature function, the if clause in the definition of the dictionary, should be Dict{Int, Tuple{String,String}}(key => feature for (key,feature) in features if feature[1] in keys(classes) && classes[feature[1]] in levels), right?

filter_features(features::Dict{Int,Tuple{String,String}}, classes::Dict{String, Int} = OpenStreetMapX.FEATURE_CLASSES; levels::Set{Int} = Set(1:length(OpenStreetMapX.FEATURE_CLASSES)))
= Dict{Int,Tuple{String,String}}(key => feature for (key,feature) in features if classes[feature[1]] in levels)

Third one: I understand that the purpose of this function is to find the closest point to a specific point which is outside of the cropped map, right? This function applies to the situation that one point is inside of the bounds, and the other point is outside of the bounds. So, I guess what the code does is to move the one which is outside of the bounds to the point on the boundary, right? What consumed me are the if conditions to do so.

function boundary_point(p1::T, p2::T, bounds::OpenStreetMapX.Bounds{T}) where T<:Union{OpenStreetMapX.LLA,OpenStreetMapX.ENU}
    x1, y1 = OpenStreetMapX.getX(p1), OpenStreetMapX.getY(p1)
    x2, y2 = OpenStreetMapX.getX(p2), OpenStreetMapX.getY(p2)

    x, y = Inf, Inf

    if bounds.min_x >  bounds.max_x && x1*x2 < 0

        if x1 < bounds.min_x && x2 < bounds.max_x || x2 < bounds.min_x && x1 < bounds.max_x
            x = bounds.min_x
            y = y1 + (y2 - y1) * (bounds.min_x - x1) / (x2 - x1)
        elseif x1 > bounds.max_x && x2 > bounds.min_x || x2 > bounds.max_x && x1 > bounds.min_x 
            x = bounds.max_x
            y = y1 + (y2 - y1) * (bounds.max_x - x1) / (x2 - x1)
        end

        p3 = T(OpenStreetMapX.XY(x, y))
        OpenStreetMapX.inbounds(p3, bounds) && return p3
    end

    # Move x to x bound if segment crosses boundary
    if x1 < bounds.min_x < x2 || x1 > bounds.min_x > x2
        x = bounds.min_x
        y = y1 + (y2 - y1) * (bounds.min_x - x1) / (x2 - x1)
    elseif x1 < bounds.max_x < x2 || x1 > bounds.max_x > x2
        x = bounds.max_x
        y = y1 + (y2 - y1) * (bounds.max_x - x1) / (x2 - x1)
    end

    p3 = T(OpenStreetMapX.XY(x, y))
    OpenStreetMapX.inbounds(p3, bounds) && return p3

    # Move y to y bound if segment crosses boundary
    if y1 < bounds.min_y < y2 || y1 > bounds.min_y > y2
        x = x1 + (x2 - x1) * (bounds.min_y - y1) / (y2 - y1)
        y = bounds.min_y
    elseif y1 < bounds.max_y < y2 || y1 > bounds.max_y > y2
        x = x1 + (x2 - x1) * (bounds.max_y - y1) / (y2 - y1)
        y = bounds.max_y
    end

    p3 = T(OpenStreetMapX.XY(x, y))
    OpenStreetMapX.inbounds(p3, bounds) && return p3

    error("Failed to find boundary point.")
end
bartoszpankratz commented 4 years ago

Question one: It just adds new nodes to the dictionary with existing ones. Regarding the hash function and IDs, osm files have specific way of identifying nodes and we don't mess around with it. IDs of nodes from the file remains the same in the library. hash is used to generate IDs for new nodes only. They obviously need to be unique and hash ensures it, by tying them to the unique localization (coordinates).

Question two: Yeah, you absolutely right - existing version works well, but it is not prone to errors. We will replace the code, thanks!

Question three: This function works in cases when one point is inside of the bounds, and the other point is outside of the bounds - it draws a line between them and find a crossing point of this line and boundary of the map. Then it adds a new node (which is this crossing point) to the nodes collection. The outside point is keeped in the map_data, user might decide whether he is interested in removing such point by using the crop function. The conditions are quite convoluted, by there is a reason why. The function is made to work with LLA and ENU coordinates systems and in case of LLA you got the problem of North/South and West/East distinction, which is coded with + and - signs. And because of that simplier conditions will not work for the maps lying on the prime or 180° meridian or on equator, this function need to check all possible cases with both positive and negative latitudes and longitudes.