Realistic vehicle wheel speed Sandcastle example #7361
Conversation
|
Thanks for the pull request @OmarShehata!
Reviewers, don't forget to make sure that:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
What's cool about it is that, you can see how the wheels slow/speed up based on moving the time, but the wheel speed is computed based on the absolute distance the entity traverses. So if you update the CZML to make it move faster/slower, or if the entity is just moved faster (as in, if it weren't controlled by the CZML) it would still have the correct speed. It's also based on the wheel's circumference, so to apply this to another model you'd just need to update the circumference. http://localhost:8080/Apps/Sandcastle/index.html?src=Multi-part%20CZML.html
|
|
Looks cool, I definitely would like to review this given the entity API implications and I already saw some things I have some feedback on (but will need to think about more). Hopefully later this week. Thanks @OmarShehata |
|
Thank you both for the feedback! While we're reviewing this, we might want to consider what all we need to expose through the Entity API for models. Here's a forum discussion where a user needs access to the https://groups.google.com/d/msg/cesium-dev/4wVIah9ZAuk/A39Gi6POBAAJ Unless the answer is "Just use a primitive for that one". Which might be okay if it's easy to position it, but I haven't tried it so I don't remember how positioning works there. |
|
Please bump to me when ready for a quick pass. |
|
@pjcozzi the actual example is ready to look at if you want to review that now (regarding things like, does the Sandcastle show this feature well enough? is it ok to put it in this Sandcastle or should I separate it out into its own?) I'm just waiting on feedback about the Entity API changes so the implementation/property names might change. |
|
Great work guys, this is a highly valuable addition! |
|
Some high level feedback, I'll dig in deeper once we hash out the details.
|
|
That all seems like great feedback @mramato. Except:
It seemed like a far simpler workflow for me to have a car with animated wheels, and just apply a speed multiplier than hardcode quaternions into my data. The fact that it is applied to all animations instead of just the wheels is a limitation of the Entity API that I think we should ameliorate. Plus that way you can easily switch it out with another animated car model and not have to go back and change all the names of the nodes in the CZML. |
|
Ok it is now a new, separate Sandcastle example, and I must say I am quite pleased with how easy it was using the Entity's properties and how clean everything's turned out! This was probably the most enjoyable experience I've had working with CesiumJS once @mramato illuminated me to the power and joy of the composability of Entity properties. All I really had to do was create a list of positions. The orientation of the vehicle is automatically computed from those at any given moment, and the animation multiplier just uses the dynamic velocity vector property to figure out the relative speed to play at. Best of all? It all works perfectly with the timeline. If you play the animation at 2x, the actual vehicle speed in meters per second does not change, but the animation speed automatically does, so it's all still accurate and perfect. It's straightforward, it's flexible, it's dynamic, it's beautiful. The only thing I didn't implement is manually rotating wheels with NodeTransformation because I still maintain that this is a better workflow and not being able to control individual animations with the Entity API is a separate issue. |
|
Sounds good, no need for me to review, @mramato please review when you are happy. @OmarShehata please update CHANGES.md |
|
CHANGES.md updated, this should be ready. |
| @@ -176,6 +179,15 @@ define([ | |||
| modelData.animationsRunning = runAnimations; | |||
| } | |||
|
|
|||
| if (modelData.animationsRunning && modelData.multiplier !== multiplier) { | |||
| var animationLength = model.activeAnimations.length; | |||
There was a problem hiding this comment.
What exactly is an activeAnimation can an animation become inactive? And if that happens will it's multiplier setting become out of date?
There was a problem hiding this comment.
activeAnimations is an instance of ModelAnimationCollection, which keeps track of all currently playing animations. Theoretically yes if you remove an animation, and add it back in its multiplier value will be out of sync. There's no way to do this for individual animations at the Entity level though. So I'd say the solution is, if/when we implement that, to set modelData.multiplier to undefined every time an animation is added/removed, (in the same way that ModelAnimationCollection currently keeps track of when the multiplier changes to recompute duration and other things).
Or we could just loop over all animations and re-set the multiplier every frame the way it sets all other properties on model every frame.
|
Just those few comments. This is looking much cleaner. Thanks again @OmarShehata! |
|
I refactored the example to use nodeTransformations because I ran into an issue when the vehicle accelerates. The animation is going to be very jumpy because triggering a change in the I think it's now cleaner and even more robust. The trick was just to think of everything as data mapped over time, as opposed to frame-based logic. This should resolve all the issues you brought up @mramato. I renamed it to |
|
Nice! Sounds awesome. I'll try to take a look soon and get this merged this week. |
|
I totally forgot about this! Bumping it again @mramato . As long as it gets merged before the next release I'll be happy. |
|
Me too! Thanks for the reminder. |
|
Definitely looks smaller and cleaner than the original example, nice job! Just two last comments:
|
@mramato I originally tried this, but ditched this approach because, correct me if I'm wrong, but in order to get the wheel's angle at a given time In any case, I gave it a shot 052d810. The example is now slightly more complicated (and might have performance issues for very long simulation times?), but it does work for data obtained on the fly. Let me know if you see a better approach here or if you think we should publish the previous pre-computed one instead. |
|
@OmarShehata That's an excellent point I hadn't thought of, and you are correct that performance would be a real problem with the latest approach. I started to think of other ways you could get a wheel rotation without having to step through previous values (like using tire circumference and total distance traveled) but nothing I thought of would work well without some sort of sample. I would recommend undoing 052d810 and going back to the pre-compute way for now, then I'll merge. We can always deal with additional complexity if users need help with it in the future. Thanks for entertaining my idea. |
|
It will show up if you haven't actually merged upstream master into your branch. Either way, it's not a big deal. Thanks again! |
|
Congratulations on closing the issue! I found these Cesium forum links in the comments above: https://groups.google.com/d/msg/cesium-dev/4wVIah9ZAuk/A39Gi6POBAAJ If this issue affects any of these threads, please post a comment like the following:
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
Cesium currently only supports time based animation. This can be inconvenient if the phase of the animation is related to something other than time (eg distance along a path of an object moving at a variable speed). This came up before in CesiumGS#7361, but the author was persuaded that it was better to use nodeTransformations to explicitly control the model. That was (just) doable with that example, because there were just 3 pairs of wheels, all of which needed the exact same, relatively trivial, transformations. The proposed solution was also cumbersome, relying on modifying `multiplier` on the fly, with the downside that modifying multiplier also reset the phase of the animation. My model has a bicycle chain with 90+ links, two wheels, two chain rings (with different rates of rotation, which are both different again from the rate of rotation of the wheels), two drive arms, and a rider. Using nodeTransformations would be extremely complex, and any changes to the model would require updating all the associated javascript to match the model - and on the site where I use the model, I actually use different models for different types of activity - so I want to just be able to drop in a new model, and have it "just work" (to make it work, I have to ensure that the animation's "duration" in seconds is equal to the distance in meters the model would move during one iteration of the animation). This adds an animationTime property to ModelAnimation. If set, it's used by ModelAnimationCollection.update to determine the localAnimationTime, rather than using the current clock time. I also added a manualAnimation property to ModelAnimationCollection so we can still do the short circuit exit from ModelAnimationCollection.update. The new sandcastle example is just a clone of Time Dynamic Wheels, rewritten to use a more complex model, and the new functionality.
Cesium currently only supports time based animation. This can be inconvenient if the phase of the animation is related to something other than time (eg distance along a path of an object moving at a variable speed). This came up before in CesiumGS#7361, but the author was persuaded that it was better to use nodeTransformations to explicitly control the model. That was (just) doable with that example, because there were just 3 pairs of wheels, all of which needed the exact same, relatively trivial, transformations. The proposed solution was also cumbersome, relying on modifying `multiplier` on the fly, with the downside that modifying multiplier also reset the phase of the animation. My model has a bicycle chain with 90+ links, two wheels, two chain rings (with different rates of rotation, which are both different again from the rate of rotation of the wheels), two drive arms, and a rider. Using nodeTransformations would be extremely complex, and any changes to the model would require updating all the associated javascript to match the model - and on the site where I use the model, I actually use different models for different types of activity - so I want to just be able to drop in a new model, and have it "just work" (to make it work, I have to ensure that the animation's "duration" in seconds is equal to the distance in meters the model would move during one iteration of the animation). This adds an animationTime property to ModelAnimation. If set, it's used by ModelAnimationCollection.update to determine the localAnimationTime, rather than using the current clock time. I also added a manualAnimation property to ModelAnimationCollection so we can still do the short circuit exit from ModelAnimationCollection.update. The new sandcastle example is just a clone of Time Dynamic Wheels, rewritten to use a more complex model, and the new functionality.
Cesium currently only supports time based animation. This can be inconvenient if the phase of the animation is related to something other than time (eg distance along a path of an object moving at a variable speed). This came up before in CesiumGS#7361, but the author was persuaded that it was better to use nodeTransformations to explicitly control the model. That was (just) doable with that example, because there were just 3 pairs of wheels, all of which needed the exact same, relatively trivial, transformations. The proposed solution was also cumbersome, relying on modifying `multiplier` on the fly, with the downside that modifying multiplier also reset the phase of the animation. For more complex models, with less uniform animations, this approach isn't really doable - especially if you want the same code to work for multiple models. This adds an animationTime function to ModelAnimation. If set, it's used by ModelAnimationCollection.update to compute the localAnimationTime, rather than using the current clock time. I also added an animateWhilePaused property to ModelAnimationCollection. When false (the default), we continue to do the short circuit exit from ModelAnimationCollection.update when the scene time hasn't changed. When true, a suitable animationTime function can continue to animate the model, even when scene time is paused. The new sandcastle example is just a clone of Time Dynamic Wheels, rewritten to use Cesium_Man.glb, and the new functionality.
Cesium currently only supports time based animation. This can be inconvenient if the phase of the animation is related to something other than time (eg distance along a path of an object moving at a variable speed). This came up before in CesiumGS#7361, but the author was persuaded that it was better to use nodeTransformations to explicitly control the model. That was (just) doable with that example, because there were just 3 pairs of wheels, all of which needed the exact same, relatively trivial, transformations. The proposed solution was also cumbersome, relying on modifying `multiplier` on the fly, with the downside that modifying multiplier also reset the phase of the animation. For more complex models, with less uniform animations, this approach isn't really doable - especially if you want the same code to work for multiple models. This adds an animationTime function to ModelAnimation. If set, it's used by ModelAnimationCollection.update to compute the localAnimationTime, rather than using the current clock time. I also added an animateWhilePaused property to ModelAnimationCollection. When false (the default), we continue to do the short circuit exit from ModelAnimationCollection.update when the scene time hasn't changed. When true, a suitable animationTime function can continue to animate the model, even when scene time is paused. The new sandcastle example is just a clone of Time Dynamic Wheels, rewritten to use Cesium_Man.glb, and the new functionality.
This is for #7295.
I've piggybacked off of the Multi-Part CZML example because it was the most relevant one. I didn't want to add a new Sandcastle example because I'm not sure that it's relevant to enough users, and I think too many Sandcastle examples make it hard to find things. I can see this being its own example if it's all about manipulating animations though.
The only thing we "lose" here is using the ground vehicle instead of the Cesium milk truck. I used it because it's easier to see the wheels spin on it.
In order to do this I had to expose an option to control animation speed through the Entity API. Initially I wanted to expose either the
Modelobject or at least theModelAnimationCollectionthrough the Entity API, but that means it starts out undefined, and only gets set after the update step, and ModelGraphics doesn't seem to have a notion of "ready".The one thing I don't like is that if you constantly update the
speedupproperty the animation keeps skipping. That's because the scene time controls the absolute position of the animation, as opposed to it just controlling how big the step is. I was going to refactor sospeedupjust scaled the delta (that way, negative speeds are allowed and the animation just goes backwards) but that approach means if you jump to a specific time the model animation may not always be at the same spot in that time. So I think this structure is intentional, we just have to not update it every frame.@lilleyse do you want to take a first review on it?
TODO
speedupAnimationsproperty.