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

Tightening up animations #704

Closed
lexaknyazev opened this issue Sep 7, 2016 · 63 comments
Closed

Tightening up animations #704

lexaknyazev opened this issue Sep 7, 2016 · 63 comments

Comments

@lexaknyazev
Copy link
Member

  1. Specify componentType for animation accessors (suggest only FLOAT).

  2. Remove count from example.

  3. To be more explicit,

    All parameters must have the same number of elements.

    could be changed to something like

    All accessors referenced by the same animation must have the same count property.

CC @pjcozzi @javagl @lasalvavida

@lexaknyazev
Copy link
Member Author

More animation stuff:

  • TIME should be the only valid value for sampler.input #573 (comment);
  • TIME values must be sorted in ascending order.

Maybe it's useful to allow some output accessors to have less elements than TIME samples (if one channel stops before others).

@javagl
Copy link
Contributor

javagl commented Sep 7, 2016

I think whether it's called TIME or not is not sooo important - it does not have a "semantic" in that sense, and could equally be called timeKeyFrames or whatever.

The constraint of time values being monotonically increasing is already pointed out for sampler.input at https://github.com/KhronosGroup/glTF/tree/master/specification#samplerinput-white_check_mark .

(One could consider requiring it to be strictly monotonic. Handling the special case of time[n+1] == time[n] , could be fiddly and error-prone. But one would have to investigate further whether this special case is really never required)

EDIT: I mean, what should

TIME     0.0  1.0  1.0  2.0
output   3.4  5.6  7.8  9.0   

actually mean? I.e. what should the output be for time=1.0 ?

@lexaknyazev
Copy link
Member Author

I think whether it's called TIME or not is not sooo important - it does not have a "semantic" in that sense, and could equally be called timeKeyFrames or whatever.

In that case we should demand all samplers to use the same input parameter. And that parameter still has to have TIME "semantic" (even when called otherwise).

The constraint of time values being monotonically increasing is already pointed out for sampler.input.

I've overlooked it, thanks.

One could consider requiring it to be strictly monotonic. Handling the special case of time[n+1] == time[n] , could be fiddly and error-prone. But one would have to investigate further whether this special case is really never required

I'd say it otherwise: we should forbid such case or define exact behavior (like always use last output value).

@pjcozzi
Copy link
Member

pjcozzi commented Sep 7, 2016

  1. Specify componentType for animation accessors (suggest only FLOAT).

Do you mean for the examples?

  1. Remove count from example.

Yes, please open a PR into master.

TIME should be the only valid value for sampler.input #573 (comment);

Looking at the Cesium implementation, I don't think it requires this to be named TIME, but it does need to have the semantic as @javagl mentioned so I'm fine with relaxing this name requirement if you guys want.

One could consider requiring it to be strictly monotonic. Handling the special case of time[n+1] == time[n] , could be fiddly and error-prone. But one would have to investigate further whether this special case is really never required
I'd say it otherwise: we should forbid such case or define exact behavior (like always use last output value).

The suggestion is to change the wording from "monotonically increasing" to "increasing", right? I think that is fine unless there is a real use case for several key frames to have the same time.

@lexaknyazev
Copy link
Member Author

Specify componentType for animation accessors (suggest only FLOAT).

Do you mean for the examples?

I mean that FLOAT must be the only allowed animation accessor componentType (for TIME input as well as for translation/rotation/scale outputs).

Looking at the Cesium implementation, I don't think it requires this to be named TIME, but it does need to have the semantic as @javagl mentioned so I'm fine with relaxing this name requirement if you guys want.

Do you propose to state that input always refers to "time" semantic?

@pjcozzi
Copy link
Member

pjcozzi commented Sep 7, 2016

I mean that FLOAT must be the only allowed animation accessor componentType (for TIME input as well as for translation/rotation/scale outputs).

+1 from me.

Do you propose to state that input always refers to "time" semantic?

Yes, so the name could be something else, but it follows the "time" semantic.

@lexaknyazev
Copy link
Member Author

Yes, so the name could be something else, but it follows the "time" semantic.

Consider this example:

{
    "animations" : {
        "an_animation": {
            "channels": [
                {
                    "sampler": "a_sampler",
                    "target": {
                        "id": "node_id",
                        "path": "rotation",
                    },

                },
                {
                    "sampler": "b_sampler",
                    "target": {
                        "id": "node_id",
                        "path": "scale",
                    },

                }
            ],
            "parameters": {
                "TIME_a": "time_accessor_a",
                "TIME_b": "time_accessor_b",
                "rotation": "rotation_accessor",
                "scale": "scale_accessor"
            },
            "samplers": {
                "a_sampler": {
                    "input": "TIME_a",
                    "interpolation": "LINEAR",
                    "output": "rotation"
                },
                "b_sampler": {
                    "input": "TIME_b",
                    "interpolation": "LINEAR",
                    "output": "scale"
                }
            }     
        }
    }
}

Is it valid? Otherwise all inputs must reference the same parameter.

@lexaknyazev
Copy link
Member Author

A bit more general question: why do we need parameters object at all? Why not reference accessors directly?

@javagl
Copy link
Contributor

javagl commented Sep 8, 2016

The suggestion is to change the wording from "monotonically increasing" to "increasing", right? I think that is fine unless there is a real use case for several key frames to have the same time.

I think it would be "strictly increasing", as of https://en.wikipedia.org/wiki/Monotonic_function#Monotonicity_in_calculus_and_analysis

Regarding the proposal to require FLOAT: Right now, the animation.target.path is constrained to rotation, translation and scale, so they are already implicitly FLOAT. But similarly to the point above, I'm wondering whether in the future there might be paths that need a different type (but can't think of one right now). I'll still have to wrap my head around the related questions, particularly the one about the use of parameters. Right now, I'm doing some archeology by reading some related issues ( #158 (the FLOAT question is discussed there!), #143 ... )

@lexaknyazev
Copy link
Member Author

I'm wondering whether in the future there might be paths that need a different type (but can't think of one right now).

With disabled interpolation, it will be possible to animate integer properties and even enums (such as switching textures).


For 1.0.1 I propose this wording (for sampler.output):

The ID of a parameter in this animation to use as keyframe output. When parameter is used with TRS target, referenced accessor's componentType must be FLOAT.

@lexaknyazev
Copy link
Member Author

A bit more general question: why do we need parameters object at all? Why not reference accessors directly?

Looks like some time ago parameters contained its own objects (very similar to current accessors). Then they were replaced with accessor references. Does parameters have any purpose aside from 1:1 links?

@javagl
Copy link
Contributor

javagl commented Sep 9, 2016

I thought about the time constraints again

I'd say it otherwise: we should forbid such case or define exact behavior (like always use last output value).

Yes, this makes more sense. Imagine someone wants to model

  • a linear motion
  • a jump
  • a linear motion

When time[n+1]==time[n] is allowed, this can simply be written as

TIME     0.0  1.0  1.0  2.0
xPos     0.0  1.0  8.0  9.0

When time[n+1]==time[n] is not allowed, then there's no proper way to express this...

TIME     0.0  0.999999999  1.0  2.0  // yuck!
xPos     0.0  1.0          8.0  9.0

So I think that there is no need to change the current wording/spec regarding this point. It should still be time[n+1] >= time[n] (greater-than or equal). The case of time[n+1] == time[n] will involve a special case treatment, but nothing that cannot be resolved (maybe with a few explanatory words in the spec).

Regarding the parameters: It looks like this had more sense once, but then "collapsed" through the use of accessors, and ... maybe it was just an oversight to not remove it. (Although I still didn't analyze whether they carry a meaning that goes beyond the 1:1 mapping)

@lexaknyazev
Copy link
Member Author

So, do you mean this?

  • no more than two equal consecutive time values
  • first is used as the previous interpolation target, second as the following interpolation start

@lexaknyazev
Copy link
Member Author

lexaknyazev commented Sep 9, 2016

Also, time values sequence can nor begin with equal values, neither end with them.

@javagl
Copy link
Contributor

javagl commented Sep 9, 2016

Yes, I think that these constraints make sense.

Unambiguously and compactly describing all this in the spec is a bit fiddly, keeping in mind the inherent ambiguitiy that is imposed by >=. The paragraph from https://github.com/KhronosGroup/glTF/tree/master/specification#samplerinput-white_check_mark would have to be a bit more elaborate...

The values represent time in seconds with time[0] >= 0.0. The values have to be monotonically increasing, i.e. time[n+1] >= time[n]. The sequence may not start or end with consecutive equal values. There may not be more than two consecutive equal values. When time[n+1] == time[n] , then the key frame of time[n+1] will be used.

And still, the last sentence sounds fuzzy....


In general, in the implementation, one has to look up a start- and an end index for a given time to find the interpolation interval. I just played a bit with my current implementation (which does not support equal keys yet), and one indeed has to be careful

EDIT The wording in the //comments below is wrong - c.f. the following discussion

// Compute the smallest index where the currentTime is greater than or equal to TIMES[index]
int startIndex = computeStartIndex(TIMES, currentTime); 
int endIndex = startIndex + 1;

// Some special cases have to be treated here ...
if (endIndex >= TIMES.length || TIMES[startIndex] == TIMES[endIndex]) ...

but of course, it's manageable once the constraints and intended meaning are clear.

@lexaknyazev
Copy link
Member Author

One more corner case (if multiple inputs are allowed):
Can we have multiple channels with the same target (suppose no)?
If yes, can their samplers have overlapping time frames (suppose no)?

Also, if different samplers can have different inputs, accessor.count values should be equal per-sampler, not per-animation.

@RemiArnaud
Copy link
Contributor

I don't think this makes sense, having two possible values for the same time. One object cannot be at two different positions at the same time.

Regards,

-- Rémi

On Sep 9, 2016, at 4:12 AM, Marco Hutter [email protected] wrote:

I thought about the time constraints again

I'd say it otherwise: we should forbid such case or define exact behavior (like always use last output value).

Yes, this makes more sense. Imagine someone wants to model

a linear motion
a jump
a linear motion
When time[n+1]==time[n] is allowed, this can simply be written as

TIME 0.0 1.0 1.0 2.0
xPos 0.0 1.0 8.0 9.0
When time[n+1]==time[n] is not allowed, then there's no proper way to express this...

TIME 0.0 0.999999999 1.0 2.0 // yuck!
xPos 0.0 1.0 8.0 9.0
So I think that there is no need to change the current wording/spec regarding. It should still be time[n+1] >= time[n](greater-than or equal). The case of time[n+1] == time[n] will involve a special case treatment, but nothing that cannot be resolved (maybe with a few explanatory words in the spec).

Regarding the parameters: It looks like this had more sense once, but then "collapsed" through the use of accessors, and ... maybe it was just an oversight to not remove it. (Although I still didn't analyze whether they carry a meaning that goes beyond the 1:1 mapping)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@lexaknyazev
Copy link
Member Author

One object cannot be at two different positions at the same time.

@javagl proposes to treat this as a special case:
image
instead of
image

@javagl
Copy link
Contributor

javagl commented Sep 9, 2016

The problem obviously is that you would see artifacts at 5.005 in the second example. If this is not considered an issue, or there is a different solution, then that's fine.

accessor.count values should be equal per-sampler

That sounds reasonable. (Right now, I don't see any constraints on the count at all. Maybe that was the count from the example that was removed recently? ;-))

@lexaknyazev
Copy link
Member Author

That sounds reasonable. (Right now, I don't see any constraints on the count at all

It's mentioned in readme, not in schemes.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 9, 2016

@lexaknyazev please let me know if I missed answering any questions above.

I have reviewed the Cesium implementation a bit (I wrote it a few years ago, and I'm not saying that it is the spec authority, but it may give some insight into how we intended things):

Otherwise all inputs must reference the same parameter.

If this constraint doesn't limit common use cases, I think it is fine. It could make the client implementation a tad easier. The Cesium implementation currently allows each time input to be separate (not sure that case has been tested though).

Does parameters have any purpose aside from 1:1 links?

It would be a prominent breaking change from 1.0 to 1.0.1 to remove this, so we would have to confirm with @tparisi and Neil, but I am also not sure what purpose they serve at this point.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 9, 2016

Also:

For 1.0.1 I propose this wording (for sampler.output):

The ID of a parameter in this animation to use as keyframe output. When parameter is used with TRS target, referenced accessor's componentType must be FLOAT.

This is OK with me; the Cesium implementation explicitly checks for this.

@lexaknyazev
Copy link
Member Author

If this constraint doesn't limit common use cases, I think it is fine. It could make the client implementation a tad easier. The Cesium implementation currently allows each time input to be separate (not sure that case has been tested though).

Different sampling rate of each target could be better for filesize. We still need to demand equal accessor.count values for each input/output pair.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2016

If this constraint doesn't limit common use cases, I think it is fine. It could make the client implementation a tad easier. The Cesium implementation currently allows each time input to be separate (not sure that case has been tested though).

Different sampling rate of each target could be better for filesize. We still need to demand equal accessor.count values for each input/output pair.

I am also OK with this and think the filesize case is a good point.

@lexaknyazev
Copy link
Member Author

@lexaknyazev please let me know if I missed answering any questions above.

This one

One more corner case (if multiple inputs are allowed):
Can we have multiple channels with the same target (suppose no)?
If yes, can their samplers have overlapping time frames (suppose no)?


And jumps handling:
#704 (comment)
#704 (comment)

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2016

One more corner case (if multiple inputs are allowed):
Can we have multiple channels with the same target (suppose no)?

No, to keep the client simple.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 10, 2016

And jumps handling:
#704 (comment)
#704 (comment)

I am still not following the solution to the special case; it sounds like the client is supposed to add an epsilon to time[n + 1] when time[n] == time[n + 1]? If that is the case, they could be done offline to keep the client simple.

@lexaknyazev
Copy link
Member Author

Please review language in #712.
Equal times are still unaddressed. I'll update readme (also with more examples) as soon as we agree on everything here.

@javagl
Copy link
Contributor

javagl commented Sep 12, 2016

Looks fine for me.

("Different channels must have different targets": One could now start (or, admittedly: continue) nitpicking and arguing whether this refers to a single animation, or globally to all animations, or how much flexibility should be allowed here. As far as I see, right now, there's nothing to prevent two animations from animating the same node property - and in fact, that would be OK, as long as the time spans that are covered by the inputs of every animation channel target are disjoint over all animations...)

@lexaknyazev
Copy link
Member Author

Same targets within one animation must be definitely forbidden.
Same targets for different animations could be possible even if they cover the same time spans, since nothing requires playing all animations at once (maybe we need to elaborate a bit more here).

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

Same targets within one animation must be definitely forbidden.
Same targets for different animations could be possible even if they cover the same time spans, since nothing requires playing all animations at once (maybe we need to elaborate a bit more here).

@lexaknyazev could you please make this more explicit as part of #712?

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

I would prefer having strict > and STEP, so we don't reduce functionality from 1.0.0 level.

Strict > is OK with me. As for STEP, do we know if any WebGL engines actually implemented the special case? I don't think Cesium did. If none of them did, perhaps we should hold off on STEP.

@lexaknyazev
Copy link
Member Author

Same targets within one animation must be definitely forbidden.
Same targets for different animations could be possible even if they cover the same time spans, since nothing requires playing all animations at once (maybe we need to elaborate a bit more here).

@lexaknyazev could you please make this more explicit as part of #712?

Do we have any implementation guideline on animations start (such as "always play everything", or "play only on-demand")?

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

Do we have any implementation guideline on animations start (such as "always play everything", or "play only on-demand")?

Do you think that is something glTF can explicitly define? I think it is too dependent on the model to make a general rule.

@lexaknyazev
Copy link
Member Author

If we allow overlapping keyframe inputs with the same target, it's inevitable. Otherwise we can state that behavior is undefined in such case.

@lexaknyazev
Copy link
Member Author

As for STEP, do we know if any WebGL engines actually implemented the special case?

I'm not 100% sure, but looks like three.js is one of them:
https://github.com/mrdoob/three.js/blob/master/src/math/interpolants/DiscreteInterpolant.js

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

As for STEP, do we know if any WebGL engines actually implemented the special case?
I'm not 100% sure, but looks like three.js is one of them:
https://github.com/mrdoob/three.js/blob/master/src/math/interpolants/DiscreteInterpolant.js

@tparisi can you confirm

@lexaknyazev or can we test with a sample model?

@pjcozzi
Copy link
Member

pjcozzi commented Sep 13, 2016

If we allow overlapping keyframe inputs with the same target, it's inevitable. Otherwise we can state that behavior is undefined in such case.

Is this related to auto start all animations or not? Seems like a separate issue that we could disallow.

@lexaknyazev
Copy link
Member Author

Is this related to auto start all animations or not? Seems like a separate issue that we could disallow.

Let's say we have two animations with the same target and overlapping (or even exact) timeranges.
There is no right answer for runtime what to do with them.

Since we allow different inputs, there is no need in multiple animation objects for simple usecases.
So to keep spec simple, we can state that if an asset contains only one animation object, it could be started automatically. In case of multiple animation objects runtime behavior is undefined.

@lexaknyazev
Copy link
Member Author

As for STEP, do we know if any WebGL engines actually implemented the special case?

I'm not 100% sure, but looks like three.js is one of them:
https://github.com/mrdoob/three.js/blob/master/src/math/interpolants/DiscreteInterpolant.js

@tparisi can you confirm

@lexaknyazev or can we test with a sample model?

Just tested with three.js own format: (AnimationTrack object):

{
  "type": "vector3",
  "interpolation": 2300, // <-- InterpolateDiscrete const
  "keys": [/* data */],
  "name": "cylinder.position"
}

@pjcozzi
Copy link
Member

pjcozzi commented Sep 15, 2016

So to keep spec simple, we can state that if an asset contains only one animation object, it could be started automatically. In case of multiple animation objects runtime behavior is undefined.

I don't think the glTF spec can require that an engine start animations even if there is only one animation. It depends on the application. For example, if the only animation for an aircraft is to lower its landing gear, we would want to start that at a specific app simulation time even if it is time "0" in the local asset timeline.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 15, 2016

Just tested with three.js own format: (AnimationTrack object):

OK, if this is in three.js, let's go with it.

@lexaknyazev
Copy link
Member Author

I don't think the glTF spec can require that an engine start animations even if there is only one animation.

Does it mean that "reference" behavior is to not auto-start any animation?

@pjcozzi
Copy link
Member

pjcozzi commented Sep 15, 2016

Does it mean that "reference" behavior is to not auto-start any animation?

It would mean to do whatever the application wants. Mac Preview, for example, will play all the animations when you double click a COLLADA file. Visual Studio (IIRC) will not. I just don't think the asset format spec can define this for all types of assets and apps.

@lexaknyazev
Copy link
Member Author

My question was about "preview" behavior.
Now, you can drag-n-drop model into Cesium viewer and see animation.

@pjcozzi
Copy link
Member

pjcozzi commented Sep 15, 2016

Cesium animates by default because many of our models want that (propellers on aircrafts, wheels on trucks, etc.), but I do not think that Cesium's behavior is the behavior for all apps. Have you checked other 3D formats? I suspect none of them define to play the animations on load or not. An asset has a local animation timeline and it is up to the application to map that to the application's timeline, which, for example, may be paused on load.

@javagl
Copy link
Contributor

javagl commented Sep 15, 2016

Sorry, maybe I'm overlooking something here, but: Isn't the question about the auto-start similar to the question of "which camera is active by default"? It's solely controlled by the app, and no information about this is contained in the glTF...

Beyond that, I'm rather with Patrick here: The animation timeline somehow has to be "embedded" into a global timeline, and this should be in the responsibility of the application.


In fact, this may already raise many questions. Imagine an app that loads 2 glTFs. The first glTF contains an animation that starts at "10 seconds". The second glTF contains an animation that starts at "20 seconds", but is loaded 5 seconds after the first one - for me it's not clear what the behavior should be here, and think that it could be difficult (and dangerously constraining) to impose requirements here.

@lexaknyazev
Copy link
Member Author

Think of gltf-emoji example. How should a random glTF asset embedded into webpage behave (e.g., HTML5 audio/video elements have autostart property).

@pjcozzi
Copy link
Member

pjcozzi commented Sep 15, 2016

For gltf-emoji, they may play by default or even pass the option on to the user. This is really outside the scope of the spec; if we try to impose something I'm sure that apps will ignore it so that they can get the behavior they specifically need.

@lexaknyazev
Copy link
Member Author

Fine.
So animations is just a collection of animation objects without any other implications (such as order of playing, auto-start, loops, etc...).

@lexaknyazev
Copy link
Member Author

See 1209c51.

@pjcozzi
Copy link
Member

pjcozzi commented Jun 15, 2017

Updated in #826

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants