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

Tile with transform not being clipped correctly #6600

Closed
lilleyse opened this issue May 18, 2018 · 3 comments · Fixed by #7034
Closed

Tile with transform not being clipped correctly #6600

lilleyse opened this issue May 18, 2018 · 3 comments · Fixed by #7034

Comments

@lilleyse
Copy link
Contributor

clipping

The tileset consists of two tiles, a root tile with a local->wgs84 transform and a child tile with a small translation/scale transform. The expected result is that both tiles get clipped in the same way but only the root tile is clipped correctly.

localhost:8080 link - uses TilesetWithTransforms in the Specs folder.

Part of the problem may be that only the root's transform is used in the Cesium3DTile/contentVisibility function. I vaguely remember the reasoning behind this came up as a PR comment but I can't find the thread.

Also I think there might be an inconsistency in how the clipping planes model matrix is handled.

Inside ClippingPlaneCollection/computeIntersectionWithBoundingVolume:

modelMatrix = Matrix4.multiply(modelMatrix, transform, scratchMatrix);

Compared to Model/createClippingPlanesMatrixFunction:

return Matrix4.multiply(model._modelViewMatrix, clippingPlanes.modelMatrix, scratchClippingPlaneMatrix);

I think the first should be modelMatrix = Matrix4.multiply(transform, modelMatrix, scratchMatrix); so that the order of operations is the same. But someone should double check that.

Brought up the forum: https://groups.google.com/forum/#!topic/cesium-dev/V3HMRAqToeU

@ggetz would you be able to look more into this?

@lilleyse lilleyse changed the title Tile with transforms not being clipped correctly Tile with transform not being clipped correctly May 18, 2018
@ppoulainpro
Copy link
Contributor

@ggetz : I have done a quick test of the fix to replace the quick & dirty bypass I have applied on model.js & PointCloud3DTileContent.js as explained at forum level and the behavior is not the same.
The clipping is changing when refresh is done and some artefacts exist on some areas where nothing should be clipped (see images below on my dataset, with the fix and with the bypass)
I can make more tests and investigations on my side during the week-end.

with fix:
issue6000_withfix6616

with bypass:
issue6000_withbypass

@OmarShehata
Copy link
Contributor

So while digging into this I found another test case where it's broken. This one has no transforms. localhost:8080 link

This is using the TilesetOfTilesets data. You can see that the children are getting clipped at a different location than the parent by the shader, and are also getting culled too early.

clipping_incorrect

I'm not sure how to confirm this yet but I believe the models in this tileset use RTC coordinates so I had to move the clipping plane to the tileset using:

clippingPlanes.modelMatrix =  Cesium.Transforms.eastNorthUpToFixedFrame(tileset.boundingSphere.center)

I think having to do this is a bit confusing since the clipping plane's coordinates are supposed to be relative to what it's clipping. It would be nice to support this case as well.

@cesium-concierge
Copy link

Congratulations on closing the issue! I found these Cesium forum links in the comments above:

https://groups.google.com/forum/#!topic/cesium-dev/V3HMRAqToeU

If this issue affects any of these threads, please post a comment like the following:

The issue at #6600 has just been closed and may resolve your issue. Look for the change in the next stable release of Cesium or get it now in the master branch on GitHub https://github.com/AnalyticalGraphicsInc/cesium.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

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

Successfully merging a pull request may close this issue.

4 participants