Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polygon filling does not preserve holes #450

Closed
artemp opened this issue Oct 11, 2011 · 6 comments
Closed

Polygon filling does not preserve holes #450

artemp opened this issue Oct 11, 2011 · 6 comments
Milestone

Comments

@artemp
Copy link
Member

artemp commented Oct 11, 2011

Testcase attached, includes actual and expected output and sample data/xml.

Mapnik does not preserve polygons or multi-polygons with holes in all cases. Rather it appears that Mapnik's AGG and Cairo renderers fill holes, which would not be the expected behavior.

Resources:
http://www.cairographics.org/manual/cairo-context.html#cairo-fill-rule-t
http://geojson.org/geojson-spec.html#id4

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] Results using data with hole from: http://geojson.org/geojson-spec.html#id4

[[BR]]
[[Image(actual.png)]]

[[Image(expected.png)]]

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] The latter image (expected.png) can be had using both renderers using this patch:

Index: src/cairo_renderer.cpp
===================================================================
--- src/cairo_renderer.cpp      (revision 1360)
+++ src/cairo_renderer.cpp      (working copy)
@@ -324,6 +324,16 @@
             context_->fill();
          }

+         void fill_preserve(void)
+         {
+            context_->fill_preserve();
+         }
+
+         void set_fill_rule(Cairo::FillRule rule)
+         {
+            context_->set_fill_rule(rule);
+         }
+         
          void paint(void)
          {
             context_->paint();
@@ -564,7 +574,8 @@
       cairo_context context(context_);

       context.set_color(sym.get_fill(), sym.get_opacity());
-
+      context.set_fill_rule(Cairo::FILL_RULE_EVEN_ODD);
+      
       for (unsigned i = 0; i < feature.num_geometries(); ++i)
       {
          geometry2d const& geom = feature.get_geometry(i);
Index: src/agg_renderer.cpp
===================================================================
--- src/agg_renderer.cpp        (revision 1360)
+++ src/agg_renderer.cpp        (working copy)
@@ -188,6 +188,7 @@
       renderer ren(renb);

       ras_ptr->reset();
+      ras_ptr->filling_rule(agg::fill_even_odd);

       for (unsigned i=0;i<feature.num_geometries();++i)
       {

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[jburgess777] I would guess that we don't see this with OSM data because osm2pgsql uses the geos normalize() function to ensure the direction of all the rings of the polygon is correct.

I have to wonder why both Agg and Cairo default to the winding mode and if this indicates that there are other circumstances when the odd/even rule fails.

To test one theory I created the following multipolygon with two intersecting squares:

{ "type": "MultiPolygon",
  "coordinates": [
    [[[101.0, 1.0], [104.0, 1.0], [104.0, 4.0], [101.0, 4.0], [101.0, 1.0]]],
    [[[100.0, 0.0], [103.0, 0.0], [103.0, 3.0], [100.0, 3.0], [100.0, 0.0]]]
    ]
}

Before the patch this renders as two completely filled squares. Afterwards the area of intersection is no longer filled. I would argue that these represent two distinct squares that should both be filled completely even if they overlap.

Perhaps mapnik needs to normalize the polygon ring directions like osm2pgsql does?

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] Based on IRC discussion it appears that this GeoJSON example may not be valid (or at least only with this shape does the even/odd rule have a positive effect), that globally using even/odd produces missing polygons in some sample shapefile data, and also creates rendering artifacts in the sample world borders data (attached).

Therefore, closing as wontfix.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] Note the sample data that looses polygons was actually AVCBIN files read using OGR.

@artemp
Copy link
Member Author

artemp commented Oct 11, 2011

[springmeyer] result with patch on the left (one reason we won't use):

[[BR]]
[[Image(even_odd_corruption.png)]]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant