From fe72a9aa084a859613c1705eb4910de615308503 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Wed, 4 Nov 2020 21:41:57 +0100 Subject: [PATCH 1/2] Change for a more strict geojson handling. - Do not handle m coordinate. - Raise for unknown geojson `type`. - Allow less strict polygons (not simple). Which is ok per spec. - Add various tests. --- lib/rgeo-geojson.rb | 5 +- lib/rgeo/geo_json.rb | 18 +++- lib/rgeo/geo_json/coder.rb | 82 +++++++++------- lib/rgeo/geo_json/entities.rb | 2 - lib/rgeo/geo_json/interface.rb | 25 ++++- test/basic_test.rb | 31 ++++-- test/coder/encode_geometry_test.rb | 147 +++++++++++++++++++++++++++++ test/test_helper.rb | 18 ++++ 8 files changed, 276 insertions(+), 52 deletions(-) create mode 100644 test/coder/encode_geometry_test.rb create mode 100644 test/test_helper.rb diff --git a/lib/rgeo-geojson.rb b/lib/rgeo-geojson.rb index 47fc549..d56ca7e 100644 --- a/lib/rgeo-geojson.rb +++ b/lib/rgeo-geojson.rb @@ -1,3 +1,6 @@ # frozen_string_literal: true -require "rgeo/geo_json" +# Helper for bundler's `require: true` option. +# See {file:lib/rgeo/geo_json/interface.rb} for documentation entry point. + +require_relative "rgeo/geo_json" diff --git a/lib/rgeo/geo_json.rb b/lib/rgeo/geo_json.rb index dec3f7c..3819787 100644 --- a/lib/rgeo/geo_json.rb +++ b/lib/rgeo/geo_json.rb @@ -1,8 +1,16 @@ # frozen_string_literal: true -require "rgeo" -require "rgeo/geo_json/version" -require "rgeo/geo_json/entities" -require "rgeo/geo_json/coder" -require "rgeo/geo_json/interface" require "multi_json" +require "rgeo" + +module RGeo + module GeoJSON + class Error < RGeo::Error::RGeoError + end + end +end + +require_relative "geo_json/version" +require_relative "geo_json/entities" +require_relative "geo_json/coder" +require_relative "geo_json/interface" diff --git a/lib/rgeo/geo_json/coder.rb b/lib/rgeo/geo_json/coder.rb index ea62964..e1b6442 100644 --- a/lib/rgeo/geo_json/coder.rb +++ b/lib/rgeo/geo_json/coder.rb @@ -6,8 +6,10 @@ module GeoJSON # the RGeo::Feature::Factory and the RGeo::GeoJSON::EntityFactory to # be used) so that you can encode and decode without specifying those # settings every time. - class Coder + class Error < RGeo::GeoJSON::Error + end + # Create a new coder settings object. The geo factory is passed as # a required argument. # @@ -23,11 +25,37 @@ class Coder # RGeo::GeoJSON::Feature or RGeo::GeoJSON::FeatureCollection. # See RGeo::GeoJSON::EntityFactory for more information. def initialize(opts = {}) - @geo_factory = opts[:geo_factory] || RGeo::Cartesian.preferred_factory - @entity_factory = opts[:entity_factory] || EntityFactory.instance + @geo_factory = opts.fetch( + :geo_factory, + RGeo::Cartesian.preferred_factory(uses_lenient_assertions: true) + ) + @entity_factory = opts.fetch(:entity_factory, EntityFactory.instance) + if @geo_factory.property(:has_m_coordinate) + # If a GeoJSON has more than 2 elements, the first one should be + # longitude and the second one latitude. M is not part of GeoJSON + # specifications and only kept here for backward compatibilities. + # + # Quote from https://tools.ietf.org/html/rfc7946#section-3.1.1: + # + # > A position is an array of numbers. There MUST be two or more + # > elements. The first two elements are longitude and latitude, or + # > easting and northing, precisely in that order and using decimal + # > numbers. Altitude or elevation MAY be included as an optional third + # > element. + # > + # > Implementations SHOULD NOT extend positions beyond three elements + # > because the semantics of extra elements are unspecified and + # > ambiguous. Historically, some implementations have used a fourth + # > element to carry a linear referencing measure (sometimes denoted as + # > "M") or a numerical timestamp, but in most situations a parser will + # > not be able to properly interpret these values. The interpretation + # > and meaning of additional elements is beyond the scope of this + # > specification, and additional elements MAY be ignored by parsers. + raise Error, "GeoJSON format cannot handle m coordinate." + end + @num_coordinates = 2 @num_coordinates += 1 if @geo_factory.property(:has_z_coordinate) - @num_coordinates += 1 if @geo_factory.property(:has_m_coordinate) end # Encode the given object as GeoJSON. The object may be one of the @@ -41,8 +69,9 @@ def initialize(opts = {}) # appropriate JSON library installed. # # Returns nil if nil is passed in as the object. - def encode(object) + return nil if object.nil? + if @entity_factory.is_feature_collection?(object) { "type" => "FeatureCollection", @@ -50,8 +79,6 @@ def encode(object) } elsif @entity_factory.is_feature?(object) encode_feature(object) - elsif object.nil? - nil else encode_geometry(object) end @@ -111,35 +138,20 @@ def encode_feature(object) end def encode_geometry(object) + return nil if object.nil? + if object.factory.property(:has_m_coordinate) + raise Error, "GeoJSON format cannot handle m coordinate." + end + case object - when RGeo::Feature::Point - { - "type" => "Point", - "coordinates" => object.coordinates - } - when RGeo::Feature::LineString + when RGeo::Feature::Point, + RGeo::Feature::LineString, + RGeo::Feature::Polygon, + RGeo::Feature::MultiPoint, + RGeo::Feature::MultiLineString, + RGeo::Feature::MultiPolygon { - "type" => "LineString", - "coordinates" => object.coordinates - } - when RGeo::Feature::Polygon - { - "type" => "Polygon", - "coordinates" => object.coordinates - } - when RGeo::Feature::MultiPoint - { - "type" => "MultiPoint", - "coordinates" => object.coordinates - } - when RGeo::Feature::MultiLineString - { - "type" => "MultiLineString", - "coordinates" => object.coordinates - } - when RGeo::Feature::MultiPolygon - { - "type" => "MultiPolygon", + "type" => object.geometry_type.type_name, "coordinates" => object.coordinates } when RGeo::Feature::GeometryCollection @@ -178,7 +190,7 @@ def decode_geometry(input) when "MultiPolygon" decode_multi_polygon_coords(input["coordinates"]) else - nil + raise Error, "'#{input['type']}' type is not part of GeoJSON spec." end end diff --git a/lib/rgeo/geo_json/entities.rb b/lib/rgeo/geo_json/entities.rb index e068dde..e99ed32 100644 --- a/lib/rgeo/geo_json/entities.rb +++ b/lib/rgeo/geo_json/entities.rb @@ -11,7 +11,6 @@ module GeoJSON # implementation need not subclass or even duck-type this class. # the entity factory mediates all interaction between the GeoJSON # engine and features. - class Feature # Create a feature wrapping the given geometry, with the given ID # and properties. @@ -95,7 +94,6 @@ def keys # FeatureCollection implementation need not subclass or even # duck-type this class. The entity factory mediates all interaction # between the GeoJSON engine and feature collections. - class FeatureCollection include Enumerable diff --git a/lib/rgeo/geo_json/interface.rb b/lib/rgeo/geo_json/interface.rb index ba46da0..9883e03 100644 --- a/lib/rgeo/geo_json/interface.rb +++ b/lib/rgeo/geo_json/interface.rb @@ -1,6 +1,30 @@ # frozen_string_literal: true module RGeo + # `RGeo::GeoJSON` is a part of `RGeo` designed to decode GeoJSON into + # `RGeo::Feature::Geometry`, or encode `RGeo::Feature::Geometry` objects as + # GeoJSON. + # + # This implementation tries to stick to GeoJSON specifications, and may raise + # when trying to decode and invalid GeoJSON string. It may also raise if one + # tries to encode a feature that cannot be handled per GeoJSON spec. + # + # @example Basic usage + # require 'rgeo/geo_json' + # + # str1 = '{"type":"Point","coordinates":[1,2]}' + # geom = RGeo::GeoJSON.decode(str1) + # geom.as_text # => "POINT (1.0 2.0)" + # + # str2 = '{"type":"Feature","geometry":{"type":"Point","coordinates":[2.5,4.0]},"properties":{"color":"red"}}' + # feature = RGeo::GeoJSON.decode(str2) + # feature['color'] # => 'red' + # feature.geometry.as_text # => "POINT (2.5 4.0)" + # + # hash = RGeo::GeoJSON.encode(feature) + # hash.to_json == str2 # => true + # + # @see https://tools.ietf.org/html/rfc7946 module GeoJSON class << self # High-level convenience routine for encoding an object as GeoJSON. @@ -13,7 +37,6 @@ class << self # RGeo::GeoJSON::EntityFactory for more information. By default, # encode supports objects of type RGeo::GeoJSON::Feature and # RGeo::GeoJSON::FeatureCollection. - def encode(object, opts = {}) Coder.new(opts).encode(object) end diff --git a/test/basic_test.rb b/test/basic_test.rb index d8d508b..e2eacce 100644 --- a/test/basic_test.rb +++ b/test/basic_test.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true -require "minitest/autorun" -require "rgeo/geo_json" +require_relative "test_helper" class BasicTest < Minitest::Test # :nodoc: def setup - @geo_factory = RGeo::Cartesian.simple_factory(srid: 4326) + @geo_factory = RGeo::GeoJSON.coder.instance_variable_get(:@geo_factory) @geo_factory_z = RGeo::Cartesian.simple_factory(srid: 4326, has_z_coordinate: true) @geo_factory_m = RGeo::Cartesian.simple_factory(srid: 4326, has_m_coordinate: true) @geo_factory_zm = RGeo::Cartesian.simple_factory(srid: 4326, has_z_coordinate: true, has_m_coordinate: true) @@ -22,7 +21,7 @@ def test_nil end def test_decode_simple_point - json = %({"type":"Point","coordinates":[1,2]}) + json = '{"type":"Point","coordinates":[1,2]}' point = RGeo::GeoJSON.decode(json) assert_equal "POINT (1.0 2.0)", point.as_text end @@ -60,8 +59,8 @@ def test_point_m "type" => "Point", "coordinates" => [10.0, 20.0, -1.0], } - assert_equal(json, RGeo::GeoJSON.encode(object)) - assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_m).eql?(object)) + assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.encode(object) } + assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_m) } end def test_point_zm @@ -70,8 +69,8 @@ def test_point_zm "type" => "Point", "coordinates" => [10.0, 20.0, -1.0, -2.0], } - assert_equal(json, RGeo::GeoJSON.encode(object)) - assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_zm).eql?(object)) + assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.encode(object) } + assert_raises(RGeo::GeoJSON::Coder::Error) { RGeo::GeoJSON.decode(json, geo_factory: @geo_factory_zm) } end def test_line_string @@ -94,6 +93,22 @@ def test_polygon assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object)) end + def test_not_simple_polygon + coordinates = [[0, 0], [2, 2], [2, 0], [0, 2], [0, 0]] + object = @geo_factory.polygon( + @geo_factory.line_string(coordinates.map { |x, y| @geo_factory.point(x, y) }) + ) + json = { + "type" => "Polygon", + "coordinates" => [coordinates] + } + assert_equal(json, RGeo::GeoJSON.encode(object)) + assert( + RGeo::GeoJSON.decode(json).eql?(object), + "It should decodes with the uses_lenient_assertions param" + ) + end + def test_polygon_complex object = @geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(0, 0), @geo_factory.point(10, 0), @geo_factory.point(10, 10), @geo_factory.point(0, 10), @geo_factory.point(0, 0)]), [@geo_factory.linear_ring([@geo_factory.point(4, 4), @geo_factory.point(6, 5), @geo_factory.point(4, 6), @geo_factory.point(4, 4)])]) json = { diff --git a/test/coder/encode_geometry_test.rb b/test/coder/encode_geometry_test.rb new file mode 100644 index 0000000..583213a --- /dev/null +++ b/test/coder/encode_geometry_test.rb @@ -0,0 +1,147 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +class CoderTest < Minitest::Test # :nodoc: + include TestHelper + def setup + @geo_factory = RGeo::Cartesian.simple_factory(srid: 4326) + @entity_factory = RGeo::GeoJSON::EntityFactory.instance + @coder = RGeo::GeoJSON::Coder.new( + geo_factory: @geo_factory, + entity_factory: @entity_factory + ) + end + + def test_encode_geometry_point + point = rand_point + assert_equal( + { "type" => "Point", "coordinates" => [point.x, point.y] }, + @coder.send(:encode_geometry, point) + ) + end + + def test_encode_geometry_linestring + point1 = rand_point + point2 = rand_point + point3 = rand_point + line = @geo_factory.line_string([point1, point2, point3]) + assert_equal( + { "type" => "LineString", "coordinates" => [ + [point1.x, point1.y], [point2.x, point2.y], [point3.x, point3.y] + ] }, + @coder.send(:encode_geometry, line) + ) + end + + def test_encode_geometry_polygon + point1 = rand_point + point2 = rand_point + point3 = rand_point + exterior = @geo_factory.linear_ring([point1, point2, point3, point1]) + polygon = @geo_factory.polygon(exterior) + assert_equal( + { "type" => "Polygon", "coordinates" => [[ + [point1.x, point1.y], + [point2.x, point2.y], + [point3.x, point3.y], + [point1.x, point1.y] + ]] }, + @coder.send(:encode_geometry, polygon) + ) + end + + def test_encode_geometry_polygon_with_one_hole + point1 = @geo_factory.point(0.1, 0.2) + point2 = @geo_factory.point(0.1, 9.2) + point3 = @geo_factory.point(10.1, 9.2) + point4 = @geo_factory.point(10.1, 0.2) + point5 = @geo_factory.point(4.1, 4.2) + point6 = @geo_factory.point(5.1, 6.2) + point7 = @geo_factory.point(6.1, 4.2) + exterior = @geo_factory.linear_ring([point1, point2, point3, point4, point1]) + interior = @geo_factory.linear_ring([point5, point6, point7, point5]) + polygon = @geo_factory.polygon(exterior, [interior]) + assert_equal( + { "type" => "Polygon", "coordinates" => [ + [ + [point1.x, point1.y], + [point2.x, point2.y], + [point3.x, point3.y], + [point4.x, point4.y], + [point1.x, point1.y] + ], + [ + [point5.x, point5.y], + [point6.x, point6.y], + [point7.x, point7.y], + [point5.x, point5.y] + ] + ] }, + @coder.send(:encode_geometry, polygon) + ) + end + + def test_encode_geometry_multipoint + point1 = rand_point + point2 = rand_point + multipoint = @geo_factory.multi_point([point1, point2]) + assert_equal( + { "type" => "MultiPoint", "coordinates" => [ + [point1.x, point1.y], [point2.x, point2.y] + ] }, + @coder.send(:encode_geometry, multipoint) + ) + end + + def test_encode_geometry_multilinestring + linestring1 = rand_linestring(2) + linestring2 = rand_linestring(3) + multilinestring = @geo_factory.multi_line_string([linestring1, linestring2]) + assert_equal( + { "type" => "MultiLineString", "coordinates" => [ + linestring1.coordinates, + linestring2.coordinates + ] }, + @coder.send(:encode_geometry, multilinestring) + ) + end + + def test_encode_geometry_multipolygon + point1 = @geo_factory.point(0, 0) + point2 = @geo_factory.point(0, 10) + point3 = @geo_factory.point(10, 10) + point4 = @geo_factory.point(10, 0) + point5 = @geo_factory.point(4, 4) + point6 = @geo_factory.point(5, 6) + point7 = @geo_factory.point(6, 4) + point8 = @geo_factory.point(0, -10) + point9 = @geo_factory.point(-10, 0) + exterior1 = @geo_factory.linear_ring([point1, point8, point9, point1]) + exterior2 = @geo_factory.linear_ring([point1, point2, point3, point4, point1]) + interior2 = @geo_factory.linear_ring([point5, point6, point7, point5]) + exterior3 = @geo_factory.linear_ring([point1, point2, point3, point1]) + poly1 = @geo_factory.polygon(exterior1) + poly2 = @geo_factory.polygon(exterior2, [interior2]) + poly3 = @geo_factory.polygon(exterior3) + multypolygon = @geo_factory.multi_polygon([poly1, poly2, poly3]) + assert_equal( + { "type" => "MultiPolygon", "coordinates" => [ + [ # poly1 + exterior1.coordinates + ], + [ # poly2 + exterior2.coordinates, + interior2.coordinates + ], + [ # poly3 + exterior3.coordinates + ] + ] }, + @coder.send(:encode_geometry, multypolygon) + ) + end + + def test_encode_geometry_geometrycollection + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb new file mode 100644 index 0000000..f5a8bf6 --- /dev/null +++ b/test/test_helper.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require "minitest/autorun" +require_relative "../lib/rgeo-geojson" + +module TestHelper + def rand_lng_lat + [rand(-180.0..180.0).round(5), rand(-90.0..90.0).round(5)] + end + + def rand_point + @geo_factory.point(*rand_lng_lat) + end + + def rand_linestring(size = 2) + @geo_factory.line_string(Array.new(size) { rand_point }) + end +end From b9bc47a4f9346497d72bcadee572423e64df4224 Mon Sep 17 00:00:00 2001 From: Ulysse Buonomo Date: Mon, 9 Nov 2020 20:50:19 +0100 Subject: [PATCH 2/2] Enforce right-hand rule when encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before (no right-hand rule) ``` Warming up -------------------------------------- big geometry 32.000 i/100ms small geometry 10.379k i/100ms Calculating ------------------------------------- big geometry 343.166 (± 2.6%) i/s - 1.728k in 5.039023s small geometry 106.304k (± 2.1%) i/s - 539.708k in 5.079398s ``` After (right-hand rule) ``` Warming up -------------------------------------- big geometry 1.000 i/100ms small geometry 2.780k i/100ms Calculating ------------------------------------- big geometry 9.085 (± 0.0%) i/s - 46.000 in 5.069703s small geometry 28.541k (± 2.9%) i/s - 144.560k in 5.069153s ``` --- lib/rgeo/geo_json/coder.rb | 39 +++++++++++++++-- test/basic_test.rb | 67 ++++++++++++++++++++++++++---- test/coder/encode_geometry_test.rb | 24 +++++------ test/test_helper.rb | 4 ++ 4 files changed, 112 insertions(+), 22 deletions(-) diff --git a/lib/rgeo/geo_json/coder.rb b/lib/rgeo/geo_json/coder.rb index e1b6442..f1fdebb 100644 --- a/lib/rgeo/geo_json/coder.rb +++ b/lib/rgeo/geo_json/coder.rb @@ -146,14 +146,25 @@ def encode_geometry(object) case object when RGeo::Feature::Point, RGeo::Feature::LineString, - RGeo::Feature::Polygon, RGeo::Feature::MultiPoint, - RGeo::Feature::MultiLineString, - RGeo::Feature::MultiPolygon + RGeo::Feature::MultiLineString { "type" => object.geometry_type.type_name, "coordinates" => object.coordinates } + when RGeo::Feature::Polygon + { + "type" => "Polygon", + "coordinates" => right_hand_ruled_coordinates(object) + } + when RGeo::Feature::MultiPolygon + coordinates = Array.new(object.num_geometries) do |i| + right_hand_ruled_coordinates(object.geometry_n(i)) + end + { + "type" => "MultiPolygon", + "coordinates" => coordinates + } when RGeo::Feature::GeometryCollection { "type" => "GeometryCollection", @@ -164,6 +175,28 @@ def encode_geometry(object) end end + def right_hand_ruled_coordinates(polygon) + # Exterior should be ccw. + exterior = if clockwise?(polygon.exterior_ring) + polygon.exterior_ring.coordinates.reverse + else + polygon.exterior_ring.coordinates + end + + interiors = polygon.interior_rings.map do |ring| + # Interiors should be cw. + next ring.coordinates if clockwise?(ring) + + ring.coordinates.reverse + end + + [exterior, *interiors] + end + + def clockwise?(ring) + RGeo::Cartesian::Analysis.ring_direction(ring) == -1 + end + def decode_feature(input) geometry = input["geometry"] if geometry diff --git a/test/basic_test.rb b/test/basic_test.rb index e2eacce..9ac33bc 100644 --- a/test/basic_test.rb +++ b/test/basic_test.rb @@ -3,6 +3,8 @@ require_relative "test_helper" class BasicTest < Minitest::Test # :nodoc: + include TestHelper + def setup @geo_factory = RGeo::GeoJSON.coder.instance_variable_get(:@geo_factory) @geo_factory_z = RGeo::Cartesian.simple_factory(srid: 4326, has_z_coordinate: true) @@ -95,9 +97,7 @@ def test_polygon def test_not_simple_polygon coordinates = [[0, 0], [2, 2], [2, 0], [0, 2], [0, 0]] - object = @geo_factory.polygon( - @geo_factory.line_string(coordinates.map { |x, y| @geo_factory.point(x, y) }) - ) + object = @geo_factory.polygon(line_string(coordinates)) json = { "type" => "Polygon", "coordinates" => [coordinates] @@ -110,10 +110,15 @@ def test_not_simple_polygon end def test_polygon_complex - object = @geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(0, 0), @geo_factory.point(10, 0), @geo_factory.point(10, 10), @geo_factory.point(0, 10), @geo_factory.point(0, 0)]), [@geo_factory.linear_ring([@geo_factory.point(4, 4), @geo_factory.point(6, 5), @geo_factory.point(4, 6), @geo_factory.point(4, 4)])]) + exterior = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]] + interior = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]] + object = @geo_factory.polygon( + line_string(exterior), + [line_string(interior)] + ) json = { "type" => "Polygon", - "coordinates" => [[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [6.0, 5.0], [4.0, 6.0], [4.0, 4.0]]], + "coordinates" => [[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]]] } assert_equal(json, RGeo::GeoJSON.encode(object)) assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object)) @@ -140,10 +145,23 @@ def test_multi_line_string end def test_multi_polygon - object = @geo_factory.multi_polygon([@geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(0, 0), @geo_factory.point(10, 0), @geo_factory.point(10, 10), @geo_factory.point(0, 10), @geo_factory.point(0, 0)]), [@geo_factory.linear_ring([@geo_factory.point(4, 4), @geo_factory.point(6, 5), @geo_factory.point(4, 6), @geo_factory.point(4, 4)])]), @geo_factory.polygon(@geo_factory.linear_ring([@geo_factory.point(-10, -10), @geo_factory.point(-15, -10), @geo_factory.point(-10, -15), @geo_factory.point(-10, -10)]))]) + exterior1 = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]] + interior1 = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]] + exterior2 = [[-10.0, -10.0], [-15.0, -10.0], [-10.0, -15.0], [-10.0, -10.0]] + polygon1 = @geo_factory.polygon( + line_string(exterior1), + [line_string(interior1)] + ) + polygon2 = @geo_factory.polygon( + line_string(exterior2) + ) + object = @geo_factory.multi_polygon([polygon1, polygon2]) json = { "type" => "MultiPolygon", - "coordinates" => [[[[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]], [[4.0, 4.0], [6.0, 5.0], [4.0, 6.0], [4.0, 4.0]]], [[[-10.0, -10.0], [-15.0, -10.0], [-10.0, -15.0], [-10.0, -10.0]]]] + "coordinates" => [ + [exterior1, interior1], + [exterior2] + ] } assert_equal(json, RGeo::GeoJSON.encode(object)) assert(RGeo::GeoJSON.decode(json, geo_factory: @geo_factory).eql?(object)) @@ -273,4 +291,39 @@ def test_feature_property assert_equal "b", feature.properties["a"] assert_equal "b", feature["a"] end + + def test_right_hand_rule + ccw_exterior = [[0.0, 0.0], [10.0, 0.0], [10.0, 10.0], [0.0, 10.0], [0.0, 0.0]] + cw_interior = [[4.0, 4.0], [4.0, 6.0], [6.0, 5.0], [4.0, 4.0]] + + json = { "type" => "Polygon", "coordinates" => [ + ccw_exterior, + cw_interior + ] } + + bad_interior = @geo_factory.polygon( + line_string(ccw_exterior), + [line_string(cw_interior.reverse)] + ) + bad_exterior = @geo_factory.polygon( + line_string(ccw_exterior.reverse), + [line_string(cw_interior)] + ) + bad_both = @geo_factory.polygon( + line_string(ccw_exterior.reverse), + [line_string(cw_interior.reverse)] + ) + [bad_exterior, bad_interior, bad_both].each do |polygon| + assert_equal(json, RGeo::GeoJSON.encode(polygon)) + end + + multi_polygon = @geo_factory.multi_polygon( + [bad_both, bad_exterior, bad_interior] + ) + assert_equal({ "type" => "MultiPolygon", "coordinates" => [ + [ccw_exterior, cw_interior], + [ccw_exterior, cw_interior], + [ccw_exterior, cw_interior] + ] }, RGeo::GeoJSON.encode(multi_polygon)) + end end diff --git a/test/coder/encode_geometry_test.rb b/test/coder/encode_geometry_test.rb index 583213a..c82795a 100644 --- a/test/coder/encode_geometry_test.rb +++ b/test/coder/encode_geometry_test.rb @@ -35,9 +35,9 @@ def test_encode_geometry_linestring end def test_encode_geometry_polygon - point1 = rand_point - point2 = rand_point - point3 = rand_point + point1 = @geo_factory.point(1.0, 2.0) + point2 = @geo_factory.point(3.4, 2.0) + point3 = @geo_factory.point(1.0, 3.0) exterior = @geo_factory.linear_ring([point1, point2, point3, point1]) polygon = @geo_factory.polygon(exterior) assert_equal( @@ -52,13 +52,13 @@ def test_encode_geometry_polygon end def test_encode_geometry_polygon_with_one_hole - point1 = @geo_factory.point(0.1, 0.2) - point2 = @geo_factory.point(0.1, 9.2) - point3 = @geo_factory.point(10.1, 9.2) + point1 = @geo_factory.point(6.1, 4.2) + point2 = @geo_factory.point(5.1, 6.2) point4 = @geo_factory.point(10.1, 0.2) - point5 = @geo_factory.point(4.1, 4.2) - point6 = @geo_factory.point(5.1, 6.2) - point7 = @geo_factory.point(6.1, 4.2) + point3 = @geo_factory.point(4.1, 4.2) + point5 = @geo_factory.point(10.1, 9.2) + point6 = @geo_factory.point(0.1, 0.2) + point7 = @geo_factory.point(0.1, 9.2) exterior = @geo_factory.linear_ring([point1, point2, point3, point4, point1]) interior = @geo_factory.linear_ring([point5, point6, point7, point5]) polygon = @geo_factory.polygon(exterior, [interior]) @@ -117,10 +117,10 @@ def test_encode_geometry_multipolygon point7 = @geo_factory.point(6, 4) point8 = @geo_factory.point(0, -10) point9 = @geo_factory.point(-10, 0) - exterior1 = @geo_factory.linear_ring([point1, point8, point9, point1]) - exterior2 = @geo_factory.linear_ring([point1, point2, point3, point4, point1]) + exterior1 = @geo_factory.linear_ring([point1, point9, point8, point1]) + exterior2 = @geo_factory.linear_ring([point1, point4, point3, point2, point1]) interior2 = @geo_factory.linear_ring([point5, point6, point7, point5]) - exterior3 = @geo_factory.linear_ring([point1, point2, point3, point1]) + exterior3 = @geo_factory.linear_ring([point1, point3, point2, point1]) poly1 = @geo_factory.polygon(exterior1) poly2 = @geo_factory.polygon(exterior2, [interior2]) poly3 = @geo_factory.polygon(exterior3) diff --git a/test/test_helper.rb b/test/test_helper.rb index f5a8bf6..e4081d3 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -15,4 +15,8 @@ def rand_point def rand_linestring(size = 2) @geo_factory.line_string(Array.new(size) { rand_point }) end + + def line_string(array) + @geo_factory.line_string(array.map { |x,y| @geo_factory.point(x, y) }) + end end