address comments

modules/aggregation-layers/src/aggregation-layer-v9/aggregation-layer.ts

@@ -9,11 +9,6 @@ import {
 } from '@deck.gl/core';
 import {Aggregator} from './aggregator';
 
-// TODO
-type GPUAggregator = Aggregator & {destroy(): void};
-// TODO
-type CPUAggregator = Aggregator;
-
 export type AggregationLayerProps<DataT> = CompositeLayerProps & {
   data: LayerDataSource<DataT>;
 };
@@ -25,19 +20,16 @@ export default abstract class AggregationLayer<
   static layerName = 'AggregationLayer';
 
   state!: {
-    gpuAggregator: GPUAggregator | null;
-    cpuAggregator: CPUAggregator | null;
+    aggregator: Aggregator;
   };
 
   /** Allow this layer to participates in the draw cycle */
   get isDrawable() {
     return true;
   }
 
-  /** Called to create a GPUAggregator instance */
-  abstract getGPUAggregator(): GPUAggregator | null;
-  /** Called to create a CPUAggregator instance if getGPUAggregator() returns null */
-  abstract getCPUAggregator(): CPUAggregator | null;
+  /** Called to create an Aggregator instance */
+  abstract createAggregator(): Aggregator;
   /** Called when some attributes change, a chance to mark Aggregator as dirty */
   abstract onAttributeChange(id: string): void;
 
@@ -50,44 +42,43 @@ export default abstract class AggregationLayer<
     super.updateState(params);
 
     if (params.changeFlags.extensionsChanged) {
-      this.state.gpuAggregator?.destroy();
-      this.state.gpuAggregator = this.getGPUAggregator();
-      if (this.state.gpuAggregator) {
-        this.getAttributeManager()!.invalidateAll();
-      } else if (!this.state.cpuAggregator) {
-        this.state.cpuAggregator = this.getCPUAggregator();
-      }
+      this.state.aggregator?.destroy();
+      this.state.aggregator = this.createAggregator();
+      this.getAttributeManager()!.invalidateAll();
     }
   }
 
   // Override Layer.finalizeState to dispose the GPUAggregator instance
   finalizeState(context: LayerContext) {
     super.finalizeState(context);
-    this.state.gpuAggregator?.destroy();
+    this.state.aggregator.destroy();
   }
 
   // Override Layer.updateAttributes to update the aggregator
   protected updateAttributes(changedAttributes: {[id: string]: Attribute}) {
-    this.getAggregator()?.setProps({
+    const {aggregator} = this.state;
+    aggregator.setProps({
       attributes: changedAttributes
     });
 
     for (const id in changedAttributes) {
       this.onAttributeChange(id);
     }
+
+    // In aggregator.update() the aggregator allocates the buffers to store its output
+    // These buffers will be exposed by aggregator.getResults() and passed to the sublayers
+    // Therefore update() must be called before renderLayers()
+    // CPUAggregator's output is populated right here in update()
+    // GPUAggregator's output is pre-allocated and populated in preDraw(), see comments below
+    aggregator.update();
   }
 
   draw({moduleParameters}) {
     // GPU aggregation needs `moduleSettings` for projection/filter uniforms which are only accessible at draw time
-    // GPUAggregator's Buffers are allocated during `updateState`/`GPUAggregator.setProps`
-    // and passed down to the sublayer attributes in renderLayers()
+    // GPUAggregator's Buffers are pre-allocated during `update()` and passed down to the sublayer attributes in renderLayers()
     // Although the Buffers have been bound to the sublayer's Model, their content are not populated yet
-    // GPUAggregator.update() is called in the draw cycle here right before Buffers are used by sublayer.draw()
-    this.state.gpuAggregator?.update({moduleSettings: moduleParameters});
-  }
-
-  protected getAggregator(): Aggregator | null {
-    return this.state.gpuAggregator || this.state.cpuAggregator;
+    // GPUAggregator.preDraw() is called in the draw cycle here right before Buffers are used by sublayer.draw()
+    this.state.aggregator.preDraw({moduleSettings: moduleParameters});
   }
 
   // override CompositeLayer._getAttributeManager to create AttributeManager instance
@@ -97,15 +88,4 @@ export default abstract class AggregationLayer<
       stats: this.context.stats
     });
   }
-
-  // Override CompositeLayer._postUpdate to update attributes and the CPUAggregator
-  protected _postUpdate(updateParams: UpdateParameters<this>, forceUpdate: boolean) {
-    this._updateAttributes();
-    // CPUAggregator.update() must be called before renderLayers()
-    // CPUAggregator's outputs are Float32Array whose content is applied during the `updateState` lifecycle
-    // The typed arrays are passed to the sublayer's attributes and uploaded to GPU Buffers during the sublayer's update
-    // therefore they must be up to date before renderLayers()
-    this.state.cpuAggregator?.update();
-    super._postUpdate(updateParams, forceUpdate);
-  }
 }

modules/aggregation-layers/src/aggregation-layer-v9/aggregator.ts

@@ -1,28 +1,43 @@
 import type {Attribute, BinaryAttribute} from '@deck.gl/core';
 
-export type AggregationOperation = 'SUM' | 'MEAN' | 'MIN' | 'MAX';
+/** Method used to reduce a list of values to one number */
+export type AggregationOperation = 'SUM' | 'MEAN' | 'MIN' | 'MAX' | 'COUNT';
 
+/** Baseline inputs to an Aggregator */
 export type AggregationProps = {
   /** Number of data points */
   pointCount: number;
   /** The input data */
   attributes: {[id: string]: Attribute};
-  /** How to aggregate getWeights, defined for each channel */
+  /** How to aggregate the values inside a bin, defined for each channel */
   operations: AggregationOperation[];
   /** Additional options to control bin sorting, e.g. bin size */
   binOptions: Record<string, number | number[]>;
 };
 
+/** Descriptor of an aggregated bin */
+export type AggregatedBin = {
+  /** The unique identifier of the bin */
+  id: number | number[];
+  /** Aggregated values by channel */
+  value: number[];
+  /** Count of data points in this bin */
+  count: number;
+};
+
 /**
+ * The Aggregator interface describes a class that performs aggregation.
+ *
  * _Aggregation_ is a 2-step process:
  * 1. Sort: Group a collection of _data points_ by some property into _bins_.
- * 2. Aggregate: for each _bin_, calculate one or more metrics (_channels_) from all its members.
+ * 2. Aggregate: for each _bin_, calculate a numeric output (_result_) from some metrics (_values_) from all its members.
+ *    Multiple results can be obtained independently (_channels_).
  *
  * An implementation of the _Aggregator_ interface takes the following inputs:
  * - The number of data points
  * - The group that each data point belongs to, by mapping each data point to a _binId_ (integer or array of integers)
- * - The value(s) to aggregate, by mapping each data point in each channel to one _weight_
- * - The method (_aggregationOperation_) to reduce a list of _weights_ to one number, such as SUM
+ * - The values to aggregate, by mapping each data point in each channel to one _value_ (number)
+ * - The method (_aggregationOperation_) to reduce a list of values to one number, such as SUM
  *
  * And yields the following outputs:
  * - The aggregated values (_result_) as a list of numbers for each channel, comprised of one number per bin
@@ -38,8 +53,14 @@ export interface Aggregator {
    */
   setNeedsUpdate(channel?: number): void;
 
-  /** Run aggregation */
-  update(params?: unknown): void;
+  /** Called after props are set and before results are accessed */
+  update(): void;
+
+  /** Called before layer is drawn to screen. */
+  preDraw(params?: {moduleSettings: any}): void;
+
+  /** Dispose all allocated resources */
+  destroy(): void;
 
   /** Get the number of bins */
   get numBins(): number;
@@ -54,12 +75,5 @@ export interface Aggregator {
   getResultDomain(channel: number): [min: number, max: number];
 
   /** Returns the information for a given bin. */
-  getBin(index: number): {
-    /** The original id */
-    id: number | number[];
-    /** Aggregated values by channel */
-    value: number[];
-    /** Count of data points in this bin */
-    count: number;
-  } | null;
+  getBin(index: number): AggregatedBin | null;
 }

modules/core/src/lib/layer.ts

@@ -1014,6 +1014,10 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
         extension.updateState.call(this, updateParams, extension);
       }
 
+      this.setNeedsRedraw();
+      // Check if attributes need recalculation
+      this._updateAttributes();
+
       const modelChanged = this.getModels()[0] !== oldModels[0];
       this._postUpdate(updateParams, modelChanged);
       // End subclass lifecycle methods
@@ -1257,10 +1261,6 @@ export default abstract class Layer<PropsT extends {} = {}> extends Component<
   protected _postUpdate(updateParams: UpdateParameters<Layer<PropsT>>, forceUpdate: boolean) {
     const {props, oldProps} = updateParams;
 
-    this.setNeedsRedraw();
-    // Check if attributes need recalculation
-    this._updateAttributes();
-
     // Note: Automatic instance count update only works for single layers
     const model = this.state.model as Model | undefined;
     if (model?.isInstanced) {