CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.92k stars 3.48k forks source link

WMS GetFeatureInfo incorrect point position #9363

Open zoran995 opened 3 years ago

zoran995 commented 3 years ago

Sandcastle example

Browser: All

Operating System: All

Reported also: https://github.com/TerriaJS/terriajs/issues/5057

When using WMS in a WebMercatorProjection instead of GeographicProjection user will get the wrong feature position in response. The issue seems to be coming from https://github.com/CesiumGS/cesium/blob/ca8b918f167421ff49966f5951ea0fc4446c1521/Source/Scene/GetFeatureInfoFormat.js#L88-L92 which incorrectly assumes that point coordinate is in GeographicProjection. Also, the issue that might affect more users is the selection indicator that is wrongly positioned. In some rare cases, it might even fly around as user rotate the globe (see photo below). Any tips on approach solving this, thinking about passing a tilingScheme or MapProjection instance to GetFeatureInfo.

image image

jjhembd commented 2 years ago

Hi @zoran995, we're looking at this again. We are getting HTTP 503 errors for the data in the example. Would you be able to restore the data for testing? Thanks!

zoran995 commented 2 years ago

You can use this URL https://gs-stable.geo-solutions.it/geoserver/unesco/wms, instead of the one in a sandcastle. Hopefully, folks from Geosolutions won't be mad for the link, I found it on google.

updated sandcastle

jjhembd commented 2 years ago

Thanks @zoran995!

I'm afraid the problem is several layers deep:

  1. The WebMapServiceImageryProvider constructor inputs an Array of GetFeatureInfoFormats.
  2. The GetFeatureInfoFormat constructor inputs a callback function to convert the WMS response to ImageryLayerFeatureInfo objects
  3. The default callback function assumes GeographicProjection, as you noticed.

The solution (as you guessed) is to include a MapProjection in the callback. Unfortunately it has to be wrapped in some custom code to handle the conversion from WMS features to ImageryLayerFeatureInfo.

Here is an updated sandcastle illustrating this approach.

Let me know if this solves your problem for this specific example. We are still thinking about how to make it easier to handle different projections.

image

ggetz commented 2 years ago

Thanks for the update @zoran995! And thanks for the explanation @jjhembd!

If we do choose to make projection handling a bit easier here, I think the plan would be to pass in an optional MapProjection parameter. If it's a GeographicProjection, we continue as before. If not, we should use the project and unproject methods to convert the resulting cartographic to Geographic.

It's a bit awkward to have an additional parameter after callback, since a callback parameter is typically the last. We could deprecate the existing parameters, and change to an options object approach.