Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix off-by-one error in expire code generating out of bounds tiles #2143

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Fix off-by-one error in expire code generating out of bounds tiles #2143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/expire-tiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,15 @@ int expire_tiles::from_bbox(geom::box_t const &box,
/* 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 =
std::clamp(int(tmp_min.x() - expire_config.buffer), 0, m_map_width);
std::clamp(int(tmp_min.x() - expire_config.buffer), 0, m_map_width - 1);
int const min_tile_y =
std::clamp(int(tmp_min.y() - expire_config.buffer), 0, m_map_width);
std::clamp(int(tmp_min.y() - expire_config.buffer), 0, m_map_width - 1);

auto const tmp_max = coords_to_tile({box.max_x(), box.min_y()});
int const max_tile_x =
std::clamp(int(tmp_max.x() + expire_config.buffer), 0, m_map_width);
std::clamp(int(tmp_max.x() + expire_config.buffer), 0, m_map_width - 1);
int const max_tile_y =
std::clamp(int(tmp_max.y() + expire_config.buffer), 0, m_map_width);
std::clamp(int(tmp_max.y() + expire_config.buffer), 0, m_map_width - 1);

for (int iterator_x = min_tile_x; iterator_x <= max_tile_x; ++iterator_x) {
uint32_t const norm_x = normalise_tile_x_coord(iterator_x);
Expand Down
64 changes: 64 additions & 0 deletions tests/test-expire-tiles.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,70 @@ TEST_CASE("simple expire z18", "[NoDB]")
CHECK(*(itr++) == tile_t(18, 131072, 131072));
}

TEST_CASE("simple expire z10 bounds 0, 0", "[NoDB]")
{
uint32_t const minzoom = 10;
uint32_t const maxzoom = 10;
expire_tiles et{minzoom, defproj};

et.from_geometry(geom::point_t{-20037508.34, 20037508.34},
expire_config_t{});

auto const tiles = get_tiles_ordered(&et, minzoom, maxzoom);
CHECK(tiles.size() == 1);

auto itr = tiles.begin();
CHECK(*(itr++) == tile_t(10, 0, 0));
}

TEST_CASE("simple expire z10 bounds 0, 1023", "[NoDB]")
{
uint32_t const minzoom = 10;
uint32_t const maxzoom = 10;
expire_tiles et{minzoom, defproj};

et.from_geometry(geom::point_t{-20037508.34, -20037508.34},
expire_config_t{});

auto const tiles = get_tiles_ordered(&et, minzoom, maxzoom);
CHECK(tiles.size() == 1);

auto itr = tiles.begin();
CHECK(*(itr++) == tile_t(10, 0, 1023));
}

TEST_CASE("simple expire z10 bounds 1023, 0", "[NoDB]")
{
uint32_t const minzoom = 10;
uint32_t const maxzoom = 10;
expire_tiles et{minzoom, defproj};

et.from_geometry(geom::point_t{20037508.34, 20037508.34},
expire_config_t{});

auto const tiles = get_tiles_ordered(&et, minzoom, maxzoom);
CHECK(tiles.size() == 1);

auto itr = tiles.begin();
CHECK(*(itr++) == tile_t(10, 1023, 0));
}

TEST_CASE("simple expire z10 bounds 1023, 1023", "[NoDB]")
{
uint32_t const minzoom = 10;
uint32_t const maxzoom = 10;
expire_tiles et{minzoom, defproj};

et.from_geometry(geom::point_t{20037508.34, -20037508.34},
expire_config_t{});

auto const tiles = get_tiles_ordered(&et, minzoom, maxzoom);
CHECK(tiles.size() == 1);

auto itr = tiles.begin();
CHECK(*(itr++) == tile_t(10, 1023, 1023));
}

TEST_CASE("expire a simple line", "[NoDB]")
{
uint32_t const zoom = 18;
Expand Down
37 changes: 37 additions & 0 deletions tests/test-reprojection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,43 @@ TEST_CASE("projection 3857", "[NoDB]")
REQUIRE(ct.y() == Approx(6982997.92));
}

TEST_CASE("projection 3857 bounds", "[NoDB]")
{
osmium::Location const loc1{0.0, 0.0};
osmium::Location const loc2{-180.0, -85.0511288};
osmium::Location const loc3{180.0, 85.0511288};
int const srs = 3857;
auto const reprojection = reprojection::create_projection(srs);

{
auto const c = reprojection->reproject(geom::point_t{loc1});
REQUIRE(c.x() == Approx(0.0));
REQUIRE(c.y() == Approx(0.0));

auto const ct = reprojection->target_to_tile(c);
REQUIRE(ct.x() == Approx(0.0));
REQUIRE(ct.y() == Approx(0.0));
}
{
auto const c = reprojection->reproject(geom::point_t{loc2});
REQUIRE(c.x() == Approx(-20037508.34));
REQUIRE(c.y() == Approx(-20037508.34));

auto const ct = reprojection->target_to_tile(c);
REQUIRE(ct.x() == Approx(-20037508.34));
REQUIRE(ct.y() == Approx(-20037508.34));
}
{
auto const c = reprojection->reproject(geom::point_t{loc3});
REQUIRE(c.x() == Approx(20037508.34));
REQUIRE(c.y() == Approx(20037508.34));

auto const ct = reprojection->target_to_tile(c);
REQUIRE(ct.x() == Approx(20037508.34));
REQUIRE(ct.y() == Approx(20037508.34));
}
}

#ifdef HAVE_GENERIC_PROJ
TEST_CASE("projection 5651", "[NoDB]")
{
Expand Down