terriajs icon indicating copy to clipboard operation
terriajs copied to clipboard

Add `clampToGround` to kml

Open nf-s opened this issue 1 year ago • 8 comments

Add clampToGround to kml

Fixes https://github.com/TerriaJS/terriajs/issues/6799

Related to

  • https://github.com/TerriaJS/terriajs/issues/4958

Terria used to clamp 2D and 3D KML polygon features to ground, and there was no way to turn it off. I have removed Terria's clamping, added a clampToGround trait, and now we pass that into KmlDataSource.load() - where cesium will now handle clamping

clampToGround will default to true - but Cesium will respect altitudeMode in KML - so it won't clamp 3D polygons that have altitudeMode set to absolute or relativeToGround.

There is a weird behaviour where KML polygons will only be clamped if in 3d (cesium) mode, this is because when loadMapItems is loaded, cesium terrain is sampled to clamp polygon features. This is now fixed.

Test me

{
  "name": "3D KML Test clampToGround = false",
  "type": "kml",
  "url": "https://gist.githubusercontent.com/nf-s/cfa170f067c8c462467558e9971f811d/raw/acf861a79fd206770029bb739fb76b5b84080e98/test-polygon.kml",
  "clampToGround": false
},
{
  "name": "3D KML Test clampToGround = true",
  "type": "kml",
  "url": "https://gist.githubusercontent.com/nf-s/cfa170f067c8c462467558e9971f811d/raw/acf861a79fd206770029bb739fb76b5b84080e98/test-polygon.kml",
  "clampToGround": true
},
{
  "name": "2D KML Test clampToGround = false",
  "type": "kml",
  "url": "https://gist.githubusercontent.com/nf-s/e0174e6f4114b9bab01307fe183be1e2/raw/7c2e5c210d8b7f0120d961d4e6086ad92b0caf44/test-2d-polygon.kml",
  "clampToGround": false
},
{
  "name": "2D KML Test clampToGround = true",
  "type": "kml",
  "url": "https://gist.githubusercontent.com/nf-s/e0174e6f4114b9bab01307fe183be1e2/raw/7c2e5c210d8b7f0120d961d4e6086ad92b0caf44/test-2d-polygon.kml",
  "clampToGround": true
}
  • Before link
    • Notice that all polygons are clamped to the ground
  • After link
    • Notice that all 3D polygons are not clamped to the ground (regardless of clampToGround value)
    • Notice that 2D polygons are clamped when clampToGround is true (set mode to 3d terrain)

Checklist

  • [ ] There are unit tests to verify my changes are correct or unit tests aren't applicable (if so, write quick reason why unit tests don't exist)
  • [ ] I've updated relevant documentation in doc/.
  • [x] I've updated CHANGES.md with what I changed.
  • [x] I've provided instructions in the PR description on how to test this PR.

nf-s avatar Oct 25 '23 09:10 nf-s

@nf-s "clampPolygonsToGround": false worked!!! Thank you for making this work. Best. Hiroo

imakihi avatar Nov 05 '23 04:11 imakihi

@nf-s Is it possible to set clampPolygonsToGround = false as a default? I wanted to locally import KML files and overlay those in the 3D mode (not being clumped). What do you think?

Hiroo

imakihi avatar Nov 10 '23 02:11 imakihi

Hi @imakihi

I looked into this a bit more and I have removed the polygon clamp feature. Cesium now has a clampToGround flag that applies its own logic to KML files.

This means that KML files with 3D polygons that have altitudeMode set to absolute or relativeToGround will never be clamped - regardless of clampToGround.

We still need to do some testing - but I think this is a better way forward than before, where Terria was manually overriding coordinate heights.

What do you think @steve9164 ?

nf-s avatar Nov 10 '23 06:11 nf-s

Hi @nf-s Thank you for looking into this. Your logic makes sense to me. Please let me know if I can test this new implementation.

imakihi avatar Nov 13 '23 06:11 imakihi

Hi @nf-s Thank you for looking into this. Your logic makes sense to me. Please let me know if I can test this new implementation.

Hi @imakihi

That would be great, thanks! If you have a few different sources of KML, can you please drag them into the test deployment (http://ci.terria.io/kml-clamp-polyhon) and let us know if you see any weird behavior with the new clampToGround (this will be set to true by default)?

nf-s avatar Nov 13 '23 06:11 nf-s

@nf-s I tested 5 kml files with different altitude modes.

  • absolute
  • clump to ground
  • clump to seafloor
  • relative to ground
  • relative to seafloor

Except relative to ground, I think, it worked. When I drag and drop my test file for the relative to ground, it doesn't clump, I think. I created my sample KML file from here. https://developers.google.com/kml/documentation/altitudemode

These are my test files. altitudemode.zip

imakihi avatar Nov 13 '23 06:11 imakihi

I tested the KMLs, but couldn't find a difference between clampToGround = true or false

To be honest, I find it quite difficult to figure out exactly how clampToGround works in cesium - the code path is pretty annoying to follow.

Basically - if clampToGround is true

Anyway, I think we should just merge this in. This respects KML features (and altitude mode), and correctly passes clampToGround into cesium.

nf-s avatar Dec 04 '23 11:12 nf-s