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

France Departments winding order incompatible with Vega #231

Open
nickpeihl opened this issue Oct 26, 2021 · 5 comments
Open

France Departments winding order incompatible with Vega #231

nickpeihl opened this issue Oct 26, 2021 · 5 comments
Labels
data Data related issues and requests

Comments

@nickpeihl
Copy link
Member

It looks like the coordinate winding order for France Departments is not compatible with Vega. This causes Vega to fill the entire area outside the polygon instead of the internal polygon.

Other layers in EMS such as China Provinces render correctly because the coordinate winding order is reversed. To see this change the data[0].url.name to China Provinces in the example code below.

I'm not sure if there is a way we can fix this in Vega or if we need to fix this in EMS. I'd rather see if we could fix this in Vega and just maintain the RFC7946 winding order for all EMS layers. But I also think Elastic Maps is more lenient on winding order. And GeoJSON Upload also appears to wind correctly the coordinate order.

If we fix this in EMS, we also need to check which other layers may cause winding issues with Vega.

Example Vega code

{
  $schema: https://vega.github.io/schema/vega/v3.0.json
  config: {
    kibana: {
      type: map
      latitude: 48
      longitude: -5
      zoom: 3
    }
  }
  data: [
    {
      name: countries
      url: {%type%: "emsfile", name: "Spain Provinces"}
      format: {type: "json", property: "features"}
    }
  ]
  marks: [
    {
      type: shape
      from: { data: "countries" }
      encode: {
        "enter": {
          "strokeWidth": {"value": 0.5},
          "stroke": {"value": "#bbb"},
          "fill": {"value": "#e5e8d3"}
        }
      }
      transform: [
        {type: "geoshape", projection: "projection"}
      ]
    }
  ]
}

@jsanz jsanz added the data Data related issues and requests label Nov 9, 2021
@jsanz
Copy link
Member

jsanz commented Nov 9, 2021

We should make this a test to find any other dataset that is breaking in Vega.

@jsanz
Copy link
Member

jsanz commented Dec 2, 2021

I've been investigating this a bit more.

With geojsonhint I've checked and 61 of our current datasets have one or more warnings about the right-hand rule:

  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule

As Elastic Maps and Landing Page, they render correctly in Vega as well. Just to take Kibana out of the equation you can test this code in the Vega Editor for Bolivia departments:

{
  "$schema": "https://vega.github.io/schema/vega-lite/v3.json",
  "width": 500,
  "height": 300,
  "data": {
    "url": "https://vector.maps.elastic.co/files/bolivia_departments_v1.geo.json?elastic_tile_service_tos=agree&my_app_name=ems-landing-page&my_app_version=7.11.0&license=643c1faf-80fc-4ab0-9323-4d9bd11f4bbc",
    "format": {"property": "features"}
  },
  "transform": [
    
  ],
  "projection": {"type": "mercator"},
  "mark": "geoshape"
}

And this file has warnings when validating it:

dist/vector/files on  master
➜ geojsonhint --format pretty bolivia_departments_v1.geo.json
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule
  1:1  warning  Polygons and MultiPolygons should follow the right-hand rule

⚠ 9 warnings

Checking the previous version of the France departments there's a big wrong polygon in the South. geojsonhint only complains about the right-hand rule but maybe this is combined with something else.

image

I will continue debugging the file when I'm back from PTO.

@jsanz
Copy link
Member

jsanz commented Dec 22, 2021

OK, I did a few more tests and the issue is as @nickpeihl stated the conformity of the France departments GeoJSON file to the RFC 7946 spec. This was already discussed in detail in 2018 on this issue with @thomasneirynck and @nyurik. I'm sorry I did not find that issue before 😞

I will explore adding a step in the build process to enforce a winding order with geojson-rewind, ensuring all our published GeoJSON files are consistent, one way or the other.

Still, the main point of the discussion remains. I'm more inclined to ensure all our GeoJSON files follow the RFC 7946 standard, finding a way for our Vega users to get them properly rendered by d3-geo.

@jsanz
Copy link
Member

jsanz commented Jan 5, 2022

After sync discussion with @nickpeihl and @thomasneirynck we agreed that EMS data needs to follow industry standards so we need to

@nyurik
Copy link
Contributor

nyurik commented Jan 5, 2022

I'm all for using "standards" (even when standards were created after an established industry practice, and contradicts it). My only concern is that we must not break existing users -- once we introduce an automatic rewinding for Vega users (should be fairly easy to do), we should introduce a new version of the EMS file service that serves different set of files (same files but different winding order). This way existing users will continue using things as is, and eventually all will migrate to the new service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data Data related issues and requests
Projects
None yet
Development

No branches or pull requests

3 participants