Added clamped and asynchronous loop handling for multiple animations #6004
Conversation
|
@likangning93, thanks for the pull request! Maintainers, we have a signed CLA from @likangning93, so you can review this at any time. I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Source/Core/CatmullRomSpline.js
Outdated
| * @function | ||
| * | ||
| * @param {Number} time The time. | ||
| * @return {Number} the time, wrapped around to the updated animation. |
There was a problem hiding this comment.
For all these change to The time
Source/Core/Spline.js
Outdated
| */ | ||
| Spline.prototype.wrapTime = function(time) { | ||
| //>>includeStart('debug', pragmas.debug); | ||
| if (!defined(time)) { |
There was a problem hiding this comment.
Use Check.typeOf.number for these.
Source/Core/Spline.js
Outdated
| } | ||
| //>>includeEnd('debug'); | ||
|
|
||
| // wrap |
There was a problem hiding this comment.
This and // clamp aren't needed.
Source/DataSources/ModelGraphics.js
Outdated
| @@ -179,6 +181,14 @@ define([ | |||
| */ | |||
| runAnimations : createPropertyDescriptor('runAnimations'), | |||
|
|
|||
| /** | |||
| * Gets or sets the boolean Property specifying if glTF animations should hold the last pose when no frames are specified. | |||
There was a problem hiding this comment.
For consistency make sure that these descriptions are the same. This and above are slightly different.
Source/Scene/Model.js
Outdated
| return function(localAnimationTime) { | ||
| if (defined(spline)) { | ||
| runtimeNode[targetPath] = spline.evaluate(spline.clampTime(localAnimationTime), runtimeNode[targetPath]); | ||
| runtimeNode.dirtyNumber = model._maxDirtyNumber; |
There was a problem hiding this comment.
Should the two cases be an if-else in the same function? That way someone would be able to change the clampAnimation property dynamically. I don't think there would be much of a performance hit. this._clampAnimations could then become this.clampAnimations and be a public property.
| @@ -7,6 +7,8 @@ Change Log | |||
| * Added ability to support touch event in Imagery Layers Split demo application. [#5948](https://github.com/AnalyticalGraphicsInc/cesium/pull/5948) | |||
There was a problem hiding this comment.
This should be simple enough to add to czml too. The only changes would be in CzmlDataSource.
CHANGES.md
Outdated
| @@ -7,6 +7,8 @@ Change Log | |||
| * Added ability to support touch event in Imagery Layers Split demo application. [#5948](https://github.com/AnalyticalGraphicsInc/cesium/pull/5948) | |||
| * Fixed `Invalid asm.js: Invalid member of stdlib` console error by recompiling crunch.js with latest emscripten toolchain. [#5847](https://github.com/AnalyticalGraphicsInc/cesium/issues/5847) | |||
| * Added CZML support for `polyline.depthFailMaterial`, `label.scaleByDistance`, `distanceDisplayCondition`, and `disableDepthTestDistance`. [#5986](https://github.com/AnalyticalGraphicsInc/cesium/pull/5986) | |||
| * Fixed a bug where models with animations of different lengths would cause an error. [#5694](https://github.com/AnalyticalGraphicsInc/cesium/issues/5694) | |||
| * Added a `clampAnimations` parameter to `Model`. Setting this to false allows different length animations to loop asynchronously. | |||
There was a problem hiding this comment.
Also mention that clampAnimations is added to Entity.model.
| @@ -326,6 +326,7 @@ define([ | |||
| * @param {Boolean} [options.allowPicking=true] When <code>true</code>, each glTF mesh and primitive is pickable with {@link Scene#pick}. | |||
| * @param {Boolean} [options.incrementallyLoadTextures=true] Determine if textures may continue to stream in after the model is loaded. | |||
| * @param {Boolean} [options.asynchronous=true] Determines if model WebGL resource creation will be spread out over several frames or block until completion once all glTF files are loaded. | |||
| * @param {Boolean} [options.clampAnimations=true] Determines if the model's animations should hold a pose over frames where no keyframes are specified. | |||
There was a problem hiding this comment.
Add this parameter to Model.fromGltf as well.
|
@lilleyse updated! |
lilleyse
force-pushed
the
multipleAnimations
branch
from
12e6bbc to
8d458cb
Compare
|
Looks good. I added a few tweaks in 8d458cb. |
Fixes #5694