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

Updates of SampleData models to glTF 2.0 #8539

Merged
merged 22 commits into from
Jan 24, 2020
Merged

Conversation

sanjeetsuhag
Copy link
Contributor

Fixes #7802

@cesium-concierge
Copy link

Thanks for the pull request @sanjeetsuhag!

  • ✔️ Signed CLA found.
  • ❔ 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.

@lilleyse
Copy link
Contributor

This is a great start @sanjeetsuhag. Thanks for wrangling together all the blender / COLLADA files I passed along.

There's a few issues I came across while testing the Sandcastle examples:

  • By convention glTF 1.0 models are +X forward. glTF 2.0 models are now +Z forward. All the models will need to be re-exported so they are facing +Z. I wish I remembered this sooner. CesiumBalloon will also need to be fixed since I forgot to fix it in my PR.
    • gltf-pipeline might fix this when it upgrades 1.0 to 2.0. Otherwise it'll need to be fixed in Blender.
  • CesiumMan looks thinner now near the torso... not sure why. What does it look like in Blender?
  • CZML Model - Node Transformations and Development/3D Models Node Explorer operate differently than master. See if re-exporting as +Z helps.
  • Remove CesiumMilkTruck-kmc.glb. See my comment here: Upgrade all SampleData models to gltf 2.0 #7802 (comment)
  • Development/Multiple Shadows throws an error when loading the model
  • Development/Shadows
    • Shadow Tester is missing its floor
    • Shadow Tester 3 - some shapes have smooth normals when they should be hard normals

Based on offline discussion the new strategy might be to use gltf-pipeline to convert 1.0 to 2.0 then import/export from Blender to remove the KHR_techniques_webgl extension. This will hopefully produce more stable results across the board. I suggest trying this before looking too deeply at any of the bullets above.

And for final testing make sure to check every Sandcastle example that loads a model and compare it side-by-side with master.

@emackey
Copy link
Contributor

emackey commented Jan 15, 2020

There's a long pause after the end of the CesiumMan animation, so it doesn't loop smoothly in this latest version.

@sanjeetsuhag
Copy link
Contributor Author

sanjeetsuhag commented Jan 23, 2020

There's a long pause after the end of the CesiumMan animation, so it doesn't loop smoothly in this latest version.

@emackey The latest update fixes it. Turns out you have to clip the Blender animation timeline to the animation's end timestep to make sure only the active loop is embedded and not the whole timeline.

@emackey
Copy link
Contributor

emackey commented Jan 23, 2020

Animation looks good! But validation fails. When you export from Blender, be sure to pick "glTF Binary" and not "glTF Embedded". The latter is a base64-encoded version that is much less space-efficient. You'll get a smaller file if you pick Binary. Thanks!

@emackey
Copy link
Contributor

emackey commented Jan 23, 2020

Looks good! One (hopefully last) comment, in the Shadow tester Sandcastle demo, if you switch to Cesium man, his feet appear to plunge into the ground. Is that expected?

All of the models validate correctly now, and look good.

@sanjeetsuhag
Copy link
Contributor Author

Looks good! One (hopefully last) comment, in the Shadow tester Sandcastle demo, if you switch to Cesium man, his feet appear to plunge into the ground. Is that expected?

All of the models validate correctly now, and look good.

@emackey Good catch! Looks like both Cesium_Man and GroundVehicle clipped through the ground so I've clamped them both to the terrain.

@sanjeetsuhag sanjeetsuhag requested a review from lilleyse January 23, 2020 21:27
Copy link
Contributor

@emackey emackey 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 to me. @lilleyse did you want to review again before merge?

@emackey
Copy link
Contributor

emackey commented Jan 24, 2020

@sanjeetsuhag Great work on this. Any chance you can share the CesiumMan Blender project files with me? I'm trying to learn more about skinning/rigging in Blender.

@emackey
Copy link
Contributor

emackey commented Jan 24, 2020

Or wait... Did you get COLLADA2GLTF working? What happened to the 90 deg rotation?

emackey added a commit to KhronosGroup/glTF-Sample-Models that referenced this pull request Jan 24, 2020
@sanjeetsuhag
Copy link
Contributor Author

sanjeetsuhag commented Jan 24, 2020

Or wait... Did you get COLLADA2GLTF working? What happened to the 90 deg rotation?

@emackey Getting it all to work with Blender and COLLADA2GLTF was a...process. The primary issue with Blender was that it applied the transformation of the first frame of the animation. This is a known issue, however, following the workaround provided did not work for my case. And the primary issue with COLLADA2GLTF was that it rotates the mesh from the skeleton by 90 on the Y axis (if we're using the glTF co-ordinate system).

To get it all to work, these are the steps I had to take:

Note: I used Blender v2.82 Beta and COLLADA2GLTF v2.1.5

  1. Take the original COLLADA file and open it in Blender.
  2. Rotate the whole model to face -Y (Rotate on Z axis by -90).
  3. Rotate the mesh (not the skeleton, only the mesh) to face -X. (Rotate on Z axis by -90)
  4. Trim Blender's animation timeline to end at whatever time the animation ends. (End it at 50, instead of 250)
  5. Export as COLLADA
  6. Run COLLADA2GLTF to export to glTF 2.0 file.

This is the Blender project that I exported to COLLADA. The model in this has steps 2-4 already applied.

@emackey
Copy link
Contributor

emackey commented Jan 24, 2020

That's quite a process, thanks for documenting. Someday the extra trip back through COLLADA needs to be destroyed in a fire. I have a lot yet to learn about skinning if I want to be the one who does that.

@emackey
Copy link
Contributor

emackey commented Jan 24, 2020

@lilleyse please disregard the red X from a different repo shown above. This is ready.

@lilleyse
Copy link
Contributor

The only sandcastle discrepancies I noticed were:

  • CZML Model - Node Transformations - the waving motion is different than master. I pushed a commit to tweak it slightly: 3092b5d
  • 3D Models Node Explorer: if you select leg_joint_R_1 you'll notice that pitch and roll have swapped behavior. Though I actually think the new behavior is more intuitive (roll literally rolls the heel).

Other than that, everything looks great!

@lilleyse lilleyse merged commit bfd66c4 into master Jan 24, 2020
@lilleyse lilleyse deleted the model-gltf2.0-update branch January 24, 2020 15:52
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.

Upgrade all SampleData models to gltf 2.0
4 participants