OrbitControls: Fix offscreen and cleanup

[RenaudRohlinger]

#27446 broke the offscreen canvas support (yes it is possible to use controls such as OrbitControls in workers by using virtual dom elements).

Also code syntax wasn't correct so I cleaned up the file to align with the lint rules.

This contribution is funded by Utsubo

[sciecode]

Attaching the ctrl intercept to the document and not domElement was done with a specific purpose of keeping the event listeners higher-up in the dom hierarchy, so stopPropagation calls wouldn't interfere with the the underlying logic.

This PR's solutions is not sufficient to guarantee correct interception, as can be shown in the following webgl_controls_map example

By clicking any of the options in UI and holding ctrl while scrolling, you can notice the zoom speed gets much faster. This is because the GUI is above domElement in the hierarchy and stops the propagation of the events controlActive logic.

This is part of the reason of why I was unsure about using this solution in the first place, it is finnicky.

#27446 broke the offscreen canvas support (yes it is possible to use controls such as OrbitControls in workers by using virtual dom elements).

Are you able to provide a bit more information on how it broke, and perhaps an example of the usage so we can see if there's a different solution here?

[RenaudRohlinger]

I see, would it work if instead of document we were using domElement.parentNode then? Otherwise it is also possible to just wrap the document with the condition to check if it is defined in the current context typeof document !== 'undefined'.

The current issue is that document is not available in web workers. /cc @sciecode

[sciecode]

I see, would it work if instead of document we were using domElement.parentNode

I'm afraid parentNode is also not sufficient, however Node.getRootNode might be exactly what we need in this scenario.

We could try scope.domElement.getRootNode().addEventListener ..., it should give us HTMLDocument/iFrame on standard DOM, and the highest available DOMNode on web workers hierarchy.

[RenaudRohlinger]

scope.domElement.getRootNode() now returns document in a normal context.
I make sure scope.domElement.getRootNode is defined as in some cases, if using a virtual element for example, it might be a hard-coded method.

@sciecode can you confirm if the behavior is now correct please?
https://raw.githack.com/renaudrohlinger/three.js/orbitcontrols-offscreen/examples/misc_controls_orbit.html

[sciecode]

Sure, the behaviour on main thread will be correct. However it may not be correct when using a ProxyDispatcher element for OffscreenCanvas. We don't want to make it optional, we want to make it mandatory (no if).

If you are implementing your virtual handler based on the manual guide, you'll need to make some changes:

Worker

• The ElementProxyReceiver needs to implement getRootNode() which returns a member dispatcher rootProxy.
• Add a new method handleRootEvent that dispatches the incoming event from rootProxy and not from this.
• Create a new type of web-worker message root-event, and modify ProxyManager so that when it receives a root-event it will call handleRootEvent.

Main-Thread

• Add a new keydown event-listener to render.domElement.getRootNode() that posts this new root-event message to the worker.

Everything should work as intended, afterwards.

I'll make these changes to the manual soon.

[RenaudRohlinger]

Alright, I removed the condition and added a comment to prevent developers from replacing the getRootNode() by document in the future:

const document = scope.domElement.getRootNode(); // offscreen canvas compatibility

Looking forward to your PR on the manual @sciecode, thanks for that.

[sciecode]

Thank you ❤️

I'll try to get the Manual changes done over the weekend, and will tag you when the PR is done.

[ben-fly2success]

Thank you ❤️

I'll try to get the Manual changes done over the weekend, and will tag you when the PR is done.

@sciecode have you made the change somewhere ? Can't see it in last release.

[davidwinter]

@sciecode @RenaudRohlinger Thanks for everyones work with this. It would be great to have the manual updated with the required changes. As it stands, following the manual does not work, as I'm encountering the issue with getRootNode not existing. I tried to follow along with the comments but I'm a little stuck in my understanding. Any help would be much appreciated.

Also, @RenaudRohlinger are you using an alternative implementation different to that of the manual? Or you were able to implement the changes that were mentioned above?