8000 Move Vec3d to its own files by isaacbrodsky · Pull Request #67 · uber/h3 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Move Vec3d to its own files #67

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.

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

Merged
merged 4 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ This project adheres to [Semantic Versioning](http://semver.org/).
The public API of this library consists of the functions declared in file
[h3api.h](./src/h3lib/include/h3api.h).

## [Unreleased]
### Added
- Generator for the faceCenterPoint table (#67)
### Changed
- Moved Vec3d structure to `vec3d.h` (#67)

## [3.0.6] - 2018-06-01
### Changed
- Changed signature of internal function h3NeighborRotations.
Expand Down
6 changes: 6 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ set(LIB_SOURCE_FILES
src/h3lib/include/h3UniEdge.h
src/h3lib/include/geoCoord.h
src/h3lib/include/vec2d.h
src/h3lib/include/vec3d.h
src/h3lib/include/linkedGeo.h
src/h3lib/include/baseCells.h
src/h3lib/include/faceijk.h
Expand All @@ -78,6 +79,7 @@ set(LIB_SOURCE_FILES
src/h3lib/lib/bbox.c
src/h3lib/lib/h3Index.c
src/h3lib/lib/vec2d.c
src/h3lib/lib/vec3d.c
src/h3lib/lib/linkedGeo.c
src/h3lib/lib/geoCoord.c
src/h3lib/lib/h3UniEdge.c
Expand Down Expand Up @@ -125,6 +127,7 @@ set(OTHER_SOURCE_FILES
src/apps/testapps/testH3SetToVertexGraph.c
src/apps/testapps/testBBox.c
src/apps/testapps/testVec2d.c
src/apps/testapps/testVec3d.c
src/apps/testapps/testH3UniEdge.c
src/apps/testapps/testLinkedGeo.c
src/apps/testapps/mkRandGeo.c
Expand All @@ -133,6 +136,7 @@ set(OTHER_SOURCE_FILES
src/apps/miscapps/h3ToGeoBoundaryHier.c
src/apps/miscapps/h3ToGeoHier.c
src/apps/miscapps/generateBaseCellNeighbors.c
src/apps/miscapps/generateFaceCenterPoint.c
src/apps/miscapps/h3ToHier.c
src/apps/benchmarks/benchmarkPolyfill.c
src/apps/benchmarks/benchmarkH3Api.c)
Expand Down Expand Up @@ -249,6 +253,7 @@ add_h3_executable(h3ToGeoBoundary src/apps/filters/h3ToGeoBoundary.c ${APP_SOURC
add_h3_executable(hexRange src/apps/filters/hexRange.c ${APP_SOURCE_FILES})
add_h3_executable(kRing src/apps/filters/kRing.c ${APP_SOURCE_FILES})
add_h3_executable(generateBaseCellNeighbors src/apps/miscapps/generateBaseCellNeighbors.c ${APP_SOURCE_FILES})
add_h3_executable(generateFaceCenterPoint src/apps/miscapps/generateFaceCenterPoint.c ${APP_SOURCE_FILES})
add_h3_executable(h3ToGeoBoundaryHier src/apps/miscapps/h3ToGeoBoundaryHier.c ${APP_SOURCE_FILES})
add_h3_executable(h3ToGeoHier src/apps/miscapps/h3ToGeoHier.c ${APP_SOURCE_FILES})
add_h3_executable(h3ToHier src/apps/miscapps/h3ToHier.c ${APP_SOURCE_FILES})
Expand Down Expand Up @@ -418,6 +423,7 @@ if(BUILD_TESTING)
add_h3_test(testGeoCoord src/apps/testapps/testGeoCoord.c)
add_h3_test(testBBox src/apps/testapps/testBBox.c)
add_h3_test(testVec2d src/apps/testapps/testVec2d.c)
add_h3_test(testVec3d src/apps/testapps/testVec3d.c)

add_h3_test_with_arg(testH3NeighborRotations src/apps/testapps/testH3NeighborRotations.c 0)
add_h3_test_with_arg(testH3NeighborRotations src/apps/testapps/testH3NeighborRotations.c 1)
Expand Down
49 changes: 49 additions & 0 deletions src/apps/miscapps/generateFaceCenterPoint.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright 2018 Uber Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/** @file generateFaceCenterPoint.c
* @brief Generates the faceCenterPoint table
*
* usage: `generateFaceCenterPoint`
*/

#include <stdlib.h>
#include "faceijk.h"
#include "vec3d.h"

/**
* Generates and prints the faceCenterPoint table.
*/
static void generate() {
printf("static const Vec3d faceCenterPoint[NUM_ICOSA_FACES] = {\n");
for (int i = 0; i < NUM_ICOSA_FACES; i++) {
GeoCoord centerCoords = faceCenterGeo[i];
Vec3d centerPoint;
_geoToVec3d(&centerCoords, &centerPoint);
printf(" {%.16f, %.16f, %.16f}, // face %2d\n", centerPoint.x,
centerPoint.y, centerPoint.z, i);
}
printf("};\n");
}

int main(int argc, char* argv[]) {
// check command line args
if (argc > 1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any need for this block, when the script takes no args?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just checks that no (unused) args were provided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as other review (#69), you can always do int main(void) in this case. No need to be "strict" about it.

fprintf(stderr, "usage: %s\n", argv[0]);
exit(1);
}

generate();
}
66 changes: 66 additions & 0 deletions src/apps/testapps/testVec3d.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright 2018 Uber Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <float.h>
#include <math.h>
#include <stdlib.h>
#include "test.h"
#include "vec3d.h"

BEGIN_TESTS(Vec3d);

TEST(_pointSquareDist) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding tests!

Vec3d v1 = {0, 0, 0};
Vec3d v2 = {1, 0, 0};
Vec3d v3 = {0, 1, 1};
Vec3d v4 = {1, 1, 1};
Vec3d v5 = {1, 1, 2};

t_assert(fabs(_pointSquareDist(&v1, &v1)) < DBL_EPSILON,
"distance to self is 0");
t_assert(fabs(_pointSquareDist(&v1, &v2) - 1) < DBL_EPSILON,
"distance to <1,0,0> is 1");
t_assert(fabs(_pointSquareDist(&v1, &v3) - 2) < DBL_EPSILON,
"distance to <0,1,1> is 2");
t_assert(fabs(_pointSquareDist(&v1, &v4) - 3) < DBL_EPSILON,
"distance to <1,1,1> is 3");
t_assert(fabs(_pointSquareDist(&v1, &v5) - 6) < DBL_EPSILON,
"distance to <1,1,2> is 6");
}

TEST(_geoToVec3d) {
Vec3d origin = {0};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my info - why not {0, 0, 0}? Doesn't this result in undefined memory for the uninitialized struct fields? Or is this functionally equivalent to {0, 0, 0}?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this should be the same as initializing all members to 0, https://stackoverflow.com/questions/11152160/initializing-a-struct-to-0


GeoCoord c1 = {0, 0};
Vec3d p1;
_geoToVec3d(&c1, &p1);
t_assert(fabs(_pointSquareDist(&origin, &p1) - 1) < EPSILON_RAD,
"Geo point is on the unit sphere");

GeoCoord c2 = {M_PI_2, 0};
Vec3d p2;
_geoToVec3d(&c2, &p2);
t_assert(fabs(_pointSquareDist(&p1, &p2) - 2) < EPSILON_RAD,
"Geo point is on another axis");

GeoCoord c3 = {M_PI, 0};
Vec3d p3;
_geoToVec3d(&c3, &p3);
t_assert(fabs(_pointSquareDist(&p1, &p3) - 4) < EPSILON_RAD,
"Geo point is the other side of the sphere");
}

END_TESTS();
2 changes: 2 additions & 0 deletions src/h3lib/include/faceijk.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ typedef struct {
/// face
} FaceOrientIJK;

extern const GeoCoord faceCenterGeo[NUM_ICOSA_FACES];

// indexes for faceNeighbors table
/** Invalid faceNeighbors table direction */
#define INVALID -1
Expand Down
13 changes: 1 addition & 12 deletions src/h3lib/include/geoCoord.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,14 @@
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
#include "constants.h"
#include "h3api.h"

/** epsilon of ~0.1mm in degrees */
#define EPSILON_DEG .000000001
/** epsilon of ~0.1mm in radians */
#define EPSILON_RAD (EPSILON_DEG * M_PI_180)

/** @struct Vec3D
* @brief 3D floating point structure
*/
typedef struct {
double x; ///< x component
double y; ///< y component
double z; ///< z component
} Vec3d;

void setGeoDegs(GeoCoord* p, double latDegs, double lonDegs);
double constrainLat(double lat);
double constrainLng(double lng);
Expand All @@ -57,7 +49,4 @@ double _geoAzimuthRads(const GeoCoord* p1, const GeoCoord* p2);
void _geoAzDistanceRads(const GeoCoord* p1, double az, double distance,
GeoCoord* p2);

void _geoToVec3d(const GeoCoord* geo, Vec3d* point);
double _pointSquareDist(const Vec3d* p1, const Vec3d* p2);

#endif
37 changes: 37 additions & 0 deletions src/h3lib/include/vec3d.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2018 Uber Technologies, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
/** @file vec3d.h
* @brief 3D floating point vector functions.
*/

#ifndef VEC3D_H
#define VEC3D_H

#include "geoCoord.h"

/** @struct Vec3D
* @brief 3D floating point structure
*/
typedef struct {
double x; ///< x component
double y; ///< y component
double z; ///< z component
} Vec3d;

void _geoToVec3d(const GeoCoord* geo, Vec3d* point);
double _pointSquareDist(const Vec3d* p1, const Vec3d* p2);

#endif
17 changes: 9 additions & 8 deletions src/h3lib/lib/faceijk.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@
#include "coordijk.h"
#include "geoCoord.h"
#include "h3Index.h"
#include "vec3d.h"

/** square root of 7 */
#define M_SQRT7 2.6457513110645905905016157536392604257102L

/** @brief icosahedron face centers in lat/lon radians */
static const GeoCoord faceCenterGeo[NUM_ICOSA_FACES] = {
const GeoCoord faceCenterGeo[NUM_ICOSA_FACES] = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my info: Why drop static?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static is incompatible with extern, because a static variable would only be visible within the same compilation unit (think .c file) and extern is needed for the generator.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need extern here if this is declared extern in a header file, which is the case here. I'd remove the extern here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. Looks good.

{0.803582649718989942, 1.248397419617396099}, // face 0
{1.307747883455638156, 2.536945009877921159}, // face 1
{1.054751253523952054, -1.347517358900396623}, // face 2
Expand All @@ -60,24 +61,24 @@ static const GeoCoord faceCenterGeo[NUM_ICOSA_FACES] = {
static const Vec3d faceCenterPoint[NUM_ICOSA_FACES] = {
{0.2199307791404606, 0.6583691780274996, 0.7198475378926182}, // face 0
{-0.2139234834501421, 0.1478171829550703, 0.9656017935214205}, // face 1
{0.1092625278784797, -0.4811951572873209, 0.8697775121287253}, // face 2
{0.1092625278784797, -0.4811951572873210, 0.8697775121287253}, // face 2
{0.7428567301586791, -0.3593941678278028, 0.5648005936517033}, // face 3
{0.8112534709140969, 0.3448953237639384, 0.4721387736413930}, // face 4
{-0.1055498149613921, 0.9794457296411413, 0.1718874610009365}, // face 5
{-0.8075407579970092, 0.1533552485898819, 0.5695261994882688}, // face 6
{-0.8075407579970092, 0.1533552485898818, 0.5695261994882688}, // face 6
{-0.2846148069787907, -0.8644080972654206, 0.4144792552473539}, // face 7
{0.7405621473854481, -0.6673299564565524, -0.0789837646326737}, // face 8
{0.7405621473854482, -0.6673299564565524, -0.0789837646326737}, // face 8
{0.8512303986474293, 0.4722343788582681, -0.2289137388687808}, // face 9
{-0.7405621473854481, 0.6673299564565525, 0.0789837646326737}, // face 10
{-0.7405621473854481, 0.6673299564565524, 0.0789837646326737}, // face 10
{-0.8512303986474292, -0.4722343788582682, 0.2289137388687808}, // face 11
{0.1055498149613920, -0.9794457296411413, -0.1718874610009365}, // face 12
{0.1055498149613919, -0.9794457296411413, -0.1718874610009365}, // face 12
{0.8075407579970092, -0.1533552485898819, -0.5695261994882688}, // face 13
{0.2846148069787908, 0.8644080972654204, -0.4144792552473539}, // face 14
{-0.7428567301586791, 0.3593941678278027, -0.5648005936517033}, // face 15
{-0.8112534709140971, -0.3448953237639383, -0.4721387736413930}, // face 16
{-0.8112534709140971, -0.3448953237639382, -0.4721387736413930}, // face 16
{-0.2199307791404607, -0.6583691780274996, -0.7198475378926182}, // face 17
{0.2139234834501420, -0.1478171829550704, -0.9656017935214205}, // face 18
{-0.1092625278784796, 0.4811951572873209, -0.8697775121287253}, // face 19
{-0.1092625278784796, 0.4811951572873210, -0.8697775121287253}, // face 19
};

/** @brief icosahedron face ijk axes as azimuth in radians from face center to
Expand Down
34 changes: 0 additions & 34 deletions src/h3lib/lib/geoCoord.c
Original file line number Diff line number Diff line change
Expand Up @@ -253,40 +253,6 @@ void _geoAzDistanceRads(const GeoCoord* p1, double az, double distance,
}
}

/**
* Square of a number
*
* @param x The input number.
* @return The square of the input number.
*/
double _square(double x) { return x * x; }

/**
* Calculate the square of the distance between two 3D coordinates.
*
* @param v1 The first 3D coordinate.
* @param v2 The second 3D coordinate.
* @return The square of the distance between the given points.
*/
double _pointSquareDist(const Vec3d* v1, const Vec3d* v2) {
return _square(v1->x - v2->x) + _square(v1->y - v2->y) +
_square(v1->z - v2->z);
}

/**
* Calculate the 3D coordinate on unit sphere from the latitude and longitude.
*
* @param geo The latitude and longitude of the point.
* @param v The 3D coordinate of the point.
*/
void _geoToVec3d(const GeoCoord* geo, Vec3d* v) {
double r = cos(geo->lat);

v->z = sin(geo->lat);
v->x = cos(geo->lon) * r;
v->y = sin(geo->lon) * r;
}

/*
* The following functions provide meta information about the H3 hexagons at
* each zoom level. Since there are only 16 total levels, these are current
Expand Down
Loading
0