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.45k stars 3.39k forks source link

Filter out duplicate points for Wall Geometry #8842

Closed iceye closed 4 years ago

iceye commented 4 years ago

Reproduced on local env and Sandcastle STEP TO REPRODUCE:

WHEN I create a wall entity with this code:

viewer.entities.add({
  name: "Red wall at height",
"wall":{
    "positions":[
        {"x":4347090.215457887,"y":1061403.4237998386,"z":4538066.036525028},
        {"x":4348147.589624987,"y":1043897.8776143644,"z":4541092.234751661},
        {"x":4348147.589882754,"y":1043897.8776762491,"z":4541092.234492364},
        {"x":4335659.882947743,"y":1047571.602084736,"z":4552098.654605664},
    ],
    granularity: Cesium.Math.toRadians(0.02),
    "material":Cesium.Color.RED},
    "outline":true,
    "outlineColor":Cesium.Color.RED,
    "outlineWidth":1
});

CURRENT BEHAVIOR: THEN I see normalize error:

An error occurred while rendering. Rendering has stopped.
DeveloperError: normalized result is not a number
Error
    at new DeveloperError (https://sandcastle.cesium.com/CesiumUnminified/Workers/Check-d404a0fe.js:62:13)
    at Function.Cartesian3.normalize (https://sandcastle.cesium.com/CesiumUnminified/Workers/Cartesian2-d59b2dc1.js:439:13)
    at Function.WallGeometry.createGeometry (https://sandcastle.cesium.com/CesiumUnminified/Workers/createWallGeometry.js:485:42)
    at createWallGeometry (https://sandcastle.cesium.com/CesiumUnminified/Workers/createWallGeometry.js:649:25)
    at createGeometry (https://sandcastle.cesium.com/CesiumUnminified/Workers/createGeometry.js:60:32)
    at callAndWrap (https://sandcastle.cesium.com/CesiumUnminified/Workers/createTaskProcessorWorker.js:61:25)
    at https://sandcastle.cesium.com/CesiumUnminified/Workers/createTaskProcessorWorker.js:107:9

EXPECTED BEHAVIOR: I see my wall ;)

Current workaround: AVOID points too close for walls. No problems with the polygons, I didn't test with other geometries.

With the workaround for positions it's working fine:

"positions":[
        {"x":4347090.215457887,"y":1061403.4237998386,"z":4538066.036525028},
        {"x":4348147.589882754,"y":1043897.8776762491,"z":4541092.234492364},
        {"x":4335659.882947743,"y":1047571.602084736,"z":4552098.654605664},
]

Sandcastle example: I just tested on https://sandcastle.cesium.com/?src=Wall.html&label=Geometries with my wall

Browser: Chrome Version 81.0.4044.122

Operating System: OSX

Additional notes: The positions array is created with Cesium.Cartesian3.fromDegreesArrayHeights, the 2 points are created from these coordinates: lat: "45.6333333333333", lng: "13.5", "height": 4000 lat: "45.63333333", lng: "13.5", "height": 4000

I hope this will help

OmarShehata commented 4 years ago

@iceye this error is happening not because these two points are close, but because they are exactly the same. You can confirm this in your example by printing out the nextTop and topPosition points here right before the crash:

https://github.com/CesiumGS/cesium/blob/389770e78ef4c7c65df1da9627c292ca187b95f3/Source/Core/WallGeometry.js#L465-L470

I'm not sure there's anything that can be fixed for this on the library side.

mramato commented 4 years ago

@OmarShehata this is still something CesiumJS should handle by filtering out duplicate points.

iceye commented 4 years ago

I understand it's an edge case, but when data are collected from an "insecure" system it may happen points are almost the same (0.2-0.3mm difference in my use case). In my test app I added a pre-filter method. It could be interesting to add a global "precision" for Cesium instance (https://en.wikipedia.org/wiki/Decimal_degrees) and automatically optimize filtering and calculation based on that. Anyway, thx a lot for your support, I understand the "enhancement" request. I added this issue just because to me it seems strange the crash behavior.

iceye commented 4 years ago

Prob this issue should be also something like: Prevent error when there are duplicate points for Wall Geometry

hpinkos commented 4 years ago

@iceye I opened a PR to fix this here: https://github.com/CesiumGS/cesium/pull/8952