Reorganize expire code

The code was a bit weird, because the decision whether to do full area
expire of boundary only is mixed up with the code to do the full area
expire. This refactoring fixes that, we decide first what to do and then
do it.

What is a bit awkward now is the way we do the box calculation in the
decide_expire_mode() function only if needed and then later if it wasn't
done already. But if all goes well this will be cleaned up in a future
commit when we don't do expire by bounding box any more but by the
actual area.

Author: joto

src/expire-tiles.cpp

@@ -114,19 +114,43 @@ void expire_tiles_t::from_polygon_boundary(geom::polygon_t const &geom,
     }
 }
 
+namespace {
+
+template <typename TGEOM>
+expire_mode decide_expire_mode(TGEOM const &geom,
+                               expire_config_t const &expire_config,
+                               geom::box_t *box)
+{
+    if (expire_config.mode != expire_mode::hybrid) {
+        return expire_config.mode;
+    }
+
+    *box = geom::envelope(geom);
+    if (box->width() > expire_config.full_area_limit ||
+        box->height() > expire_config.full_area_limit) {
+        return expire_mode::boundary_only;
+    }
+
+    return expire_mode::full_area;
+}
+
+} // anonymous namespace
+
 void expire_tiles_t::from_geometry(geom::polygon_t const &geom,
                                    expire_config_t const &expire_config)
 {
-    if (expire_config.mode == expire_mode::boundary_only) {
+    geom::box_t box;
+    auto const mode = decide_expire_mode(geom, expire_config, &box);
+
+    if (mode == expire_mode::boundary_only) {
         from_polygon_boundary(geom, expire_config);
         return;
     }
 
-    geom::box_t const box = geom::envelope(geom);
-    if (from_bbox(box, expire_config)) {
-        /* Bounding box too big - just expire tiles on the boundary */
-        from_polygon_boundary(geom, expire_config);
+    if (!box.valid()) {
+        box = geom::envelope(geom);
     }
+    from_bbox(box, expire_config);
 }
 
 void expire_tiles_t::from_polygon_boundary(geom::multipolygon_t const &geom,
@@ -140,16 +164,18 @@ void expire_tiles_t::from_polygon_boundary(geom::multipolygon_t const &geom,
 void expire_tiles_t::from_geometry(geom::multipolygon_t const &geom,
                                    expire_config_t const &expire_config)
 {
-    if (expire_config.mode == expire_mode::boundary_only) {
+    geom::box_t box;
+    auto const mode = decide_expire_mode(geom, expire_config, &box);
+
+    if (mode == expire_mode::boundary_only) {
         from_polygon_boundary(geom, expire_config);
         return;
     }
 
-    geom::box_t const box = geom::envelope(geom);
-    if (from_bbox(box, expire_config)) {
-        /* Bounding box too big - just expire tiles on the boundary */
-        from_polygon_boundary(geom, expire_config);
+    if (!box.valid()) {
+        box = geom::envelope(geom);
     }
+    from_bbox(box, expire_config);
 }
 
 // False positive: Apparently clang-tidy can not see through the visit()
@@ -238,15 +264,6 @@ void expire_tiles_t::from_line_segment(geom::point_t const &a,
 int expire_tiles_t::from_bbox(geom::box_t const &box,
                               expire_config_t const &expire_config)
 {
-    double const width = box.width();
-    double const height = box.height();
-
-    if (expire_config.mode == expire_mode::hybrid &&
-        (width > expire_config.full_area_limit ||
-         height > expire_config.full_area_limit)) {
-        return -1;
-    }
-
     /* Convert the box's Mercator coordinates into tile coordinates */
     auto const tmp_min = coords_to_tile({box.min_x(), box.max_y()});
     int const min_tile_x =

src/geom-box.hpp

@@ -72,6 +72,11 @@ class box_t
         return !(a == b);
     }
 
+    bool valid() const noexcept
+    {
+        return m_min_x != std::numeric_limits<double>::max();
+    }
+
 private:
     double m_min_x = std::numeric_limits<double>::max();
     double m_min_y = std::numeric_limits<double>::max();

tests/test-geom-box.cpp

@@ -16,6 +16,7 @@ TEST_CASE("box_t getter/setter", "[NoDB]")
 {
     geom::box_t box{1.0, 2.0, 3.0, 4.0};
 
+    REQUIRE(box.valid());
     REQUIRE(box.min_x() == Approx(1.0));
     REQUIRE(box.max_x() == Approx(3.0));
     REQUIRE(box.min_y() == Approx(2.0));
@@ -57,7 +58,9 @@ TEST_CASE("Extend box_t with points", "[NoDB]")
 {
     geom::box_t box;
 
+    REQUIRE_FALSE(box.valid());
     box.extend(geom::point_t{1.0, 2.0});
+    REQUIRE(box.valid());
 
     REQUIRE(box.min_x() == Approx(1.0));
     REQUIRE(box.max_x() == Approx(1.0));