fix: filter stacking bug (#199)

* fix: enforce consistent point ordering w/ `filter()`

* chore: bump biome schema version

* docs: update changelog

Author: flekschas

CHANGELOG.md

@@ -1,3 +1,7 @@
+## 1.11.1
+
+- Fix: ensure that the drawing order of points cannot be manipulated via `scatterplot.filter()` ([#197](https://github.com/flekschas/regl-scatterplot/issues/197))
+
 ## 1.11.0
 
 - Feat: add support for linear and constant point scaling via a new property called `pointScaleMode`.

biome.json

@@ -1,5 +1,5 @@
 {
-  "$schema": "https://biomejs.dev/schemas/1.8.3/schema.json",
+  "$schema": "https://biomejs.dev/schemas/1.9.4/schema.json",
   "organizeImports": {
     "enabled": true
   },

src/index.js

@@ -131,6 +131,7 @@ import {
   dist,
   flipObj,
   getBBox,
+  insertionSort,
   isConditionalArray,
   isDomRect,
   isHorizontalLine,
@@ -2218,34 +2219,37 @@ const createScatterplot = (
    * @param {import('./types').ScatterplotMethodOptions['filter']}
    */
   const filter = (pointIdxs, { preventEvent = false } = {}) => {
-    const filteredPoints = Array.isArray(pointIdxs) ? pointIdxs : [pointIdxs];
-
     isPointsFiltered = true;
     filteredPointsSet.clear();
 
+    const pointIdxsArray = Array.isArray(pointIdxs) ? pointIdxs : [pointIdxs];
+    const filteredPoints = [];
     const filteredPointsBuffer = [];
     const filteredSelectedPoints = [];
 
-    for (let i = filteredPoints.length - 1; i >= 0; i--) {
-      const pointIdx = filteredPoints[i];
-
+    for (const pointIdx of pointIdxsArray) {
       if (pointIdx < 0 || pointIdx >= numPoints) {
-        // Remove invalid filtered points
-        filteredPoints.splice(i, 1);
+        // Skip invalid filtered points
         continue;
       }
 
+      filteredPoints.push(pointIdx);
       filteredPointsSet.add(pointIdx);
-      filteredPointsBuffer.push.apply(
-        filteredPointsBuffer,
-        indexToStateTexCoord(pointIdx),
-      );
 
       if (selectedPointsSet.has(pointIdx)) {
         filteredSelectedPoints.push(pointIdx);
       }
     }
 
+    const sortedFilteredPoints = insertionSort([...filteredPoints]);
+
+    for (const pointIdx of sortedFilteredPoints) {
+      filteredPointsBuffer.push.apply(
+        filteredPointsBuffer,
+        indexToStateTexCoord(pointIdx),
+      );
+    }
+
     // Update the normal points index buffers
     normalPointsIndexBuffer.subdata(filteredPointsBuffer);
 

src/utils.js

@@ -526,3 +526,19 @@ export const isRect = (annotation) =>
 
 export const isPolygon = (annotation) =>
   'vertices' in annotation && annotation.vertices.length > 1;
+
+export const insertionSort = (array) => {
+  const end = array.length;
+  for (let i = 1; i < end; i++) {
+    // Choosing the first element in our unsorted subarray
+    const current = array[i];
+    // The last element of our sorted subarray
+    let j = i - 1;
+    while (j > -1 && current < array[j]) {
+      array[j + 1] = array[j];
+      j--;
+    }
+    array[j + 1] = current;
+  }
+  return array;
+};

tests/index.js

@@ -2802,18 +2802,88 @@ test('regl-scatterplot', async (t2) => {
         'should have not filter down to any point because -1 is an invalid point index'
       );
 
-      await scatterplot.filter([0, -1, 2, 4, 6]);
+      const pointsToBeFiltered = [0, -1, 2, 4, 6];
+
+      await scatterplot.filter(pointsToBeFiltered);
       await wait(0);
 
       t.ok(
         isSameElements(filteredPoints, [0, 2, 4]),
         'should have filtered down to valid points (0, 2, and 4) only'
       );
 
+      // We're testing this due to the following bug where we accidentically
+      // spliced of from the input argument, which we should never do.
+      // @see https://github.com/flekschas/regl-scatterplot/issues/197
+      t.equal(
+        pointsToBeFiltered.length,
+        5,
+        'should have not altered `pointsToBeFiltered`'
+      );
+
       scatterplot.destroy();
     })
   );
 
+  await t2.test(
+    '`filter() point order consistency`',
+    catchError(async (t) => {
+      const scatterplot = createScatterplot({
+        canvas: createCanvas(50, 50),
+        width: 50,
+        height: 50,
+        pointColor: ['#FF0000', '#00FF00', '#0000FF'],
+        pointSize: 50,
+        opacity: 1,
+        colorBy: "valueA"
+      });
+
+      const dpr = window.devicePixelRatio;
+      const middlePixel = (50 * dpr * 25 * dpr + 25 * dpr) * 4;
+
+      await scatterplot.draw({
+        x: [0, 0, 0],
+        y: [0, 0, 0],
+        valueA: [0, 1, 2]
+      });
+
+      await wait(25);
+
+      let image = scatterplot.export();
+
+      t.equal(
+        Array.from(image.data.slice(middlePixel, middlePixel + 4)),
+        [0, 0, 255, 255],
+        'the center pixel should be blue'
+      );
+
+      await scatterplot.filter([1, 0]);
+
+      await wait(25);
+
+      image = scatterplot.export();
+
+      t.equal(
+        Array.from(image.data.slice(middlePixel, middlePixel + 4)),
+        [0, 255, 0, 255],
+        'the center pixel should be green'
+      );
+
+      await scatterplot.unfilter();
+      await scatterplot.filter([0, 1]);
+
+      await wait(25);
+
+      image = scatterplot.export();
+
+      t.equal(
+        Array.from(image.data.slice(middlePixel, middlePixel + 4)),
+        [0, 255, 0, 255],
+        'the center pixel should be green'
+      );
+    })
+  );
+
   await t2.test(
     'test hover, select, and filter options of `draw()`',
     catchError(async (t) => {