-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Camera input mapping system with backward-compatible legacy flag support #18379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
georginahalpern
wants to merge
52
commits into
BabylonJS:master
Choose a base branch
from
georginahalpern:inputMapperBackCompat
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 35 commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
5500624
cmd key and shift key
7502ed6
alt
df67ac6
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js
e8c823d
merge
00f14d6
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js
2bfee09
input plan
81fe852
architecture plan and adopting to geocam
885289e
introduce arcrotate and simplify handlers api
744b7f1
nonpartial
055df6f
remove partial
3711be5
update doc
caf208a
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
078af0e
Task 01: Add resetInputMap() to CameraMovement base class and subclasses
4016fa6
Task 02: Add useMovementSystem flag to ArcRotateCamera and wire compu…
d5cb4fe
Task 03: Map legacy deprecated flags to inputMap modifications
9903026
Task 04: Add unit tests for CameraMovement base class and ArcRotateCa…
e32e184
merge
c4e41e7
add shift, separate modifiers
85a06f0
allow for configurable modifiers
cee59d0
negation confusion
261412f
rotate direction changes
b65423d
continue iterating and making improvements to api
2196e8c
applypandelta extracted
e48a14d
remove more duplication
4090e2e
speed calibration
62a6639
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
85ad317
inputmapper
ff8eb61
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
c129870
matching behavior 60fps
b8fe7d9
commit
6061bd8
merge
53888a3
latest
26885b3
Remove dev artifacts and revert package-lock.json
b5418ef
Remove specs/inputSystem dev notes
75918bd
Code review fixes (automated by code-review skill)
b59a8e1
Address PR review comments
31825cf
Fix tsc build: narrow getEntry/resolveInteraction return type by source
9480522
retrigger CI to verify viewer test stability
e240684
Add InputMapper.setInteraction() for targeted input remapping
474948e
Remove arcRotateCamera interaction tests and revert playwright config
1a023ef
Address PR review feedback from Kevin
4555093
Merge branch 'upstream-master' into inputMapperBackCompat
498cd6f
Improve inertial offset docstrings with decay coefficient explanation
f4f1ca9
Merge branch 'master' into inputMapperBackCompat
ee15bcb
rename
b0b00f2
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
922aef5
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
f6bee9d
optimization
e2a5540
obj allocation and getentries
5dce2e5
Address PR review comments from RaananW
bba5493
Merge branch 'master' of https://github.com/BabylonJS/Babylon.js into…
7fc2ad1
Fix TypeDoc errors flagged by tightened CI check (#18428)
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,8 +98,11 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
| // If zooming in, estimate the target radius and use that to compute the delta for inertia | ||
| // this will stop multiple scroll events zooming in from adding too much inertia | ||
| if (delta > 0) { | ||
| // When zoomToMouseLocation is active, the accumulating state lives in our private | ||
| // `_zoomToMouseRadiusImpulse` field. Otherwise fall back to the legacy surface. | ||
| const currentAccumulated = this.zoomToMouseLocation ? this._zoomToMouseRadiusImpulse : this.camera.inertialRadiusOffset; | ||
| let estimatedTargetRadius = this.camera.radius; | ||
| let targetInertia = this.camera.inertialRadiusOffset + delta; | ||
| let targetInertia = currentAccumulated + delta; | ||
| for (let i = 0; i < 20; i++) { | ||
| // 20 iterations should be enough to converge | ||
| if (estimatedTargetRadius <= targetInertia) { | ||
|
|
@@ -117,7 +120,11 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
| delta = this._computeDeltaFromMouseWheelLegacyEvent(wheelDelta, estimatedTargetRadius); | ||
| } | ||
| } else { | ||
| delta = wheelDelta / (this.wheelPrecision * 40); | ||
| // The inputMap entry's `sensitivity` takes precedence so consumers can tune | ||
| // wheel zoom feel declaratively; fall back to the legacy `wheelPrecision`. | ||
| const wheelEntry = this.camera.movement.input.getEntry("wheel", "zoom"); | ||
| const wheelScale = wheelEntry?.sensitivity ?? 1 / (this.wheelPrecision * 40); | ||
| delta = wheelDelta * wheelScale; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -132,10 +139,9 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
|
|
||
| this._zoomToMouse(delta); | ||
| } else { | ||
| this.camera.inertialRadiusOffset += delta; | ||
| this.camera.movement.zoomAccumulatedPixels += delta; | ||
| } | ||
| } | ||
|
|
||
| if (event.preventDefault) { | ||
| if (!noPreventDefault) { | ||
| event.preventDefault(); | ||
|
|
@@ -171,18 +177,38 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
| } | ||
|
|
||
| const camera = this.camera; | ||
| const motion = 0.0 + camera.inertialAlphaOffset + camera.inertialBetaOffset + camera.inertialRadiusOffset; | ||
| // Motion check based on our private impulse state + the legacy inertialRadiusOffset | ||
| // surface (so user code that still writes to it continues to animate). Legacy also | ||
| // included alpha/beta offsets in the motion check — keep that for compatibility so | ||
| // the hit plane stays updated while the camera rotates. | ||
| const motion = | ||
| Math.abs(this._zoomToMouseRadiusImpulse) + | ||
| this._inertialPanning.lengthSquared() + | ||
| Math.abs(camera.inertialAlphaOffset) + | ||
| Math.abs(camera.inertialBetaOffset) + | ||
| Math.abs(camera.inertialRadiusOffset); | ||
| if (motion) { | ||
| // if zooming is still happening as a result of inertia, then we also need to update | ||
| // the hit plane. | ||
| this._updateHitPlane(); | ||
|
|
||
| // Note we cannot use arcRotateCamera.inertialPlanning here because arcRotateCamera panning | ||
| // uses a different panningInertia which could cause this panning to get out of sync with | ||
| // the zooming, and for this to work they must be exactly in sync. | ||
| // Apply this frame's coupled radius + pan impulse to the camera. They must stay | ||
| // in lockstep so the cursor remains at the zoom focal point — keep pan here instead | ||
| // of routing through `arcRotateCamera.inertialPanning` which uses a different | ||
| // `panningInertia`. | ||
| camera.target.addInPlace(this._inertialPanning); | ||
| this._inertialPanning.scaleInPlace(camera.inertia); | ||
| camera.radius -= this._zoomToMouseRadiusImpulse; | ||
|
|
||
| // Framerate-independent decay — matches legacy `* camera.inertia` exactly at 60fps | ||
| // and preserves glide duration at any other fps. | ||
| const decay = camera.movement.getFrameIndependentDecay(camera.inertia); | ||
| this._inertialPanning.scaleInPlace(decay); | ||
| this._zoomToMouseRadiusImpulse *= decay; | ||
|
|
||
| this._zeroIfClose(this._inertialPanning); | ||
| if (Math.abs(this._zoomToMouseRadiusImpulse) < Epsilon) { | ||
| this._zoomToMouseRadiusImpulse = 0; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -237,19 +263,30 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
|
|
||
| private _inertialPanning: Vector3 = Vector3.Zero(); | ||
|
|
||
| /** | ||
| * Private impulse state for the zoomToMouseLocation path. Replaces the legacy use of | ||
| * `camera.inertialRadiusOffset` so that the coupled radius/pan impulses can be decayed | ||
| * with framerate-independent semantics (via `CameraMovement.getFrameIndependentDecay`) | ||
| * without disturbing the general inertialRadiusOffset back-compat surface. | ||
| */ | ||
| private _zoomToMouseRadiusImpulse: number = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The term impulse is confusing to me here, isn't this just an accumulation of delta? I think of impulse as the rate of change of acceleration, but that's not what this is right? Maybe more detail in the docstring would be helpful, particularly about the units of this value. |
||
|
|
||
| private _zoomToMouse(delta: number) { | ||
| const camera = this.camera; | ||
| const inertiaComp = 1 - camera.inertia; | ||
| // inputScale corrects per-frame impulse so that the decayed sum matches the legacy 60fps total | ||
| // at any actual framerate. At 60fps inputScale = 1 (no-op). | ||
| const inputScale = camera.movement.getFrameIndependentInputScale(camera.inertia); | ||
| if (camera.lowerRadiusLimit) { | ||
| const lowerLimit = camera.lowerRadiusLimit ?? 0; | ||
| if (camera.radius - (camera.inertialRadiusOffset + delta) / inertiaComp < lowerLimit) { | ||
| delta = (camera.radius - lowerLimit) * inertiaComp - camera.inertialRadiusOffset; | ||
| if (camera.radius - (this._zoomToMouseRadiusImpulse + delta * inputScale) / inertiaComp < lowerLimit) { | ||
| delta = ((camera.radius - lowerLimit) * inertiaComp - this._zoomToMouseRadiusImpulse) / inputScale; | ||
| } | ||
| } | ||
| if (camera.upperRadiusLimit) { | ||
| const upperLimit = camera.upperRadiusLimit ?? 0; | ||
| if (camera.radius - (camera.inertialRadiusOffset + delta) / inertiaComp > upperLimit) { | ||
| delta = (camera.radius - upperLimit) * inertiaComp - camera.inertialRadiusOffset; | ||
| if (camera.radius - (this._zoomToMouseRadiusImpulse + delta * inputScale) / inertiaComp > upperLimit) { | ||
| delta = ((camera.radius - upperLimit) * inertiaComp - this._zoomToMouseRadiusImpulse) / inputScale; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -264,9 +301,11 @@ export class ArcRotateCameraMouseWheelInput implements ICameraInput<ArcRotateCam | |
| vec.subtractToRef(camera.target, directionToZoomLocation); | ||
| directionToZoomLocation.scaleInPlace(ratio); | ||
| directionToZoomLocation.scaleInPlace(inertiaComp); | ||
|
|
||
| directionToZoomLocation.scaleInPlace(inputScale); | ||
| this._inertialPanning.addInPlace(directionToZoomLocation); | ||
|
|
||
| camera.inertialRadiusOffset += delta; | ||
| this._zoomToMouseRadiusImpulse += delta * inputScale; | ||
| } | ||
|
|
||
| // Sets x y or z of passed in vector to zero if less than Epsilon. | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great if we could avoid this non-null assertion operator, maybe store the modifiers object seperately?