Fix Looker Classifications handing (#5322)

* fix looker classifications

* cleanup

* update tsconfig

* enhance filterView

* add viewsAreEqual tests

* use sample overlays

* empty state overlay array buffer tests

* temporal detections

app/packages/looker/src/lookers/abstract.ts

@@ -23,7 +23,7 @@ import {
 import { Events } from "../elements/base";
 import { COMMON_SHORTCUTS, LookerElement } from "../elements/common";
 import { ClassificationsOverlay, loadOverlays } from "../overlays";
-import { CONTAINS, LabelMask, Overlay } from "../overlays/base";
+import { CONTAINS, Overlay } from "../overlays/base";
 import processOverlays from "../processOverlays";
 import {
   BaseState,
@@ -44,6 +44,7 @@ import {
 } from "../util";
 import { ProcessSample } from "../worker";
 import { LookerUtils } from "./shared";
+import { retrieveArrayBuffers } from "./utils";
 
 const LABEL_LISTS_PATH = new Set(withPath(LABELS_PATH, LABEL_LISTS));
 const LABEL_LIST_KEY = Object.fromEntries(
@@ -516,44 +517,7 @@ export abstract class AbstractLooker<
   abstract updateOptions(options: Partial<State["options"]>): void;
 
   updateSample(sample: Sample) {
-    // collect any mask targets array buffer that overlays might have
-    // we'll transfer that to the worker instead of copying it
-    const arrayBuffers: ArrayBuffer[] = [];
-
-    for (const overlay of this.pluckedOverlays ?? []) {
-      let overlayData: LabelMask = null;
-
-      if ("mask" in overlay.label) {
-        overlayData = overlay.label.mask as LabelMask;
-      } else if ("map" in overlay.label) {
-        overlayData = overlay.label.map as LabelMask;
-      }
-
-      const buffer = overlayData?.data?.buffer;
-
-      if (!buffer) {
-        continue;
-      }
-
-      // check for detached buffer (happens if user is switching colors too fast)
-      // note: ArrayBuffer.prototype.detached is a new browser API
-      if (typeof buffer.detached !== "undefined") {
-        if (buffer.detached) {
-          // most likely sample is already being processed, skip update
-          return;
-        } else {
-          arrayBuffers.push(buffer);
-        }
-      } else if (buffer.byteLength) {
-        // hope we don't run into this edge case (old browser)
-        // sometimes detached buffers have bytelength > 0
-        // if we run into this case, we'll just attempt to transfer the buffer
-        // might get a DataCloneError if user is switching colors too fast
-        arrayBuffers.push(buffer);
-      }
-    }
-
-    this.loadSample(sample, arrayBuffers.flat());
+    this.loadSample(sample, retrieveArrayBuffers(this.sampleOverlays));
   }
 
   getSample(): Promise<Sample> {
@@ -739,7 +703,7 @@ export abstract class AbstractLooker<
 
   protected cleanOverlays() {
     for (const overlay of this.sampleOverlays ?? []) {
-      overlay.cleanup();
+      overlay.cleanup?.();
     }
   }
 

app/packages/looker/src/lookers/utils.test.ts

@@ -1,6 +1,13 @@
 import { describe, expect, it } from "vitest";
+import { ClassificationsOverlay } from "../overlays";
+import { TemporalDetectionOverlay } from "../overlays/classifications";
+import DetectionOverlay from "../overlays/detection";
+import HeatmapOverlay from "../overlays/heatmap";
+import KeypointOverlay from "../overlays/keypoint";
+import PolylineOverlay from "../overlays/polyline";
+import SegmentationOverlay from "../overlays/segmentation";
 import type { Buffers } from "../state";
-import { hasFrame } from "./utils";
+import { hasFrame, retrieveArrayBuffers } from "./utils";
 
 describe("looker utilities", () => {
   it("determines frame availability given a buffer list", () => {
@@ -16,4 +23,46 @@ describe("looker utilities", () => {
       expect(hasFrame(BUFFERS, frameNumber)).toBe(false);
     }
   });
+
+  it("retrieves array buffers without errors", () => {
+    expect(
+      retrieveArrayBuffers([new ClassificationsOverlay([])])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([new DetectionOverlay("ground_truth", {})])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([
+        new HeatmapOverlay("ground_truth", { id: "", tags: [] }),
+      ])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([new KeypointOverlay("ground_truth", {})])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([
+        new PolylineOverlay("ground_truth", {
+          id: "",
+          closed: false,
+          filled: false,
+          points: [],
+          tags: [],
+        }),
+      ])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([
+        new SegmentationOverlay("ground_truth", { id: "", tags: [] }),
+      ])
+    ).toStrictEqual([]);
+
+    expect(
+      retrieveArrayBuffers([new TemporalDetectionOverlay([])])
+    ).toStrictEqual([]);
+  });
 });

app/packages/looker/src/lookers/utils.ts

@@ -1,7 +1,57 @@
-import type { Buffers } from "../state";
+import type { LabelMask, Overlay } from "../overlays/base";
+import DetectionOverlay from "../overlays/detection";
+import HeatmapOverlay from "../overlays/heatmap";
+import SegmentationOverlay from "../overlays/segmentation";
+import type { BaseState, Buffers } from "../state";
 
 export const hasFrame = (buffers: Buffers, frameNumber: number) => {
   return buffers.some(
     ([start, end]) => start <= frameNumber && frameNumber <= end
   );
 };
+
+export const retrieveArrayBuffers = <State extends BaseState>(
+  overlays?: Overlay<State>[]
+) => {
+  // collect any mask targets array buffer that overlays might have
+  // we'll transfer that to the worker instead of copying it
+  const arrayBuffers: ArrayBuffer[] = [];
+
+  for (const overlay of overlays ?? []) {
+    let overlayData: LabelMask = null;
+
+    if (
+      overlay instanceof DetectionOverlay ||
+      overlay instanceof SegmentationOverlay
+    ) {
+      overlayData = overlay.label.mask;
+    } else if (overlay instanceof HeatmapOverlay) {
+      overlayData = overlay.label.map;
+    }
+
+    const buffer = overlayData?.data?.buffer;
+
+    if (!buffer) {
+      continue;
+    }
+
+    // check for detached buffer (happens if user is switching colors too fast)
+    // note: ArrayBuffer.prototype.detached is a new browser API
+    if (typeof buffer.detached !== "undefined") {
+      if (buffer.detached) {
+        // most likely sample is already being processed, skip update
+        return [];
+      }
+
+      arrayBuffers.push(buffer);
+    } else if (buffer.byteLength) {
+      // hope we don't run into this edge case (old browser)
+      // sometimes detached buffers have bytelength > 0
+      // if we run into this case, we'll just attempt to transfer the buffer
+      // might get a DataCloneError if user is switching colors too fast
+      arrayBuffers.push(buffer);
+    }
+  }
+
+  return arrayBuffers;
+};

app/packages/looker/tsconfig.json

@@ -1,8 +1,8 @@
 {
     "compilerOptions": {
-        "target": "ES2023",
+        "target": "ES2024",
         "module": "ES2022",
-        "lib": ["ES2023", "DOM", "DOM.Iterable"],
+        "lib": ["ES2024", "DOM", "DOM.Iterable"],
         "moduleResolution": "Node",
         "sourceMap": true,
         "resolveJsonModule": true,

app/packages/state/src/utils.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { convertTargets } from "./utils";
+import { convertTargets, viewsAreEqual } from "./utils";
 
 describe("convertTargets", () => {
   it("upper cases rgb hex targets", () => {
@@ -8,3 +8,13 @@ describe("convertTargets", () => {
     ).toStrictEqual({ "#FFFFFF": { label: "white", intTarget: 1 } });
   });
 });
+
+describe("filterView", () => {
+  it("handles saved view string names and undefined values", () => {
+    expect(viewsAreEqual("one", "one")).toBe(true);
+    expect(viewsAreEqual("one", "two")).toBe(false);
+    expect(viewsAreEqual("one", undefined)).toBe(false);
+    expect(viewsAreEqual("one", [])).toBe(false);
+    expect(viewsAreEqual([], [])).toBe(true);
+  });
+});

app/packages/state/src/utils.ts

@@ -57,15 +57,26 @@ export const stringifyObj = (obj) => {
   );
 };
 
-export const filterView = (stages) =>
+export const filterView = (stages: State.Stage[]) =>
   JSON.stringify(
     stages.map(({ kwargs, _cls }) => ({
       kwargs: kwargs.filter((ka) => !ka[0].startsWith("_")),
       _cls,
     }))
   );
 
-export const viewsAreEqual = (viewOne, viewTwo) => {
+export const viewsAreEqual = (
+  viewOne?: string | State.Stage[],
+  viewTwo?: string | State.Stage[]
+) => {
+  if (viewOne === viewTwo) {
+    return true;
+  }
+
+  if (!Array.isArray(viewOne) || !Array.isArray(viewTwo)) {
+    return false;
+  }
+
   return filterView(viewOne) === filterView(viewTwo);
 };