Conversation
8b20d90 to
7172bbf
Compare
|
The V3 coordinates are wrong (at least on iOS). Changing render to fixes it so it seems like the wrong coordinate space is being used. This needs to be investigated. |
There was a problem hiding this comment.
Pull request overview
This PR adds a new, more advanced “Transformations” example to the New API example set (extracted from #2919) and updates the legacy v2 transformations example to use the same matrix-based approach for pan/pinch/rotate composition.
Changes:
- Register a new “Transformations” entry in the New API “Complicated” examples list.
- Add a new New API transformations example that composes pan/pinch/rotation/double-tap using a 4×4 matrix accumulator.
- Rework the legacy v2 transformations example to use the same matrix/origin approach (instead of simple translate/scale/rotate values).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| apps/common-app/src/new_api/index.tsx | Adds the Transformations example to the New API list. |
| apps/common-app/src/new_api/complicated/transformations/index.tsx | New complex matrix-based transformations example using hook gestures. |
| apps/common-app/src/legacy/v2_api/transformations/index.tsx | Updates legacy example to match the complex matrix/origin approach and add layout sizing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!isRotating.value && !isScaling.value) { | ||
| origin.value = { | ||
| x: -(e.anchorX - size.width / 2), | ||
| y: -(e.anchorY - size.height / 2), | ||
| }; |
There was a problem hiding this comment.
On web, anchorX/anchorY are not in the view’s local coordinate space (issue #2929), so computing origin from these values makes rotations behave incorrectly. Consider excluding this example on web or adding a web-specific conversion from window to local coordinates before using anchorX/anchorY.
| if (!isRotating.value && !isScaling.value) { | ||
| origin.value = { | ||
| x: -(e.focalX - size.width / 2), | ||
| y: -(e.focalY - size.height / 2), | ||
| }; |
There was a problem hiding this comment.
On web, focalX/focalY are not in the view’s local coordinate space (issue #2929), so this origin calculation will be incorrect and scaling will drift relative to the view. Consider excluding this example on web or adding a web-specific conversion from window to local coordinates before using focalX/focalY.
| if (!isRotating.value && !isScaling.value) { | ||
| origin.value = { | ||
| x: -(e.anchorX - size.width / 2), | ||
| y: -(e.anchorY - size.height / 2), | ||
| }; |
There was a problem hiding this comment.
On web, anchorX/anchorY are not in the view’s local coordinate space (issue #2929), so computing origin from these values makes rotations behave incorrectly. Consider excluding this example on web or adding a web-specific conversion from window to local coordinates before using anchorX/anchorY.
| if (!isRotating.value && !isScaling.value) { | ||
| origin.value = { | ||
| x: -(e.focalX - size.width / 2), | ||
| y: -(e.focalY - size.height / 2), | ||
| }; |
There was a problem hiding this comment.
On web, focalX/focalY are not in the view’s local coordinate space (issue #2929), so this origin calculation will be incorrect and scaling will drift relative to the view. Consider excluding this example on web or adding a web-specific conversion from window to local coordinates before using focalX/focalY.
| .onEnd(() => { | ||
| 'worklet'; | ||
| transform.value = applyTransformations( | ||
| translation.value, | ||
| scale.value, |
There was a problem hiding this comment.
Cleanup/commit logic is attached to .onEnd, which won’t run for cancelled/failed gestures. That can leave isRotating/transient values stuck and the matrix uncommitted. Prefer .onFinalize (or handle success in .onEnd and also reset state in .onFinalize) so the reset/commit runs for END/CANCELLED/FAILED.
| .onEnd(() => { | ||
| 'worklet'; | ||
| transform.value = applyTransformations( | ||
| translation.value, | ||
| scale.value, |
There was a problem hiding this comment.
Cleanup/commit logic is attached to .onEnd, which won’t run for cancelled/failed gestures. That can leave isScaling/transient values stuck and the matrix uncommitted. Prefer .onFinalize (or add a matching .onFinalize cleanup) so the reset/commit runs for END/CANCELLED/FAILED.
| .onEnd(() => { | ||
| 'worklet'; | ||
| transform.value = applyTransformations( | ||
| translation.value, | ||
| scale.value, |
There was a problem hiding this comment.
Cleanup/commit logic is attached to .onEnd, which won’t run for cancelled/failed gestures. If the pan is interrupted (e.g., pointer count changes during a simultaneous gesture), translation/scale/rotation may not be reset and the matrix may not be committed. Prefer .onFinalize (or add a matching .onFinalize cleanup) so cleanup runs for END/CANCELLED/FAILED.
| { name: 'Velocity Test', component: VelocityExample }, | ||
| { name: 'Chat Heads', component: ChatHeadsExample }, | ||
| { name: 'Camera', component: CameraExample }, | ||
| { name: 'Transformations', component: TransformationsExample }, |
There was a problem hiding this comment.
This example relies on rotation anchor / pinch focal coordinates being in the view’s coordinate space, which is currently not true on web (see issue #2929). As-is, the Transformations example will behave incorrectly on web; consider marking it as unsupportedPlatforms: new Set(['web']) (or applying a web-specific coordinate-space workaround) until the underlying web behavior is fixed.
| { name: 'Transformations', component: TransformationsExample }, | |
| { | |
| name: 'Transformations', | |
| component: TransformationsExample, | |
| unsupportedPlatforms: new Set(['web']), | |
| }, |
Description
Extracted new transformations example from #2919, so that the entire thing isn't blocked by #2929