coordinate system fixes for embedded graph (#768)

* change pan speed to 1 per issue #722

* correct handle scaling of graph when aspect ratio less than one

* add package lock

* add invert to our scale functions

* correctly transform to/from gl coordinates

* remove unused import

* fix naming of import

* update smoke tests

client/__tests__/e2e/data.js

@@ -26,8 +26,8 @@ export const datasets = {
     cellsets: {
       lasso: [
         {
-          "coordinates-as-percent": { x1: 0.25, y1: 0.25, x2: 0.35, y2: 0.35 },
+          "coordinates-as-percent": { x1: 0.05, y1: 0.25, x2: 0.15, y2: 0.35 },
-          count: "26"
+          count: "43"
         }
       ],
       categorical: [
@@ -91,8 +91,8 @@ export const datasets = {
         }
       },
       lasso: {
-        "coordinates-as-percent": { x1: 0.45, y1: 0.45, x2: 0.5, y2: 0.5 },
+        "coordinates-as-percent": { x1: 0.45, y1: 0.05, x2: 0.5, y2: 0.1 },
-        count: "67"
+        count: "65"
       }
     },
     scatter: {

client/__tests__/e2e/e2e.test.js

@@ -272,7 +272,7 @@ describe("scatter plot", async () => {
 
 describe("clipping", async () => {
   test("clip continuous", async () => {
-    await cxgActions.clip(data.clip.min, data.clip.max)
+    await cxgActions.clip(data.clip.min, data.clip.max);
     const histId = `histogram-${data.clip.metadata}-plot-brush`;
     const coords = await cxgActions.calcDragCoordinates(
       histId,
@@ -281,16 +281,13 @@ describe("clipping", async () => {
     await cxgActions.drag(histId, coords.start, coords.end);
     const cellCount = await cxgActions.cellSet(1);
     expect(cellCount).toBe(data.clip.count);
-    
   });
 
   test("clip gene", async () => {
     await utils.typeInto("gene-search", data.clip.gene);
     await page.keyboard.press("Enter");
-    await page.waitForSelector(
+    await page.waitForSelector(`[data-testid='histogram-${data.clip.gene}']`);
-      `[data-testid='histogram-${data.clip.gene}']`
+    await cxgActions.clip(data.clip.min, data.clip.max);
-    );
-    await cxgActions.clip(data.clip.min, data.clip.max)
     const histId = `histogram-${data.clip.gene}-plot-brush`;
     const coords = await cxgActions.calcDragCoordinates(
       histId,

client/package-lock.json

@@ -8324,6 +8324,11 @@
       "resolved": "https://registry.npmjs.org/gl-matrix/-/gl-matrix-3.0.0.tgz",
       "integrity": "sha512-PD4mVH/C/Zs64kOozeFnKY8ybhgwxXXQYGWdB4h68krAHknWJgk9uKOn6z8YElh5//vs++90pb6csrTIDWnexA=="
     },
+    "gl-vec3": {
+      "version": "1.1.3",
+      "resolved": "https://registry.npmjs.org/gl-vec3/-/gl-vec3-1.1.3.tgz",
+      "integrity": "sha512-jduKUqT0SGH02l8Yl+mV1yVsDfYgQAJyXGxkJQGyxPLHRiW25DwVIRPt6uvhrEMHftJfqhqKthRcyZqNEl9Xdw=="
+    },
     "glob": {
       "version": "7.1.3",
       "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz",

client/package.json

@@ -41,6 +41,7 @@
     "font-color-contrast": "^1.0.3",
     "fuzzysort": "^1.1.4",
     "gl-mat4": "^1.1.4",
+    "gl-vec3": "^1.1.3",
     "gl-matrix": "^3.0.0",
     "is-number": "^7.0.0",
     "key-pressed": "0.0.1",

client/src/components/graph/drawPointsRegl.js

@@ -1,5 +1,6 @@
 // jshint esversion: 6
 const mat4 = require("gl-mat4");
+const vec3 = require("gl-vec3");
 
 // opacity: https://github.com/spacetx/starfish/blob/master/viz/draw/regions.js
 
@@ -38,7 +39,20 @@ export default function(regl) {
     uniforms: {
       distance: regl.prop("distance"),
       view: regl.prop("view"),
-      projection: ({viewportWidth, viewportHeight}) => mat4.perspective([], Math.PI / 2, viewportWidth / viewportHeight, 0.01, 1000)
+      projection: ({ viewportWidth, viewportHeight }) => {
+        const aspectRatio = viewportWidth / viewportHeight;
+        let m = mat4.perspective(
+          [],
+          Math.PI / 2,
+          viewportWidth / viewportHeight,
+          0.01,
+          1000
+        );
+        if (aspectRatio < 1) {
+          m = mat4.scale(m, m, vec3.fromValues(1, 1, 1 / aspectRatio));
+        }
+        return m;
+      }
     },
 
     count: regl.prop("count"),

client/src/components/graph/graph.js

@@ -175,18 +175,19 @@ class Graph extends React.Component {
         const glScaleX = scaleLinear([0, 1], [-1, 1]);
         const glScaleY = scaleLinear([0, 1], [1, -1]);
 
-        const offset = [d3.mean(X) - 0.5, d3.mean(Y) - 0.5];
-
         for (let i = 0, { positions } = renderCache; i < nObs; i += 1) {
-          positions[2 * i] = glScaleX(X[i] - offset[0]);
+          positions[2 * i] = glScaleX(X[i]);
-          positions[2 * i + 1] = glScaleY(Y[i] - offset[1]);
+          positions[2 * i + 1] = glScaleY(Y[i]);
         }
         pointBuffer({
           data: renderCache.positions,
           dimension: 2
         });
 
-        stateChanges.offset = offset;
+        stateChanges.transform = {
+          glScaleX,
+          glScaleY
+        };
       }
 
       // Colors for each point - a cached value that only changes when
@@ -271,11 +272,11 @@ class Graph extends React.Component {
       mode !== prevState.mode ||
       stateChanges.svg
     ) {
-      const { tool, container, offset } = this.state;
+      const { tool, container, transform } = this.state;
       this.selectionToolUpdate(
         stateChanges.tool ? stateChanges.tool : tool,
         stateChanges.container ? stateChanges.container : container,
-        stateChanges.offset ? stateChanges.offset : offset
+        stateChanges.transform ? stateChanges.transform : transform
       );
     }
 
@@ -435,7 +436,7 @@ class Graph extends React.Component {
     this.setState({ pendingClipPercentiles: null });
   };
 
-  brushToolUpdate(tool, container, offset) {
+  brushToolUpdate(tool, container, transform) {
     /*
     this is called from componentDidUpdate(), so be very careful using
     anything from this.state, which may be updated asynchronously.
@@ -449,8 +450,14 @@ class Graph extends React.Component {
         if there is a selection, make sure the brush tool matches
         */
         const screenCoords = [
-          this.mapPointToScreen(currentSelection.brushCoords.northwest, offset),
+          this.mapPointToScreen(
-          this.mapPointToScreen(currentSelection.brushCoords.southeast, offset)
+            currentSelection.brushCoords.northwest,
+            transform
+          ),
+          this.mapPointToScreen(
+            currentSelection.brushCoords.southeast,
+            transform
+          )
         ];
         if (!toolCurrentSelection) {
           /* tool is not selected, so just move the brush */
@@ -477,7 +484,7 @@ class Graph extends React.Component {
     }
   }
 
-  lassoToolUpdate(tool, container, offset) {
+  lassoToolUpdate(tool, container, transform) {
     /*
     this is called from componentDidUpdate(), so be very careful using
     anything from this.state, which may be updated asynchronously.
@@ -488,7 +495,7 @@ class Graph extends React.Component {
       if there is a current selection, make sure the lasso tool matches
       */
       const polygon = currentSelection.polygon.map(p =>
-        this.mapPointToScreen(p, offset)
+        this.mapPointToScreen(p, transform)
       );
       tool.move(polygon);
     } else {
@@ -496,7 +503,7 @@ class Graph extends React.Component {
     }
   }
 
-  selectionToolUpdate(tool, container, offset) {
+  selectionToolUpdate(tool, container, transform) {
     /*
     this is called from componentDidUpdate(), so be very careful using
     anything from this.state, which may be updated asynchronously.
@@ -504,10 +511,10 @@ class Graph extends React.Component {
     const { selectionTool } = this.props;
     switch (selectionTool) {
       case "brush":
-        this.brushToolUpdate(tool, container, offset);
+        this.brushToolUpdate(tool, container, transform);
         break;
       case "lasso":
-        this.lassoToolUpdate(tool, container, offset);
+        this.lassoToolUpdate(tool, container, transform);
         break;
       default:
         /* punt? */
@@ -564,7 +571,8 @@ class Graph extends React.Component {
     accounting for current pan/zoom camera.
     */
     const { responsive } = this.props;
-    const { regl, camera, offset } = this.state;
+    const { regl, camera, transform } = this.state;
+    const { glScaleX, glScaleY } = transform;
 
     const gl = regl._gl;
 
@@ -579,19 +587,20 @@ class Graph extends React.Component {
     const y = 2 * (1 - pin[1] / (responsive.height - this.graphPaddingTop)) - 1;
     const pout = [
       x * inverse[14] * aspect + inverse[12],
-      y * inverse[14] + inverse[13]
+      -(y * inverse[14] + inverse[13])
     ];
 
-    return [(pout[0] + 1) / 2 + offset[0], (pout[1] + 1) / 2 + offset[1]];
+    return [glScaleX.invert(pout[0]), glScaleY.invert(pout[1])];
   }
 
-  mapPointToScreen(xyCell, offset) {
+  mapPointToScreen(xyCell, transform) {
     /*
     Map an XY coordinate from cell/point domain to screen range.  Inverse
     of mapScreenToPoint()
     */
     const { responsive } = this.props;
     const { regl, camera } = this.state;
+    const { glScaleX, glScaleY } = transform;
 
     const gl = regl._gl;
 
@@ -603,12 +612,9 @@ class Graph extends React.Component {
 
     // variable names are choosen to reflect inverse of those used
     // in mapScreenToPoint().
-    const pout = [
+    const pout = [glScaleX(xyCell[0]), glScaleY(xyCell[1])];
-      (xyCell[0] - offset[0]) * 2 - 1,
-      (xyCell[1] - offset[1]) * 2 - 1
-    ];
     const x = (pout[0] - inverse[12]) / aspect / inverse[14];
-    const y = (pout[1] - inverse[13]) / inverse[14];
+    const y = (-pout[1] - inverse[13]) / inverse[14];
 
     const pin = [
       Math.round(((x + 1) * (responsive.width - this.graphPaddingRight)) / 2),

client/src/util/camera.js

@@ -5,10 +5,9 @@ const mp = require("mouse-position");
 const mb = require("mouse-pressed");
 const key = require("key-pressed");
 
-const panSpeed = 0.4;
+const panSpeed = 1.0; // changed from 0.4 to 1.0 per issue #722
 const scaleSpeed = 0.5;
 const scaleMax = 3;
-// const scaleMin = 1.15
 const scaleMin = 1.03;
 
 function attachCamera(canvas, opts) {

client/src/util/scaleLinear.js

@@ -9,8 +9,14 @@
 // this is is equivalent to d3.scaleLinear().domain([0,1]).range([-1,1])
 
 export default (domain, range) => {
-  const domainStart = domain[0];
+	const domainStart = domain[0];
-  const scale = (range[1] - range[0]) / (domain[1] - domain[0]);
+	const scale = (range[1] - range[0]) / (domain[1] - domain[0]);
-  const rangeStart = range[0];
+	const invScale = 1 / scale;
-  return value => (value - domainStart) * scale + rangeStart;
+	const rangeStart = range[0];
+	const f = value => (value - domainStart) * scale + rangeStart;
+
+	// inverter
+	f.invert = value => (value - rangeStart) * invScale + domainStart;
+
+	return f;
 };