Added damping to OrbitControls

[WestLangley]

This has been a long-requested feature.

Some time ago, the property names of OrbitControls and TrackballControls were made to match. I have (reluctantly) continued that convention, and added the following properties to OrbitControls:

this.staticMoving = false;
this.dynamicDampingFactor = 0.2;

[WestLangley]

Thing is, these property names are dreadful, IMO, as are the following property names requiring double-negatives:

this.noZoom = false;
this.noRotate = false;
this.noPan = false;
this.noKeys = false;

Perhaps we can change them, and eliminate the double-negatives, to something like the following -- even it if results in breakage:

noZoom        ->  allowZoom (default true)
noRotate      ->  allowRotate (default true)
noPan         ->  allowPan (default true)
noKeys        ->  allowKeys (default true)
staticMoving  ->  inertia (default true)
dynamicDampingFactor  ->  inertiaFactor (default 0.2) or dampingFactor

Regarding the factor, by convention so far, small values -> less friction (longer spinning).

Suggestions welcome.

[WestLangley]

@dubejf The OrbitControls refactoring is not making sense to me. I understand your vision, and it is a noble one, but IMO the layering isn't right, as is evidenced by the numerous defineProperties required to share property names between the classes.

Plus, properties a user would care about are scattered within the file.

Were you planning on re-working this? As I mentioned elsewhere, a finite-state-machine seems the way to go, but it is not clear to me that it would be "better" than the linear structure we had before.

[mrdoob]

noZoom        ->  allowZoom (default true)
noRotate      ->  allowRotate (default true)
noPan         ->  allowPan (default true)
noKeys        ->  allowKeys (default true)
staticMoving  ->  inertia (default true)
dynamicDampingFactor  ->  inertiaFactor (default 0.2) or dampingFactor

How about enableZoom, enableRotate, enablePan...? inertia and inertiaFactor sounds great!

[WestLangley]

How about enableZoom, enableRotate, enablePan...? inertia and inertiaFactor sounds great!

That sounds good -- in a separate PR, though.

The only thing I don't like about inertiaFactor is that lower values imply more inertia.

How do you want to handle warnings -- just a console warning that the property is deprecated, or re-map the old to the new, somehow?

[erichlof]

@WestLangley
Can I put a vote in for dampingFactor rather than inertia or inertiaFactor? dampingFactor is really clear IMO as to what the input value does. A lower value means less damping, so the orbitControls will be moving about more freely. A higher damping value means you want to dampen the motion more, and therefore the orbitControls will be more restricted in motion, or harder to move because of the extra damping.

I know this relates to the concept of inertia in physics, but I have trouble remembering all my high school physics terms. :-) On the other hand, I would guess that most people intuitively know what dampening means (squashing down, restricting, or limiting).

Edit: .friction could just as easily be used to mean the same thing as .dampingFactor, but I don't know if that's the correct physical term for what's going on under the hood.

[WestLangley]

Yes, I proposed dampingFactor as an option, also.

Maybe enableDamping and dampingFactor or dampingCoefficient ?

[erichlof]

enableDamping and dampingFactor are clear and easy to spell, more so than coefficient.

[WestLangley]

@mrdoob Would you find this acceptable?

noZoom        ->  enableZoom (default true)
noRotate      ->  enableRotate (default true)
noPan         ->  enablePan (default true)
noKeys        ->  enableKeys (default true)
staticMoving  ->  enableDamping (default true)
dynamicDampingFactor  ->  dampingFactor (default 0.2)

And as I asked above,

How do you want to handle warnings -- just a console warning that the property is deprecated, or re-map the old to the new, somehow?

[mrdoob]

These names looks good to me. As per renaming approach. I think I would use getter/setter and show a warning.

Something like this: https://github.com/mrdoob/three.js/blob/dev/src/renderers/WebGLRenderer.js#L3679-L3732

[WestLangley]

Got it. Next PR.

[dubejf]

@WestLangley I had no plan to continue this refactoring until the next release. Should I prepare a patch to revert the commit that created OrbitConstraint?

The defineProperties are for backward compatibility. Otherwise, control.constraint.minPolarAngle = xxx should be the preferred method to set a limit.

I'm not sure about FSM. Could you make an example by overhauling a simpler controls?

[WestLangley]

Should I prepare a patch to revert the commit that created OrbitConstraint?

Oh, no. That is not necessary, unless you decide you want to.

FSM is very high on my interest list, but low on my critical list. So I probably will not pursue it -- unless a miracle happens. I was kind of hoping you would. : - )