From 94bb380c51972afe3ca55690976cb66aae17390a Mon Sep 17 00:00:00 2001
From: Jochen Topf <jochen@topf.org>
Date: Thu, 30 Oct 2025 17:34:20 +0100
Subject: [PATCH] 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.
---
 src/expire-tiles.cpp    | 55 +++++++++++++++++++++++++++--------------
 src/geom-box.hpp        |  5 ++++
 tests/test-geom-box.cpp |  3 +++
 3 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/src/expire-tiles.cpp b/src/expire-tiles.cpp
index 249365070..69ffceda2 100644
--- a/src/expire-tiles.cpp
+++ b/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 =
diff --git a/src/geom-box.hpp b/src/geom-box.hpp
index 7bb5f9c76..cd6279ceb 100644
--- a/src/geom-box.hpp
+++ b/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();
diff --git a/tests/test-geom-box.cpp b/tests/test-geom-box.cpp
index dec7a96fe..3d1cdd437 100644
--- a/tests/test-geom-box.cpp
+++ b/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));