Don't consider negative domain as valid in log/sqrt scale

Author: axelboc

packages/app/src/providers/mock/mock-file.ts

@@ -137,6 +137,7 @@ export function makeMockFile(): GroupWithChildren {
           valueId: 'twoD',
           attributes: [scalar('_FillValue', 400)],
         }),
+        array('twoD_neg'),
         array('twoD_bigint'),
         array('twoD_cplx'),
         array('twoD_compound', {

packages/lib/src/vis/shared/ViewportCenterer.tsx

@@ -17,9 +17,14 @@ function ViewportCenterer() {
   });
 
   useEffect(() => {
+    // On resize, move camera to the latest saved viewport center coordinates
     if (viewportCenter.current) {
-      // On resize, move camera to the latest saved viewport center coordinates
-      moveCameraTo(dataToWorld(viewportCenter.current));
+      const newPos = dataToWorld(viewportCenter.current);
+
+      // Ignore previous center if no longer valid with current scale
+      if (Number.isFinite(newPos.x) && Number.isFinite(newPos.y)) {
+        moveCameraTo(newPos);
+      }
     }
   }, [viewportCenter, moveCameraTo, dataToWorld, camera]);
 

packages/lib/src/vis/utils.test.ts

@@ -39,60 +39,44 @@ describe('getSizeToFit', () => {
 
 describe('getDomain', () => {
   it('should return min and max values of data array', () => {
-    const data = [2, 0, 10, 5, 2, -1];
-    const domain = getDomain(data);
-    expect(domain).toEqual([-1, 10]);
+    expect(getDomain([2, 0, 10, 5, 2, -1])).toEqual([-1, 10]);
   });
 
   it('should return `undefined` if data is empty', () => {
-    const domain = getDomain([]);
-    expect(domain).toBeUndefined();
+    expect(getDomain([])).toBeUndefined();
   });
 
   it('should ignore NaN and Infinity', () => {
-    const domain = getDomain([2, NAN, 10, INFINITY, 2, -1]);
-    expect(domain).toEqual([-1, 10]);
+    expect(getDomain([2, NAN, 10, INFINITY, 2, -1])).toEqual([-1, 10]);
   });
 
   it('should return `undefined` if data contains only NaN and Infinity', () => {
-    const domain = getDomain([NAN, NAN, -INFINITY, INFINITY]);
-    expect(domain).toBeUndefined();
+    expect(getDomain([NAN, NAN, -INFINITY, INFINITY])).toBeUndefined();
   });
 
   describe('with log scale', () => {
-    it('should support negative domain', () => {
-      const domain = getDomain([-2, -10, -5, -2, -1], ScaleType.Log);
-      expect(domain).toEqual([-10, -1]);
-    });
-
-    it('should clamp domain min to first strict positive value when domain crosses zero', () => {
-      const domain = getDomain([2, 0, 10, 5, 2, -1], ScaleType.Log);
-      expect(domain).toEqual([2, 10]);
+    it('should clamp domain min to first strictly positive value when domain crosses zero', () => {
+      expect(getDomain([2, 0, 10, -1], ScaleType.Log)).toEqual([2, 10]);
     });
 
     it('should return `undefined` if domain is not supported', () => {
-      const domain = getDomain([-2, 0, -10, -5, -2, -1], ScaleType.Log);
-      expect(domain).toBeUndefined();
+      expect(getDomain([-2, 0, -10], ScaleType.Log)).toBeUndefined();
+      expect(getDomain([-2, -5, -10], ScaleType.Log)).toBeUndefined();
     });
   });
 
   describe('with sqrt scale', () => {
-    it('should support negative domain', () => {
-      const domain = getDomain([-2, -10, -5, -2, -1], ScaleType.Sqrt);
-      expect(domain).toEqual([-10, -1]);
-    });
-
-    it('should support negative domain including 0', () => {
-      const domain = getDomain([-2, 0, -10, -5, -2, -1], ScaleType.Sqrt);
-      expect(domain).toEqual([-10, 0]);
+    it('should support positive domain including 0', () => {
+      expect(getDomain([1, 3, 0, 2], ScaleType.Sqrt)).toEqual([0, 3]);
     });
 
     it('should clamp domain min to first positive value when domain crosses zero', () => {
-      const domain = getDomain([2, 0, 10, 5, 2, -1], ScaleType.Sqrt);
-      expect(domain).toEqual([0, 10]);
+      expect(getDomain([2, 0, 10, -1], ScaleType.Sqrt)).toEqual([0, 10]);
+      expect(getDomain([2, 5, 10, -1], ScaleType.Sqrt)).toEqual([2, 10]);
+    });
 
-      const domain2 = getDomain([2, 10, 5, 2, -1], ScaleType.Sqrt);
-      expect(domain2).toEqual([2, 10]);
+    it('should return `undefined` if domain is fully negative', () => {
+      expect(getDomain([-2, -10], ScaleType.Sqrt)).toBeUndefined();
     });
   });
 });
@@ -108,12 +92,11 @@ describe('getDomains', () => {
   });
 
   it('should return domains of multiple arrays in log scale', () => {
-    const arr1 = [-2, -10, -5, -2, -1];
-    const arr2 = [2, 0, 10, 5, 2, -1];
-    const arr3 = [-2, 0, -10, -5, -2, -1];
+    const arr1 = [2, 0, 10, 5, 2, -1];
+    const arr2 = [-2, 0, -10, -5, -2, -1];
 
-    const domain = getDomains([arr1, arr2, arr3], ScaleType.Log);
-    expect(domain).toEqual([[-10, -1], [2, 10], undefined]);
+    const domain = getDomains([arr1, arr2], ScaleType.Log);
+    expect(domain).toEqual([[2, 10], undefined]);
   });
 });
 

packages/shared/src/mock-values.ts

@@ -174,6 +174,7 @@ export const mockValues = {
       [20, 41],
     );
   },
+  twoD_neg: () => ndarray(range(-10, 0), [1, 10]),
   threeD,
   threeD_cplx: () =>
     ndarray(

packages/shared/src/vis-utils.ts

@@ -167,16 +167,17 @@ export function getValidDomainForScale(
   }
 
   const { min, max, positiveMin, strictPositiveMin } = bounds;
-  if (scaleType === ScaleType.Log && min * max <= 0) {
-    // Clamp domain minimum to first positive value,
-    // or return `undefined` if domain is not unsupported: `[-x, 0]`
+
+  if (scaleType === ScaleType.Log && min <= 0) {
     return Number.isFinite(strictPositiveMin)
-      ? [strictPositiveMin, max]
-      : undefined;
+      ? [strictPositiveMin, max] // clamp min to first strictly positive value, if any
+      : undefined; // can't make valid domain - e.g. [-5, -2], [-10, 0]
   }
 
-  if (scaleType === ScaleType.Sqrt && min * max < 0) {
-    return [positiveMin, max];
+  if (scaleType === ScaleType.Sqrt && min < 0) {
+    return Number.isFinite(positiveMin)
+      ? [positiveMin, max] // clamp min to first positive value, if any
+      : undefined; // can't make valid domain - e.g. [-5, -2]
   }
 
   return [min, max];