Update resolveLinksTransform() for parsed XVIZ changes

XVIZ is separating pose streams from others to better support
the scene graph.

Maintain compatibility, but support the new 'poses' field from
parsed XVIZ messages.  Also ensure we handle missing poses
and cycles.

Author: twojtasz

modules/core/src/utils/transform.js

@@ -29,30 +29,51 @@ import {COORDINATE} from '../constants';
 const DEFAULT_ORIGIN = [0, 0, 0];
 
 // Export only for testing
-export function resolveLinksTransform(links, streams, streamName) {
+export function resolveLinksTransform(links, poses, streamName) {
   const transforms = [];
-  let parentPose = links[streamName] && links[streamName].target_pose;
+
+  // If streamName has a pose entry, ensure we capture it
+  if (poses[streamName]) {
+    transforms.push(poses[streamName]);
+  }
 
   // TODO(twojtasz): we could cache the resulting transform based on the entry
   // into the link structure.
+  let missingPose = '';
+  let cycleDetected = false;
+  if (links) {
+    let parentPoseName = links[streamName] && links[streamName].target_pose;
+    const seen = new Set();
+
+    // Collect all poses from child to root
+    while (parentPoseName) {
+      cycleDetected = seen.has(parentPoseName);
+      if (cycleDetected) {
+        break;
+      }
+      seen.add(parentPoseName);
+
+      if (!poses[parentPoseName]) {
+        missingPose = parentPoseName;
+        break;
+      }
+
+      transforms.push(poses[parentPoseName]);
+      parentPoseName = links[parentPoseName] && links[parentPoseName].target_pose;
+    }
+  }
 
-  let missingPose = false;
+  if (missingPose) {
+    // TODO(twojtasz): report issue
+    return null;
+  }
 
-  // Collect all poses from child to root
-  while (parentPose) {
-    if (!streams[parentPose]) {
-      missingPose = true;
-      break;
-    }
-    transforms.push(streams[parentPose]);
-    parentPose = links[parentPose] && links[parentPose].target_pose;
+  if (cycleDetected) {
+    // TODO(twojtasz): report issue
+    return null;
   }
 
-  // Resolve pose transforms. If missingPose is true, which can happen if a
-  // persistent link is defined before normal state has been sent, ignore it
-  // TODO(twojtasz): Flag stream affected by missingPose so it can be reported
-  // by application
-  if (!missingPose && transforms.length) {
+  if (transforms.length) {
     // process from root to child
     return transforms.reduceRight((acc, val) => {
       return acc.multiplyRight(new Pose(val).getTransformationMatrix());
@@ -69,13 +90,14 @@ export function resolveLinksTransform(links, streams, streamName) {
  * streamMetadata - Anym metadata associated with the stream
  * getTransformMatrix - A callback function for when stream metadata specifieds a DYNAMIC coordinate system
  */
+/* eslint-disable complexity */
 export function resolveCoordinateTransform(
   frame,
   streamName,
   streamMetadata = {},
   getTransformMatrix
 ) {
-  const {origin, links = {}, streams, transforms = {}, vehicleRelativeTransform} = frame;
+  const {origin, links, poses, streams, transforms = {}, vehicleRelativeTransform} = frame;
   const {coordinate, transform, pose} = streamMetadata;
 
   let coordinateSystem = COORDINATE_SYSTEM.METER_OFFSETS;
@@ -105,7 +127,7 @@ export function resolveCoordinateTransform(
 
     default:
     case COORDINATE.IDENTITY:
-      modelMatrix = resolveLinksTransform(links, streams, streamName);
+      modelMatrix = resolveLinksTransform(links, poses || streams, streamName);
       break;
   }
 
@@ -126,6 +148,7 @@ export function resolveCoordinateTransform(
     modelMatrix
   };
 }
+/* eslint-enable complexity */
 
 export function positionToLngLat([x, y, z], {coordinateSystem, coordinateOrigin, modelMatrix}) {
   if (modelMatrix) {

package.json

@@ -26,14 +26,14 @@
     "@babel/preset-flow": "^7.0.0",
     "@babel/preset-react": "^7.0.0",
     "@babel/register": "^7.0.0",
+    "@deck.gl/test-utils": "^8.1.5",
+    "@luma.gl/test-utils": "^8.1.2",
+    "@probe.gl/test-utils": "^3.2.1",
     "@storybook/addon-actions": "^4.0.7",
     "@storybook/addon-knobs": "^4.0.7",
     "@storybook/addon-links": "^4.0.7",
     "@storybook/addon-options": "^4.0.7",
     "@storybook/react": "^4.0.7",
-    "@deck.gl/test-utils": "^8.1.5",
-    "@luma.gl/test-utils": "^8.1.2",
-    "@probe.gl/test-utils": "^3.2.1",
     "babel-loader": "^8.0.0",
     "coveralls": "^3.0.0",
     "enzyme": "^3.7.0",
@@ -58,5 +58,6 @@
     "copyright-check",
     "test-fast"
   ],
-  "private": true
+  "private": true,
+  "dependencies": {}
 }

test/modules/core/utils/transform.spec.js

@@ -24,13 +24,16 @@ import test from 'tape';
 import {resolveLinksTransform} from '@streetscape.gl/core/utils/transform';
 
 test('resolveLinksTransform', t => {
-  // transation links
+  // translation links
   // A -> B -> C
-  // A -> D
+  // A -> D -> E
   //
   // rotation links
   // r90 -> m10 -> DD
   // rn90 -> m20 -> CC
+  //
+  // cycle links
+  // cycleA -> cycleB -> cycleA
   const links = {
     C: {
       target_pose: 'B'
@@ -41,6 +44,9 @@ test('resolveLinksTransform', t => {
     D: {
       target_pose: 'A'
     },
+    E: {
+      target_pose: 'D'
+    },
     CC: {
       target_pose: 'm20'
     },
@@ -52,10 +58,16 @@ test('resolveLinksTransform', t => {
     },
     m20: {
       target_pose: 'rn90'
+    },
+    cycleA: {
+      target_pose: 'cycleB'
+    },
+    cycleB: {
+      target_pose: 'cycleA'
     }
   };
 
-  const streams = {
+  const poses = {
     A: {
       yaw: 0,
       pitch: 0,
@@ -72,6 +84,22 @@ test('resolveLinksTransform', t => {
       y: 20,
       z: 0
     },
+    D: {
+      yaw: 0,
+      pitch: 0,
+      roll: 0,
+      x: 0,
+      y: 0,
+      z: 0
+    },
+    E: {
+      yaw: 0,
+      pitch: 0,
+      roll: 0,
+      x: 10,
+      y: 10,
+      z: 0
+    },
     r90: {
       yaw: Math.PI / 2,
       pitch: 0,
@@ -103,19 +131,40 @@ test('resolveLinksTransform', t => {
       x: 20,
       y: 20,
       z: 0
+    },
+    cycleA: {
+      yaw: 0,
+      pitch: 0,
+      roll: 0,
+      x: 5,
+      y: 5,
+      z: 0
+    },
+    cycleB: {
+      yaw: 0,
+      pitch: 0,
+      roll: 0,
+      x: 5,
+      y: 5,
+      z: 0
     }
   };
 
   const testCases = [
-    {stream: 'C', expected: [30, 30, 0]},
-    {stream: 'D', expected: [10, 10, 0]},
-    {stream: 'E', expected: null},
-    {stream: 'CC', expected: [20, -20, 0]},
-    {stream: 'DD', expected: [-10, 10, 0]}
+    {stream: 'A', expected: [10, 10, 0]}, // no link, just pose
+    {stream: 'C', expected: [30, 30, 0]}, // 2 links, 2 poses
+    {stream: 'D', expected: [10, 10, 0]}, // 1 link, 1 pose
+    {stream: 'E', expected: [20, 20, 0]}, // 2 links, 3 poses
+    {stream: 'Z', expected: null}, // missing pose
+    {stream: 'CC', expected: [20, -20, 0]}, // rotation & translation path
+    {stream: 'DD', expected: [-10, 10, 0]}, // rotation & translation path
+    {stream: 'cycleB', expected: null} // cycle in path
   ];
 
+  // For each testcase, resolve the transform for the named stream
+  // and verify the expected
   for (const testcase of testCases) {
-    const transformTo = resolveLinksTransform(links, streams, testcase.stream);
+    const transformTo = resolveLinksTransform(links, poses, testcase.stream);
     if (transformTo) {
       const resultOrigin = transformTo.transformVector([0, 0, 0]);
 
@@ -130,7 +179,7 @@ test('resolveLinksTransform', t => {
       t.equal(
         transformTo,
         testcase.expected,
-        `missing links entry matches expected '${testcase.expected}'`
+        `null return matches expected '${testcase.expected}'`
       );
     }
   }