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

Crash when 3D Tiles bounding region has 0 volume #7933

Closed
verybigzhouhai opened this issue Jun 12, 2019 · 4 comments
Closed

Crash when 3D Tiles bounding region has 0 volume #7933

verybigzhouhai opened this issue Jun 12, 2019 · 4 comments

Comments

@verybigzhouhai
Copy link
Contributor

verybigzhouhai commented Jun 12, 2019

Hi,
Recently I loaded a lot of 3dtiles layers from different sources. During the browsing process, I sometimes get an error.
image
image
I checked this in detail. I found that when a cesium3dtile bounding box has a volume of 0, for example, it is A face or a point in point cloud data, there is no height in a certain direction, which causes this error to be thrown when calculating the distance from the camera to the tile.

image
Should we add a validation to the program, when creating a cesium3dtile object, if I get a bounding box that will cause problems, initialize it to a fixed unit quantity, for example, when I create the box, I will detect it. The following is the case.
image
The correct operation of the program does not depend entirely on the legitimacy of the data.

@OmarShehata OmarShehata changed the title about cesium3dtile boundingvolume Crash when 3D Tiles bounding region has 0 volume Jun 12, 2019
@OmarShehata
Copy link
Contributor

This seems like a good idea to me, to make the client more robust. The specification doesn't say anything about a 0 volume bounding volume being invalid.

I'm not sure if it's going to cause problems further down the chain if you fix this one, but if this really is a valid bounding volume, we should fix it, or if it's not, we should throw a more meaningful error.

@lilleyse thoughts?

@lilleyse
Copy link
Contributor

Yeah we should fix this in the client. I think it's fine that the spec allows 0 volume bounding volumes for the cases @verybigzhouhai mentioned.

@verybigzhouhai your change looks mostly good, however I would set the axis length to something smaller like CesiumMath.Epsilon.EPSILON7 instead of 1.0. I would also make sure that the axes are still orthogonal to each other when one or two of the half-axes are non-zero length.

Do you want to open a pull request? If not we could make the change.

@verybigzhouhai
Copy link
Contributor Author

verybigzhouhai commented Jun 12, 2019

@lilleyse @OmarShehata By resetting the new value for 0 volume, the program is running fine so far, and no effect has been found. Just because we modified it at the top of the chain, it should not cause other problems. Maybe I can set the value even more. Smaller, make it closer to 0 instead of unit_x , unit_y or unit_z

@verybigzhouhai
Copy link
Contributor Author

verybigzhouhai commented Jun 12, 2019

@lilleyse haha,we have the same idea,I will open a pull request.

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

3 participants