Repository: valkey-io/valkey
File: src/geohash_helper.c (C)

Code before:
    return step;
}

/* Return the bounding box of the search area by shape (see geohash.h GeoShape)
 * bounds[0] - bounds[2] is the minimum and maximum longitude
 * while bounds[1] - bounds[3] is the minimum and maximum latitude.
 * since the higher the latitude, the shorter the arc length, the box shape is as follows
 * (left and right edges are actually bent), as shown in the following diagram:
 *
 *    \-----------------/          --------               \-----------------/
 *     \               /         /          \              \               /
 *      \  (long,lat) /         / (long,lat) \              \  (long,lat) /
 *       \           /         /              \             /             \
 *         ---------          /----------------\           /---------------\
 *  Northern Hemisphere       Southern Hemisphere         Around the equator
 */
int geohashBoundingBox(GeoShape *shape, double *bounds) {
    if (!bounds) return 0;
    double height = 0.0, width = 0.0;
    if (shape->type == CIRCULAR_TYPE) {
        // For circular shapes, use the given radius directly.
        height = shape->conversion * shape->t.radius;
        width = shape->conversion * shape->t.radius;
    } else if (shape->type == RECTANGLE_TYPE) {
        // For rectangles, calculate the diagonal as the radius.
        height = shape->conversion * shape->t.r.height / 2;
        width = shape->conversion * shape->t.r.width / 2;
    } else if (shape->type == POLYGON_TYPE) {
        int num_vertices = shape->t.polygon.num_vertices;
        double x = 0.0, y = 0.0, z = 0.0;
        double min_x = EARTH_RADIUS_IN_METERS, max_x = -EARTH_RADIUS_IN_METERS;
        double min_y = EARTH_RADIUS_IN_METERS, max_y = -EARTH_RADIUS_IN_METERS;
        double min_z = EARTH_RADIUS_IN_METERS, max_z = -EARTH_RADIUS_IN_METERS;
        // For polygons, calculate the bounding box & centroid based on the vertices.
        for (int i = 0; i < shape->t.polygon.num_vertices; i++) {
            double cur_x = 0, cur_y = 0, cur_z = 0;
            double longitude = shape->t.polygon.points[i][0];
            double latitude = shape->t.polygon.points[i][1];
            // Code below in this loop is for calculating centroid.
            // Convert degree to radians.
            double lon_rad = deg_rad(longitude);
            double lat_rad = deg_rad(latitude);
            // Convert to Cartesian coordinates
            cur_x = EARTH_RADIUS_IN_METERS * cos(lat_rad) * cos(lon_rad);
            cur_y = EARTH_RADIUS_IN_METERS * cos(lat_rad) * sin(lon_rad);
            cur_z = EARTH_RADIUS_IN_METERS * sin(lat_rad);
            x += cur_x;
            y += cur_y;
            z += cur_z;
            if (cur_x < min_x) min_x = cur_x;
            if (cur_x > max_x) max_x = cur_x;

Review comment (suggestion):
PR changes at time of comment:
This block of code converts Lon & Lat to Cartesian Coordinates to find the Bounding Box **and** Centroid of the polygon.

Proposed PR changes:
* Use Lon and Lat directly to find the Bounding Box
* Convert to Cartesian Coordinates to compute the Centroid

PR changes at time of comment:
```
        double x = 0.0, y = 0.0, z = 0.0;
        double min_x = EARTH_RADIUS_IN_METERS, max_x = -EARTH_RADIUS_IN_METERS;
        double min_y = EARTH_RADIUS_IN_METERS, max_y = -EARTH_RADIUS_IN_METERS;
        double min_z = EARTH_RADIUS_IN_METERS, max_z = -EARTH_RADIUS_IN_METERS;
        // For polygons, calculate the bounding box & centroid based on the vertices.
        for (int i = 0; i < shape->t.polygon.num_vertices; i++) {
            double cur_x = 0, cur_y = 0, cur_z = 0;
            double longitude = shape->t.polygon.points[i][0];
            double latitude = shape->t.polygon.points[i][1];
            // Code below in this loop is for calculating centroid.
            // Convert degree to radians.
            double lon_rad = deg_rad(longitude);
            double lat_rad = deg_rad(latitude);
            // Convert to Cartesian coordinates
            cur_x = EARTH_RADIUS_IN_METERS * cos(lat_rad) * cos(lon_rad);
            cur_y = EARTH_RADIUS_IN_METERS * cos(lat_rad) * sin(lon_rad);
            cur_z = EARTH_RADIUS_IN_METERS * sin(lat_rad);
            x += cur_x;
            y += cur_y;
            z += cur_z;
            if (cur_x < min_x) min_x = cur_x;
            if (cur_x > max_x) max_x = cur_x;
            if (cur_y < min_y) min_y = cur_y;
            if (cur_y > max_y) max_y = cur_y;
            if (cur_z < min_z) min_z = cur_z;
            if (cur_z > max_z) max_z = cur_z;
        }
        // Code block below is for setting the final bounding box values.
        double min_lon = atan2(min_y, min_x);
        double min_hyp = sqrt(min_x * min_x + min_y * min_y);
        double min_lat = atan2(min_z, min_hyp);
        double max_lon = atan2(max_y, max_x);
        double max_hyp = sqrt(max_x * max_x + max_y * max_y);
        double max_lat = atan2(max_z, max_hyp);
        bounds[0] = rad_deg(min_lon);
        bounds[1] = rad_deg(min_lat);
        bounds[2] = rad_deg(max_lon);
        bounds[3] = rad_deg(max_lat);
        // Code block below is for calculating centroid.
        // Average the Cartesian coordinates
        x /= num_vertices;
        y /= num_vertices;
        z /= num_vertices;
        // Convert back to latitude and longitude
        double central_lon = atan2(y, x);           // Longitude in radians
        double central_hyp = sqrt(x * x + y * y);   // Hypotenuse
        double central_lat = atan2(z, central_hyp); // Latitude in radians
        // Convert back to degrees
        shape->xy[0] = rad_deg(central_lon);
        shape->xy[1] = rad_deg(central_lat);
        printf("Geo centroid coordinates lon lat: %f, %f\r\n", shape->xy[0], shape->xy[1]);
        return 1;
    }
```

Proposed: See the PR now

Code after:

/* Return the bounding box of the search area by shape (see geohash.h GeoShape)
 * bounds[0] - bounds[2] is the minimum and maximum longitude
 * while bounds[1] - bounds[3] is the minimum and maximum latitude.
 * since the higher the latitude, the shorter the arc length, the box shape is as follows
 * (left and right edges are actually bent), as shown in the following diagram:
 *
 *    \-----------------/          --------               \-----------------/
 *     \               /         /          \              \               /
 *      \  (long,lat) /         / (long,lat) \              \  (long,lat) /
 *       \           /         /              \             /             \
 *         ---------          /----------------\           /---------------\
 *  Northern Hemisphere       Southern Hemisphere         Around the equator
 *
 * Note: In case of the BYPOLYGON search, this function also sets the centroid coordinates in the shape.
 */
int geohashBoundingBox(GeoShape *shape, double *bounds) {
    if (!bounds) return 0;
    double height = 0.0, width = 0.0;
    if (shape->type == CIRCULAR_TYPE) {
        height = shape->conversion * shape->t.radius;
        width = shape->conversion * shape->t.radius;
    } else if (shape->type == RECTANGLE_TYPE) {
        height = shape->conversion * shape->t.r.height / 2;
        width = shape->conversion * shape->t.r.width / 2;
    } else if (shape->type == POLYGON_TYPE) {
        int num_vertices = shape->t.polygon.num_vertices;
        double x = 0.0, y = 0.0, z = 0.0;
        /* Bounding box directly from lon & lat. */
        double min_lon = GEO_LONG_MAX, max_lon = GEO_LONG_MIN;
        double min_lat = GEO_LAT_MAX, max_lat = GEO_LAT_MIN;
        for (int i = 0; i < num_vertices; i++) {
            double longitude = shape->t.polygon.points[i][0];
            double latitude = shape->t.polygon.points[i][1];
            /* Calculate the bounding box (in LON/LAT). */
            if (longitude < min_lon) min_lon = longitude;
            if (longitude > max_lon) max_lon = longitude;
            if (latitude < min_lat) min_lat = latitude;
            if (latitude > max_lat) max_lat = latitude;
            /* Convert to cartesian coordinates and accumulate for centroid.
             * Note: We do not need to divide the x, y & z values by num_vertices because the magnitude is not needed
             * for centroid calculation. Summing the cartesian coordinates is all that is needed for computing the angle
             * which can be converted back into the LON/LAT format. */
            double lon_rad = deg_rad(longitude);
            double lat_rad = deg_rad(latitude);
            double cur_x = cos(lat_rad) * cos(lon_rad);
            double cur_y = cos(lat_rad) * sin(lon_rad);
            double cur_z = sin(lat_rad);
            x += cur_x;
            y += cur_y;
            z += cur_z;