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

Fix for Point Cloud show style #5598

Closed
sir-chaos opened this issue Jul 7, 2017 · 7 comments
Closed

Fix for Point Cloud show style #5598

sir-chaos opened this issue Jul 7, 2017 · 7 comments

Comments

@sir-chaos
Copy link
Contributor

sir-chaos commented Jul 7, 2017

In PointCloud3DTileContent.js, in line executing when a "show" style parameter is defined to hide points with show == false, there might be incorrect nullification of gl_Position, which in fact does not remove the point. I suggest it was meant to nullify gl_PointSize instead (which effectively hides the point).
Link to the line in code (just change the variable):
https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Scene/PointCloud3DTileContent.js#L1025
(actual for commit 337ad9d)

@lilleyse
Copy link
Contributor

lilleyse commented Jul 7, 2017

Setting gl_Position *= 0.0 should discard the point properly. The resulting NDC position will be outside the [1, 1, 1] to [-1, -1, -1] NDC-space cube and the GPU will cull the point. Whether its bad for the NDC position to be [infinity, infinity, infinity] is a different question, but we use this trick throughout the code base, though I suppose it's less relevant for points.

For clarity sake I would be ok with changing it to gl_PointSize = 0. Feel free to open a PR.

Semi related post here: https://stackoverflow.com/a/33970293

@sir-chaos
Copy link
Contributor Author

sir-chaos commented Jul 7, 2017

Thank you for your response!
I will revisit this issue with a PR shortly, but anyway I must point out that for some reason in my case the present code leads to a single ugly point in the center of the screen instead of discard (obviously at [0,0,0]). I think this is subject to some investigation and would appreciate if someone could verify that.

@lilleyse
Copy link
Contributor

lilleyse commented Jul 7, 2017

Ok that's interesting, I don't see that but it is worth the fix.

Do you also see a stray point when show is false for non point-cloud tilesets? Like for this example: http://cesiumjs.org/Cesium/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=e79e2d6547c4c5a017372e8a91776769

@sir-chaos
Copy link
Contributor Author

Non point-cloud tilesets seem OK. For sand castle the issue reproduced, try inserting
tileset.style = new Cesium.Cesium3DTileStyle({show: "${POSITION}.x > 10.0"})
line somewhere (The point might be tricky to see but it is definitely there)

@lilleyse
Copy link
Contributor

lilleyse commented Jul 7, 2017

I've tried a similar setup here and don't see a point: http://cesiumjs.org/Cesium/Apps/Sandcastle/?src=Hello%20World.html&label=Showcases&gist=2fdd2a88b3bce29db2f51515c08a5d79

This inconsistency is probably explained by hardware or driver differences.

@sir-chaos
Copy link
Contributor Author

In the last example the point is very distinct for me.
Created pull request #5599

@hpinkos
Copy link
Contributor

hpinkos commented Sep 19, 2019

Fixed in #5599

@hpinkos hpinkos closed this as completed Sep 19, 2019
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

4 participants