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

Improve outlining for solid geometry in glTFs #8776

Merged
merged 35 commits into from
Apr 28, 2020
Merged

Conversation

kring
Copy link
Member

@kring kring commented Apr 21, 2020

Fixes #8689

Adds explicit solid geometry outlining for glTF, roughly following the technique described in this paper: https://www.researchgate.net/publication/220067637_Fast_and_versatile_texture-based_wireframe_rendering

With this technique, we get high quality, anti-aliased, non-depth-fighting edges without any additional draw calls.

The edges to outline are specified using a glTF extension, a rough draft of which is here:
https://github.com/kring/glTF/blob/CESIUM_primitive_outline/extensions/2.0/Vendor/CESIUM_primitive_outline/README.md

In short, the edges are specified using an indexed line list, much like you'd do with a separate GL_LINES primitive. There's a bit of extra work at load time to turn this edge list into additional vertex attributes, and sometimes it even requires duplicating vertices. I initially thought I'd have to do this in a web worker, but the performance seems quite good, so I'm going to try to get away without. Doing it in a web worker would requiring shuffling vertex and index buffers around, which would be a hassle and possibly cost more time than it saves.

Bonus features in this PR:

  • Basic support for creating a texture with explicit mipmaps.
  • A new GltfBuilder class in the tests, which is a handy way of building glTF models on the fly. It's a bit half-baked right now, but it made the ModelOutlineLoader tests easier to write and (hopefully) understand.

@cesium-concierge
Copy link

cesium-concierge commented Apr 21, 2020

Thanks for the pull request @kring!

  • ✔️ 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.

@likangning93 likangning93 self-requested a review April 26, 2020 16:21
Copy link
Contributor

@likangning93 likangning93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kring some preliminary feedback for the Renderer and Scene changes, mostly either small or philosophical things. From what I can tell this looks pretty solid 👌

I'll take a look at the Specs changes soon.

Source/Scene/Model.js Outdated Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Outdated Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Outdated Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Outdated Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Show resolved Hide resolved
Source/Scene/ModelOutlineLoader.js Show resolved Hide resolved
@kring
Copy link
Member Author

kring commented Apr 27, 2020

Thanks for the review @likangning93! I think I've addressed everything, except that I need to write a test or two for the index enbiggening.

Copy link
Contributor

@likangning93 likangning93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kring! Just one more place where doc needs updating. I took a look at the specs too, those pretty much look good to me.

Source/Scene/ModelOutlineLoader.js Outdated Show resolved Hide resolved
@likangning93
Copy link
Contributor

I need to write a test or two for the index enbiggening.

Oof. That sounds like it's going to be an impressive spec.

@kring
Copy link
Member Author

kring commented Apr 28, 2020

Ready for another look!

Copy link
Contributor

@likangning93 likangning93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kring! The new spec looks great 👍, just one more thing:

Source/Scene/ModelOutlineLoader.js Outdated Show resolved Hide resolved
@likangning93
Copy link
Contributor

likangning93 commented Apr 28, 2020

Thanks @kring! I can merge this once CI passes.

@weiyinggh
Copy link

@likangning93 Can you provide an example program to realize the effect of contour line in cesium? thank you!

This pull request was closed.
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.

Better outlining for solid geometry
4 participants