From c5516269167744fb65c08757335d5c2a54ad9b27 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 21 Dec 2011 12:22:05 -0800 Subject: [PATCH 1/3] finish geometry cleanup work by ensuring proper behavior in the ogr plugin with multigeometries --- bindings/python/mapnik/__init__.py | 5 - plugins/input/geos/geos_featureset.hpp | 1 - plugins/input/ogr/ogr_converter.cpp | 143 +++------------------ plugins/input/ogr/ogr_converter.hpp | 7 +- plugins/input/ogr/ogr_datasource.cpp | 18 ++- plugins/input/ogr/ogr_datasource.hpp | 1 - plugins/input/ogr/ogr_featureset.cpp | 10 +- plugins/input/ogr/ogr_featureset.hpp | 7 +- plugins/input/ogr/ogr_index_featureset.cpp | 8 +- plugins/input/ogr/ogr_index_featureset.hpp | 4 +- tests/data/good_maps/line_symbolizer2.xml | 1 - 11 files changed, 34 insertions(+), 171 deletions(-) diff --git a/bindings/python/mapnik/__init__.py b/bindings/python/mapnik/__init__.py index 49744a53e..17ccb0686 100644 --- a/bindings/python/mapnik/__init__.py +++ b/bindings/python/mapnik/__init__.py @@ -381,7 +381,6 @@ def PostGIS(**keywords): srid -- specify srid to use (default: auto-detected from geometry_field) row_limit -- integer limit of rows to return (default: 0) cursor_size -- integer size of binary cursor to use (default: 0, no binary cursor is used) - multiple_geometries -- boolean, direct the Mapnik wkb reader to interpret as multigeometries (default False) >>> from mapnik import PostGIS, Layer >>> params = dict(dbname='mapnik',table='osm',user='postgres',password='gis') @@ -467,7 +466,6 @@ def Occi(**keywords): encoding -- file encoding (default 'utf-8') geometry_field -- specify geometry field (default 'GEOLOC') use_spatial_index -- boolean, force the use of the spatial index (default True) - multiple_geometries -- boolean, direct the Mapnik wkb reader to interpret as multigeometries (default False) >>> from mapnik import Occi, Layer >>> params = dict(host='myoracle',user='scott',password='tiger',table='test') @@ -492,7 +490,6 @@ def Ogr(**keywords): layer_by_sql -- choose layer by sql query number instead of by layer name or index. base -- path prefix (default None) encoding -- file encoding (default 'utf-8') - multiple_geometries -- boolean, direct the Mapnik wkb reader to interpret as multigeometries (default False) >>> from mapnik import Ogr, Layer >>> datasource = Ogr(base='/home/mapnik/data',file='rivers.geojson',layer='OGRGeoJSON') @@ -520,7 +517,6 @@ def SQLite(**keywords): row_offset -- specify a custom integer row offset (default 0) row_limit -- specify a custom integer row limit (default 0) wkb_format -- specify a wkb type of 'spatialite' (default None) - multiple_geometries -- boolean, direct the Mapnik wkb reader to interpret as multigeometries (default False) use_spatial_index -- boolean, instruct sqlite plugin to use Rtree spatial index (default True) >>> from mapnik import SQLite, Layer @@ -601,7 +597,6 @@ def Geos(**keywords): wkt -- inline WKT text of the geometry Optional keyword arguments: - multiple_geometries -- boolean, direct the GEOS wkt reader to interpret as multigeometries (default False) extent -- manually specified data extent (comma delimited string, default None) >>> from mapnik import Geos, Layer diff --git a/plugins/input/geos/geos_featureset.hpp b/plugins/input/geos/geos_featureset.hpp index d68352329..713f9ff4c 100644 --- a/plugins/input/geos/geos_featureset.hpp +++ b/plugins/input/geos/geos_featureset.hpp @@ -55,7 +55,6 @@ private: int identifier_; std::string field_; std::string field_name_; - bool multiple_geometries_; bool already_rendered_; geos_featureset(const geos_featureset&); diff --git a/plugins/input/ogr/ogr_converter.cpp b/plugins/input/ogr/ogr_converter.cpp index 4188e1fab..59f1b2095 100644 --- a/plugins/input/ogr/ogr_converter.cpp +++ b/plugins/input/ogr/ogr_converter.cpp @@ -36,57 +36,34 @@ using mapnik::feature_ptr; using mapnik::geometry_utils; using mapnik::geometry_type; -void ogr_converter::convert_geometry(OGRGeometry* geom, feature_ptr feature, bool multiple_geometries) +void ogr_converter::convert_geometry(OGRGeometry* geom, feature_ptr feature) { + // NOTE: wkbFlatten macro in ogr flattens 2.5d types into base 2d type switch (wkbFlatten(geom->getGeometryType())) { case wkbPoint: convert_point(static_cast(geom), feature); break; + case wkbMultiPoint: + convert_multipoint(static_cast(geom), feature); + break; + case wkbLinearRing: + convert_linestring(static_cast(geom), feature); + break; case wkbLineString: convert_linestring(static_cast(geom), feature); break; + case wkbMultiLineString: + convert_multilinestring(static_cast(geom), feature); + break; case wkbPolygon: convert_polygon(static_cast(geom), feature); break; - case wkbMultiPoint: - if (multiple_geometries) - { - convert_multipoint_2(static_cast(geom), feature); - } - else - { - // Todo - using convert_multipoint_2 until we have proper multipoint handling in convert_multipoint - // http://trac.mapnik.org/ticket/458 - //convert_multipoint(static_cast(geom), feature); - convert_multipoint_2(static_cast(geom), feature); - } - break; - case wkbMultiLineString: - if (multiple_geometries) - { - convert_multilinestring_2(static_cast(geom), feature); - } - else - { - convert_multilinestring(static_cast(geom), feature); - } - break; case wkbMultiPolygon: - if (multiple_geometries) - { - convert_multipolygon_2(static_cast(geom), feature); - } - else - { - convert_multipolygon(static_cast(geom), feature); - } + convert_multipolygon(static_cast(geom), feature); break; case wkbGeometryCollection: - convert_collection(static_cast(geom), feature, multiple_geometries); - break; - case wkbLinearRing: - convert_linestring(static_cast(geom), feature); + convert_collection(static_cast(geom), feature); break; case wkbNone: case wkbUnknown: @@ -151,22 +128,6 @@ void ogr_converter::convert_polygon(OGRPolygon* geom, feature_ptr feature) } void ogr_converter::convert_multipoint(OGRMultiPoint* geom, feature_ptr feature) -{ - int num_geometries = geom->getNumGeometries(); - geometry_type* point = new geometry_type(mapnik::Point); - - for (int i = 0; i < num_geometries; i++) - { - OGRPoint* ogrpoint = static_cast(geom->getGeometryRef(i)); - point->move_to(ogrpoint->getX(), ogrpoint->getY()); - //Todo - need to accept multiple points per mapnik::Point - } - - // Todo - this only gets last point - feature->add_geometry(point); -} - -void ogr_converter::convert_multipoint_2(OGRMultiPoint* geom, feature_ptr feature) { int num_geometries = geom->getNumGeometries(); for (int i = 0; i < num_geometries; i++) @@ -176,34 +137,6 @@ void ogr_converter::convert_multipoint_2(OGRMultiPoint* geom, feature_ptr featur } void ogr_converter::convert_multilinestring(OGRMultiLineString* geom, feature_ptr feature) -{ - int num_geometries = geom->getNumGeometries(); - - int num_points = 0; - for (int i = 0; i < num_geometries; i++) - { - OGRLineString* ls = static_cast(geom->getGeometryRef(i)); - num_points += ls->getNumPoints(); - } - - geometry_type* line = new geometry_type(mapnik::LineString); - line->set_capacity(num_points); - - for (int i = 0; i < num_geometries; i++) - { - OGRLineString* ls = static_cast(geom->getGeometryRef(i)); - num_points = ls->getNumPoints(); - line->move_to(ls->getX(0), ls->getY(0)); - for (int i = 1; i < num_points; ++i) - { - line->line_to(ls->getX(i), ls->getY(i)); - } - } - - feature->add_geometry(line); -} - -void ogr_converter::convert_multilinestring_2(OGRMultiLineString* geom, feature_ptr feature) { int num_geometries = geom->getNumGeometries(); for (int i = 0; i < num_geometries; i++) @@ -213,52 +146,6 @@ void ogr_converter::convert_multilinestring_2(OGRMultiLineString* geom, feature_ } void ogr_converter::convert_multipolygon(OGRMultiPolygon* geom, feature_ptr feature) -{ - int num_geometries = geom->getNumGeometries(); - - int capacity = 0; - for (int i = 0; i < num_geometries; i++) - { - OGRPolygon* p = static_cast(geom->getGeometryRef(i)); - OGRLinearRing* exterior = p->getExteriorRing(); - capacity += exterior->getNumPoints(); - for (int r = 0; r < p->getNumInteriorRings(); r++) - { - OGRLinearRing* interior = p->getInteriorRing(r); - capacity += interior->getNumPoints(); - } - } - - geometry_type* poly = new geometry_type(mapnik::Polygon); - poly->set_capacity(capacity); - - for (int i = 0; i < num_geometries; i++) - { - OGRPolygon* p = static_cast(geom->getGeometryRef(i)); - OGRLinearRing* exterior = p->getExteriorRing(); - int num_points = exterior->getNumPoints(); - int num_interior = p->getNumInteriorRings(); - poly->move_to(exterior->getX(0), exterior->getY(0)); - for (int i = 1; i < num_points; ++i) - { - poly->line_to(exterior->getX(i), exterior->getY(i)); - } - for (int r = 0; r < num_interior; r++) - { - OGRLinearRing* interior = p->getInteriorRing(r); - num_points = interior->getNumPoints(); - poly->move_to(interior->getX(0), interior->getY(0)); - for (int i = 1; i < num_points; ++i) - { - poly->line_to(interior->getX(i), interior->getY(i)); - } - } - } - - feature->add_geometry(poly); -} - -void ogr_converter::convert_multipolygon_2(OGRMultiPolygon* geom, feature_ptr feature) { int num_geometries = geom->getNumGeometries(); for (int i = 0; i < num_geometries; i++) @@ -267,7 +154,7 @@ void ogr_converter::convert_multipolygon_2(OGRMultiPolygon* geom, feature_ptr fe } } -void ogr_converter::convert_collection(OGRGeometryCollection* geom, feature_ptr feature, bool multiple_geometries) +void ogr_converter::convert_collection(OGRGeometryCollection* geom, feature_ptr feature) { int num_geometries = geom->getNumGeometries(); for (int i = 0; i < num_geometries; i++) @@ -275,7 +162,7 @@ void ogr_converter::convert_collection(OGRGeometryCollection* geom, feature_ptr OGRGeometry* g = geom->getGeometryRef(i); if (g != NULL) { - convert_geometry(g, feature, multiple_geometries); + convert_geometry(g, feature); } } } diff --git a/plugins/input/ogr/ogr_converter.hpp b/plugins/input/ogr/ogr_converter.hpp index 0871fce6d..94b198d54 100644 --- a/plugins/input/ogr/ogr_converter.hpp +++ b/plugins/input/ogr/ogr_converter.hpp @@ -33,17 +33,14 @@ class ogr_converter { public: - static void convert_geometry (OGRGeometry* geom, mapnik::feature_ptr feature, bool multiple_geometries); - static void convert_collection (OGRGeometryCollection* geom, mapnik::feature_ptr feature, bool multiple_geometries); + static void convert_geometry (OGRGeometry* geom, mapnik::feature_ptr feature); + static void convert_collection (OGRGeometryCollection* geom, mapnik::feature_ptr feature); static void convert_point (OGRPoint* geom, mapnik::feature_ptr feature); static void convert_linestring (OGRLineString* geom, mapnik::feature_ptr feature); static void convert_polygon (OGRPolygon* geom, mapnik::feature_ptr feature); static void convert_multipoint (OGRMultiPoint* geom, mapnik::feature_ptr feature); - static void convert_multipoint_2 (OGRMultiPoint* geom, mapnik::feature_ptr feature); static void convert_multilinestring (OGRMultiLineString* geom, mapnik::feature_ptr feature); - static void convert_multilinestring_2 (OGRMultiLineString* geom, mapnik::feature_ptr feature); static void convert_multipolygon (OGRMultiPolygon* geom, mapnik::feature_ptr feature); - static void convert_multipolygon_2 (OGRMultiPolygon* geom, mapnik::feature_ptr feature); }; #endif // OGR_CONVERTER_HPP diff --git a/plugins/input/ogr/ogr_datasource.cpp b/plugins/input/ogr/ogr_datasource.cpp index 7fbca6486..230244655 100644 --- a/plugins/input/ogr/ogr_datasource.cpp +++ b/plugins/input/ogr/ogr_datasource.cpp @@ -66,8 +66,6 @@ ogr_datasource::ogr_datasource(parameters const& params, bool bind) throw datasource_exception("missing or parameter"); } - multiple_geometries_ = *params.get("multiple_geometries", false); - if (string) { dataset_name_ = *string; @@ -403,16 +401,16 @@ featureset_ptr ogr_datasource::features(query const& q) const *layer, filter, index_name_, - desc_.get_encoding(), - multiple_geometries_)); + desc_.get_encoding() + )); } else { return featureset_ptr(new ogr_featureset (*dataset_, *layer, q.get_bbox(), - desc_.get_encoding(), - multiple_geometries_)); + desc_.get_encoding() + )); } } @@ -435,8 +433,8 @@ featureset_ptr ogr_datasource::features_at_point(coord2d const& pt) const *layer, filter, index_name_, - desc_.get_encoding(), - multiple_geometries_)); + desc_.get_encoding() + )); } else { @@ -447,8 +445,8 @@ featureset_ptr ogr_datasource::features_at_point(coord2d const& pt) const return featureset_ptr(new ogr_featureset (*dataset_, *layer, point, - desc_.get_encoding(), - multiple_geometries_)); + desc_.get_encoding() + )); } } diff --git a/plugins/input/ogr/ogr_datasource.hpp b/plugins/input/ogr/ogr_datasource.hpp index 25612f173..694af5d7f 100644 --- a/plugins/input/ogr/ogr_datasource.hpp +++ b/plugins/input/ogr/ogr_datasource.hpp @@ -58,7 +58,6 @@ private: mutable ogr_layer_ptr layer_; mutable std::string layer_name_; mutable mapnik::layer_descriptor desc_; - bool multiple_geometries_; mutable bool indexed_; }; diff --git a/plugins/input/ogr/ogr_featureset.cpp b/plugins/input/ogr/ogr_featureset.cpp index 9f3efc05b..2e916a4bb 100644 --- a/plugins/input/ogr/ogr_featureset.cpp +++ b/plugins/input/ogr/ogr_featureset.cpp @@ -49,14 +49,12 @@ using mapnik::feature_factory; ogr_featureset::ogr_featureset(OGRDataSource & dataset, OGRLayer & layer, OGRGeometry & extent, - const std::string& encoding, - const bool multiple_geometries) + const std::string& encoding) : dataset_(dataset), layer_(layer), layerdef_(layer.GetLayerDefn()), tr_(new transcoder(encoding)), fidcolumn_(layer_.GetFIDColumn ()), - multiple_geometries_(multiple_geometries), count_(0) { layer_.SetSpatialFilter (&extent); @@ -65,14 +63,12 @@ ogr_featureset::ogr_featureset(OGRDataSource & dataset, ogr_featureset::ogr_featureset(OGRDataSource & dataset, OGRLayer & layer, const mapnik::box2d & extent, - const std::string& encoding, - const bool multiple_geometries) + const std::string& encoding) : dataset_(dataset), layer_(layer), layerdef_(layer.GetLayerDefn()), tr_(new transcoder(encoding)), fidcolumn_(layer_.GetFIDColumn()), - multiple_geometries_(multiple_geometries), count_(0) { layer_.SetSpatialFilterRect (extent.minx(), @@ -99,7 +95,7 @@ feature_ptr ogr_featureset::next() OGRGeometry* geom = (*feat)->GetGeometryRef(); if (geom && ! geom->IsEmpty()) { - ogr_converter::convert_geometry(geom, feature, multiple_geometries_); + ogr_converter::convert_geometry(geom, feature); } #ifdef MAPNIK_DEBUG else diff --git a/plugins/input/ogr/ogr_featureset.hpp b/plugins/input/ogr/ogr_featureset.hpp index 0f4fbeb1c..7e30a1ab1 100644 --- a/plugins/input/ogr/ogr_featureset.hpp +++ b/plugins/input/ogr/ogr_featureset.hpp @@ -40,14 +40,12 @@ public: ogr_featureset(OGRDataSource & dataset, OGRLayer & layer, OGRGeometry & extent, - const std::string& encoding, - const bool multiple_geometries); + const std::string& encoding); ogr_featureset(OGRDataSource & dataset, OGRLayer & layer, const mapnik::box2d & extent, - const std::string& encoding, - const bool multiple_geometries); + const std::string& encoding); virtual ~ogr_featureset(); mapnik::feature_ptr next(); @@ -60,7 +58,6 @@ private: OGRFeatureDefn* layerdef_; boost::scoped_ptr tr_; const char* fidcolumn_; - bool multiple_geometries_; mutable int count_; }; diff --git a/plugins/input/ogr/ogr_index_featureset.cpp b/plugins/input/ogr/ogr_index_featureset.cpp index 2bace05bb..399a0aa65 100644 --- a/plugins/input/ogr/ogr_index_featureset.cpp +++ b/plugins/input/ogr/ogr_index_featureset.cpp @@ -56,15 +56,13 @@ ogr_index_featureset::ogr_index_featureset(OGRDataSource & dataset, OGRLayer & layer, const filterT& filter, const std::string& index_file, - const std::string& encoding, - const bool multiple_geometries) + const std::string& encoding) : dataset_(dataset), layer_(layer), layerdef_(layer.GetLayerDefn()), filter_(filter), tr_(new transcoder(encoding)), - fidcolumn_(layer_.GetFIDColumn()), - multiple_geometries_(multiple_geometries) + fidcolumn_(layer_.GetFIDColumn()) { boost::optional memory = mapnik::mapped_memory_cache::find(index_file.c_str(),true); @@ -108,7 +106,7 @@ feature_ptr ogr_index_featureset::next() OGRGeometry* geom=(*feat)->GetGeometryRef(); if (geom && !geom->IsEmpty()) { - ogr_converter::convert_geometry (geom, feature, multiple_geometries_); + ogr_converter::convert_geometry (geom, feature); } #ifdef MAPNIK_DEBUG else diff --git a/plugins/input/ogr/ogr_index_featureset.hpp b/plugins/input/ogr/ogr_index_featureset.hpp index 03ab3626c..1d76f914b 100644 --- a/plugins/input/ogr/ogr_index_featureset.hpp +++ b/plugins/input/ogr/ogr_index_featureset.hpp @@ -36,8 +36,7 @@ public: OGRLayer& layer, const filterT& filter, const std::string& index_file, - const std::string& encoding, - const bool multiple_geometries); + const std::string& encoding); virtual ~ogr_index_featureset(); mapnik::feature_ptr next(); @@ -53,7 +52,6 @@ private: std::vector::iterator itr_; boost::scoped_ptr tr_; const char* fidcolumn_; - bool multiple_geometries_; }; #endif // OGR_INDEX_FEATURESET_HPP diff --git a/tests/data/good_maps/line_symbolizer2.xml b/tests/data/good_maps/line_symbolizer2.xml index 1f0430aa5..7133720fa 100644 --- a/tests/data/good_maps/line_symbolizer2.xml +++ b/tests/data/good_maps/line_symbolizer2.xml @@ -3,7 +3,6 @@ sqlite ../sqlite/qgis_spatiallite.sqlite - true