Hello all,
As the title explains, we are currently having an issue where changing the lockedTarget of an instantiated ArcRotateCamera breaks the panning functionality on that camera. Google hasn’t shed any light on this. Any help would be much appreciated.
Please see the linked playground:
@PolygonalSun would be great if you could have a look soon ? and check if it is a regression ?
According to my understanding of our cameras and lockedTarget, when you provide a value for lockedTarget, it should prevent changes to the camera’s target. Since panning moves the camera’s target, it shouldn’t work by design when a lockedTarget is specified.
Just out of curiosity, is there particular reason for using lockedTarget vs target (eg. camera.target = sphere2.position.clone())? I just want to make sure that I’m fully understanding the situation.
Firstly, thank you so much for your timely help with all this.
Using target doesn’t seem to preserve alpha, beta & radius properties of the ArcRotateCamera. It purely points the camera at the target position.
Please see the following playground:
A potential solution can be found in this playground:
This second playground example clones the alpha, beta & radius across, which solves our issue. But, it would be nice to have a less verbose way of achieving this.
However, a potential bug can be seen regarding camera.target.
According to the docs, “you can set the target to a mesh and thus the target will be copied from mesh.position”.
However, if we were to do this, panning is again broken. This is the trap we fell into before we tried lockedTarget. Please see this playground:
Also, according to the docs, lockedTarget is to “Define the current target of the camera as an object or a position”. It makes no explicit mention of disabling panning.
To double check things, if we were to use “camera.lockedTarget = sphere2.position.clone();”, instead of “camera.lockedTarget = sphere2;”, panning is still disabled. Please see this playground:
There is also inconsistency in how camera.target doesn’t clone alpha, beta & radius, but camera.lockedTarget does.
As far as I’m seeing, this isn’t a regression. I’m gonna be out of the office for a few days so I won’t be able to properly dig in on this until next week. Any chance either @sebavan or @RaananW have some free cycles to look into this? If not, I’ll investigate this when I get back.
About this one, unfortunately, I can not afford to break backcompat but can propose to add a flag in the setTarget functions to prevent the verbose way ?
This sounds to be by design but I ll confirm with @Deltakosh and might introduce a new option to support it as I agree the difference of behavior is not consistent at all
I ll update the doc to precise that lockedTarget disables panning which is actually by design here.
All three points sound like good ways forward. Below are some suggestions of extra flags to complement your points. Please note the scaleRadius flag which is something that has come to mind just now.
setTarget()
Optional cloneAlphaBetaRadius: boolean = false
If cloneAlphaBetaRadius === true, clones across alpha, beta and radius properties rather than simply pointing camera at newTarget.
Optional scaleRadius: boolean = false
If cloneAlphaBetaRadius === true && scaleRadius === true && typeof newTarget === ‘AbstractMesh’ && typeof oldTarget === ‘AbstractMesh’, scales radius by the difference in bounding box size between oldTarget and newTarget.
Optional lockTarget: boolean
See [1] if you want to preserve backwards compatibility.
See [2] if you want to introduce consistency.
If false, assigns newTarget to target, null to lockedTarget and enables panning. If true, assigns newTarget to both target and lockedTarget, and disables panning.
[1] = If typeof newTarget === ‘Vector3’, defaults to false. If typeof newTarget === ‘AbstractMesh’, defaults to true.
[2] = Defaults to false.
Would you be willing to do a PR for it ?
@nima Thank you for the input, but I’m afraid I’m not following. Could you explain in more depth how this assists with preserving alpha, beta & radius values when changing target of an ArcRotateCamera, or preventing the disabling of panning when target is set to a AbstractMesh and not a Vector3?
If, you mean just apply a Vector3 rather than an AbstractMesh, isn’t that more of a workaround for setting an AbstractMesh breaking panning rather than addressing the root issue?
@sebavan I’m afraid I’m already working overtime and weekends with company projects. I would like to help, but I just don’t have the time. Anyone can feel free to take those flag suggestions and run with them.
One thing I did notice is that _targetHost seems to control whether or not panning is enabled. If passed an AbstractMesh, setTarget sets _targetHost to AbstractMesh and that seems to disable panning. If passed a Vector3, setTarget sets _targetHost to null and panning works. Following the inheritance structure, I’m not sure that lockedTarget actually restricts panning. Though, I could only take a very brief look into this.
Implementing cloneAlphaBetaRadius and scaleRadius seem straightforward. lockTarget will need looking into this _targetHost private property.
@nima Makes sense. And, agreed, your workaround of setting a Vector3 either via new Vector3() or mesh.position.clone() seems to be the only way to preserve panning currently, short of perhaps directly messing around with private properties like _targetHost.
Thinking about this more, it could be that it is intended behaviour that passing setTarget() an AbstractMesh locks the camera to said mesh. Think camera tracking the mesh. If this is the case, the docs probably need to be updated to explain this as “you can set the target to a mesh and thus the target will be copied from mesh.position” doesn’t fully explain this behaviour and can lead to other users falling into the same trap.
There is still a case for adding a flag to override this behaviour, as well as the other flags discussed.
I added the parameter for replicating the setup.
About target, this is only to set Vectors and not meshes. The property targetHost should be used to set meshes or directly the function setTarget(…) I updated the documentation about the panning limitation.
The inconsistency with LockedTarget comes from its usage in the base class TargetCamera. where targetHost is basically called LockedTarget not relying on the arcRotateCamera one.