Skip to content

Refactor expire by geometry code#1809

Merged
lonvia merged 1 commit intoosm2pgsql-dev:masterfrom
joto:refactor-expire-by-geom
Nov 3, 2022
Merged

Refactor expire by geometry code#1809
lonvia merged 1 commit intoosm2pgsql-dev:masterfrom
joto:refactor-expire-by-geom

Conversation

@joto
Copy link
Copy Markdown
Collaborator

@joto joto commented Nov 2, 2022

Split up code into functions for each geometry type. Adds support for geometry types that have been missing so far (multipoint, geometry collection).

Copy link
Copy Markdown
Collaborator

@lonvia lonvia left a comment

Choose a reason for hiding this comment

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

There is one more call to from_geometry() here: https://github.com/openstreetmap/osm2pgsql/blob/bd8d41fcdf6814404132795f0e571c7357cd22ad/src/expire-tiles.cpp#L291 That could be non-3857, too, or am I reading that wrong?

Comment thread src/expire-tiles.cpp Outdated
geom::box_t box;
for (auto const &sgeom : geom) {
box.extend(sgeom.outer());
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn't that the same as box = geom::envelope(geom)?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I missed that one. Changed.

Split up code into functions for each geometry type. Adds support for
geometry types that have been missing so far (multipoint, geometry
collection).

When using a target SRS other than Mercator (EPSG:3857) expire didn't
work before, it is now explicitly disabled (with a warning) in this
case. (This applies only to the pgsql output, the flex output sets
the SRS per table. There is currently no warning in this case, but
we are moving away from the expire configuration on the command line
for the flex output anyway, so I'll leave it at that for the moment.)
@joto joto force-pushed the refactor-expire-by-geom branch from 3c12abb to 4bfa132 Compare November 3, 2022 09:39
@joto
Copy link
Copy Markdown
Collaborator Author

joto commented Nov 3, 2022

I have changed the code some more based on the comment above regarding the use of from_geometry() vs. from_geometry_if_3857(). In that case the check should be done before we even reach that code, otherwise we are doing too much work (requesting the geometry from the database and then ignoring the result).

I added a check in the command line options parsing code to detect the case where expire is enabled but a projection other than Mercator used and disable the expire then. That makes checking for that condition easier later on and the user gets a nice warning.

@lonvia lonvia merged commit d6013c4 into osm2pgsql-dev:master Nov 3, 2022
@joto joto deleted the refactor-expire-by-geom branch November 4, 2022 10:16
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