virtual-world-framework / vwf

Virtual World Framework
https://virtual.wf
Other
142 stars 53 forks source link

Branch/no tick interp #451

Closed rchadwic closed 9 years ago

rchadwic commented 9 years ago

Tickless interpolation for view/threejs. @eric79, can you re-merge in the fix for the navObject?

eric79 commented 9 years ago

Awesome! Thank you for this. I've got a bunch of other things (unfortunately, not VWF-related) on my plate right now, but I will try to merge in the navObject changes as soon as I have a chance.

rchadwic commented 9 years ago

No Prob. I'd do it, if you could point me at a good test scene to exercise it. This is tested with duck,plane,and pong, but I don't think any of those use the navobject. Also, you happen to have a scene that uses animationFrame?

davideaster commented 9 years ago

GitHub has some hints on the pull request page at the link in the "You can also merge branches on the command line" sentence.

Step 1: From your project repository, check out a new branch and test the changes.

git checkout -b adlnet-branch/no-tick-interp development
git pull https://github.com/adlnet/vwf.git branch/no-tick-interp

Step 2: Merge the changes and update on GitHub.

git checkout development
git merge --no-ff adlnet-branch/no-tick-interp
git push origin development

You could also add https://github.com/adlnet/vwf.git as a remote named adlnet and check out adlnet/branch/no-tick-interp.

eric79 commented 9 years ago

Huh ... we seem to have lost @scottnc27603 's original question that led to @davideaster 's response so it seems like a bit of a comical non sequitur. :smile:

scottnc27603 commented 9 years ago

Haha, I finally figured it out, so I went ahead and deleted the message. I finally found the commit on adl's vwf repo. I ended up setting up adl as a remote.

scottnc27603 commented 9 years ago

I removed all of the Object3 nodes from being stored all of the time. Now the nodes are added to the map when an animation ends and begins. I've gotten the node3 animations( to, and by ) working well, but I need to test the frame animations.

scottnc27603 commented 9 years ago

After looking at this code for a while now, why can't the view just drive frame based animations? I'm assuming that the answer is that the model would not be shared across all clients? I don't believe that would be true though. Getting the model state of the animation at time t would result in the same model state across all clients. animationTime, which is used to calculate the current frame, is based off of this.time and that is the same across all clients. One issue might be that the events could be sent at different times, so I guess that is a potential problem. Thoughts? Getting and setting the currentFrame requires 'vwf' from the view since the frame calculation is stored in the model, and not the threejs driver(threejs).

davideaster commented 9 years ago

I think that's right. The model driver just needs to ensure that the property being animated returns the correct value for the current time when the application asks for it. Animations can still occur in the model in the abstract--and in the view at 60 fps--without propagating animation increments through the model.

The node3 animation implementations are there to ensure that animations run regardless of which drivers are installed. But a driver could implement those method calls itself and perform the animations without ticking through the model. The only difference would be that property-changed events wouldn't occur on each increment. The model would still get the right answer every time it chose to look at the property being animated though.

scottnc27603 commented 9 years ago

A couple of other issues have come up.

  1. The properties 'playing' and 'looping' in node3 are never used(possibly 'speed' as well). Should they be removed?
  2. The view really needs to know if the animation is a frame based animation. Currently, both frame based and transform interpolation animations are started, and stopped with the same method, animationPlay. I'm trying to clean up the list of each style of animation as they start and stop, but in the view driver I can't tell the difference between the two types of animations. If I start a transform interpolation of an object that is playing a framed based animation, then bad things occur.
davideaster commented 9 years ago
  1. Sure.
  2. Doesn't the model side do something when it starts a frame-based animation that would allow you to recognize which ones the driver is running? On animationPlay, the driver either recognizes the animation as one it implements and starts to manage it, or it doesn't and ignores it. I would think that would give the view side enough information to go on. We don't have any support for blending animations, so we just need to avoid running interpolated transforms and frame-based animations at the same time.
scottnc27603 commented 9 years ago

The model knows if the object has an animation attached, but I can't find a way to tell in the driver when I need to know. The property 'animationTimeUpdated' is how the model updates threejs, but that property is set in both cases. The animation component doesn't know either. The only way to tell is by the method that is called and that only works for when a transform interpolations starts. The view needs to know when each starts and stops. The only thing I can think of is to have the transform interpolation add an additional parameter to the animationPlay. That brings up an entire other issue, will the current setup actually support executing a transform interpolation and a frame animation at the same time. I don't think it will.

scottnc27603 commented 9 years ago

When I run the callMethod in the console below, the order of the methods called is listed below(there are a few other methods that are left out).

vwf.callMethod( "index-vwf:1-85-duck", "translateBy", [ [ 0, -1000, 0 ], 1 ] ) event: animationStarted index-vwf:1-85-duck model.callingMethod( index-vwf:1-85-duck, animationPlay ) view.calledMethod( index-vwf:1-85-duck, animationPlay ) model.callingMethod( index-vwf:1-85-duck, translateBy ) view.calledMethod( index-vwf:1-85-duck, translateBy ) event: animationStopped index-vwf:1-85-duck

Is there anyway a render() can be called in the view between the methods above?

davideaster commented 9 years ago

No. JavaScript is single-threaded, so unless there's an async operation or a future call somewhere in the call thread, the thread runs to completion and returns to the browser before the browser executes a render, setTimeout handler, UI event handler, or anything like that.

There is a gap before animationStopped though, isn't there? That should happen 1 second after the animation starts.

rchadwic commented 9 years ago

Hey, sorry for leaving such a long thread without comment. I've been traveling. This implementation includes interpolation for the animationFrame property, but on for the property itself. You'll actually see in here that, since there is not good way to trigger the set animationFrame logic in the model driver without going through VWF, we touch the kernel, which is a big no-no. I have another layer where it makes more sense to set this property, but maybe we just remove interpolation of the animationFrame property, as it was before.

rchadwic commented 9 years ago

Also, Davids point above about moving transforms and animating models is spot-on. In Cosmos3D, every bone in the character rig is attached to a VWF node, but top level avatar plays an animation that moves their transforms.

I think that the solution here should not be attempting to interpolate the transforms of the bones because there is no setProperty for their transforms.

Scott - you're right. I've forgotten to merge something here - that lastTransformStep should be keeping track of the last time a setProperty was called for the transform of an object, allowing this to ignore objects that are not moving. I'll need to add another commit.

scottnc27603 commented 9 years ago

Ok, thanks. I'll check it out. I ended up stopping the interpolation on 'animationStopped'. I think that causes the object to snap to the last position. I also added the nodes map to the shared state, because of the way we store the transforms in the model. We had issues requesting the transform of any object that was being animated, so that nodes map could be helpful in the model.

allisoncorey commented 9 years ago

This PR is worked in #452 Closing.

rchadwic commented 9 years ago

Ah, bad news. I need to push another commit to this PR. undefined + 1 == NaN, which is less than 1. Thus, the code will run some math for every object if those objects never see a setProperty for transform. This has massive performance implications for Cosmos. lastTransformStep needs to initialize to 0.

allisoncorey commented 9 years ago

Ok, cool. @scottnc27603 pulled these changes over to #452 We'll want to move your new commit over there as well.

allisoncorey commented 9 years ago

@rchadwic I have moved all of the no-tick-interp code over to branch/tickless-model. We are now using that branch/PR (#450) for testing and solving remaining issues. Let us know if you have any new commits and we can add them there. Thanks!