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

Cesium stops rendering on bad terrain #7936

Closed
gberaudo opened this issue Jun 12, 2019 · 24 comments
Closed

Cesium stops rendering on bad terrain #7936

gberaudo opened this issue Jun 12, 2019 · 24 comments

Comments

@gberaudo
Copy link
Contributor

gberaudo commented Jun 12, 2019

Hi,

I am upgrading an application from Cesium 1.44 to Cesium 1.58.
After some navigation an error is triggered from the terrain rendering code and Cesium freezes.
I would expect this custom terrain mesh to continue working with newer Cesium versions. Ideally the Cesium render would also not freeze.

The issue seems to trigger only when navigating near the Swiss border.
We generate tiles only inside Switzerland, outside we return 204.

Do you know of a change since 1.44 that could cause this issue?
Any hint would be welcome.

Here is a stack trace:
at new DeveloperError (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:540:19)
at Object.Check.typeOf.number (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:1793:19)
at Function.Cartesian3.multiplyByScalar (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:7859:22)
at Ellipsoid.cartographicToCartesian (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:9173:20)
at addVertexWithComputedPosition (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217648:34)
at createFillMesh (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217515:25)
at propagateEdge (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217193:17)
at visitTile (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217180:9)
at visitRenderedTiles (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217119:13)
at Function.TerrainFillMesh.updateFillTiles (https://cesiumjs.org/releases/1.58/Build/CesiumUnminified/Cesium.js:217057:13)

Here is the state of the variables (see the undefined and NaN values):
createFillMeshNaN

The issue can be reproduced using these 2 codepens:

Browsers / OS:
Chromium & Firefox on Linux/Ubuntu.

@gberaudo
Copy link
Contributor Author

I tested several releases and the issue might have been introduced in Cesium 1.55.
I will do more testings to confirm the issue does not appear in 1.54.

@OmarShehata
Copy link
Contributor

Thanks for looking into this @gberaudo . There have been two major changes recently. Version 1.55 changed the way terrain & imagery load to skip levels of detail to make it faster and use less bandwidth:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CHANGES.md#155---2019-03-01

And version 1.56 changed the way image decoding works by implementing ImageBitmap:

https://github.com/AnalyticalGraphicsInc/cesium/blob/master/CHANGES.md#156---2019-04-01

My guess is one of those changes is the culprit. This is the error you're getting right?

DeveloperError: Expected scalar to be typeof number, actual typeof was undefined

@gberaudo
Copy link
Contributor Author

Thanks @OmarShehata for the feedback. Indeed this is the message I get.
I tested again with Cesium 1.54 and I could not reproduce the issue. It really looks like a consequence of the changes in Cesium 1.55.

@OmarShehata
Copy link
Contributor

OmarShehata commented Jun 14, 2019

Thanks for confirming @gberaudo. I think the next step here would be to git bisect to find the exact commit that caused this issue now that we've narrowed it down to versions 1.54 and 1.55. If you can give that a shot and update this issue that would be super helpful in getting a fix in time for the next release.

@gberaudo
Copy link
Contributor Author

Hi @OmarShehata, hi @kring,

I took the time to bisect the error. It looks like being introduced either in 955c996 or in 0328cc9.
@kring, I mention you since you are the author of these commits, and you might have some idea of the issue / a solution.

gberaudo@wrk29:~/dev/cesium (master $%)$ git log 955c996334167fd299ef1115d0068d641f1ce0f2...aa4b8063299f3c7f7ae4424031d0b5ebd7b6b857
commit 955c996334167fd299ef1115d0068d641f1ce0f2
Author: Kevin Ring <[email protected]>
Date:   Tue Sep 11 18:00:18 2018 +1000

    New fill tile code kinda working.

commit 0328cc917dfd001f6a8ac0cb27e265635263d47f
Author: Kevin Ring <[email protected]>
Date:   Wed Sep 5 14:09:32 2018 +1000

    Don't regenerate fill tiles every frame.

commit 237f8deccf89a298c08237344996625fdad7d097
Author: Kevin Ring <[email protected]>
Date:   Wed Aug 8 22:01:17 2018 +1000

    Fix a bug that made fill tiles flicker out after partial load.

I was not able to test commit 0328cc9 as it immediately triggers the following error:

An error occurred while rendering. Rendering has stopped.
TypeError: Cannot read property 'center' of undefined
TypeError: Cannot read property 'center' of undefined
    at addDrawCommandsForTile (http://localhost:8080/Build/CesiumUnminified/Cesium.js:220910:24)
    at GlobeSurfaceTileProvider.endUpdate (http://localhost:8080/Build/CesiumUnminified/Cesium.js:219505:17)
    at QuadtreePrimitive.render (http://localhost:8080/Build/CesiumUnminified/Cesium.js:222799:26)
    at Globe.render (http://localhost:8080/Build/CesiumUnminified/Cesium.js:224243:21)
    at updateAndRenderPrimitives (http://localhost:8080/Build/CesiumUnminified/Cesium.js:242220:26)
    at executeCommandsInViewport (http://localhost:8080/Build/CesiumUnminified/Cesium.js:242074:13)
    at updateAndExecuteCommands (http://localhost:8080/Build/CesiumUnminified/Cesium.js:241937:13)
    at render (http://localhost:8080/Build/CesiumUnminified/Cesium.js:242501:9)
    at tryAndCatchError (http://localhost:8080/Build/CesiumUnminified/Cesium.js:242521:13)
    at Scene.render (http://localhost:8080/Build/CesiumUnminified/Cesium.js:242565:13)

Here is the procedure to check by yourself:

git checkout <some_hash>
npm i && npm run clean && npm run combine && npm run start
Open http://localhost:8080/Apps/bisect.html (be sure files are not cached) and navigate around Swiss borders:
- on bad commits an error in the createMesh code happens after some time;
- on good commits no such error happens (and you get bored of navigating).

Here is the Apps/bisect.html file:

<head>
  <link rel="stylesheet" src="./Sandcastle/templates/bucket.css"></link>
  <script src="/Build/CesiumUnminified/Cesium.js"></script>
  <script src="./bisect.js"></script>
  <style>
      @import url(/Build/CesiumUnminified/Widgets/widgets.css);
      html, body, #cesiumContainer {
          width: 100%; height: 100%; margin: 0; padding: 0; overflow: hidden;
      }
  </style>
</head>
<div id="cesiumContainer" class="fullSize"></div>
<div id="toolbar"></div>

And the Apps/bisect.js file:

window.onload = function() {
  // Swiss extent
  var rectangle = Cesium.Rectangle.fromDegrees(
    5.013926957923385,
    45.35600133779394,
    11.477436312994008,
    48.27502358353741
  );

  try {
    var viewer = new Cesium.Viewer("cesiumContainer", {
      baseLayerPicker: false,
      terrainProvider: new Cesium.CesiumTerrainProvider({
        url:
          "//3d.geo.admin.ch/1.0.0/ch.swisstopo.terrain.3d/default/20180601/4326/"
      }),
      imageryProvider: new Cesium.UrlTemplateImageryProvider({
        url:
          "//wmts10.geo.admin.ch/1.0.0/ch.swisstopo.swisstlm3d-karte-farbe.3d/default/current/4326/{z}/{x}/{y}.jpeg",
        minimumLevel: 6,
        maximumLevel: 17,
        tilingScheme: new Cesium.GeographicTilingScheme({
          numberOfLevelZeroTilesX: 2,
          numberOfLevelZeroTilesY: 1
        }),
        rectangle: rectangle
      }),
      fullscreenButton: false,
      homeButton: false,
      sceneModePicker: false,
      selectionIndicator: false,
      timeline: false,
      animation: false,
      geocoder: true,
      navigationInstructionsInitiallyVisible: false,
      navigationHelpButton: false,
      scene3DOnly: true
    });

    // Zoom to Mürrn
    viewer.camera.setView({
      destination: Cesium.Rectangle.fromDegrees(7.87, 46.58, 7.88, 46.59), // Mürren
      orientation: {
        heading: Cesium.Math.toRadians(175.0),
        pitch: Cesium.Math.toRadians(-35.0),
        roll: 0.0
      }
    });

    viewer.scene.globe.baseColor = Cesium.Color.WHITE;
    viewer.scene.backgroundColor = Cesium.Color.WHITE;
  } catch (e) {
    console.log(e.message);
  }
};

@kring
Copy link
Member

kring commented Jun 24, 2019

Hi @gberaudo, I played with your 1.58 codepen and wasn't able to trigger the error in a few minutes of navigating around. Any more specific advice about where or how to move?

Based on your debugger screen shot, my best guess is that you have a quantized-mesh tile that is missing a corner point, or where the corner vertices are not quite exactly in the corners. This is tripping up the fill tile generation code. We could perhaps handle this case, but, if I'm right, you really should fix your tileset if at all possible. The missing/misplaced corner will cause cracking artifacts between tiles, too.

@gberaudo
Copy link
Contributor Author

Thanks @kring for the tips.

I can probably rely on https://quantized-mesh-tile.readthedocs.io to iterate the whole tileset.
For each tile, get the edge vertices and check:

  • corners: (0, 32767), (0, 0), (32767, 0), (32767, 32767) are present;
  • tile edges: coordinates pair contain at least 0 or 32767.
    Or do you know of a tool doing it already?
    If I find something wrong then I would check the code generating the tiles and hopefully fix it.

I can probably add some detection logics in Cesium to find the bad tiles (and test your hypothesis).
If there is a way to fix it in the client it would be great too.

We only generate part of the world (Switzerland with a border). Where there is no content we send a "204 No content" HTTP response; could it also trigger the issue?

To trigger the issue I navigate like this (triggered with the 1.58 codepen):
output

@kring
Copy link
Member

kring commented Jun 25, 2019

I think the black areas you see in your GIF are the problem. Check those tiles first. Enabling Cesium Inspector might help identify which they are.

I wouldn't expect a missing tile (204 or otherwise) to cause that problem, Cesium should be able to handle that. But you should definitely include the available property in your layer.json. Doing so will improve performance by allowing Cesium to skip loading levels of the tile hierarchy. It will also allow Cesium to avoid even trying to load the missing tiles, improving performance and avoiding errors in the console.

@OmarShehata
Copy link
Contributor

@gberaudo in case it's relevant, there was a bug I introduced, that was recently fixed in this PR #7914 where imagery requests that came back with 204 would prevent future requests from happening. This doesn't look like what's happening here, but I wonder if it's worth also testing the latest master branch here.

@gberaudo
Copy link
Contributor Author

@kring, we actually define the available property in the returned layer.json file. But some values might be wrong! The black areas also occur in Cesium 1.44, without harm so far; getting ride of them would be great though.

In the console logs I see: "An error occurred in "CesiumTerrainProvider": Failed to obtain terrain tile X: 4276 Y: 988 Level: 12". Now if I put a breakpoint in CesiumTerrainProvider.prototype.requestTileGeometry and call
this._layers[0].availability.isTileAvailable(12, 4276, 988) I get true.
=> is this the correct way to check or should I first do some transforms on the coordinates (Y = YCountAtLevel - Y -1)?

@OmarShehata, I just tested with master and the issue is still present.

@kring
Copy link
Member

kring commented Jun 26, 2019

Yes the black areas wouldn't have caused problems in older versions because we weren't trying to generate "fill tiles" that join up at the (non-existent in this case) edges.

Your call to isTileAvailable is correct relative to that error message, but you will need to flip the Y as you described when looking for the tile file in your tileset.

@gberaudo
Copy link
Contributor Author

@kring

I tried producing black areas by navigating after switching my browser to offline in the network panel. Then I see many tile retrieval failures in the console but no black area appears on the screen.
=> even if we have an incorrect layer.json and some tiles are missing it should not be the cause of the exception. So it should be the data in some of the tiles that is wrong.

You said "my best guess is that you have a quantized-mesh tile that is missing a corner point, or where the corner vertices are not quite exactly in the corners".
But is it required to have vertices on terrain tile corners / edges? I do not see this in the terrain spec.

@kring
Copy link
Member

kring commented Jun 27, 2019

Right, missing tiles won't break anything. Incorrect layer.json won't cause a crash, though it can cause unnecessary requests, or cause Cesium to not display terrain that actually exists.

However, missing corner vertices in a tile can cause a crash in the latest versions of Cesium. I would consider corners required even though I didn't state it explicitly in the spec. Missing corners have always caused holes in the Earth, through which you can see stars, bits of the atmosphere, and other artifacts. When navigating with the mouse, accidentally clicking on a crack caused by missing corners would cause wild camera behavior. The crash is new, but missing corners have always been a problem.

@gberaudo
Copy link
Contributor Author

Thank you very much @kring for the precisions.
I remember there were some details about how to handle edges when generating terrain tiles in some old talk by @pjcozzi but I can not find it again.

@OmarShehata
Copy link
Contributor

I'm going to remove the priority - next release tag because it sounds like this can be avoided by fixing the terrain generation to have complete corners, since that has always produced artifacts. It does sound like it's something CesiumJS could catch without crashing, so we can leave this open.

@gberaudo , we do keep all of our old talks up, might be worth looking there: https://cesium.com/presentations/

@gberaudo
Copy link
Contributor Author

I activated the Cesium inspector and used an external viewer to display a broken tile. Here are the findings:

Large portions of tiles are missing:
bisect_11_2133_1546_and_around

This is confirmed when displaying one of the broken tile 11x2133x501 (/11/2132/1546.terrain):
debugger_11_2133_1546

For the curious, here is how to download it:
curl 'https://terrain0.geo.admin.ch/1.0.0/ch.swisstopo.terrain.3d/default/20180601/4326/11/2132/1546.terrain?v=1.0.0' -H 'Accept: application/vnd.quantized-mesh,application/octet-stream;q=0.9,*/*;q=0.01' -H 'Referer: http://localhost:8080/Apps/bisect.html' -H 'Origin: http://localhost:8080' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/75.0.3770.90 Chrome/75.0.3770.90 Safari/537.36' --compressed > 11_2132_1546.terrain

@gberaudo
Copy link
Contributor Author

Hi @kring, we are using partial terrain (we only cover Switzerland). Is non-global terrain still supported? As long as all generated tiles have corner filled?

I see it now exists a globe.cartographicLimitRectangle property. Is it related?

@kring
Copy link
Member

kring commented Jan 22, 2020

@gberaudo I don't think non-global terrain has ever been supported. You at least need to provide the root tile(s), even if their heights are all zeros. Otherwise Cesium can't draw a globe (or at least it would have to fill with zeros itself). As discussed before, every tile must at least have vertices at its four corners as well.

@gberaudo
Copy link
Contributor Author

OK, thanks @kring. I will try that and see what happens.

@imagoiq
Copy link

imagoiq commented May 26, 2020

@gberaudo Have you found a workaround ? We experience the same issue with the same terrain (from Swisstopo), for example on this page when you zoom out: https://smapshot-beta.heig-vd.ch/visit/1790

Have you or should we report the issue to Swisstopo ?

@gberaudo
Copy link
Contributor Author

@imagoiq, unfortunately not.
Swisstopo is investigating a fix but it may take time to land.
At HEIG-VD, you might (???) have free access to the 3D data used for generating the terrain: https://shop.swisstopo.admin.ch/fr/products/height_models/alti3D If yes you could try one of the opensource solutions to generate your own Cesium terrain tiles (the best is quantized-mesh format). If you do let me know, I would be very interested in the result.
If you can not, you might want to use SRTM, which might be better than nothing.

Another point is that Cesium is not supposed to work with partial terrain.
I think it is a standard use case though and would be cool to be officially supported.
It looks like globe.cartographicLimitRectangle can help.

@imagoiq
Copy link

imagoiq commented May 27, 2020

Thanks @gberaudo for your reply, so we'll further investigate this issue and contact Swisstopo to coordinate.

@jjhembd
Copy link
Contributor

jjhembd commented Apr 13, 2023

Hi @gberaudo, have you tried rendering this terrain as a 3D Tileset? See the 3D Tiles Next S2 Globe for an example.

I think 3DTiles will be a better way to handle non-global datasets, so I will close this issue for now. Feel free to re-open if you think it's still an issue.

@jjhembd jjhembd closed this as completed Apr 13, 2023
@gberaudo
Copy link
Contributor Author

gberaudo commented Jun 7, 2023

Hi @jjhembd,

Thanks for the idea, I did not notice that Cesium had advanced so much in this direction.
I am happy to see the 3d-tiles ecosystem has matured so much that Cesium can now work on this core evolution.

There are still lots of things to address before it is plainly usable:

  • imagery layers on top a 3d tiles
  • GroundPrimitives on top of 3d tiles;
  • occlusion tests, and probably many more things.

For everyone, there is a list of related issues here: https://github.com/CesiumGS/cesium/labels/3D%20Tiles%20as%20Terrain

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

No branches or pull requests

5 participants