OrbitControls can send nested start and end events

[nkallen]

Describe the bug

OrbitControls is an EventDispatcher. It sends 'start', 'change', and 'end' events. If you mouse wheel while rotating it will send nested 'start' and 'end' events. This is (probably) incorrect, because clients relying on this code to e.g., enable/disable functionality in their app will incorrectly reenable/re-disable things.

To Reproduce

1. Add an event listener for 'start' and 'end' events
2. Rotate, holding down the mouse button (default: left)
3. While holding down the button use the mouse wheel

Live example

• jsfiddle-latest-release

Expected behavior

It should send change events during mouse wheel, not start stop

Screenshots

If applicable, add screenshots to help explain your problem (drag and drop the image).

Platform:

• Device: [Desktop, Mobile]
• OS: [Windows, MacOS, Linux, Android, iOS]
• Browser: [Chrome, Firefox, Safari, Edge]
• Three.js version: [dev, r???]

[Mugen87]

It seems you did not provide a live example as shown in the screenshot. Let's use this one: https://jsfiddle.net/b610qohy/

[Mugen87]

Thinking about this line:

three.js/examples/jsm/controls/OrbitControls.js

Line 1029 in 38d0b96

if ( scope.enabled === false || scope.enableZoom === false || ( state !== STATE.NONE && state !== STATE.ROTATE ) ) return;

Especially state !== STATE.ROTATE. Why is this in place? Wouldn't it be better to only allow zooming if the controls are in the NONE state?

[Mugen87]

I vote to revert #8634. The reported issue is a side effect of the PR which was not clear when it was merged.

[nkallen]

I'm ok with that. If you're hesitant to make a breaking change, this will work obviously

            if (state === STATE.NONE) scope.dispatchEvent(_startEvent);

etc

[Mugen87]

I guess a note in the migration guide is sufficient.