osm2pgsql-dev / osm2pgsql

OpenStreetMap data to PostgreSQL converter
https://osm2pgsql.org
GNU General Public License v2.0
1.48k stars 473 forks source link

Flex: call `:get_bbox()` on geometry, not on parameter table (object) #2219

Closed mdorda closed 1 month ago

mdorda commented 1 month ago

This is not a bug report, but rather a feature request, or a request for advice.

According to your documentation, method :get_bbox() is available for the parameter table (object). Which works great for the most cases. But I find it very limiting when I need to work with multipolygons.

I need to add centroid and bboxes to all (multi)polygons. It seems to be very straightforward:

function Place:get_geometry_bbox()
    if (self.object.type == 'node') then
        return nil
    end

    local xmin, ymin, xmax, ymax = self.object:get_bbox()
    if xmin == nil then
        return nil
    end
    return 'BOX(' .. tostring(xmin) .. ' ' .. tostring(ymin) .. ',' .. tostring(xmax) .. ' ' .. tostring(ymax) .. ')'
end

function Place:get_geometry_loc()
        return self.geometry:centroid()
end

and then just call it when writting:

function Place:write_row(k, v, save_extra_mains)

    -- some logic

    place_table:insert{
        -- more props
        geometry_bbox = self:get_geometry_bbox(),
        geometry_loc = self:get_geometry_loc()
    }

    -- some logic

    return 1
end

But it gets tricky when you need to work a bit more complex with multipolygons. Specifically: to get a centroid and bbox only from the largest part of the multipolygon.

It is easy for centroid:

function Place:get_geometry_loc()
    if not self.geometry:geometry_type() == 'MULTIPOLYGON' then
        return self.geometry:centroid()
    end

    -- for multipolygon return centroid of the biggest geometry part
    local max_geometry = nil
    local max_geometry_area = nil

    for g in self.geometry:geometries() do
        local geometry_area = g:spherical_area()
        if max_geometry_area == nil or max_geometry_area < geometry_area then
            max_geometry = g
            max_geometry_area = geometry_area
        end
    end

    return max_geometry:centroid();
end

But it cannot be done for bbox, becasue it is not a geometry method.

I admit that I have a very specific requirement, but purely from the logic of things, :get_bbox() seems to me to be a geometry thing and I would expect this method to be directly on geometry. Similar to :centroid() or :area() (documentation). It looks pretty inconsistent to me that way.

Did I miss something? Is there any way to achieve this?

Thank you so much!

joto commented 1 month ago

I agree that the :get_bbox() handling is inconsistent. There are two reasons for that:

  1. get_bbox() existed before the advanced geometry handling, so in some sense it is a left-over.
  2. It is more effort to first create the geometry and then get the bounding box from it than just get the bounding box directly from the object.

All of this doesn't mean that it wouldn't make sense to have an :envelope() function that works on any geometry and returns the envelope.

All of this would be very easy to implement. Frankly the biggest problem with this is a question of data types. Usually (i.e. in PostGIS) the envelope function returns another geometry object. This would be great for importing into the database. But what if you need the values in Lua, in the same way get_bbox() returns them? It is unclear to me what users want here and how best to handle that. Any suggestions?

mdorda commented 1 month ago

I can only speak for myself. And I find it much more practical to return single values: xmin, ymin, xmax, ymax. You can still work with them. For example, increase the bbox according to the individual geometries in a multipolygon under certain conditions (which in effect means, to filter out some polygons from multipolygon and get bbox for them).

But there could be two methods: :envelope() and :envelope_box() to get Postgis format for example. I have never been good at naming.

lonvia commented 1 month ago

Maybe follow PostGIS here. Have envelope() returning a Polygon and then four additional functions X/Ymin/max. Or would that require too much calling back and forth?

mdorda commented 1 month ago

Sounds great to me. That would cover all my requirements.

joto commented 1 month ago

The get_bbox() function is now available for geometries. Give it a try.

mdorda commented 1 month ago

Thank you! That was pretty fast :-) Works great. For example, you can now bbox over filtered geometry, for example ignoring parts of the geometry that are less than 0.1 of the total area.

 local total_area = self.geometry:spherical_area()

for g in self.geometry:geometries() do
    local area_ratio = g:spherical_area() / total_area
    if area_ratio > 0.1 then
        if xmin == nil then
            xmin, ymin, xmax, ymax = g:get_bbox()
        else
            local gxmin, gymin, gxmax, gymax = g:get_bbox()
            xmin = math.min(xmin, gxmin)
            ymin = math.min(ymin, gymin)
            xmax = math.max(xmax, gxmax)
            ymax = math.max(ymax, gymax)
        end
    end
end