Better heatmap range handling (#4865)

* ensure heatmap value is within range

* add test

* lint

app/packages/looker/src/overlays/heatmap.ts

@@ -10,13 +10,14 @@ import {
 import { ARRAY_TYPES, OverlayMask, TypedArray } from "../numpy";
 import { BaseState, Coordinates } from "../state";
 import { isFloatArray } from "../util";
+import { clampedIndex } from "../worker/painter";
 import {
   BaseLabel,
   CONTAINS,
-  isShown,
   Overlay,
   PointInfo,
   SelectData,
+  isShown,
 } from "./base";
 import { sizeBytes, strokeCanvasRect, t } from "./util";
 
@@ -204,12 +205,13 @@ export default class HeatmapOverlay<State extends BaseState>
     }
 
     if (state.options.coloring.by === "value") {
-      const index = Math.round(
-        (Math.max(value - start, 0) / (stop - start)) *
-          (state.options.coloring.scale.length - 1)
+      const index = clampedIndex(
+        value,
+        start,
+        stop,
+        state.options.coloring.scale.length
       );
-
-      return get32BitColor(state.options.coloring.scale[index]);
+      return index < 0 ? 0 : get32BitColor(state.options.coloring.scale[index]);
     }
 
     const color = getColor(
@@ -219,9 +221,9 @@ export default class HeatmapOverlay<State extends BaseState>
     );
     const max = Math.max(Math.abs(start), Math.abs(stop));
 
-    value = Math.min(max, Math.abs(value)) / max;
+    const result = Math.min(max, Math.abs(value)) / max;
 
-    return get32BitColor(color, value / max);
+    return get32BitColor(color, result / max);
   }
 
   private getTarget(state: Readonly<State>): number {

app/packages/looker/src/worker/painter.test.ts

@@ -63,3 +63,13 @@ describe("filter resolves correctly", () => {
     ).toBeUndefined();
   });
 });
+
+describe("heatmap utils", () => {
+  it("clamps for heatmaps", async () => {
+    // A value below a heatmap range returns -1
+    expect(painter.clampedIndex(1, 2, 3, 4)).toBe(-1);
+
+    // A value above a heatmap range return the max
+    expect(painter.clampedIndex(4, 2, 3, 4)).toBe(3);
+  });
+});

app/packages/looker/src/worker/painter.ts

@@ -206,23 +206,28 @@ export const PainterFactory = (requestColor) => ({
       }
 
       // 0 is background image
-      if (value !== 0) {
-        let r;
-        if (coloring.by === COLOR_BY.FIELD) {
-          color =
-            fieldSetting?.fieldColor ??
-            (await requestColor(coloring.pool, coloring.seed, field));
-
-          r = get32BitColor(color, Math.min(max, Math.abs(value)) / max);
-        } else {
-          const index = Math.round(
-            (Math.max(value - start, 0) / (stop - start)) * (scale.length - 1)
-          );
-          r = get32BitColor(scale[index]);
+      if (value === 0) {
+        continue;
+      }
+      let r: number;
+      if (coloring.by === COLOR_BY.FIELD) {
+        color =
+          fieldSetting?.fieldColor ??
+          (await requestColor(coloring.pool, coloring.seed, field));
+
+        r = get32BitColor(color, Math.min(max, Math.abs(value)) / max);
+      } else {
+        const index = clampedIndex(value, start, stop, scale.length);
+
+        if (index < 0) {
+          // values less than range start are background
+          continue;
         }
 
-        overlay[i] = r;
+        r = get32BitColor(scale[index]);
       }
+
+      overlay[i] = r;
     }
   },
   Segmentation: async (
@@ -386,8 +391,23 @@ export const convertToHex = (color: string) =>
 const convertMaskColorsToObject = (array: MaskColorInput[]) => {
   const result = {};
   if (!array) return {};
-  array.forEach((item) => {
+  for (const item of array) {
     result[item.intTarget.toString()] = item.color;
-  });
+  }
   return result;
 };
+
+export const clampedIndex = (
+  value: number,
+  start: number,
+  stop: number,
+  length: number
+) => {
+  if (value < start) {
+    return -1;
+  }
+  const clamped = Math.min(value, stop);
+  return Math.round(
+    (Math.max(clamped - start, 0) / (stop - start)) * (length - 1)
+  );
+};