-
Notifications
You must be signed in to change notification settings - Fork 367
Add double-tap zoom for Environment Controls #1439
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,6 +104,9 @@ export class EnvironmentControls extends EventDispatcher { | |
| this.adjustHeight = true; | ||
| this.enableDamping = false; | ||
| this.dampingFactor = 0.15; | ||
| this.enableDoubleTapZoom = true; | ||
| this.doubleTapZoomScale = 2.0; | ||
| this.doubleTapAnimationDuration = 0.3; | ||
|
|
||
| this.fallbackPlane = new Plane( new Vector3( 0, 1, 0 ), 0 ); | ||
| this.useFallbackPlane = true; | ||
|
|
@@ -127,6 +130,11 @@ export class EnvironmentControls extends EventDispatcher { | |
| this.zoomPoint = new Vector3(); | ||
| this.zoomDelta = 0; | ||
|
|
||
| // double-tap state | ||
| this._lastTapTime = 0; | ||
| this._lastTapPoint = new Vector2(); | ||
| this._doubleTapAnim = null; // stores { startTime, startValue, targetValue, zoomPoint } | ||
|
|
||
| // fields used for inertia | ||
| this.rotationInertiaPivot = new Vector3(); | ||
| this.rotationInertia = new Vector2(); | ||
|
|
@@ -431,6 +439,16 @@ export class EnvironmentControls extends EventDispatcher { | |
|
|
||
| } | ||
|
|
||
| // Check for double-tap before deleting pointer | ||
| const isTouch = pointerTracker.getPointerType() === 'touch'; | ||
| const isSingleTouch = pointerTracker.getPointerCount() === 1; | ||
|
|
||
| if ( this.enableDoubleTapZoom && isTouch && isSingleTouch ) { | ||
|
|
||
| this._detectDoubleTap( pointerTracker ); | ||
|
|
||
| } | ||
|
|
||
| pointerTracker.deletePointer( e ); | ||
|
|
||
| if ( | ||
|
|
@@ -442,7 +460,13 @@ export class EnvironmentControls extends EventDispatcher { | |
|
|
||
| } | ||
|
|
||
| this.resetState(); | ||
| // Don't reset state if we're animating a double-tap zoom | ||
| if ( ! this._doubleTapAnim ) { | ||
|
|
||
| this.resetState(); | ||
|
|
||
|
Comment on lines
+463
to
+467
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. Why is this necessary? It doesn't seem to impact anything relating to the animation |
||
| } | ||
|
|
||
| this.needsUpdate = true; | ||
|
|
||
| }; | ||
|
|
@@ -687,27 +711,33 @@ export class EnvironmentControls extends EventDispatcher { | |
|
|
||
| // update the actions | ||
| const inertiaNeedsUpdate = this._inertiaNeedsUpdate(); | ||
| const adjustCameraRotation = this.needsUpdate || inertiaNeedsUpdate; | ||
| if ( this.needsUpdate || inertiaNeedsUpdate ) { | ||
| const doubleTapNeedsUpdate = this._updateDoubleTapAnimation(); | ||
| const adjustCameraRotation = this.needsUpdate || inertiaNeedsUpdate || doubleTapNeedsUpdate; | ||
| if ( this.needsUpdate || inertiaNeedsUpdate || doubleTapNeedsUpdate ) { | ||
|
|
||
| const zoomDelta = this.zoomDelta; | ||
|
|
||
| this._updateZoom(); | ||
| this._updatePosition( deltaTime ); | ||
| this._updateRotation( deltaTime ); | ||
| // Don't process normal zoom/position/rotation while animating double-tap | ||
| if ( ! this._doubleTapAnim ) { | ||
|
|
||
| this._updateZoom(); | ||
| this._updatePosition( deltaTime ); | ||
| this._updateRotation( deltaTime ); | ||
|
|
||
| } | ||
|
Comment on lines
+721
to
+727
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. This guard shouldn't be needed, either. If the zoom animation has begun / is animating then the mouse / touch should be up and there should be no way for these functions to do anything |
||
|
|
||
| if ( state === DRAG || state === ROTATE ) { | ||
|
|
||
| _forward.set( 0, 0, - 1 ).transformDirection( camera.matrixWorld ); | ||
| this.inertiaTargetDistance = _vec.copy( pivotPoint ).sub( camera.position ).dot( _forward ); | ||
|
|
||
| } else if ( state === NONE ) { | ||
| } else if ( state === NONE && ! this._doubleTapAnim ) { | ||
|
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. Same here - this check should not be needed. if the double tap has begun then all the residual inertia field should have been reset. Otherwise we'll run into a case where the "inertia" just begins again once the animation completes. |
||
|
|
||
| this._updateInertia( deltaTime ); | ||
|
|
||
| } | ||
|
|
||
| if ( state !== NONE || zoomDelta !== 0 || inertiaNeedsUpdate ) { | ||
| if ( state !== NONE || zoomDelta !== 0 || inertiaNeedsUpdate || doubleTapNeedsUpdate ) { | ||
|
|
||
| this.dispatchEvent( _changeEvent ); | ||
|
|
||
|
|
@@ -1114,6 +1144,112 @@ export class EnvironmentControls extends EventDispatcher { | |
|
|
||
| } | ||
|
|
||
| _detectDoubleTap( pointerTracker ) { | ||
|
|
||
| const currentPoint = new Vector2(); | ||
|
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. Lets use an existing scratch variable for this. |
||
| pointerTracker.getLatestPoint( currentPoint ); | ||
|
|
||
| const now = performance.now(); | ||
| const timeSinceLastTap = now - this._lastTapTime; | ||
| const moveDistance = pointerTracker.getMoveDistance(); | ||
|
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. "getMoveDistance" seems to always be "0" because the "pointerup" event is fired without any "pointerMove" events on the same frame. And the "pointerTracker.updateFrame()" call in "update()" resets the distances. Have you checked if this is returning what you expect it to? |
||
| const tapMoveThreshold = 10; | ||
| const doubleTapTimeThreshold = 300; | ||
|
|
||
| const isDoubleTap = timeSinceLastTap < doubleTapTimeThreshold && | ||
| currentPoint.distanceTo( this._lastTapPoint ) < tapMoveThreshold && | ||
| moveDistance < tapMoveThreshold; | ||
|
|
||
| if ( isDoubleTap ) { | ||
|
|
||
| this._startDoubleTapZoom( currentPoint ); | ||
| this._lastTapTime = 0; // prevent triple-tap | ||
|
|
||
| } else if ( moveDistance < tapMoveThreshold ) { | ||
|
|
||
| this._lastTapTime = now; | ||
| this._lastTapPoint.copy( currentPoint ); | ||
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| _startDoubleTapZoom( tapPoint ) { | ||
|
|
||
| const { camera, domElement, raycaster } = this; | ||
|
|
||
| // Find zoom target point | ||
| _pointer.copy( tapPoint ); | ||
| adjustedPointerToCoords( _pointer, domElement, _pointer ); | ||
| setRaycasterFromCamera( raycaster, _pointer, camera ); | ||
| const hit = this._raycast( raycaster ); | ||
| const zoomPoint = hit ? hit.point.clone() : camera.position.clone().addScaledVector( raycaster.ray.direction, 100 ); | ||
|
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. Please use the scratch variables |
||
|
|
||
| // Setup animation state | ||
| const isOrtho = camera.isOrthographicCamera; | ||
| const startValue = isOrtho ? camera.zoom : camera.position.distanceTo( zoomPoint ); | ||
| let targetValue = isOrtho ? startValue * this.doubleTapZoomScale : startValue / this.doubleTapZoomScale; | ||
|
|
||
| // Clamp target value | ||
| targetValue = MathUtils.clamp( | ||
| targetValue, | ||
| isOrtho ? this.minZoom : this.minDistance, | ||
| isOrtho ? this.maxZoom : this.maxDistance | ||
| ); | ||
|
|
||
| this._doubleTapAnim = { | ||
| startTime: this.clock.getElapsedTime(), | ||
| startValue, | ||
| targetValue, | ||
| zoomPoint, | ||
| }; | ||
|
|
||
| // Reset inertia and dispatch event | ||
| this.rotationInertia.set( 0, 0 ); | ||
| this.dragInertia.set( 0, 0, 0 ); | ||
| this.dispatchEvent( _startEvent ); | ||
|
|
||
| } | ||
|
|
||
| _updateDoubleTapAnimation() { | ||
|
|
||
| if ( ! this._doubleTapAnim ) return false; | ||
|
|
||
| const { camera } = this; | ||
| const { startTime, startValue, targetValue, zoomPoint } = this._doubleTapAnim; | ||
| const elapsed = this.clock.getElapsedTime() - startTime; | ||
|
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. We should take a "deltaTime" passed into the function and thread it through the "update" function, as is done elsewhere in the class. Then the animation time can be counted down on each update. |
||
| const duration = this.doubleTapAnimationDuration; | ||
|
|
||
| // Ease-out cubic: t = 1 - (1-t)³ | ||
| const progress = Math.min( elapsed / duration, 1 ); | ||
| const t = 1 - Math.pow( 1 - progress, 3 ); | ||
| const currentValue = startValue + ( targetValue - startValue ) * t; | ||
|
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. Can we use MathUtils.lerp here? |
||
|
|
||
| if ( camera.isOrthographicCamera ) { | ||
|
|
||
| camera.zoom = currentValue; | ||
| camera.updateProjectionMatrix(); | ||
|
|
||
| } else { | ||
|
|
||
| _vec.subVectors( camera.position, zoomPoint ).normalize(); | ||
| camera.position.copy( zoomPoint ).addScaledVector( _vec, currentValue ); | ||
| camera.updateMatrixWorld(); | ||
|
|
||
| } | ||
|
|
||
| // Check completion | ||
| if ( progress >= 1 ) { | ||
|
|
||
| this._doubleTapAnim = null; | ||
| this.dispatchEvent( _endEvent ); | ||
| return false; | ||
|
|
||
| } | ||
|
|
||
| return true; | ||
|
|
||
| } | ||
|
|
||
| // returns the point below the camera | ||
| _getPointBelowCamera( point = this.camera.position, up = this.up ) { | ||
|
|
||
|
|
||
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.
Lets initialize this during initialization rather than setting it to null.