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 artifacts in BIM demo #7181

Merged
merged 5 commits into from
Oct 26, 2018
Merged

Fix artifacts in BIM demo #7181

merged 5 commits into from
Oct 26, 2018

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Oct 24, 2018

Fixes #5722

Updates the tileset used in the BIM demo. Live Demo.

  • No more line flicker artifacts
  • Tried to remove as much z-fighting as possible
    • Ideally this would be fixed in the tileset rather than patched in the example.
  • Updated glTFs so that the REPLACE blend mode works.
  • Upgraded to glTF 2

Also adds hover-over picking to the demo. Because we don't have global batch ids there is a bit of boilerplate in there to get it to work.

I wanted to add silhouetting but I felt like there were too many artifacts. Example

To do:

  • Don't use tileset out of my personal ion account
  • Update other demos to use the new asset

@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

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

🌍 🌎 🌏

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

Also adds hover-over picking to the demo

Please make this a check box that defaults to false; it is too distracting for initial navigation.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

Tried to remove as much z-fighting as possible
Ideally this would be fixed in the tileset rather than patched in the example.

Where is the code doing the patching? I don't see it at glance.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

I wanted to add silhouetting but I felt like there were too many artifacts. Example

Not too bad but just depends on the view; agree to stay with just highlight for now.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

Also adds hover-over picking to the demo. Because we don't have global batch ids there is a bit of boilerplate in there to get it to work.

Not ideal but OK for now.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

Update CHANGES.md mainly to just build awareness of BIM in Cesium and per-feature selection.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 24, 2018

Thanks for upgrading this, bump when ready.

@lilleyse
Copy link
Contributor Author

Where is the code doing the patching? I don't see it at glance.

Here and here

@lilleyse
Copy link
Contributor Author

@pjcozzi Ready

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2018

Here and here

ah, lol

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2018

@lilleyse please submit a separate cleanup issue for this model so we can remove the workarounds. The workarounds are fine for demoing BIM / interiors, but not a useful code example.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2018

Should we move the near plane up?

image

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 25, 2018

Minor, but show unchecking remove the highlight? Perhaps a good idea if it is only a line or code or two

image

@lilleyse
Copy link
Contributor Author

Should we move the near plane up?

I believe that is a log-depth problem rather than the near plane being too far - #6573 (comment)

@lilleyse
Copy link
Contributor Author

@pjcozzi Updated. Opened #7191 to address the z-fighting.

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 26, 2018

Thanks!!!

@pjcozzi pjcozzi merged commit 31296f9 into master Oct 26, 2018
@pjcozzi pjcozzi deleted the update-bim-demo branch October 26, 2018 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bad artifacts in 3D Tiles BIM example
3 participants