From 8b228bdecbae6aeca1d889cbdb884ee23d2a3c32 Mon Sep 17 00:00:00 2001 From: Jesse Rosenstock Date: Thu, 19 May 2022 11:49:07 +0200 Subject: [PATCH] Update to latest google3 version * Fix bug moving empty S2Polygon * Update Vector to be C++20 compatible * Minor improvements in test coverage * Remove some deprecated functions --- src/python/s2_common.i | 4 +- src/s2/encoded_s2point_vector.cc | 4 +- src/s2/encoded_s2point_vector_test.cc | 2 +- src/s2/encoded_s2shape_index_test.cc | 8 ++ src/s2/s2boolean_operation.cc | 2 +- src/s2/s2boolean_operation_test.cc | 6 +- src/s2/s2buffer_operation.cc | 4 +- src/s2/s2builder.cc | 2 +- src/s2/s2builder_graph_test.cc | 2 +- src/s2/s2builder_test.cc | 25 ++-- ...s2builderutil_find_polygon_degeneracies.cc | 3 +- ...lderutil_get_snapped_winding_delta_test.cc | 1 + .../s2builderutil_lax_polygon_layer_test.cc | 2 +- .../s2builderutil_lax_polyline_layer_test.cc | 3 +- ...s2builderutil_s2point_vector_layer_test.cc | 2 +- src/s2/s2builderutil_s2polygon_layer.cc | 2 +- src/s2/s2builderutil_s2polygon_layer_test.cc | 2 +- src/s2/s2builderutil_s2polyline_layer_test.cc | 3 +- ...uilderutil_s2polyline_vector_layer_test.cc | 2 +- src/s2/s2cell_id.cc | 1 + src/s2/s2cell_id_test.cc | 1 - src/s2/s2cell_union.h | 1 + src/s2/s2closest_cell_query_test.cc | 2 +- src/s2/s2closest_edge_query_test.cc | 16 ++- src/s2/s2edge_clipping_test.cc | 7 + src/s2/s2edge_crossings_test.cc | 11 +- src/s2/s2furthest_edge_query_test.cc | 31 ++++- src/s2/s2lax_polygon_shape.cc | 4 +- src/s2/s2lax_polygon_shape_test.cc | 2 +- src/s2/s2lax_polyline_shape.cc | 2 +- src/s2/s2loop.cc | 2 +- src/s2/s2loop.h | 9 -- src/s2/s2loop_test.cc | 14 +- src/s2/s2max_distance_targets_test.cc | 3 +- src/s2/s2polygon.cc | 10 +- src/s2/s2polygon.h | 16 --- src/s2/s2polygon_test.cc | 19 ++- src/s2/s2polyline.cc | 2 +- src/s2/s2polyline_alignment_test.cc | 16 ++- src/s2/s2polyline_test.cc | 2 +- src/s2/s2predicates_test.cc | 35 ++--- src/s2/s2shape_index_buffered_region_test.cc | 2 +- src/s2/s2shape_index_measures_test.cc | 4 +- src/s2/s2shape_index_region_test.cc | 1 - src/s2/s2shape_nesting_query.h | 1 - src/s2/s2shape_nesting_query_test.cc | 3 +- src/s2/s2shapeutil_coding.cc | 2 +- src/s2/s2testing.cc | 1 + src/s2/s2testing.h | 3 +- src/s2/s2text_format.cc | 2 +- src/s2/s2winding_operation_test.cc | 9 +- src/s2/util/bitmap/bitmap.h | 65 +++++++--- src/s2/util/bits/bit-interleave.cc | 122 ++++++++---------- src/s2/util/coding/varint.h | 10 +- src/s2/util/math/exactfloat/exactfloat.cc | 1 + src/s2/util/math/exactfloat/exactfloat.h | 1 + src/s2/util/math/vector.h | 22 ++-- 57 files changed, 324 insertions(+), 210 deletions(-) diff --git a/src/python/s2_common.i b/src/python/s2_common.i index b75045e3..8b63d996 100755 --- a/src/python/s2_common.i +++ b/src/python/s2_common.i @@ -615,11 +615,11 @@ class S2Point { %define USE_EQUALS_FN_FOR_EQ_AND_NE(type) %extend type { bool __eq__(const type& other) { - return $self->Equals(&other); + return $self->Equals(other); } bool __ne__(const type& other) { - return !$self->Equals(&other); + return !$self->Equals(other); } } %enddef diff --git a/src/s2/encoded_s2point_vector.cc b/src/s2/encoded_s2point_vector.cc index 1c2e878c..4c3d149c 100644 --- a/src/s2/encoded_s2point_vector.cc +++ b/src/s2/encoded_s2point_vector.cc @@ -175,8 +175,8 @@ void EncodeS2PointVectorFast(Span points, Encoder* encoder) { bool EncodedS2PointVector::InitUncompressedFormat(Decoder* decoder) { #if !defined(IS_LITTLE_ENDIAN) || defined(__arm__) || \ defined(ABSL_INTERNAL_NEED_ALIGNED_LOADS) - // TODO(ericv): Make this work on platforms that don't support unaligned - // 64-bit little-endian reads, e.g. by falling back to + // TODO(b/231674214): Make this work on platforms that don't support + // unaligned 64-bit little-endian reads, e.g. by falling back to // // bit_cast(little_endian::Load64()). // diff --git a/src/s2/encoded_s2point_vector_test.cc b/src/s2/encoded_s2point_vector_test.cc index e3a7b3ca..a7b840d8 100644 --- a/src/s2/encoded_s2point_vector_test.cc +++ b/src/s2/encoded_s2point_vector_test.cc @@ -383,7 +383,7 @@ TEST(EncodedS2PointVectorTest, SnappedFractalLoops) { lax_polygon_size += TestEncodedS2PointVector(points, CodingHint::COMPACT, -1) + 2; } - printf("n=%5d s2=%9" PRIuS " lax=%9" PRIuS "\n", + printf("n=%5d s2=%9zu lax=%9zu\n", num_points, s2polygon_size, lax_polygon_size); } } diff --git a/src/s2/encoded_s2shape_index_test.cc b/src/s2/encoded_s2shape_index_test.cc index dd5dce39..e0ba4bbd 100644 --- a/src/s2/encoded_s2shape_index_test.cc +++ b/src/s2/encoded_s2shape_index_test.cc @@ -117,6 +117,11 @@ TEST(EncodedS2ShapeIndex, RegularLoops) { } } +#ifndef __EMSCRIPTEN__ +// TODO(b/232496949): This test relies on `random()` return values because +// it tests an exact encoded byte size. Either change it to accept a range +// of sizes, or decode and check either the number of shapes, or possibly +// the points themselves by resetting the RNG state. TEST(EncodedS2ShapeIndex, OverlappingPointClouds) { struct TestCase { int num_shapes, num_points_per_shape; @@ -144,6 +149,7 @@ TEST(EncodedS2ShapeIndex, OverlappingPointClouds) { } } +// TODO(b/232496949): This test relies on `random()` return values. TEST(EncodedS2ShapeIndex, OverlappingPolylines) { struct TestCase { int num_shapes, num_shape_edges; @@ -174,6 +180,7 @@ TEST(EncodedS2ShapeIndex, OverlappingPolylines) { } } +// TODO(b/232496949): This test relies on `random()` return values. TEST(EncodedS2ShapeIndex, OverlappingLoops) { struct TestCase { int num_shapes, max_edges_per_loop; @@ -203,6 +210,7 @@ TEST(EncodedS2ShapeIndex, OverlappingLoops) { index, test_case.expected_bytes); } } +#endif // defined(__EMSCRIPTEN__) // Like S2PolylineLayer, but converts the polyline to an S2LaxPolylineShape // and adds it to an S2ShapeIndex (if the polyline is non-empty). diff --git a/src/s2/s2boolean_operation.cc b/src/s2/s2boolean_operation.cc index 57f70b6b..42ccac81 100644 --- a/src/s2/s2boolean_operation.cc +++ b/src/s2/s2boolean_operation.cc @@ -91,9 +91,9 @@ extern bool s2builder_verbose; namespace { // Anonymous namespace for helper classes. using absl::flat_hash_map; -using absl::make_unique; using std::lower_bound; using std::make_pair; +using absl::make_unique; using std::max; using std::min; using std::pair; diff --git a/src/s2/s2boolean_operation_test.cc b/src/s2/s2boolean_operation_test.cc index 12d763e2..f8cd81e3 100644 --- a/src/s2/s2boolean_operation_test.cc +++ b/src/s2/s2boolean_operation_test.cc @@ -56,15 +56,15 @@ namespace { using absl::ByAnyChar; using absl::SkipEmpty; using absl::StrContains; -using absl::StrSplit; -using absl::make_unique; using absl::string_view; +using absl::StrSplit; using s2builderutil::IndexMatchingLayer; using s2builderutil::LaxPolygonLayer; using s2shapeutil::ContainsBruteForce; +using absl::make_unique; using std::pair; -using std::unique_ptr; using std::string; +using std::unique_ptr; using std::vector; using EdgeType = S2Builder::EdgeType; diff --git a/src/s2/s2buffer_operation.cc b/src/s2/s2buffer_operation.cc index ab053f9c..1de78ee1 100644 --- a/src/s2/s2buffer_operation.cc +++ b/src/s2/s2buffer_operation.cc @@ -69,13 +69,13 @@ #include "s2/s2shapeutil_contains_brute_force.h" #include "s2/util/math/mathutil.h" -using absl::make_unique; using s2pred::DBL_ERR; using std::ceil; +using absl::make_unique; using std::max; using std::min; -using std::vector; using std::unique_ptr; +using std::vector; // The errors due to buffering can be categorized as follows: // diff --git a/src/s2/s2builder.cc b/src/s2/s2builder.cc index 4b443643..1802a437 100644 --- a/src/s2/s2builder.cc +++ b/src/s2/s2builder.cc @@ -106,9 +106,9 @@ #include "s2/s2text_format.h" #include "s2/util/gtl/dense_hash_set.h" -using absl::make_unique; using gtl::compact_array; using gtl::dense_hash_set; +using absl::make_unique; using std::max; using std::pair; using std::unique_ptr; diff --git a/src/s2/s2builder_graph_test.cc b/src/s2/s2builder_graph_test.cc index 1bfd00f1..39356dd0 100644 --- a/src/s2/s2builder_graph_test.cc +++ b/src/s2/s2builder_graph_test.cc @@ -31,11 +31,11 @@ #include "s2/s2lax_polyline_shape.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::GraphClone; using s2builderutil::GraphCloningLayer; using s2textformat::MakeLaxPolylineOrDie; using s2textformat::ParsePointsOrDie; +using absl::make_unique; using std::vector; using EdgeType = S2Builder::EdgeType; diff --git a/src/s2/s2builder_test.cc b/src/s2/s2builder_test.cc index fd69d797..d8f7b500 100644 --- a/src/s2/s2builder_test.cc +++ b/src/s2/s2builder_test.cc @@ -59,15 +59,6 @@ using absl::StrAppend; using absl::StrCat; -using absl::make_unique; -using std::cout; -using std::endl; -using std::make_pair; -using std::min; -using std::pair; -using std::string; -using std::unique_ptr; -using std::vector; using s2builderutil::GraphClone; using s2builderutil::IdentitySnapFunction; using s2builderutil::IntLatLngSnapFunction; @@ -79,6 +70,15 @@ using s2builderutil::S2PolylineVectorLayer; using s2textformat::MakePointOrDie; using s2textformat::MakePolygonOrDie; using s2textformat::MakePolylineOrDie; +using std::cout; +using std::endl; +using std::make_pair; +using absl::make_unique; +using std::min; +using std::pair; +using std::string; +using std::unique_ptr; +using std::vector; using EdgeType = S2Builder::EdgeType; using InputEdgeId = S2Builder::Graph::InputEdgeId; using Graph = S2Builder::Graph; @@ -1714,4 +1714,11 @@ TEST(S2Builder, IncorrectSeparationSiteBug) { ASSERT_TRUE(builder.Build(&error)) << error; } +TEST(S2Builder, PushPopLabel) { + // TODO(b/232074544): Test more thoroughly. + S2Builder builder; + builder.push_label(S2Builder::Label{1}); + builder.pop_label(); +} + } // namespace diff --git a/src/s2/s2builderutil_find_polygon_degeneracies.cc b/src/s2/s2builderutil_find_polygon_degeneracies.cc index 16ea2f46..97f74b31 100644 --- a/src/s2/s2builderutil_find_polygon_degeneracies.cc +++ b/src/s2/s2builderutil_find_polygon_degeneracies.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include @@ -32,8 +33,8 @@ #include "s2/s2pointutil.h" #include "s2/s2predicates.h" -using absl::make_unique; using std::make_pair; +using absl::make_unique; using std::pair; using std::vector; diff --git a/src/s2/s2builderutil_get_snapped_winding_delta_test.cc b/src/s2/s2builderutil_get_snapped_winding_delta_test.cc index b270772e..c58325e2 100644 --- a/src/s2/s2builderutil_get_snapped_winding_delta_test.cc +++ b/src/s2/s2builderutil_get_snapped_winding_delta_test.cc @@ -18,6 +18,7 @@ #include "s2/s2builderutil_get_snapped_winding_delta.h" #include +#include #include #include "absl/memory/memory.h" diff --git a/src/s2/s2builderutil_lax_polygon_layer_test.cc b/src/s2/s2builderutil_lax_polygon_layer_test.cc index b10be076..9117604f 100644 --- a/src/s2/s2builderutil_lax_polygon_layer_test.cc +++ b/src/s2/s2builderutil_lax_polygon_layer_test.cc @@ -31,13 +31,13 @@ #include "s2/s2debug.h" #include "s2/s2text_format.h" -using absl::make_unique; using absl::string_view; using s2builderutil::IndexedLaxPolygonLayer; using s2builderutil::LaxPolygonLayer; using s2textformat::MakeLaxPolygonOrDie; using s2textformat::MakePointOrDie; using s2textformat::MakePolylineOrDie; +using absl::make_unique; using std::map; using std::set; using std::string; diff --git a/src/s2/s2builderutil_lax_polyline_layer_test.cc b/src/s2/s2builderutil_lax_polyline_layer_test.cc index 4a9a360b..f70b0985 100644 --- a/src/s2/s2builderutil_lax_polyline_layer_test.cc +++ b/src/s2/s2builderutil_lax_polyline_layer_test.cc @@ -17,6 +17,7 @@ #include "s2/s2builderutil_lax_polyline_layer.h" +#include #include #include "s2/base/casts.h" @@ -26,10 +27,10 @@ #include "s2/s2builderutil_snap_functions.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IndexedLaxPolylineLayer; using s2builderutil::LaxPolylineLayer; using s2textformat::MakeLaxPolylineOrDie; +using absl::make_unique; using std::string; using std::vector; diff --git a/src/s2/s2builderutil_s2point_vector_layer_test.cc b/src/s2/s2builderutil_s2point_vector_layer_test.cc index 343b0026..e848c7de 100644 --- a/src/s2/s2builderutil_s2point_vector_layer_test.cc +++ b/src/s2/s2builderutil_s2point_vector_layer_test.cc @@ -28,10 +28,10 @@ #include "s2/mutable_s2shape_index.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IndexedS2PointVectorLayer; using s2builderutil::S2PointVectorLayer; using s2textformat::MakePointOrDie; +using absl::make_unique; using std::string; using std::vector; diff --git a/src/s2/s2builderutil_s2polygon_layer.cc b/src/s2/s2builderutil_s2polygon_layer.cc index 44794eed..5a8ac0a5 100644 --- a/src/s2/s2builderutil_s2polygon_layer.cc +++ b/src/s2/s2builderutil_s2polygon_layer.cc @@ -24,8 +24,8 @@ #include "absl/memory/memory.h" #include "s2/s2debug.h" -using absl::make_unique; using std::make_pair; +using absl::make_unique; using std::pair; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2builderutil_s2polygon_layer_test.cc b/src/s2/s2builderutil_s2polygon_layer_test.cc index f9e94469..94e3b926 100644 --- a/src/s2/s2builderutil_s2polygon_layer_test.cc +++ b/src/s2/s2builderutil_s2polygon_layer_test.cc @@ -31,10 +31,10 @@ #include "s2/s2debug.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IndexedS2PolygonLayer; using s2builderutil::S2PolygonLayer; using s2textformat::MakePolylineOrDie; +using absl::make_unique; using std::map; using std::set; using std::string; diff --git a/src/s2/s2builderutil_s2polyline_layer_test.cc b/src/s2/s2builderutil_s2polyline_layer_test.cc index 225d9f59..b6aa628a 100644 --- a/src/s2/s2builderutil_s2polyline_layer_test.cc +++ b/src/s2/s2builderutil_s2polyline_layer_test.cc @@ -17,6 +17,7 @@ #include "s2/s2builderutil_s2polyline_layer.h" +#include #include #include "s2/base/casts.h" @@ -27,10 +28,10 @@ #include "s2/s2debug.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IndexedS2PolylineLayer; using s2builderutil::S2PolylineLayer; using s2textformat::MakePolylineOrDie; +using absl::make_unique; using std::string; using std::vector; diff --git a/src/s2/s2builderutil_s2polyline_vector_layer_test.cc b/src/s2/s2builderutil_s2polyline_vector_layer_test.cc index d96755a6..9440f326 100644 --- a/src/s2/s2builderutil_s2polyline_vector_layer_test.cc +++ b/src/s2/s2builderutil_s2polyline_vector_layer_test.cc @@ -29,10 +29,10 @@ #include "s2/s2builderutil_snap_functions.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IndexedS2PolylineVectorLayer; using s2builderutil::S2PolylineVectorLayer; using s2textformat::MakePolylineOrDie; +using absl::make_unique; using std::string; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2cell_id.cc b/src/s2/s2cell_id.cc index d857d1da..796d4b14 100644 --- a/src/s2/s2cell_id.cc +++ b/src/s2/s2cell_id.cc @@ -30,6 +30,7 @@ #include "absl/base/casts.h" #include "absl/strings/str_cat.h" #include "s2/util/bits/bits.h" +#include "s2/util/coding/coder.h" #include "s2/r1interval.h" #include "s2/s2coords.h" #include "s2/s2latlng.h" diff --git a/src/s2/s2cell_id_test.cc b/src/s2/s2cell_id_test.cc index fd5e3fb7..40cb7251 100644 --- a/src/s2/s2cell_id_test.cc +++ b/src/s2/s2cell_id_test.cc @@ -342,7 +342,6 @@ TEST(S2CellId, DecodeFailsWithTruncatedBuffer) { EXPECT_FALSE(decoded_id.Decode(&decoder)); } - static const int kMaxExpandLevel = 3; static void ExpandCell( diff --git a/src/s2/s2cell_union.h b/src/s2/s2cell_union.h index 2b4fcb2b..5f7a9751 100644 --- a/src/s2/s2cell_union.h +++ b/src/s2/s2cell_union.h @@ -28,6 +28,7 @@ #include "s2/base/logging.h" #include "absl/base/macros.h" #include "absl/flags/flag.h" +#include "absl/hash/hash.h" #include "s2/_fp_contract_off.h" #include "s2/s2cell_id.h" #include "s2/s2region.h" diff --git a/src/s2/s2closest_cell_query_test.cc b/src/s2/s2closest_cell_query_test.cc index 86369efa..4005a2c8 100644 --- a/src/s2/s2closest_cell_query_test.cc +++ b/src/s2/s2closest_cell_query_test.cc @@ -38,10 +38,10 @@ namespace { -using absl::make_unique; using s2testing::FractalLoopShapeIndexFactory; using s2textformat::MakeCellIdOrDie; using s2textformat::MakePointOrDie; +using absl::make_unique; using std::pair; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2closest_edge_query_test.cc b/src/s2/s2closest_edge_query_test.cc index 6e77323a..2064852d 100644 --- a/src/s2/s2closest_edge_query_test.cc +++ b/src/s2/s2closest_edge_query_test.cc @@ -41,10 +41,10 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2shapeutil::ShapeEdgeId; using s2textformat::MakeIndexOrDie; using s2textformat::MakePointOrDie; +using absl::make_unique; using std::min; using std::pair; using std::string; @@ -208,12 +208,26 @@ TEST(S2ClosestEdgeQuery, TargetPolygonContainingIndexedPoints) { target.set_include_interiors(true); auto results = query.FindClosestEdges(&target); ASSERT_EQ(2, results.size()); + EXPECT_EQ(S1ChordAngle::Zero(), results[0].distance()); EXPECT_EQ(0, results[0].shape_id()); EXPECT_EQ(2, results[0].edge_id()); // 1:11 + EXPECT_FALSE(results[0].is_interior()); + S2Shape::Edge e0 = query.GetEdge(results[0]); + EXPECT_TRUE(S2::ApproxEquals(e0.v0, S2LatLng::FromDegrees(1, 11).ToPoint())) + << S2LatLng(e0.v0); + EXPECT_TRUE(S2::ApproxEquals(e0.v1, S2LatLng::FromDegrees(1, 11).ToPoint())) + << S2LatLng(e0.v1); + EXPECT_EQ(S1ChordAngle::Zero(), results[1].distance()); EXPECT_EQ(0, results[1].shape_id()); EXPECT_EQ(3, results[1].edge_id()); // 3:13 + EXPECT_FALSE(results[1].is_interior()); + S2Shape::Edge e1 = query.GetEdge(results[1]); + EXPECT_TRUE(S2::ApproxEquals(e1.v0, S2LatLng::FromDegrees(3, 13).ToPoint())) + << S2LatLng(e1.v0); + EXPECT_TRUE(S2::ApproxEquals(e1.v1, S2LatLng::FromDegrees(3, 13).ToPoint())) + << S2LatLng(e1.v1); } TEST(S2ClosestEdgeQuery, EmptyTargetOptimized) { diff --git a/src/s2/s2edge_clipping_test.cc b/src/s2/s2edge_clipping_test.cc index 4a92ca42..5071e59d 100644 --- a/src/s2/s2edge_clipping_test.cc +++ b/src/s2/s2edge_clipping_test.cc @@ -65,6 +65,13 @@ void TestFaceClipping(const S2Point& a_raw, const S2Point& b_raw) { EXPECT_LE(b.Angle(S2::FaceUVtoXYZ(segments[n-1].face, segments[n-1].b)), kErrorRadians); + // Similarly, in UV space. + R2Point a_uv, b_uv; + EXPECT_TRUE(S2::FaceXYZtoUV(segments[0].face, a, &a_uv)); + EXPECT_TRUE(S2::FaceXYZtoUV(segments[n-1].face, b, &b_uv)); + EXPECT_LE((a_uv - segments[0].a).Norm(), S2::kFaceClipErrorUVDist); + EXPECT_LE((b_uv - segments[n-1].b).Norm(), S2::kFaceClipErrorUVDist); + S2Point norm = S2::RobustCrossProd(a, b).Normalize(); S2Point a_tangent = norm.CrossProd(a); S2Point b_tangent = b.CrossProd(norm); diff --git a/src/s2/s2edge_crossings_test.cc b/src/s2/s2edge_crossings_test.cc index 67b2bd37..9c985d8b 100644 --- a/src/s2/s2edge_crossings_test.cc +++ b/src/s2/s2edge_crossings_test.cc @@ -162,7 +162,8 @@ void TestRobustCrossProd(const S2Point& a, const S2Point& b, Precision expected_prec) { EXPECT_EQ(s2pred::Sign(a, b, expected_result), 1); EXPECT_EQ(S2::RobustCrossProd(a, b).Normalize(), expected_result); - EXPECT_EQ(TestRobustCrossProdError(a, b), expected_prec); + EXPECT_EQ(TestRobustCrossProdError(a, b), expected_prec) + << "a: " << a << " b: " << b; } TEST(S2, RobustCrossProdCoverage) { @@ -177,12 +178,16 @@ TEST(S2, RobustCrossProdCoverage) { S2Point(0, 0, 1), DOUBLE); TestRobustCrossProd(S2Point(20 * DBL_ERR, 1, 0), S2Point(0, 1, 0), S2Point(0, 0, 1), DOUBLE); + // If `long double` is the same as `double`, such as on armv7, we'll + // get `EXACT` instead of `LONG_DOUBLE`. + constexpr Precision kLongDoublePrecision = + s2pred::kHasLongDouble ? LONG_DOUBLE : EXACT; TestRobustCrossProd(S2Point(16 * DBL_ERR, 1, 0), S2Point(0, 1, 0), - S2Point(0, 0, 1), LONG_DOUBLE); + S2Point(0, 0, 1), kLongDoublePrecision); // The following bounds hold for both 80-bit and "double double" precision. TestRobustCrossProd(S2Point(5 * LD_ERR, 1, 0), S2Point(0, 1, 0), - S2Point(0, 0, 1), LONG_DOUBLE); + S2Point(0, 0, 1), kLongDoublePrecision); TestRobustCrossProd(S2Point(4 * LD_ERR, 1, 0), S2Point(0, 1, 0), S2Point(0, 0, 1), EXACT); diff --git a/src/s2/s2furthest_edge_query_test.cc b/src/s2/s2furthest_edge_query_test.cc index b238ed93..85c526ec 100644 --- a/src/s2/s2furthest_edge_query_test.cc +++ b/src/s2/s2furthest_edge_query_test.cc @@ -58,6 +58,8 @@ TEST(S2FurthestEdgeQuery, NoEdges) { EXPECT_EQ(S1ChordAngle::Negative(), edge.distance()); EXPECT_EQ(-1, edge.edge_id()); EXPECT_EQ(-1, edge.shape_id()); + EXPECT_FALSE(edge.is_interior()); + EXPECT_TRUE(edge.is_empty()); EXPECT_EQ(S1ChordAngle::Negative(), query.GetDistance(&target)); } @@ -82,6 +84,24 @@ TEST(S2FurthestEdgeQuery, OptionsNotModified) { EXPECT_EQ(options.max_error(), query.options().max_error()); } +TEST(S2FurthestEdgeQuery, OptionsS1AngleSetters) { + // Verify that the S1Angle and S1ChordAngle versions do the same thing. + // This is mainly to prevent the (so far unused) S1Angle versions from + // being detected as dead code. + S2FurthestEdgeQuery::Options angle_options, chord_angle_options; + angle_options.set_min_distance(S1Angle::Degrees(1)); + chord_angle_options.set_min_distance(S1ChordAngle::Degrees(1)); + EXPECT_EQ(chord_angle_options.min_distance(), angle_options.min_distance()); + + angle_options.set_inclusive_min_distance(S1Angle::Degrees(1)); + chord_angle_options.set_inclusive_min_distance(S1ChordAngle::Degrees(1)); + EXPECT_EQ(chord_angle_options.min_distance(), angle_options.min_distance()); + + angle_options.set_conservative_min_distance(S1Angle::Degrees(1)); + chord_angle_options.set_conservative_min_distance(S1ChordAngle::Degrees(1)); + EXPECT_EQ(chord_angle_options.min_distance(), angle_options.min_distance()); +} + // In furthest edge queries, the following distance computation is used when // updating max distances. S1ChordAngle GetMaxDistanceToEdge( @@ -159,6 +179,8 @@ TEST(S2FurthestEdgeQuery, AntipodalPointInsideIndexedPolygon) { EXPECT_EQ(1, results[0].shape_id()); // Should find the interior, so no specific edge id. EXPECT_EQ(-1, results[0].edge_id()); + EXPECT_TRUE(results[0].is_interior()); + EXPECT_FALSE(results[0].is_empty()); // Next check that with include_interiors set to false, the distance is less // than 180 for the same target and index. @@ -168,7 +190,14 @@ TEST(S2FurthestEdgeQuery, AntipodalPointInsideIndexedPolygon) { EXPECT_LE(results[0].distance(), S1ChordAngle::Straight()); EXPECT_EQ(1, results[0].shape_id()); // Found a specific edge, so id should be positive. - EXPECT_NE(-1, results[0].edge_id()); + EXPECT_EQ(3, results[0].edge_id()); + EXPECT_FALSE(results[0].is_interior()); + EXPECT_FALSE(results[0].is_empty()); + S2Shape::Edge e0 = query.GetEdge(results[0]); + EXPECT_TRUE(S2::ApproxEquals(e0.v0, S2LatLng::FromDegrees(5, 10).ToPoint())) + << S2LatLng(e0.v0); + EXPECT_TRUE(S2::ApproxEquals(e0.v1, S2LatLng::FromDegrees(0, 10).ToPoint())) + << S2LatLng(e0.v1); } TEST(S2FurthestEdgeQuery, AntipodalPointOutsideIndexedPolygon) { diff --git a/src/s2/s2lax_polygon_shape.cc b/src/s2/s2lax_polygon_shape.cc index 94bdfa16..308b2047 100644 --- a/src/s2/s2lax_polygon_shape.cc +++ b/src/s2/s2lax_polygon_shape.cc @@ -25,9 +25,9 @@ #include "s2/util/coding/varint.h" #include "s2/s2shapeutil_get_reference_point.h" -using absl::make_unique; using absl::MakeSpan; using absl::Span; +using absl::make_unique; using std::vector; using ChainPosition = S2Shape::ChainPosition; @@ -125,7 +125,7 @@ void S2LaxPolygonShape::Init(Span> loops) { // since "new T[]" stores its own copy of the array size. // // Note that even absl::make_unique_for_overwrite<> and c++20's - // std::make_unique_for_overwrite default-construct all elements when + // absl::make_unique_for_overwrite default-construct all elements when // T is a class type. vertices_ = make_unique(num_vertices_); std::copy(loops[0].begin(), loops[0].end(), vertices_.get()); diff --git a/src/s2/s2lax_polygon_shape_test.cc b/src/s2/s2lax_polygon_shape_test.cc index 8ab028d2..098ef689 100644 --- a/src/s2/s2lax_polygon_shape_test.cc +++ b/src/s2/s2lax_polygon_shape_test.cc @@ -38,8 +38,8 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2textformat::MakePolygonOrDie; +using absl::make_unique; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2lax_polyline_shape.cc b/src/s2/s2lax_polyline_shape.cc index e8e6a7fd..2089549b 100644 --- a/src/s2/s2lax_polyline_shape.cc +++ b/src/s2/s2lax_polyline_shape.cc @@ -26,9 +26,9 @@ #include "absl/utility/utility.h" #include "s2/s2polyline.h" -using absl::make_unique; using absl::MakeSpan; using absl::Span; +using absl::make_unique; S2LaxPolylineShape::S2LaxPolylineShape(S2LaxPolylineShape&& other) : S2Shape(std::move(other)), diff --git a/src/s2/s2loop.cc b/src/s2/s2loop.cc index 8182ce62..90423782 100644 --- a/src/s2/s2loop.cc +++ b/src/s2/s2loop.cc @@ -61,9 +61,9 @@ #include "s2/s2wedge_relations.h" #include "s2/util/math/matrix3x3.h" -using absl::make_unique; using absl::MakeSpan; using absl::Span; +using absl::make_unique; using std::pair; using std::set; using std::vector; diff --git a/src/s2/s2loop.h b/src/s2/s2loop.h index ed35e99c..d27d1923 100644 --- a/src/s2/s2loop.h +++ b/src/s2/s2loop.h @@ -338,15 +338,6 @@ class S2Loop final : public S2Region { bool BoundaryNear(const S2Loop& b, S1Angle max_error = S1Angle::Radians(1e-15)) const; -#ifndef SWIG - ABSL_DEPRECATED("Inline the implementation") - bool Contains(const S2Loop* b) const { return Contains(*b); } - ABSL_DEPRECATED("Inline the implementation") - bool Equals(const S2Loop* b) const { return Equals(*b); } - ABSL_DEPRECATED("Inline the implementation") - bool BoundaryEquals(const S2Loop* b) const { return BoundaryEquals(*b); } -#endif - // This method computes the oriented surface integral of some quantity f(x) // over the loop interior, given a function f_tri(A,B,C) that returns the // corresponding integral over the spherical triangle ABC. Here "oriented diff --git a/src/s2/s2loop_test.cc b/src/s2/s2loop_test.cc index 553aa7ce..53024e62 100644 --- a/src/s2/s2loop_test.cc +++ b/src/s2/s2loop_test.cc @@ -55,9 +55,9 @@ #include "s2/util/math/matrix3x3.h" #include "s2/util/math/vector.h" -using absl::make_unique; using ::s2textformat::MakeLoopOrDie; using std::fabs; +using absl::make_unique; using std::map; using std::max; using std::min; @@ -1193,16 +1193,20 @@ TEST(S2Loop, IsValidDetectsInvalidLoops) { // Some edges cross CheckLoopIsInvalid("20:20, 21:21, 21:20.5, 21:20, 20:21", "crosses"); + // Adjacent antipodal vertices + CheckLoopIsInvalid({S2Point(1, 0, 0), S2Point(-1, 0, 0), S2Point(0, 0, 1)}, + "antipodal"); +} + +#if GTEST_HAS_DEATH_TEST +TEST(S2LoopDeathTest, IsValidDetectsInvalidLoops) { // Points with non-unit length (triggers S2_DCHECK failure in debug) EXPECT_DEBUG_DEATH( CheckLoopIsInvalid({S2Point(2, 0, 0), S2Point(0, 1, 0), S2Point(0, 0, 1)}, "unit length"), "IsUnitLength"); - - // Adjacent antipodal vertices - CheckLoopIsInvalid({S2Point(1, 0, 0), S2Point(-1, 0, 0), S2Point(0, 0, 1)}, - "antipodal"); } +#endif // Helper function for testing the distance methods. "boundary_x" is the // expected result of projecting "x" onto the loop boundary. For convenience diff --git a/src/s2/s2max_distance_targets_test.cc b/src/s2/s2max_distance_targets_test.cc index b803b96b..8c218bc8 100644 --- a/src/s2/s2max_distance_targets_test.cc +++ b/src/s2/s2max_distance_targets_test.cc @@ -15,6 +15,7 @@ #include "s2/s2max_distance_targets.h" +#include #include #include @@ -31,10 +32,10 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2textformat::MakeIndexOrDie; using s2textformat::MakePointOrDie; using s2textformat::ParsePointsOrDie; +using absl::make_unique; using std::vector; TEST(CellTarget, GetCapBound) { diff --git a/src/s2/s2polygon.cc b/src/s2/s2polygon.cc index 7ba97f4d..3d23e9e8 100644 --- a/src/s2/s2polygon.cc +++ b/src/s2/s2polygon.cc @@ -71,13 +71,13 @@ #include "s2/s2shapeutil_visit_crossing_edge_pairs.h" using absl::flat_hash_set; -using absl::make_unique; using s2builderutil::IdentitySnapFunction; using s2builderutil::S2CellIdSnapFunction; using s2builderutil::S2PolygonLayer; using s2builderutil::S2PolylineLayer; using s2builderutil::S2PolylineVectorLayer; using std::fabs; +using absl::make_unique; using std::pair; using std::sqrt; using std::unique_ptr; @@ -138,7 +138,9 @@ S2Polygon::S2Polygon(S2Polygon&& b) // `index_` has a pointer to an S2Polygon::Shape which points to S2Polygon. // But, we've moved to a new address, so get the Shape back out of the index // and update it to point to our new location. - down_cast(*index_.begin())->polygon_ = this; + if (index_.begin() != index_.end()) { + down_cast(*index_.begin())->polygon_ = this; + } } S2Polygon& S2Polygon::operator=(S2Polygon&& b) { @@ -161,7 +163,9 @@ S2Polygon& S2Polygon::operator=(S2Polygon&& b) { // `index_` has a pointer to an S2Polygon::Shape which points to S2Polygon. // But, we've moved to a new address, so get the Shape back out of the index // and update it to point to our new location. - down_cast(*index_.begin())->polygon_ = this; + if (index_.begin() != index_.end()) { + down_cast(*index_.begin())->polygon_ = this; + } return *this; } diff --git a/src/s2/s2polygon.h b/src/s2/s2polygon.h index db3a0dd8..9b7f9123 100644 --- a/src/s2/s2polygon.h +++ b/src/s2/s2polygon.h @@ -517,14 +517,6 @@ class S2Polygon final : public S2Region { int snap_level = S2CellId::kMaxLevel) { return InitToSnapped(*polygon, snap_level); } - ABSL_DEPRECATED("Inline the implementation") - void InitToSimplifiedInCell( - const S2Polygon* a, const S2Cell& cell, S1Angle snap_radius, - S1Angle boundary_tolerance = S1Angle::Radians(1e-15)) { - return InitToSimplifiedInCell(*a, cell, snap_radius, boundary_tolerance); - } - ABSL_DEPRECATED("Inline the implementation") - void InitToComplement(const S2Polygon* a) { return InitToComplement(*a); } #endif // Invert the polygon (replace it by its complement). @@ -680,8 +672,6 @@ class S2Polygon final : public S2Region { bool ApproxEquals(const S2Polygon* b, S1Angle tolerance) const { return ApproxEquals(*b, tolerance); } - ABSL_DEPRECATED("Inline the implementation") - bool BoundaryEquals(const S2Polygon* b) const { return BoundaryEquals(*b); } #endif // Return true if two polygons have the same boundary except for vertex @@ -949,12 +939,6 @@ class S2Polygon final : public S2Region { // order of loop vertices. This function is used to choose which loop to // invert in the case where several loops have exactly the same area. static int CompareLoops(const S2Loop& a, const S2Loop& b); -#ifndef SWIG - ABSL_DEPRECATED("Inline the implementation") - static int CompareLoops(const S2Loop* a, const S2Loop* b) { - return CompareLoops(*a, *b); - } -#endif std::vector > loops_; diff --git a/src/s2/s2polygon_test.cc b/src/s2/s2polygon_test.cc index f946ea4e..432ed9b5 100644 --- a/src/s2/s2polygon_test.cc +++ b/src/s2/s2polygon_test.cc @@ -70,9 +70,9 @@ #include "s2/util/math/matrix3x3.h" using absl::StrCat; -using absl::make_unique; using s2builderutil::IntLatLngSnapFunction; using s2builderutil::S2PolygonLayer; +using absl::make_unique; using std::max; using std::min; using std::numeric_limits; @@ -737,6 +737,18 @@ TEST_F(S2PolygonTestBase, ValidAfterMove) { S2_CHECK(!moved.is_empty()); EXPECT_TRUE(moved.IsValid()); } + + { + S2Polygon polygon; + S2Polygon moved(std::move(polygon)); + EXPECT_TRUE(moved.is_empty()); + } + + { + S2Polygon polygon; + S2Polygon moved = std::move(polygon); + EXPECT_TRUE(moved.is_empty()); + } } struct TestCase { @@ -2645,7 +2657,7 @@ TEST(InitToSimplifiedInCell, PointsOnCellBoundaryKept) { EXPECT_TRUE(simplified.is_empty()); S2Polygon simplified_in_cell; simplified_in_cell.InitToSimplifiedInCell(*polygon, cell, tolerance); - EXPECT_TRUE(simplified_in_cell.BoundaryEquals(polygon.get())); + EXPECT_TRUE(simplified_in_cell.BoundaryEquals(*polygon)); EXPECT_EQ(3, simplified_in_cell.num_vertices()); EXPECT_EQ(-1, simplified.GetSnapLevel()); } @@ -2861,6 +2873,8 @@ TEST_F(S2PolygonDecodeTest, FuzzUncompressedEncoding) { #endif } +#ifndef __EMSCRIPTEN__ +// TODO(b/231695412): Investigate further. TEST_F(S2PolygonDecodeTest, FuzzCompressedEncoding) { // Some parts of the S2 library S2_DCHECK on invalid data, even if we set // FLAGS_s2debug to false or use S2Polygon::set_s2debug_override. So we @@ -2872,6 +2886,7 @@ TEST_F(S2PolygonDecodeTest, FuzzCompressedEncoding) { } #endif } +#endif TEST_F(S2PolygonDecodeTest, FuzzEverything) { // Some parts of the S2 library S2_DCHECK on invalid data, even if we set diff --git a/src/s2/s2polyline.cc b/src/s2/s2polyline.cc index 157c5e67..12c5f3e4 100644 --- a/src/s2/s2polyline.cc +++ b/src/s2/s2polyline.cc @@ -50,10 +50,10 @@ #include "s2/s2predicates.h" #include "s2/util/math/matrix3x3.h" -using absl::make_unique; using absl::Span; using s2builderutil::S2CellIdSnapFunction; using s2builderutil::S2PolylineLayer; +using absl::make_unique; using std::max; using std::min; using std::set; diff --git a/src/s2/s2polyline_alignment_test.cc b/src/s2/s2polyline_alignment_test.cc index 4c99b71d..24ac1127 100644 --- a/src/s2/s2polyline_alignment_test.cc +++ b/src/s2/s2polyline_alignment_test.cc @@ -395,26 +395,28 @@ std::vector> GenPolylines( return polylines; } -TEST(S2PolylineAlignmentTest, ExactLengthZeroInputs) { +#if GTEST_HAS_DEATH_TEST +TEST(S2PolylineAlignmentDeathTest, ExactLengthZeroInputs) { const auto a = s2textformat::MakePolylineOrDie(""); const auto b = s2textformat::MakePolylineOrDie(""); const WarpPath correct_path = {}; EXPECT_DEATH(VerifyPath(*a, *b, correct_path), ""); } -TEST(S2PolylineAlignmentTest, ExactLengthZeroInputA) { +TEST(S2PolylineAlignmentDeathTest, ExactLengthZeroInputA) { const auto a = s2textformat::MakePolylineOrDie(""); const auto b = s2textformat::MakePolylineOrDie("0:0, 1:1, 2:2"); const WarpPath correct_path = {}; EXPECT_DEATH(VerifyPath(*a, *b, correct_path), ""); } -TEST(S2PolylineAlignmentTest, ExactLengthZeroInputB) { +TEST(S2PolylineAlignmentDeathTest, ExactLengthZeroInputB) { const auto a = s2textformat::MakePolylineOrDie("0:0, 1:1, 2:2"); const auto b = s2textformat::MakePolylineOrDie(""); const WarpPath correct_path = {}; EXPECT_DEATH(VerifyPath(*a, *b, correct_path), ""); } +#endif TEST(S2PolylineAlignmentTest, ExactLengthOneInputs) { const auto a = s2textformat::MakePolylineOrDie("1:1"); @@ -465,11 +467,13 @@ TEST(S2PolylineAlignmentTest, FuzzedWithBruteForce) { // TESTS FOR TRAJECTORY CONSENSUS ALGORITHMS // Tests for GetMedoidPolyline -TEST(S2PolylineAlignmentTest, MedoidPolylineNoPolylines) { +#if GTEST_HAS_DEATH_TEST +TEST(S2PolylineAlignmentDeathTest, MedoidPolylineNoPolylines) { std::vector> polylines; const MedoidOptions default_opts; EXPECT_DEATH(GetMedoidPolyline(polylines, default_opts), ""); } +#endif TEST(S2PolylineAlignmentTest, MedoidPolylineOnePolyline) { std::vector> polylines; @@ -572,11 +576,13 @@ TEST(S2PolylineAlignmentTest, MedoidPolylineFewLargePolylines) { } // Tests for GetConsensusPolyline -TEST(S2PolylineAlignmentTest, ConsensusPolylineNoPolylines) { +#if GTEST_HAS_DEATH_TEST +TEST(S2PolylineAlignmentDeathTest, ConsensusPolylineNoPolylines) { std::vector> polylines; const ConsensusOptions default_opts; EXPECT_DEATH(GetConsensusPolyline(polylines, default_opts), ""); } +#endif TEST(S2PolylineAlignmentTest, ConsensusPolylineOnePolyline) { std::vector> polylines; diff --git a/src/s2/s2polyline_test.cc b/src/s2/s2polyline_test.cc index 8e167677..467c243e 100644 --- a/src/s2/s2polyline_test.cc +++ b/src/s2/s2polyline_test.cc @@ -39,10 +39,10 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using absl::StrCat; using s2builderutil::S2CellIdSnapFunction; using std::fabs; +using absl::make_unique; using std::string; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2predicates_test.cc b/src/s2/s2predicates_test.cc index 74022a29..984c29d5 100644 --- a/src/s2/s2predicates_test.cc +++ b/src/s2/s2predicates_test.cc @@ -485,6 +485,11 @@ static const char* kPrecisionNames[] = { "double", "long double", "exact", "symbolic" }; +// If `sizeof(long double) == sizeof(double)`, then we will never do +// calculations with `long double` and instead fall back to exact. +constexpr Precision kLongDoublePrecision = + s2pred::kHasLongDouble ? LONG_DOUBLE : EXACT; + // A helper class that keeps track of how often each precision was used and // generates a string for logging purposes. class PrecisionStats { @@ -604,7 +609,7 @@ TEST(CompareDistances, Coverage) { 1, DOUBLE); TestCompareDistances( S2Point(2, 0, 0), S2Point(2, -1, 0), S2Point(2, 1, 1e-8), - -1, LONG_DOUBLE); + -1, kLongDoublePrecision); TestCompareDistances( S2Point(2, 0, 0), S2Point(2, -1, 0), S2Point(2, 1, 1e-100), -1, EXACT); @@ -624,7 +629,7 @@ TEST(CompareDistances, Coverage) { -1, DOUBLE); TestCompareDistances( S2Point(1, 1, 1), S2Point(1, -1, 0), S2Point(-1, 1, 3e-18), - 1, LONG_DOUBLE); + 1, kLongDoublePrecision); TestCompareDistances( S2Point(1, 1, 1), S2Point(1, -1, 0), S2Point(-1, 1, 1e-100), 1, EXACT); @@ -644,7 +649,7 @@ TEST(CompareDistances, Coverage) { S2Point(1, 1 - 1e-15, 1e-21), 1, DOUBLE); TestCompareDistances( S2Point(-1, -1, 0), S2Point(2, 1, 0), S2Point(2, 1, 1e-8), - 1, LONG_DOUBLE); + 1, kLongDoublePrecision); TestCompareDistances( S2Point(-1, -1, 0), S2Point(2, 1, 0), S2Point(2, 1, 1e-30), 1, EXACT); @@ -789,7 +794,7 @@ TEST(CompareDistance, Coverage) { S1ChordAngle::Radians(1e-15), -1, DOUBLE); TestCompareDistance( S2Point(1, 0, 0), S2Point(1, 1, 0), - S1ChordAngle::Radians(M_PI_4), -1, LONG_DOUBLE); + S1ChordAngle::Radians(M_PI_4), -1, kLongDoublePrecision); TestCompareDistance( S2Point(1, 1e-40, 0), S2Point(1 + DBL_EPSILON, 1e-40, 0), S1ChordAngle::Radians(0.9 * DBL_EPSILON * 1e-40), 1, EXACT); @@ -812,7 +817,7 @@ TEST(CompareDistance, Coverage) { S1ChordAngle::Right(), 1, DOUBLE); TestCompareDistance( S2Point(1, 1, 0), S2Point(1, -1 - DBL_EPSILON, 0), - S1ChordAngle::Right(), 1, LONG_DOUBLE); + S1ChordAngle::Right(), 1, kLongDoublePrecision); TestCompareDistance( S2Point(1, 1, 0), S2Point(1, -1, 1e-30), S1ChordAngle::Right(), 0, EXACT); @@ -901,7 +906,7 @@ TEST(CompareEdgeDistance, Coverage) { S1ChordAngle::Radians(1e-15 + DBL_EPSILON), -1, DOUBLE); TestCompareEdgeDistance( S2Point(1, 1, 1e-15), S2Point(1, 0, 0), S2Point(0, 1, 0), - S1ChordAngle::Radians(1e-15 + DBL_EPSILON), -1, LONG_DOUBLE); + S1ChordAngle::Radians(1e-15 + DBL_EPSILON), -1, kLongDoublePrecision); TestCompareEdgeDistance( S2Point(1, 1, 1e-40), S2Point(1, 0, 0), S2Point(0, 1, 0), S1ChordAngle::Radians(1e-40), -1, EXACT); @@ -917,7 +922,7 @@ TEST(CompareEdgeDistance, Coverage) { TestCompareEdgeDistance( S2Point(1e-15, 0, 1), S2Point(1, 0, 0), S2Point(0, 1, 0), S1ChordAngle::Radians(M_PI_2 - 1e-15 - DBL_EPSILON), - 1, LONG_DOUBLE); + 1, kLongDoublePrecision); TestCompareEdgeDistance( S2Point(1e-40, 0, 1), S2Point(1, 0, 0), S2Point(0, 1, 0), S1ChordAngle::Right(), -1, EXACT); @@ -934,7 +939,7 @@ TEST(CompareEdgeDistance, Coverage) { S1ChordAngle::Right(), 1, DOUBLE); TestCompareEdgeDistance( S2Point(1e-18, -1, 0), S2Point(1, 0, 0), S2Point(1, 1, 0), - S1ChordAngle::Right(), -1, LONG_DOUBLE); + S1ChordAngle::Right(), -1, kLongDoublePrecision); TestCompareEdgeDistance( S2Point(1e-100, -1, 0), S2Point(1, 0, 0), S2Point(1, 1, 0), S1ChordAngle::Right(), -1, EXACT); @@ -948,7 +953,7 @@ TEST(CompareEdgeDistance, Coverage) { S1ChordAngle::Right(), 1, DOUBLE); TestCompareEdgeDistance( S2Point(-1, 0, 0), S2Point(1, 0, 0), S2Point(1e-18, 1, 0), - S1ChordAngle::Right(), 1, LONG_DOUBLE); + S1ChordAngle::Right(), 1, kLongDoublePrecision); TestCompareEdgeDistance( S2Point(-1, 0, 0), S2Point(1, 0, 0), S2Point(1e-100, 1, 0), S1ChordAngle::Right(), 1, EXACT); @@ -1099,7 +1104,7 @@ TEST(CompareEdgeDirections, Coverage) { 1, DOUBLE); TestCompareEdgeDirections(S2Point(1, 0, 1e-18), S2Point(1, 1, 0), S2Point(0, -1, 0), S2Point(0, 0, 1), - 1, LONG_DOUBLE); + 1, kLongDoublePrecision); TestCompareEdgeDirections(S2Point(1, 0, 1e-50), S2Point(1, 1, 0), S2Point(0, -1, 0), S2Point(0, 0, 1), 1, EXACT); @@ -1221,7 +1226,7 @@ TEST(EdgeCircumcenterSign, Coverage) { TestEdgeCircumcenterSign( S2Point(1, -1, 0), S2Point(1, 1, 0), S2Point(1, -1e-5, 1), S2Point(1, 1e-5, -1), S2Point(1, 1 - 1e-9, 1e-5), - -1, LONG_DOUBLE); + -1, kLongDoublePrecision); TestEdgeCircumcenterSign( S2Point(1, -1, 0), S2Point(1, 1, 0), S2Point(1, -1e-5, 1), S2Point(1, 1e-5, -1), S2Point(1, 1 - 1e-15, 1e-5), @@ -1380,11 +1385,11 @@ TEST(VoronoiSiteExclusion, Coverage) { TestVoronoiSiteExclusion( S2Point(1, -1e-10, 1e-5), S2Point(1, 1e-10, -1e-5), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1e-5), - Excluded::NEITHER, LONG_DOUBLE); + Excluded::NEITHER, kLongDoublePrecision); TestVoronoiSiteExclusion( S2Point(1, -1e-17, 1e-5), S2Point(1, 1e-17, -1e-5), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1e-4), - Excluded::NEITHER, LONG_DOUBLE); + Excluded::NEITHER, kLongDoublePrecision); TestVoronoiSiteExclusion( S2Point(1, -1e-20, 1e-5), S2Point(1, 1e-20, -1e-5), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1e-5), @@ -1399,11 +1404,11 @@ TEST(VoronoiSiteExclusion, Coverage) { TestVoronoiSiteExclusion( S2Point(1, -1.00105e-6, 1.0049999999e-5), S2Point(1, 0, -1e-5), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1.005e-5), - Excluded::FIRST, LONG_DOUBLE); + Excluded::FIRST, kLongDoublePrecision); TestVoronoiSiteExclusion( S2Point(1, -1e-6, 1.005e-5), S2Point(1, 0, -1e-5), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1.005e-5), - Excluded::FIRST, LONG_DOUBLE); + Excluded::FIRST, kLongDoublePrecision); TestVoronoiSiteExclusion( S2Point(1, -1e-31, 1.005e-30), S2Point(1, 0, -1e-30), S2Point(1, -1, 0), S2Point(1, 1, 0), S1ChordAngle::Radians(1.005e-30), diff --git a/src/s2/s2shape_index_buffered_region_test.cc b/src/s2/s2shape_index_buffered_region_test.cc index c3b8fcd0..8445bbce 100644 --- a/src/s2/s2shape_index_buffered_region_test.cc +++ b/src/s2/s2shape_index_buffered_region_test.cc @@ -31,10 +31,10 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2textformat::MakeIndexOrDie; using s2textformat::MakePointOrDie; using std::cout; +using absl::make_unique; using std::string; TEST(S2ShapeIndexBufferedRegion, EmptyIndex) { diff --git a/src/s2/s2shape_index_measures_test.cc b/src/s2/s2shape_index_measures_test.cc index dd088d6e..3e1ddf60 100644 --- a/src/s2/s2shape_index_measures_test.cc +++ b/src/s2/s2shape_index_measures_test.cc @@ -21,6 +21,8 @@ #include "s2/s2shape_index_measures.h" +#include + #include #include "absl/memory/memory.h" #include "s2/mutable_s2shape_index.h" @@ -30,8 +32,8 @@ #include "s2/s2pointutil.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2textformat::MakeIndexOrDie; +using absl::make_unique; namespace { diff --git a/src/s2/s2shape_index_region_test.cc b/src/s2/s2shape_index_region_test.cc index 49a9986e..bcc3aa19 100644 --- a/src/s2/s2shape_index_region_test.cc +++ b/src/s2/s2shape_index_region_test.cc @@ -29,7 +29,6 @@ #include "s2/s2testing.h" #include "s2/s2wrapped_shape.h" - using absl::make_unique; using std::map; using std::string; diff --git a/src/s2/s2shape_nesting_query.h b/src/s2/s2shape_nesting_query.h index 3f774315..2f790345 100644 --- a/src/s2/s2shape_nesting_query.h +++ b/src/s2/s2shape_nesting_query.h @@ -18,7 +18,6 @@ #define S2_S2SHAPE_NESTING_QUERY_H_ #include -#include #include #include "absl/container/inlined_vector.h" diff --git a/src/s2/s2shape_nesting_query_test.cc b/src/s2/s2shape_nesting_query_test.cc index b2fd6199..fe901817 100644 --- a/src/s2/s2shape_nesting_query_test.cc +++ b/src/s2/s2shape_nesting_query_test.cc @@ -17,6 +17,7 @@ #include "s2/s2shape_nesting_query.h" #include +#include #include #include @@ -29,8 +30,8 @@ #include "s2/s2text_format.h" #include "s2/util/gtl/legacy_random_shuffle.h" -using absl::make_unique; using absl::Span; +using absl::make_unique; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2shapeutil_coding.cc b/src/s2/s2shapeutil_coding.cc index b27ab75c..95e0cb49 100644 --- a/src/s2/s2shapeutil_coding.cc +++ b/src/s2/s2shapeutil_coding.cc @@ -30,8 +30,8 @@ #include "s2/s2polyline.h" #include "s2/s2wrapped_shape.h" -using absl::make_unique; using std::make_shared; +using absl::make_unique; using std::unique_ptr; using std::vector; diff --git a/src/s2/s2testing.cc b/src/s2/s2testing.cc index 8d83d068..ad0845e7 100644 --- a/src/s2/s2testing.cc +++ b/src/s2/s2testing.cc @@ -33,6 +33,7 @@ #include "absl/memory/memory.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_split.h" +#include "absl/strings/string_view.h" #include "s2/r1interval.h" #include "s2/s1angle.h" #include "s2/s1interval.h" diff --git a/src/s2/s2testing.h b/src/s2/s2testing.h index 1357cf91..e2de4f20 100644 --- a/src/s2/s2testing.h +++ b/src/s2/s2testing.h @@ -27,6 +27,7 @@ #include "s2/base/commandlineflags.h" #include "s2/base/integral_types.h" #include "absl/base/macros.h" +#include "absl/strings/string_view.h" #include "s2/_fp_contract_off.h" #include "s2/r2.h" #include "s2/s1angle.h" @@ -317,7 +318,7 @@ bool CheckResultSet(const std::vector>& x, int max_size, Distance max_distance, typename Distance::Delta max_error, typename Distance::Delta max_pruning_error, - const std::string& label) { + absl::string_view label) { using Result = std::pair; // Results should be sorted by distance, but not necessarily then by Id. EXPECT_TRUE(std::is_sorted(x.begin(), x.end(), diff --git a/src/s2/s2text_format.cc b/src/s2/s2text_format.cc index 011b5caf..3cf4211c 100644 --- a/src/s2/s2text_format.cc +++ b/src/s2/s2text_format.cc @@ -37,8 +37,8 @@ #include "s2/s2polyline.h" using absl::Span; -using absl::make_unique; using absl::string_view; +using absl::make_unique; using std::pair; using std::string; using std::unique_ptr; diff --git a/src/s2/s2winding_operation_test.cc b/src/s2/s2winding_operation_test.cc index 2a38f927..cebb7900 100644 --- a/src/s2/s2winding_operation_test.cc +++ b/src/s2/s2winding_operation_test.cc @@ -33,9 +33,9 @@ #include "s2/s2testing.h" #include "s2/s2text_format.h" -using absl::make_unique; using s2builderutil::IdentitySnapFunction; using s2builderutil::IntLatLngSnapFunction; +using absl::make_unique; using std::string; using std::unique_ptr; using std::vector; @@ -271,4 +271,11 @@ TEST(S2WindingOperation, SymmetricDifferenceDegeneracies) { "", "2:2; 5:5"); } +TEST(S2WindingOperationOptions, SetGetSnapFunction) { + // Prevent these from being detected as dead code. + S2WindingOperation::Options opts; + opts.set_snap_function(IdentitySnapFunction()); + EXPECT_EQ(opts.snap_function().snap_radius(), S1Angle::Zero()); +} + } // namespace diff --git a/src/s2/util/bitmap/bitmap.h b/src/s2/util/bitmap/bitmap.h index 343d79f4..7de736a3 100644 --- a/src/s2/util/bitmap/bitmap.h +++ b/src/s2/util/bitmap/bitmap.h @@ -31,6 +31,7 @@ #include #include #include +#include #include "s2/base/integral_types.h" #include "s2/base/logging.h" @@ -149,7 +150,7 @@ class BasicBitmap { // element, you will be left with only the valid, defined bits (the // others will be 0) Word HighOrderMapElementMask() const { - return (size_ == 0) ? 0 : (~W{0}) >> (-size_ & (kIntBits - 1)); + return (size_ == 0) ? 0 : kAllOnesWord >> (-size_ & (kIntBits - 1)); } bool Get(size_type index) const { @@ -197,7 +198,7 @@ class BasicBitmap { // Returns true if all bits are set bool IsAllOnes() const { return std::all_of(map_, map_ + array_size() - 1, - [](Word w) { return w == ~W{0}; }) && + [](Word w) { return w == kAllOnesWord; }) && ((~map_[array_size() - 1]) & HighOrderMapElementMask()) == W{0}; } @@ -343,7 +344,7 @@ class BasicBitmap { // Sets all the bits to true or false void SetAll(bool value) { - std::fill(map_, map_ + array_size(), value ? ~W{0} : W{0}); + std::fill(map_, map_ + array_size(), value ? kAllOnesWord : W{0}); } // Clears all bits in the bitmap @@ -372,7 +373,8 @@ class BasicBitmap { // Sets "this" to be the "~" (Complement) of "this". void Complement() { - std::transform(map_, map_ + array_size(), map_, [](Word w) { return ~w; }); + std::transform(map_, map_ + array_size(), map_, + [](Word w) -> Word { return ~w; }); } // Sets "this" to be the set of bits in "this" but not in "other" @@ -584,6 +586,26 @@ class BasicBitmap { BitIndexRange TrueBitIndices() const { return BitIndexRange(this); } private: + // An unsigned integral type that is not promoted and can represent all values + // of `Word` (assuming `Word` is unsigned). Note that if `Word` does not get + // promoted, then this type is the same as `Word`. We occaisionally use this + // type to perform arithmetic, to avoid having `Word`-typed values promoted to + // a signed type via integral promotions. + // TODO(b/228178585) + using ArithmeticWord = absl::make_unsigned_t())>; + + // A value of type `Word` with all bits set to one. If `Word` != + // `ArithmeticWord`, i.e. `Word` gets promoted, then this is a `Word` with all + // bits set to one then promoted to `ArithmeticWord`. In other words, the + // lowest N bits are one, and all other bits are zero, where N is the width of + // `Word`. + // + // For example, for uint32, this is 0xFFFFFFFF. However, if `Word` is + // `unit8_t`, and `ArithmeticWord` is equal to `uint32` and kAllOnesWord is + // 0x000000FF. + static constexpr auto kAllOnesWord = + ArithmeticWord{static_cast(~ArithmeticWord{0})}; + // Implements FindNextSetBitInVector if 'complement' is false, // and FindNextUnsetBitInVector if 'complement' is true. static bool FindNextBitInVector(bool complement, const Word* words, @@ -624,13 +646,18 @@ inline std::ostream& operator<<(std::ostream& out, namespace util { namespace bitmap { +using Bitmap8 = ::util::bitmap::internal::BasicBitmap; +using Bitmap16 = ::util::bitmap::internal::BasicBitmap; using Bitmap32 = ::util::bitmap::internal::BasicBitmap; using Bitmap64 = ::util::bitmap::internal::BasicBitmap; } // namespace bitmap } // namespace util -// Legacy definition, use Bitmap32/Bitmap64 instead in new code. +// Legacy definition, use Bitmap32/Bitmap64 instead in new code. Do not use +// this to instantiate the BasicBitmap template (i.e. do not spell +// `::Bitmap::BasicBitmap`). Instead, use one of the aliases above (e.g. +// ::util::bitmap::Bitmap32). // TODO(b/145388656): remove this class, once all forward declarations are gone. class ABSL_DEPRECATED( "Legacy definition, use util::bitmap::{Bitmap32|Bitmap64} instead in new " @@ -883,20 +910,22 @@ typename BasicBitmap::size_type BasicBitmap::GetOnesCountInRange( size_t end_word = (end - 1) / kIntBits; // Word containing the last bit. Word* p = map_ + start_word; - Word c = (*p & (~W{0} << (start & (kIntBits - 1)))); + ArithmeticWord c = static_cast(*p) & + (kAllOnesWord << (start & (kIntBits - 1))); - Word endmask = (~W{0} >> ((end_word + 1) * kIntBits - end)); + ArithmeticWord endmask = (kAllOnesWord >> ((end_word + 1) * kIntBits - end)); if (end_word == start_word) { // Only one word? - return absl::popcount(c & endmask); + return absl::popcount(static_cast(c & endmask)); } - size_type sum = absl::popcount(c); + size_type sum = absl::popcount(static_cast(c)); for (++p; p < map_ + end_word; ++p) { sum += absl::popcount(*p); } - return sum + absl::popcount(*p & endmask); + return sum + absl::popcount(static_cast( + static_cast(*p) & endmask)); } template @@ -942,7 +971,7 @@ bool BasicBitmap::FindNextBitInVector(bool complement, const Word* words, if (one_word & (W{1} << first_bit_offset)) return true; // First word is special - we need to mask off leading bits - one_word &= (~W{0} << first_bit_offset); + one_word &= (kAllOnesWord << first_bit_offset); // Loop through all but the last word. Note that 'limit' is one // past the last bit we want to check, and we don't want to read @@ -962,7 +991,7 @@ bool BasicBitmap::FindNextBitInVector(bool complement, const Word* words, // Last word is special - we may need to mask off trailing bits. Note that // 'limit' is one past the last bit we want to check, and if limit is a // multiple of kIntBits we want to check all bits in this word. - one_word &= ~((~W{0} - 1) << ((limit - 1) & (kIntBits - 1))); + one_word &= ~((kAllOnesWord - 1) << ((limit - 1) & (kIntBits - 1))); if (one_word != 0) { *bit_index_inout = (int_index << kLogIntBits) + Bits::FindLSBSetNonZero64(one_word); @@ -979,7 +1008,7 @@ bool BasicBitmap::FindPreviousBitInVector(bool complement, const Word* words, const size_type bit_index = *bit_index_inout; size_t map_index = bit_index >> kLogIntBits; const size_t map_limit = limit >> kLogIntBits; - const size_t bit_limit_mask = ~W{0} << (limit & (kIntBits - 1)); + const size_t bit_limit_mask = kAllOnesWord << (limit & (kIntBits - 1)); Word one_word = complement ? ~words[map_index] : words[map_index]; if (limit > *bit_index_inout) return false; @@ -990,7 +1019,7 @@ bool BasicBitmap::FindPreviousBitInVector(bool complement, const Word* words, if (one_word & (W{1} << bit_offset)) return true; // First word is special - we need to mask off trailing bits - one_word &= ~((~W{0} - 1) << bit_offset); + one_word &= ~((kAllOnesWord - 1) << bit_offset); // Then any leading bits if the limit is within this word if (map_index == map_limit) one_word &= bit_limit_mask; @@ -1043,11 +1072,11 @@ void BasicBitmap::SetRange(size_type begin, size_type end, bool value) { const size_type begin_bit = begin % kIntBits; const size_type end_element = end / kIntBits; const size_type end_bit = end % kIntBits; - Word initial_mask = ~W{0} << begin_bit; + Word initial_mask = kAllOnesWord << begin_bit; if (end_element == begin_element) { // The range is contained in a single element of the array, so // adjust both ends of the mask. - initial_mask = initial_mask & (~W{0} >> (kIntBits - end_bit)); + initial_mask = initial_mask & (kAllOnesWord >> (kIntBits - end_bit)); } if (value) { map_[begin_element] |= initial_mask; @@ -1058,14 +1087,14 @@ void BasicBitmap::SetRange(size_type begin, size_type end, bool value) { // Set all the bits in the array elements between the begin // and end elements. std::fill(map_ + begin_element + 1, map_ + end_element, - value ? ~W{0} : W{0}); + value ? kAllOnesWord : W{0}); // Update the appropriate bit-range in the last element. // Note end_bit is an exclusive bound, so if it's 0 none of the // bits in end_element are contained in the range (and we don't // have to modify it). if (end_bit != 0) { - const Word final_mask = ~W{0} >> (kIntBits - end_bit); + const Word final_mask = kAllOnesWord >> (kIntBits - end_bit); if (value) { map_[end_element] |= final_mask; } else { diff --git a/src/s2/util/bits/bit-interleave.cc b/src/s2/util/bits/bit-interleave.cc index 552a0dd8..8a43e035 100644 --- a/src/s2/util/bits/bit-interleave.cc +++ b/src/s2/util/bits/bit-interleave.cc @@ -24,6 +24,17 @@ // gcc-4.3.1-glibc-2.3.6-grte-k8. // TODO(user): Inlining InterleaveUint32 yields a measurable speedup (5 // ns vs. 8 ns). Consider cost/benefit of moving implementations inline. +// 2022-04-21: nywang@google.com microbenchmarked DeinterleaveUint32/16/8 with +// Lut and table-free implementations. DeinterleaveUint32/8 would benefit +// from table-free implementations with 3% - 8% performance improvement on a +// Xeon E5 2690 V3, while DeinterleaveUint16 has 6% performance regression +// with table-free implementations. +// Some client of this lib like util/geometry/s2point_compression.cc even +// see up to 28% performance improvment by adopting the table-free +// DeinterleaveUint32 implementation. +// TODO(user): As of 2022, Xeon E5 2690 V3 is also an ancient +// architecture(Haswell). We need to consider benchmarking it on more +// recent architectures. #include "s2/util/bits/bit-interleave.h" @@ -93,81 +104,58 @@ uint64 InterleaveUint32(const uint32 val0, const uint32 val1) { (static_cast(kInterleaveLut[val1 >> 24]) << 49); } -// The lookup table below can convert a sequence of interleaved 8 bits into -// non-interleaved 4 bits. The table can convert both odd and even bits at the -// same time, and lut[x & 0x55] converts the even bits (bits 0, 2, 4 and 6), -// while lut[x & 0xaa] converts the odd bits (bits 1, 3, 5 and 7). -// -// The lookup table below was generated using the following python code: -// -// def deinterleave(bits): -// if bits == 0: return 0 -// if bits < 4: return 1 -// return deinterleave(bits / 4) * 2 + deinterleave(bits & 3) -// -// for i in range(256): print "0x%x," % deinterleave(i), -// -static const uint8 kDeinterleaveLut[256] = { - 0x0, 0x1, 0x1, 0x1, 0x2, 0x3, 0x3, 0x3, 0x2, 0x3, 0x3, 0x3, 0x2, - 0x3, 0x3, 0x3, 0x4, 0x5, 0x5, 0x5, 0x6, 0x7, 0x7, 0x7, 0x6, 0x7, - 0x7, 0x7, 0x6, 0x7, 0x7, 0x7, 0x4, 0x5, 0x5, 0x5, 0x6, 0x7, 0x7, - 0x7, 0x6, 0x7, 0x7, 0x7, 0x6, 0x7, 0x7, 0x7, 0x4, 0x5, 0x5, 0x5, - 0x6, 0x7, 0x7, 0x7, 0x6, 0x7, 0x7, 0x7, 0x6, 0x7, 0x7, 0x7, - - 0x8, 0x9, 0x9, 0x9, 0xa, 0xb, 0xb, 0xb, 0xa, 0xb, 0xb, 0xb, 0xa, - 0xb, 0xb, 0xb, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, - 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, - 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, - 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, - - 0x8, 0x9, 0x9, 0x9, 0xa, 0xb, 0xb, 0xb, 0xa, 0xb, 0xb, 0xb, 0xa, - 0xb, 0xb, 0xb, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, - 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, - 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, - 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, - - 0x8, 0x9, 0x9, 0x9, 0xa, 0xb, 0xb, 0xb, 0xa, 0xb, 0xb, 0xb, 0xa, - 0xb, 0xb, 0xb, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, - 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, 0xe, 0xf, 0xf, - 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xc, 0xd, 0xd, 0xd, - 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, 0xe, 0xf, 0xf, 0xf, -}; +// Extracting the even bits (bit 0, 2, ...). +inline uint8 ExtractEvenBits(uint16 bits) { + bits &= 0x5555; + bits |= bits >> 1; + bits &= 0x3333; + bits |= bits >> 2; + bits &= 0x0f0f; + bits |= bits >> 4; + return static_cast(bits); +} void DeinterleaveUint8(uint16 val, uint8 *val0, uint8 *val1) { - *val0 = ((kDeinterleaveLut[val & 0x55]) | - (kDeinterleaveLut[(val >> 8) & 0x55] << 4)); - *val1 = ((kDeinterleaveLut[val & 0xaa]) | - (kDeinterleaveLut[(val >> 8) & 0xaa] << 4)); + *val0 = ExtractEvenBits(val); + *val1 = ExtractEvenBits(val >> 1); +} + +// Extracting the even bits (bit 0, 2, ...). +inline uint16 ExtractEvenBits(uint32 bits) { + bits &= 0x55555555; + bits |= bits >> 1; + bits &= 0x33333333; + bits |= bits >> 2; + bits &= 0x0f0f0f0f; + bits |= bits >> 4; + bits &= 0x00ff00ff; + bits |= bits >> 8; + return static_cast(bits); } void DeinterleaveUint16(uint32 code, uint16 *val0, uint16 *val1) { - *val0 = ((kDeinterleaveLut[code & 0x55]) | - (kDeinterleaveLut[(code >> 8) & 0x55] << 4) | - (kDeinterleaveLut[(code >> 16) & 0x55] << 8) | - (kDeinterleaveLut[(code >> 24) & 0x55] << 12)); - *val1 = ((kDeinterleaveLut[code & 0xaa]) | - (kDeinterleaveLut[(code >> 8) & 0xaa] << 4) | - (kDeinterleaveLut[(code >> 16) & 0xaa] << 8) | - (kDeinterleaveLut[(code >> 24) & 0xaa] << 12)); + *val0 = ExtractEvenBits(code); + *val1 = ExtractEvenBits(code >> 1); +} + +// Extracting the even bits (bit 0, 2, ...). +inline uint32 ExtractEvenBits(uint64 bits) { + bits &= 0x5555555555555555; + bits |= bits >> 1; + bits &= 0x3333333333333333; + bits |= bits >> 2; + bits &= 0x0f0f0f0f0f0f0f0f; + bits |= bits >> 4; + bits &= 0x00ff00ff00ff00ff; + bits |= bits >> 8; + bits &= 0x0000ffff0000ffff; + bits |= bits >> 16; + return static_cast(bits); } void DeinterleaveUint32(uint64 code, uint32 *val0, uint32 *val1) { - *val0 = ((kDeinterleaveLut[code & 0x55]) | - (kDeinterleaveLut[(code >> 8) & 0x55] << 4) | - (kDeinterleaveLut[(code >> 16) & 0x55] << 8) | - (kDeinterleaveLut[(code >> 24) & 0x55] << 12) | - (kDeinterleaveLut[(code >> 32) & 0x55] << 16) | - (kDeinterleaveLut[(code >> 40) & 0x55] << 20) | - (kDeinterleaveLut[(code >> 48) & 0x55] << 24) | - (kDeinterleaveLut[(code >> 56) & 0x55] << 28)); - *val1 = ((kDeinterleaveLut[code & 0xaa]) | - (kDeinterleaveLut[(code >> 8) & 0xaa] << 4) | - (kDeinterleaveLut[(code >> 16) & 0xaa] << 8) | - (kDeinterleaveLut[(code >> 24) & 0xaa] << 12) | - (kDeinterleaveLut[(code >> 32) & 0xaa] << 16) | - (kDeinterleaveLut[(code >> 40) & 0xaa] << 20) | - (kDeinterleaveLut[(code >> 48) & 0xaa] << 24) | - (kDeinterleaveLut[(code >> 56) & 0xaa] << 28)); + *val0 = ExtractEvenBits(code); + *val1 = ExtractEvenBits(code >> 1); } // Derivation of the multiplication based interleave algorithm: diff --git a/src/s2/util/coding/varint.h b/src/s2/util/coding/varint.h index 905efaaf..beea7145 100644 --- a/src/s2/util/coding/varint.h +++ b/src/s2/util/coding/varint.h @@ -290,11 +290,11 @@ inline const char* Varint::Parse32WithLimit(const char* p, const char* l, inline const char* Varint::Parse64(const char* p, uint64* OUTPUT) { #if defined(__x86_64__) - auto ptr = reinterpret_cast(p); - int64 byte = *ptr; - if (byte >= 0) { - *OUTPUT = static_cast(byte); - return reinterpret_cast(ptr) + 1; + auto ptr = reinterpret_cast(p); + int64 byte = *ptr; + if (byte >= 0) { + *OUTPUT = static_cast(byte); + return reinterpret_cast(ptr) + 1; } else { auto tmp = Parse64FallbackPair(p, byte); if (ABSL_PREDICT_TRUE(tmp.first)) *OUTPUT = tmp.second; diff --git a/src/s2/util/math/exactfloat/exactfloat.cc b/src/s2/util/math/exactfloat/exactfloat.cc index 8f6365b9..1709e81c 100644 --- a/src/s2/util/math/exactfloat/exactfloat.cc +++ b/src/s2/util/math/exactfloat/exactfloat.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include "s2/base/integral_types.h" #include "s2/base/logging.h" diff --git a/src/s2/util/math/exactfloat/exactfloat.h b/src/s2/util/math/exactfloat/exactfloat.h index 8f5840e5..3742072b 100644 --- a/src/s2/util/math/exactfloat/exactfloat.h +++ b/src/s2/util/math/exactfloat/exactfloat.h @@ -115,6 +115,7 @@ #include #include #include +#include #include #include "s2/base/integral_types.h" diff --git a/src/s2/util/math/vector.h b/src/s2/util/math/vector.h index d2cbfc5b..0f6548ab 100644 --- a/src/s2/util/math/vector.h +++ b/src/s2/util/math/vector.h @@ -98,20 +98,20 @@ class BasicVector { } // TODO(user): Relationals should be nonmembers. - bool operator==(const D& b) const { - const T* ap = static_cast(*this).Data(); - return std::equal(ap, ap + this->Size(), b.Data()); - } - bool operator!=(const D& b) const { return !(AsD() == b); } - bool operator<(const D& b) const { - const T* ap = static_cast(*this).Data(); - const T* bp = b.Data(); + bool operator==(const BasicVector& b) const { + const T* ap = AsD().Data(); + return std::equal(ap, ap + this->Size(), b.AsD().Data()); + } + bool operator!=(const BasicVector& b) const { return !(*this == b); } + bool operator<(const BasicVector& b) const { + const T* ap = AsD().Data(); + const T* bp = b.AsD().Data(); return std::lexicographical_compare(ap, ap + this->Size(), bp, bp + b.Size()); } - bool operator>(const D& b) const { return b < AsD(); } - bool operator<=(const D& b) const { return !(AsD() > b); } - bool operator>=(const D& b) const { return !(AsD() < b); } + bool operator>(const BasicVector& b) const { return b < *this; } + bool operator<=(const BasicVector& b) const { return !(*this > b); } + bool operator>=(const BasicVector& b) const { return !(*this < b); } D& operator+=(const D& b) { PlusEq(static_cast(*this).Data(), b.Data(), IdxSeqN{});