From bfbf4d1e1cbc8850566a9befc626608680c3ef3a Mon Sep 17 00:00:00 2001 From: Erica Fischer Date: Wed, 12 Jul 2023 14:02:44 -0700 Subject: [PATCH] Searching for floating point conversion bugs --- Makefile | 2 +- geometry.cpp | 78 +++++++++++++++++++++++++++------------------------- geometry.hpp | 2 +- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/Makefile b/Makefile index 0eea3294..364fb455 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ CXX := $(CXX) CFLAGS := $(CFLAGS) -fPIE CXXFLAGS := $(CXXFLAGS) -std=c++17 -fPIE LDFLAGS := $(LDFLAGS) -WARNING_FLAGS := -Wall -Wshadow -Wsign-compare -Wextra -Wunreachable-code -Wuninitialized -Wshadow +WARNING_FLAGS := -Wall -Wshadow -Wsign-compare -Wextra -Wunreachable-code -Wuninitialized -Wshadow -Wfloat-conversion RELEASE_FLAGS := -O3 -DNDEBUG DEBUG_FLAGS := -O0 -DDEBUG -fno-inline-functions -fno-omit-frame-pointer diff --git a/geometry.cpp b/geometry.cpp index a888e9d2..49951d85 100644 --- a/geometry.cpp +++ b/geometry.cpp @@ -23,6 +23,8 @@ #include "errors.hpp" #include "projection.hpp" +#define ROUND(n) ((long long) std::round(n)) + static int clip(double *x0, double *y0, double *x1, double *y1, double xmin, double ymin, double xmax, double ymax); drawvec decode_geometry(char **meta, int z, unsigned tx, unsigned ty, long long *bbox, unsigned initial_x, unsigned initial_y) { @@ -85,8 +87,8 @@ drawvec decode_geometry(char **meta, int z, unsigned tx, unsigned ty, long long void to_tile_scale(drawvec &geom, int z, int detail) { for (size_t i = 0; i < geom.size(); i++) { - geom[i].x = std::round((double) geom[i].x / (1LL << (32 - detail - z))); - geom[i].y = std::round((double) geom[i].y / (1LL << (32 - detail - z))); + geom[i].x = ROUND((double) geom[i].x / (1LL << (32 - detail - z))); + geom[i].y = ROUND((double) geom[i].y / (1LL << (32 - detail - z))); } } @@ -108,7 +110,7 @@ drawvec remove_noop(drawvec geom, int type, int shift) { drawvec out; for (size_t i = 0; i < geom.size(); i++) { - if (geom[i].op == VT_LINETO && std::round((double) geom[i].x / (1LL << shift)) == x && std::round((double) geom[i].y / (1LL << shift)) == y) { + if (geom[i].op == VT_LINETO && ROUND((double) geom[i].x / (1LL << shift)) == x && ROUND((double) geom[i].y / (1LL << shift)) == y) { continue; } @@ -116,8 +118,8 @@ drawvec remove_noop(drawvec geom, int type, int shift) { out.push_back(geom[i]); } else { /* moveto or lineto */ out.push_back(geom[i]); - x = std::round((double) geom[i].x / (1LL << shift)); - y = std::round((double) geom[i].y / (1LL << shift)); + x = ROUND((double) geom[i].x / (1LL << shift)); + y = ROUND((double) geom[i].y / (1LL << shift)); } } @@ -156,7 +158,7 @@ drawvec remove_noop(drawvec geom, int type, int shift) { for (size_t i = 0; i < geom.size(); i++) { if (geom[i].op == VT_MOVETO) { - if (i > 0 && geom[i - 1].op == VT_LINETO && std::round((double) geom[i - 1].x / (1LL << shift)) == std::round((double) geom[i].x / (1LL << shift)) && std::round((double) geom[i - 1].y / (1LL << shift)) == std::round((double) geom[i].y / (1LL << shift))) { + if (i > 0 && geom[i - 1].op == VT_LINETO && ROUND((double) geom[i - 1].x / (1LL << shift)) == ROUND((double) geom[i].x / (1LL << shift)) && ROUND((double) geom[i - 1].y / (1LL << shift)) == ROUND((double) geom[i].y / (1LL << shift))) { continue; } } @@ -282,7 +284,7 @@ static void decode_clipped(mapbox::geometry::multi_polygon &t, drawve drawvec ring; for (size_t k = 0; k < t[i][j].size(); k++) { - ring.push_back(draw((k == 0) ? VT_MOVETO : VT_LINETO, std::round(t[i][j][k].x / scale), std::round(t[i][j][k].y / scale))); + ring.push_back(draw((k == 0) ? VT_MOVETO : VT_LINETO, ROUND(t[i][j][k].x / scale), ROUND(t[i][j][k].y / scale))); } if (ring.size() > 0 && ring[ring.size() - 1] != ring[0]) { @@ -328,7 +330,7 @@ drawvec clean_or_clip_poly(drawvec &geom, int z, int buffer, bool clip) { mapbox::geometry::linear_ring lr; for (size_t k = i; k < j; k++) { - lr.push_back(mapbox::geometry::point(geom[k].x * scale, geom[k].y * scale)); + lr.push_back(mapbox::geometry::point(ROUND(geom[k].x * scale), ROUND(geom[k].y * scale))); } if (lr.size() >= 3) { @@ -349,11 +351,11 @@ drawvec clean_or_clip_poly(drawvec &geom, int z, int buffer, bool clip) { mapbox::geometry::linear_ring lr; - lr.push_back(mapbox::geometry::point(scale * -clip_buffer, scale * -clip_buffer)); - lr.push_back(mapbox::geometry::point(scale * -clip_buffer, scale * (area + clip_buffer))); - lr.push_back(mapbox::geometry::point(scale * (area + clip_buffer), scale * (area + clip_buffer))); - lr.push_back(mapbox::geometry::point(scale * (area + clip_buffer), scale * -clip_buffer)); - lr.push_back(mapbox::geometry::point(scale * -clip_buffer, scale * -clip_buffer)); + lr.push_back(mapbox::geometry::point(ROUND(scale * -clip_buffer), ROUND(scale * -clip_buffer))); + lr.push_back(mapbox::geometry::point(ROUND(scale * -clip_buffer), ROUND(scale * (area + clip_buffer)))); + lr.push_back(mapbox::geometry::point(ROUND(scale * (area + clip_buffer)), ROUND(scale * (area + clip_buffer)))); + lr.push_back(mapbox::geometry::point(ROUND(scale * (area + clip_buffer)), ROUND(scale * -clip_buffer))); + lr.push_back(mapbox::geometry::point(ROUND(scale * -clip_buffer), ROUND(scale * -clip_buffer))); wagyu.add_ring(lr, mapbox::geometry::wagyu::polygon_type_clip); } @@ -414,8 +416,8 @@ drawvec clean_or_clip_poly(drawvec &geom, int z, int buffer, bool clip) { for (auto const &outer : result) { for (auto const &ring : outer) { for (auto const &p : ring) { - if (p.x / scale != std::round(p.x / scale) || - p.y / scale != std::round(p.y / scale)) { + if (p.x / scale != ROUND(p.x / scale) || + p.y / scale != ROUND(p.y / scale)) { scale = 1; again = true; break; @@ -489,7 +491,7 @@ void check_polygon(drawvec &geom) { fprintf(stderr, "Internal error: self-intersecting polygon\n"); } - size_t outer_start = -1; + ssize_t outer_start = -1; size_t outer_len = 0; for (size_t i = 0; i < geom.size(); i++) { @@ -515,7 +517,7 @@ void check_polygon(drawvec &geom) { if (!pnpoly(geom, outer_start, outer_len, geom[k].x, geom[k].y)) { bool on_edge = false; - for (size_t l = outer_start; l < outer_start + outer_len; l++) { + for (ssize_t l = outer_start; l < outer_start + (ssize_t) outer_len; l++) { if (geom[k].x == geom[l].x || geom[k].y == geom[l].y) { on_edge = true; break; @@ -635,7 +637,7 @@ static std::vector> clip_poly1(std::vector> clip_poly1(std::vector> clip_poly1(std::vector 0 && *accum_area > pixel * pixel) { // XXX use centroid; - out.push_back(draw(VT_MOVETO, geom[i].x - pixel / 2, geom[i].y - pixel / 2)); - out.push_back(draw(VT_LINETO, geom[i].x - pixel / 2 + pixel, geom[i].y - pixel / 2)); - out.push_back(draw(VT_LINETO, geom[i].x - pixel / 2 + pixel, geom[i].y - pixel / 2 + pixel)); - out.push_back(draw(VT_LINETO, geom[i].x - pixel / 2, geom[i].y - pixel / 2 + pixel)); - out.push_back(draw(VT_LINETO, geom[i].x - pixel / 2, geom[i].y - pixel / 2)); + out.push_back(draw(VT_MOVETO, ROUND(geom[i].x - pixel / 2), ROUND(geom[i].y - pixel / 2))); + out.push_back(draw(VT_LINETO, ROUND(geom[i].x - pixel / 2 + pixel), ROUND(geom[i].y - pixel / 2))); + out.push_back(draw(VT_LINETO, ROUND(geom[i].x - pixel / 2 + pixel), ROUND(geom[i].y - pixel / 2 + pixel))); + out.push_back(draw(VT_LINETO, ROUND(geom[i].x - pixel / 2), ROUND(geom[i].y - pixel / 2 + pixel))); + out.push_back(draw(VT_LINETO, ROUND(geom[i].x - pixel / 2), ROUND(geom[i].y - pixel / 2))); includes_dust = true; *accum_area -= pixel * pixel; @@ -964,8 +966,8 @@ drawvec clip_lines(drawvec &geom, long long minx, long long miny, long long maxx int c = clip(&x1, &y1, &x2, &y2, minx, miny, maxx, maxy); if (c > 1) { // clipped - out.push_back(draw(VT_MOVETO, std::round(x1), std::round(y1))); - out.push_back(draw(VT_LINETO, std::round(x2), std::round(y2))); + out.push_back(draw(VT_MOVETO, ROUND(x1), ROUND(y1))); + out.push_back(draw(VT_LINETO, ROUND(x2), ROUND(y2))); out.push_back(draw(VT_MOVETO, geom[i].x, geom[i].y)); } else if (c == 1) { // unchanged out.push_back(geom[i]); @@ -992,8 +994,8 @@ static long long square_distance_from_line(long long point_x, long long point_y, u = 0; } - long long x = std::round(segA_x + u * p2x); - long long y = std::round(segA_y + u * p2y); + long long x = ROUND(segA_x + u * p2x); + long long y = ROUND(segA_y + u * p2y); long long dx = x - point_x; long long dy = y - point_y; @@ -1095,12 +1097,12 @@ drawvec impose_tile_boundaries(drawvec &geom, long long extent) { int c = clip(&x1, &y1, &x2, &y2, 0, 0, extent, extent); if (c > 1) { // clipped - if (std::round(x1) != geom[i - 1].x || std::round(y1) != geom[i - 1].y) { - out.push_back(draw(VT_LINETO, std::round(x1), std::round(y1))); + if (ROUND(x1) != geom[i - 1].x || ROUND(y1) != geom[i - 1].y) { + out.push_back(draw(VT_LINETO, ROUND(x1), ROUND(y1))); out[out.size() - 1].necessary = 1; } - if (std::round(x2) != geom[i - 0].x || std::round(y2) != geom[i - 0].y) { - out.push_back(draw(VT_LINETO, std::round(x2), std::round(y2))); + if (ROUND(x2) != geom[i - 0].x || ROUND(y2) != geom[i - 0].y) { + out.push_back(draw(VT_LINETO, ROUND(x2), ROUND(y2))); out[out.size() - 1].necessary = 1; } } @@ -1528,8 +1530,8 @@ drawvec stairstep(drawvec &geom, int z, int detail) { double scale = 1 << (32 - detail - z); for (size_t i = 0; i < geom.size(); i++) { - geom[i].x = std::round(geom[i].x / scale); - geom[i].y = std::round(geom[i].y / scale); + geom[i].x = ROUND(geom[i].x / scale); + geom[i].y = ROUND(geom[i].y / scale); } for (size_t i = 0; i < geom.size(); i++) { diff --git a/geometry.hpp b/geometry.hpp index b416abda..75ba7a2f 100644 --- a/geometry.hpp +++ b/geometry.hpp @@ -26,7 +26,7 @@ struct draw { draw(int nop, long long nx, long long ny) : x(nx), - op(nop), + op((signed char) nop), y(ny), necessary(0) { }