Support all dragAxis #15844
Support all dragAxis #15844
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/15844/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/15844/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/15844/merge#BCU1XR#0 |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU (Experimental) |
| @@ -452,7 +452,7 @@ export class PointerDragBehavior implements Behavior<AbstractMesh> { | |||
|
|
|||
| // Project delta drag from the drag plane onto the drag axis | |||
| pickedPoint.subtractToRef(this.lastDragPosition, this._tmpVector); | |||
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis); | |||
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis) / (Vector3.Dot(this._worldDragAxis, this._worldDragAxis) || 1); | |||
There was a problem hiding this comment.
Seems a little unintuitive to dot a vector with itself. To me it would be a little more clear to do:
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis) / (Vector3.Dot(this._worldDragAxis, this._worldDragAxis) || 1); | |
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis) / (this._worldDragAxis.lengthSquared() || 1); | |
There was a problem hiding this comment.
This looks weird to me. What is the intended computation? get lengh of this._tmpVector projected on this._worldDragAxis ? in that case, is normalizing this._worldDragAxis just enough?
There was a problem hiding this comment.
I used the dot because this is the notation you can find in Vector projection formulas but I understand it may be more readable with lengthSquared instead.
There was a problem hiding this comment.
@CedricGuillemet Yes it could work too, let me know which solution you think is the best.
There was a problem hiding this comment.
@CedricGuillemet if I normalized it's in place, so I have to clone, I know it's a negligible impact, but maybe the lengthSquared is also more readable ?
There was a problem hiding this comment.
I was still thinking about it...And now I think it's more understandable with a normalize() ^^
| @@ -452,7 +452,7 @@ export class PointerDragBehavior implements Behavior<AbstractMesh> { | |||
|
|
|||
| // Project delta drag from the drag plane onto the drag axis | |||
| pickedPoint.subtractToRef(this.lastDragPosition, this._tmpVector); | |||
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis); | |||
| dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis) / (Vector3.Dot(this._worldDragAxis, this._worldDragAxis) || 1); | |||
There was a problem hiding this comment.
Thanks for the bugfix! Instead of dividing the dot product by the length of this._worldDragAxis squared, could you please normalize this._worldDragAxis before this line? Something like:
Vector3.NormalizeToRef(this._worldDragAxis, this._worldDragAxis);
Then this line can remain unchanged:
dragLength = Vector3.Dot(this._tmpVector, this._worldDragAxis);
Thanks again!
#15843
https://forum.babylonjs.com/t/pointerdragbehavior-dragaxis-inconstant-behavior-with-dragaxis-other-than-axis/54781/3