Fix countItems in DataFilterExtension (#9158)

visgl · Sep 26, 2024 · 230104f · 230104f
1 parent aa2537b
commit 230104f
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 32 deletions.
diff --git a/modules/extensions/src/data-filter/aggregator.ts b/modules/extensions/src/data-filter/aggregator.ts
@@ -1,6 +1,5 @@
-import {Device, DeviceFeature, Framebuffer} from '@luma.gl/core';
-import {Model} from '@luma.gl/engine';
-import {GL} from '@luma.gl/constants';
+import {Device, DeviceFeature, Framebuffer, RenderPipelineParameters} from '@luma.gl/core';
+import {Model, ModelProps} from '@luma.gl/engine';
 
 const AGGREGATE_VS = `\
 #version 300 es
@@ -84,7 +83,12 @@ export function getFramebuffer(device: Device, useFloatTarget: boolean): Framebu
 }
 
 // Increments the counter based on dataFilter_value
-export function getModel(device: Device, shaderOptions: any, useFloatTarget: boolean): Model {
+export function getModel(
+  device: Device,
+  bufferLayout: ModelProps['bufferLayout'],
+  shaderOptions: any,
+  useFloatTarget: boolean
+): Model {
   shaderOptions.defines.NON_INSTANCED_MODEL = 1;
   if (useFloatTarget) {
     shaderOptions.defines.FLOAT_TARGET = 1;
@@ -94,16 +98,21 @@ export function getModel(device: Device, shaderOptions: any, useFloatTarget: boo
     id: 'data-filter-aggregation-model',
     vertexCount: 1,
     isInstanced: false,
-    drawMode: GL.POINTS,
+    topology: 'point-list',
     vs: AGGREGATE_VS,
     fs: AGGREGATE_FS,
+    bufferLayout,
     ...shaderOptions
   });
 }
 
-export const parameters = {
+export const parameters: RenderPipelineParameters = {
   blend: true,
-  blendFunc: [GL.ONE, GL.ONE, GL.ONE, GL.ONE],
-  blendEquation: [GL.FUNC_ADD, GL.FUNC_ADD],
-  depthTest: false
+  blendColorSrcFactor: 'one',
+  blendColorDstFactor: 'one',
+  blendAlphaSrcFactor: 'one',
+  blendAlphaDstFactor: 'one',
+  blendColorOperation: 'add',
+  blendAlphaOperation: 'add',
+  depthCompare: 'never'
 } as const;
diff --git a/modules/extensions/src/data-filter/data-filter-extension.ts b/modules/extensions/src/data-filter/data-filter-extension.ts
@@ -18,7 +18,7 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
-import type {Framebuffer} from '@luma.gl/core';
+import type {Buffer, Framebuffer} from '@luma.gl/core';
 import type {Model} from '@luma.gl/engine';
 import type {Layer, LayerContext, Accessor, UpdateParameters} from '@deck.gl/core';
 import {_deepEqual as deepEqual, LayerExtension, log} from '@deck.gl/core';
@@ -30,6 +30,7 @@ import {
   dataFilter64
 } from './shader-module';
 import * as aggregator from './aggregator';
+import {NumberArray4} from '@math.gl/core';
 
 const defaultProps = {
   getFilterValue: {type: 'accessor', value: 0},
@@ -204,7 +205,7 @@ export default class DataFilterExtension extends LayerExtension<
       // The vertex shader checks if a vertex has the same "index" as the previous vertex
       // so that we only write one count cross multiple vertices of the same object
       attributeManager.add({
-        filterIndices: {
+        filterVertexIndices: {
           size: useFloatTarget ? 1 : 2,
           vertexOffset: 1,
           type: 'unorm8',
@@ -226,6 +227,7 @@ export default class DataFilterExtension extends LayerExtension<
       const filterFBO = aggregator.getFramebuffer(device, useFloatTarget);
       const filterModel = aggregator.getModel(
         device,
+        attributeManager.getBufferLayouts({isInstanced: false}),
         extension.getShaders.call(this, extension),
         useFloatTarget
       );
@@ -311,32 +313,35 @@ export default class DataFilterExtension extends LayerExtension<
 
     /* eslint-disable-next-line camelcase */
     if (filterNeedsUpdate && onFilteredItemsChange && filterModel) {
+      const attributeManager = this.getAttributeManager()!;
       const {
-        attributes: {filterValues, filterCategoryValues, filterIndices}
-      } = this.getAttributeManager()!;
+        attributes: {filterValues, filterCategoryValues, filterVertexIndices}
+      } = attributeManager;
       filterModel.setVertexCount(this.getNumInstances());
 
-      this.context.device.clearWebGL({framebuffer: filterFBO, color: [0, 0, 0, 0]});
-
-      filterModel.updateModuleSettings(params.moduleParameters);
-      // @ts-expect-error filterValue and filterIndices should always have buffer value
-      filterModel.setAttributes({
+      // @ts-expect-error filterValue and filterVertexIndices should always have buffer value
+      const attributes: Record<string, Buffer> = {
         ...filterValues?.getValue(),
         ...filterCategoryValues?.getValue(),
-        ...filterIndices?.getValue()
+        ...filterVertexIndices?.getValue()
+      };
+      filterModel.setAttributes(attributes);
+      filterModel.shaderInputs.setProps({
+        dataFilter: dataFilterProps
       });
-      filterModel.shaderInputs.setProps({dataFilter: dataFilterProps});
-      filterModel.device.withParametersWebGL(
-        {
-          framebuffer: filterFBO,
-          // ts-ignore 'readonly' cannot be assigned to the mutable type '[GLBlendEquation, GLBlendEquation]'
-          ...(aggregator.parameters as any),
-          viewport: [0, 0, filterFBO.width, filterFBO.height]
-        },
-        () => {
-          filterModel.draw(this.context.renderPass);
-        }
-      );
+
+      const viewport = [0, 0, filterFBO.width, filterFBO.height] as NumberArray4;
+
+      const renderPass = filterModel.device.beginRenderPass({
+        id: 'data-filter-aggregation',
+        framebuffer: filterFBO,
+        parameters: {viewport},
+        clearColor: [0, 0, 0, 0]
+      });
+      filterModel.setParameters(aggregator.parameters);
+      filterModel.draw(renderPass);
+      renderPass.end();
+
       const color = filterModel.device.readPixelsToArrayWebGL(filterFBO);
       let count = 0;
       for (let i = 0; i < color.length; i++) {

diff --git a/test/modules/extensions/data-filter.spec.ts b/test/modules/extensions/data-filter.spec.ts
@@ -140,6 +140,7 @@ test('DataFilterExtension#categories', t => {
 
 test('DataFilterExtension#countItems', t => {
   let cbCalled = 0;
+  let cbCount = -1;
 
   const testCases = [
     {
@@ -150,12 +151,16 @@ test('DataFilterExtension#countItems', t => {
         ],
         getPosition: d => d.position,
         getFilterValue: d => d.timestamp,
-        onFilteredItemsChange: () => cbCalled++,
+        onFilteredItemsChange: event => {
+          cbCalled++;
+          cbCount = event.count;
+        },
         filterRange: [80, 160],
         extensions: [new DataFilterExtension({filterSize: 1, countItems: true})]
       },
       onAfterUpdate: () => {
         t.is(cbCalled, 1, 'onFilteredItemsChange is called');
+        t.is(cbCount, 2, 'count is correct');
       }
     },
     {
@@ -172,6 +177,7 @@ test('DataFilterExtension#countItems', t => {
       },
       onAfterUpdate: () => {
         t.is(cbCalled, 2, 'onFilteredItemsChange is called');
+        t.is(cbCount, 0, 'count is correct');
       }
     }
   ];