Skip to content

Initial support for AGI_articulations#7835

Merged
shunter merged 12 commits intomasterfrom
articulations
May 30, 2019
Merged

Initial support for AGI_articulations#7835
shunter merged 12 commits intomasterfrom
articulations

Conversation

@emackey
Copy link
Copy Markdown
Contributor

@emackey emackey commented May 13, 2019

This PR has initial support for 3D model articulations from the AGI_articulations vendor extension to glTF 2.0. There is no pointing vector or attach point support here, just articulations.

This isn't polished yet, but I'm opening it for early review from @shunter, @abwood, and @gbeatty.

TODO

  • There should be a better way to expose the available articulation names, stage name, min, max, and get/set current stage value. Let's push this until a more complete solution for 3D Model Entity API at a disadvantage #7831 is designed.
  • Add unit tests.
  • Possibly in a separate PR, add support at the Entity layer. Also pushed.

@cesium-concierge
Copy link
Copy Markdown

cesium-concierge commented May 13, 2019

Thanks for the pull request @emackey!

  • ✔️ 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.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

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

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 13, 2019

Here's a live demo.

CesiumArticulations

@TJKoury
Copy link
Copy Markdown
Contributor

TJKoury commented May 23, 2019

This is awesome!

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 23, 2019

The scope of this PR has been dialed in a little. It's write-only (same as nodeTransformations) support for articulations at the model primitive level only.

Ready for full review & merge.

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 28, 2019

Any chance this can be merged in time for release? Scott doesn't know the contents of Model.js well enough to be comfortable merging this himself. We're confident in the articulations functionality, just need to make sure we're not breaking some conventions or established patterns in that file.

@lilleyse
Copy link
Copy Markdown
Contributor

I took a quick look at the Model.js changes. They look totally fine to me. Should this change be mentioned in CHANGES.md?

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 30, 2019

CHANGES.md updated, thanks.

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 30, 2019

@shunter Can you do the honors, or does this need additional reviewers?

@abwood
Copy link
Copy Markdown
Contributor

abwood commented May 30, 2019

The application of articulation transformations looks to be consistent with STK's implementation. Here's the sand castle example using facility.glb:
image

and here are those same articulations applied to the default facility in STK:
image

Copy link
Copy Markdown
Contributor

@abwood abwood left a comment

Choose a reason for hiding this comment

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

Looks good on my end once the Travis CI issue is adjudicated.

@emackey
Copy link
Copy Markdown
Contributor Author

emackey commented May 30, 2019

Thanks! Travis is a fluke, some unrelated test flaked out.

@shunter shunter merged commit 8a7939f into master May 30, 2019
@shunter shunter deleted the articulations branch May 30, 2019 17:32
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.

6 participants