From 420968e2d46279b763700ea3e5d7c2b7814c386e Mon Sep 17 00:00:00 2001 From: Jacob Parker Date: Wed, 21 May 2025 16:50:05 +0100 Subject: [PATCH] Scene perf --- .../src/chart/chartAxis.ts | 14 +- .../src/chart/legend/legendMarkerLabel.ts | 2 +- .../src/chart/pagination/pagination.ts | 5 +- .../src/chart/series/cartesian/areaSeries.ts | 9 +- .../chart/series/cartesian/cartesianSeries.ts | 8 +- .../chart/series/cartesian/histogramSeries.ts | 3 +- .../src/chart/series/polar/donutSeries.ts | 10 +- .../src/chart/series/polar/pieSeries.ts | 10 +- .../src/chart/series/series.ts | 4 +- .../ag-charts-community/src/scene/group.ts | 158 ++++++++++++++++-- .../src/scene/node.test.ts | 3 +- .../ag-charts-community/src/scene/node.ts | 148 ++-------------- .../ag-charts-community/src/scene/scene.ts | 4 +- .../src/scene/sceneDebug.ts | 75 +++++---- .../src/scene/selection.test.ts | 4 +- .../src/scene/selection.ts | 11 +- .../src/features/crosshair/crosshair.ts | 6 +- .../src/features/error-bar/errorBarNode.ts | 2 +- .../src/gradient-legend/axisTicks.ts | 2 +- 19 files changed, 245 insertions(+), 233 deletions(-) diff --git a/packages/ag-charts-community/src/chart/chartAxis.ts b/packages/ag-charts-community/src/chart/chartAxis.ts index 5e59d44c9a4..88799bc1a2d 100644 --- a/packages/ag-charts-community/src/chart/chartAxis.ts +++ b/packages/ag-charts-community/src/chart/chartAxis.ts @@ -13,7 +13,7 @@ import type { ModuleContextWithParent } from '../module/moduleContext'; import type { ModuleMap } from '../module/moduleMap'; import type { Scale } from '../scale/scale'; import type { BBox } from '../scene/bbox'; -import type { Node } from '../scene/node'; +import type { Group } from '../scene/group'; import type { TransformableText } from '../scene/shape/text'; import type { Padding } from '../util/padding'; import type { AxisPrimaryTickCount } from '../util/secondaryAxisTicks'; @@ -43,12 +43,12 @@ interface AxisLayoutConstraints { } export interface AxisGroups { - axisNode: Node; - gridNode: Node; - crossLineRangeNode: Node; - crossLineLineNode: Node; - crossLineLabelNode: Node; - labelNode: Node; + axisNode: Group; + gridNode: Group; + crossLineRangeNode: Group; + crossLineLineNode: Group; + crossLineLabelNode: Group; + labelNode: Group; } export interface ChartAxis { diff --git a/packages/ag-charts-community/src/chart/legend/legendMarkerLabel.ts b/packages/ag-charts-community/src/chart/legend/legendMarkerLabel.ts index 076facba63a..e4655d690ea 100644 --- a/packages/ag-charts-community/src/chart/legend/legendMarkerLabel.ts +++ b/packages/ag-charts-community/src/chart/legend/legendMarkerLabel.ts @@ -141,7 +141,7 @@ export class LegendMarkerLabel extends Translatable(Group) { this.label.x = length + spacing; } - protected override computeBBox(): BBox { + protected override computeBBox(): BBox | undefined { this.layout(); return super.computeBBox(); } diff --git a/packages/ag-charts-community/src/chart/pagination/pagination.ts b/packages/ag-charts-community/src/chart/pagination/pagination.ts index 57e319bb4d7..c8525d3bd0b 100644 --- a/packages/ag-charts-community/src/chart/pagination/pagination.ts +++ b/packages/ag-charts-community/src/chart/pagination/pagination.ts @@ -1,8 +1,7 @@ import { clamp, createId } from 'ag-charts-core'; import type { AgChartLegendOrientation, AgMarkerShape, FontStyle, FontWeight } from 'ag-charts-types'; -import { TranslatableGroup } from '../../scene/group'; -import type { Node } from '../../scene/node'; +import { Group, TranslatableGroup } from '../../scene/group'; import { Text } from '../../scene/shape/text'; import { type RotatableType, Transformable } from '../../scene/transformable'; import { BaseProperties } from '../../util/properties'; @@ -308,7 +307,7 @@ export class Pagination extends BaseProperties { this.chartUpdateCallback(ChartUpdateType.SCENE_RENDER); } - attachPagination(node: Node) { + attachPagination(node: Group) { node.append(this.group); } diff --git a/packages/ag-charts-community/src/chart/series/cartesian/areaSeries.ts b/packages/ag-charts-community/src/chart/series/cartesian/areaSeries.ts index d5a28ee7959..0c8a1e4c2f5 100644 --- a/packages/ag-charts-community/src/chart/series/cartesian/areaSeries.ts +++ b/packages/ag-charts-community/src/chart/series/cartesian/areaSeries.ts @@ -8,7 +8,6 @@ import { pathMotion } from '../../../motion/pathMotion'; import { resetMotion } from '../../../motion/resetMotion'; import { BBox } from '../../../scene/bbox'; import { Group } from '../../../scene/group'; -import type { Node } from '../../../scene/node'; import { PointerEvents } from '../../../scene/node'; import type { SizedPoint } from '../../../scene/point'; import type { Selection } from '../../../scene/selection'; @@ -127,16 +126,16 @@ export class AreaSeries extends CartesianSeries< ); } - override attachSeries(seriesContentNode: Node, seriesNode: Node, annotationNode: Node | undefined): void { + override attachSeries(seriesContentNode: Group, seriesNode: Group, annotationNode: Group | undefined): void { super.attachSeries(seriesContentNode, seriesNode, annotationNode); seriesContentNode.appendChild(this.backgroundGroup); } override detachSeries( - seriesContentNode: Node | undefined, - seriesNode: Node, - annotationNode: Node | undefined + seriesContentNode: Group | undefined, + seriesNode: Group, + annotationNode: Group | undefined ): void { super.detachSeries(seriesContentNode, seriesNode, annotationNode); diff --git a/packages/ag-charts-community/src/chart/series/cartesian/cartesianSeries.ts b/packages/ag-charts-community/src/chart/series/cartesian/cartesianSeries.ts index fc0c6733600..0a6b7852ab2 100644 --- a/packages/ag-charts-community/src/chart/series/cartesian/cartesianSeries.ts +++ b/packages/ag-charts-community/src/chart/series/cartesian/cartesianSeries.ts @@ -305,16 +305,16 @@ export abstract class CartesianSeries< ); } - override attachSeries(seriesContentNode: Node, seriesNode: Node, annotationNode: Node | undefined): void { + override attachSeries(seriesContentNode: Group, seriesNode: Group, annotationNode: Group | undefined): void { super.attachSeries(seriesContentNode, seriesNode, annotationNode); this.attachPaths(this.paths, seriesNode, annotationNode); } override detachSeries( - seriesContentNode: Node | undefined, - seriesNode: Node, - annotationNode: Node | undefined + seriesContentNode: Group | undefined, + seriesNode: Group, + annotationNode: Group | undefined ): void { super.detachSeries(seriesContentNode, seriesNode, annotationNode); diff --git a/packages/ag-charts-community/src/chart/series/cartesian/histogramSeries.ts b/packages/ag-charts-community/src/chart/series/cartesian/histogramSeries.ts index 7ec1a0ac076..3688fe43ba8 100644 --- a/packages/ag-charts-community/src/chart/series/cartesian/histogramSeries.ts +++ b/packages/ag-charts-community/src/chart/series/cartesian/histogramSeries.ts @@ -4,6 +4,7 @@ import type { AgHistogramBinDatum } from 'ag-charts-types'; import type { ModuleContext } from '../../../module/moduleContext'; import { fromToMotion } from '../../../motion/fromToMotion'; import type { BBox } from '../../../scene/bbox'; +import { Group } from '../../../scene/group'; import { PointerEvents } from '../../../scene/node'; import type { Point } from '../../../scene/point'; import type { Selection } from '../../../scene/selection'; @@ -485,7 +486,7 @@ export class HistogramSeries extends CartesianSeries< protected override initQuadTree(quadtree: QuadtreeNearest) { const { value: childNode } = this.contentGroup.children().next(); - if (childNode) { + if (childNode instanceof Group) { addHitTestersToQuadtree(quadtree, childNode.children() as Iterable); } } diff --git a/packages/ag-charts-community/src/chart/series/polar/donutSeries.ts b/packages/ag-charts-community/src/chart/series/polar/donutSeries.ts index 05b171bfbc1..9a718b69693 100644 --- a/packages/ag-charts-community/src/chart/series/polar/donutSeries.ts +++ b/packages/ag-charts-community/src/chart/series/polar/donutSeries.ts @@ -7,7 +7,7 @@ import { LinearScale } from '../../../scale/linearScale'; import { BBox } from '../../../scene/bbox'; import type { GradientParams } from '../../../scene/gradient/gradient'; import { Group, TranslatableGroup } from '../../../scene/group'; -import { Node, PointerEvents } from '../../../scene/node'; +import { PointerEvents } from '../../../scene/node'; import type { Point } from '../../../scene/point'; import { Selection } from '../../../scene/selection'; import { Line } from '../../../scene/shape/line'; @@ -174,16 +174,16 @@ export class DonutSeries extends PolarSeries extends Node { return compareZIndex(a.zIndex, b.zIndex) || a.serialNumber - b.serialNumber; } + private readonly childNodes = new Set(); + public dirty = false; private dirtyZIndex: boolean = false; - private clipRect?: BBox; + private clipRect: BBox | undefined = undefined; @SceneChangeDetection({ convertor: (v: number) => clamp(0, v, 1) }) opacity: number = 1; @@ -63,13 +65,13 @@ export class Group extends Node { return true; } - protected override computeBBox(): BBox { + protected override computeBBox(): BBox | undefined { return Group.computeChildrenBBox(this.children()); } private computeSafeClippingBBox(pixelRatio: number): BBox | undefined { const bbox = this.computeBBox(); - if (!bbox.isFinite()) return; + if (bbox?.isFinite() !== true) return; let strokeWidth = 0; const strokeMiterAmount = 4; @@ -105,6 +107,19 @@ export class Group extends Node { return sharedOffscreenCanvas; } + override setScene(scene?: IScene) { + super.setScene(scene); + + if (this.layer) { + this.scene?.layersManager.removeLayer(this.layer); + this.layer = undefined; + } + + for (const child of this.children()) { + child.setScene(scene); + } + } + override markDirty(property?: string): void { this.dirty = true; super.markDirty(property); @@ -116,6 +131,99 @@ export class Group extends Node { this.markDirty(); } + /** + * Appends one or more new node instances to this parent. + * If one needs to: + * - move a child to the end of the list of children + * - move a child from one parent to another (including parents in other scenes) + * one should use the {@link insertBefore} method instead. + * @param nodes A node or nodes to append. + */ + append(nodes: Iterable | Node) { + for (const node of toIterable(nodes)) { + node.parentNode?.removeChild(node); + this.childNodes.add(node); + + node.parentNode = this; + node.setScene(this.scene); + } + + this.markDirtyChildrenOrder(); + this.markDirty(); + } + + appendChild(node: T): T { + this.append(node); + return node; + } + + override removeChild(node: Node) { + if (!this.childNodes?.delete(node)) { + throw new Error( + `AG Charts - internal error, unknown child node ${node.name ?? node.id} in $${this.name ?? this.id}` + ); + } + + node.parentNode = undefined; + node.setScene(); + + this.markDirtyChildrenOrder(); + this.markDirty(); + } + + clear() { + for (const child of this.children()) { + delete child.parentNode; + child.setScene(); + } + this.childNodes?.clear(); + this.markDirty(); + } + + /** + * Hit testing method. + * Recursively checks if the given point is inside this node or any of its children. + * Returns the first matching node or `undefined`. + * Nodes that render later (show on top) are hit tested first. + */ + override pickNode(x: number, y: number): Node | undefined { + if (!this.visible || this.pointerEvents === PointerEvents.None || !this.containsPoint(x, y)) { + return; + } + + if (this.childNodes != null && this.childNodes.size !== 0) { + const children = [...this.children()]; + // Nodes added later should be hit-tested first, + // as they are rendered on top of the previously added nodes. + for (let i = children.length - 1; i >= 0; i--) { + const child = children[i]; + const hit = child.pickNode(x, y); + if (hit != null) { + return hit; + } + } + } else if (!this.isContainerNode) { + // a leaf node, but not a container leaf + return this; + } + } + + override pickNodes(x: number, y: number, into: Node[] = []): Node[] { + if (!this.visible || this.pointerEvents === PointerEvents.None || !this.containsPoint(x, y)) { + return into; + } + + if (!this.isContainerNode) { + into.push(this); + } + + for (const child of this.children()) { + child.pickNodes(x, y, into); + } + + return into; + } + private _lastWidth = NaN; private _lastHeight = NaN; private _lastDevicePixelRatio = NaN; @@ -134,6 +242,14 @@ export class Group extends Node { let counts: ChildNodeCounts; if (this.dirty) { counts = super.preRender(renderCtx, 0); + + for (const child of this.children()) { + const childCounts = child.preRender(renderCtx); + counts.groups += childCounts.groups; + counts.nonGroups += childCounts.nonGroups; + counts.complexity += childCounts.complexity; + } + // Correct counts for this group. counts.groups += 1; counts.nonGroups -= 1; @@ -277,6 +393,30 @@ export class Group extends Node { ctx.restore(); } + private sortChildren(compareFn?: (a: Node, b: Node) => number) { + if (!this.childNodes) return; + + // Sort children, and re-add in new order (Set preserves insertion order). + const sortedChildren = [...this.childNodes].sort(compareFn); + this.childNodes.clear(); + for (const child of sortedChildren) { + this.childNodes.add(child); + } + } + + *children(): Generator { + yield* this.childNodes; + } + + *descendants(): Generator { + for (const child of this.children()) { + yield child; + if (child instanceof Group) { + yield* child.descendants(); + } + } + } + /** * Transforms bbox given in the canvas coordinate space to bbox in this group's coordinate space and * sets this group's clipRect to the transformed bbox. @@ -294,14 +434,6 @@ export class Group extends Node { this.clipRect = bbox; } - override setScene(scene?: IScene) { - if (this.layer) { - this.scene?.layersManager.removeLayer(this.layer); - this.layer = undefined; - } - super.setScene(scene); - } - private getVisibility() { for (const node of this.traverseUp(true)) { if (!node.visible) { diff --git a/packages/ag-charts-community/src/scene/node.test.ts b/packages/ag-charts-community/src/scene/node.test.ts index 272e0dd64fd..b1e80dc334f 100644 --- a/packages/ag-charts-community/src/scene/node.test.ts +++ b/packages/ag-charts-community/src/scene/node.test.ts @@ -1,9 +1,10 @@ import { describe, expect, it } from '@jest/globals'; import { BBox } from './bbox'; +import { Group } from './group'; import { Node } from './node'; -class TestNode extends Node { +class TestNode extends Group { protected override computeBBox(): BBox | undefined { return BBox.merge(Array.from(this.children(), (c) => c.getBBox())); } diff --git a/packages/ag-charts-community/src/scene/node.ts b/packages/ag-charts-community/src/scene/node.ts index e4477b03734..196be122d63 100644 --- a/packages/ag-charts-community/src/scene/node.ts +++ b/packages/ag-charts-community/src/scene/node.ts @@ -1,4 +1,4 @@ -import { createId, createSvgElement, toIterable } from 'ag-charts-core'; +import { createId, createSvgElement } from 'ag-charts-core'; import { objectsEqual } from '../util/object'; import { BBox } from './bbox'; @@ -98,27 +98,25 @@ export abstract class Node { /** Unique node ID in the form `ClassName-NaturalNumber`. */ readonly id = createId(this); - readonly name?: string; + readonly name: string | undefined = undefined; /** * Some number to identify this node, typically within a `Group` node. * Usually this will be some enum value used as a selector. */ tag: number; - transitionOut?: boolean; + transitionOut: boolean | undefined = undefined; pointerEvents: PointerEvents = PointerEvents.All; - protected _datum?: D; - protected _previousDatum?: D; + protected _datum: D | undefined = undefined; + protected _previousDatum: D | undefined = undefined; - protected _debug?: (...args: any[]) => void; protected scene: IScene | undefined = undefined; - private readonly _debugDirtyProperties?: Map; + private readonly _debugDirtyProperties: Map | undefined = undefined; - private parentNode?: Node; - private childNodes?: Set; + parentNode: Node | undefined = undefined; - private cachedBBox?: BBox; + private cachedBBox: BBox | undefined = undefined; /** * To simplify the type system (especially in Selections) we don't have the `Parent` node @@ -181,19 +179,12 @@ export abstract class Node { } /** Perform any pre-rendering initialization. */ - preRender(renderCtx: RenderContext, thisComplexity = 1): ChildNodeCounts { + preRender(_renderCtx: RenderContext, thisComplexity = 1): ChildNodeCounts { this.childNodeCounts.groups = 0; this.childNodeCounts.nonGroups = 1; // Assume this node isn't a group. this.childNodeCounts.complexity = thisComplexity; this.childNodeCounts.thisComplexity = thisComplexity; - for (const child of this.children()) { - const childCounts = child.preRender(renderCtx); - this.childNodeCounts.groups += childCounts.groups; - this.childNodeCounts.nonGroups += childCounts.nonGroups; - this.childNodeCounts.complexity += childCounts.complexity; - } - return this.childNodeCounts; } @@ -217,22 +208,6 @@ export abstract class Node { setScene(scene?: IScene) { this.scene = scene; - this._debug = scene?.layersManager?.debug; - - for (const child of this.children()) { - child.setScene(scene); - } - } - - protected sortChildren(compareFn?: (a: Node, b: Node) => number) { - if (!this.childNodes) return; - - // Sort children, and re-add in new order (Set preserves insertion order). - const sortedChildren = [...this.childNodes].sort(compareFn); - this.childNodes.clear(); - for (const child of sortedChildren) { - this.childNodes.add(child); - } } *traverseUp(includeSelf?: boolean): Generator { @@ -245,27 +220,6 @@ export abstract class Node { } } - *children(): Generator { - if (!this.childNodes) return; - for (const child of this.childNodes) { - yield child; - } - } - - *descendants(): Generator { - for (const child of this.children()) { - yield child; - yield* child.descendants(); - } - } - - /** - * Checks if the node is a leaf (has no children). - */ - isLeaf() { - return !this.childNodes?.size; - } - /** * Checks if the node is the root (has no parent). */ @@ -273,60 +227,16 @@ export abstract class Node { return !this.parentNode; } - /** - * Appends one or more new node instances to this parent. - * If one needs to: - * - move a child to the end of the list of children - * - move a child from one parent to another (including parents in other scenes) - * one should use the {@link insertBefore} method instead. - * @param nodes A node or nodes to append. - */ - append(nodes: Iterable | Node) { - this.childNodes ??= new Set(); - for (const node of toIterable(nodes)) { - node.parentNode?.removeChild(node); - this.childNodes.add(node); - - node.parentNode = this; - node.setScene(this.scene); - } - - this.markDirtyChildrenOrder(); - this.markDirty(); - } - - appendChild(node: T): T { - this.append(node); - return node; - } - removeChild(node: Node) { - if (!this.childNodes?.delete(node)) { - throw new Error( - `AG Charts - internal error, unknown child node ${node.name ?? node.id} in $${this.name ?? this.id}` - ); - } - - delete node.parentNode; - node.setScene(); - - this.markDirtyChildrenOrder(); - this.markDirty(); + throw new Error( + `AG Charts - internal error, unknown child node ${node.name ?? node.id} in $${this.name ?? this.id}` + ); } remove() { this.parentNode?.removeChild(this); } - clear() { - for (const child of this.children()) { - delete child.parentNode; - child.setScene(); - } - this.childNodes?.clear(); - this.markDirty(); - } - destroy(): void { this.parentNode?.removeChild(this); } @@ -346,46 +256,16 @@ export abstract class Node { return false; } - /** - * Hit testing method. - * Recursively checks if the given point is inside this node or any of its children. - * Returns the first matching node or `undefined`. - * Nodes that render later (show on top) are hit tested first. - */ pickNode(x: number, y: number): Node | undefined { - if (!this.visible || this.pointerEvents === PointerEvents.None || !this.containsPoint(x, y)) { - return; - } - - if (this.childNodes != null && this.childNodes.size !== 0) { - const children = [...this.children()]; - // Nodes added later should be hit-tested first, - // as they are rendered on top of the previously added nodes. - for (let i = children.length - 1; i >= 0; i--) { - const hit = children[i].pickNode(x, y); - if (hit) { - return hit; - } - } - } else if (!this.isContainerNode) { - // a leaf node, but not a container leaf + if (this.containsPoint(x, y)) { return this; } } pickNodes(x: number, y: number, into: Node[] = []): Node[] { - if (!this.visible || this.pointerEvents === PointerEvents.None || !this.containsPoint(x, y)) { - return into; - } - - if (!this.isContainerNode) { + if (this.containsPoint(x, y)) { into.push(this); } - - for (const child of this.children()) { - child.pickNodes(x, y, into); - } - return into; } diff --git a/packages/ag-charts-community/src/scene/scene.ts b/packages/ag-charts-community/src/scene/scene.ts index 7d9c75e35fd..33e69651599 100644 --- a/packages/ag-charts-community/src/scene/scene.ts +++ b/packages/ag-charts-community/src/scene/scene.ts @@ -30,7 +30,7 @@ export class Scene extends EventEmitter { readonly layersManager: LayersManager; readonly imageLoader = new ImageLoader(); - private root: Node | null = null; + private root: Group | null = null; private pendingSize: [number, number, number] | null = null; private isDirty: boolean = false; @@ -77,7 +77,7 @@ export class Scene extends EventEmitter { return this; } - setRoot(node: Node | null) { + setRoot(node: Group | null) { if (this.root === node) { return this; } diff --git a/packages/ag-charts-community/src/scene/sceneDebug.ts b/packages/ag-charts-community/src/scene/sceneDebug.ts index 6d11809cd75..cb4deda2761 100644 --- a/packages/ag-charts-community/src/scene/sceneDebug.ts +++ b/packages/ag-charts-community/src/scene/sceneDebug.ts @@ -191,43 +191,42 @@ export function buildTree(node: Node, mode: 'json' | 'console'): BuildTree { node: mode === 'json' ? nodeProps(node) : node, name: node.name ?? node.id, dirty: node instanceof Group ? node.dirty : undefined, - ...Array.from(node.children(), (c) => buildTree(c, mode)).reduce>( - (result, childTree) => { - let { name: treeNodeName } = childTree; - const { - node: { visible, opacity, zIndex, translationX, translationY, rotation, scalingX, scalingY }, - node: childNode, - } = childTree; - if (!visible || opacity <= 0) { - treeNodeName = `(${treeNodeName})`; - } - if (Group.is(childNode) && childNode.renderToOffscreenCanvas) { - treeNodeName = `*${treeNodeName}*`; - } - const zIndexString = Array.isArray(zIndex) ? `(${zIndex.join(', ')})` : zIndex; - const key = [ - `${(order++).toString().padStart(3, '0')}|`, - `${treeNodeName ?? ''}`, - `z: ${zIndexString}`, - translationX && `x: ${translationX}`, - translationY && `y: ${translationY}`, - rotation && `r: ${rotation}`, - scalingX != null && scalingX !== 1 && `sx: ${scalingX}`, - scalingY != null && scalingY !== 1 && `sy: ${scalingY}`, - ] - .filter((v) => !!v) - .join(' '); + ...Array.from(node instanceof Group ? node.children() : [], (c) => buildTree(c, mode)).reduce< + Record + >((result, childTree) => { + let { name: treeNodeName } = childTree; + const { + node: { visible, opacity, zIndex, translationX, translationY, rotation, scalingX, scalingY }, + node: childNode, + } = childTree; + if (!visible || opacity <= 0) { + treeNodeName = `(${treeNodeName})`; + } + if (Group.is(childNode) && childNode.renderToOffscreenCanvas) { + treeNodeName = `*${treeNodeName}*`; + } + const zIndexString = Array.isArray(zIndex) ? `(${zIndex.join(', ')})` : zIndex; + const key = [ + `${(order++).toString().padStart(3, '0')}|`, + `${treeNodeName ?? ''}`, + `z: ${zIndexString}`, + translationX && `x: ${translationX}`, + translationY && `y: ${translationY}`, + rotation && `r: ${rotation}`, + scalingX != null && scalingX !== 1 && `sx: ${scalingX}`, + scalingY != null && scalingY !== 1 && `sy: ${scalingY}`, + ] + .filter((v) => !!v) + .join(' '); - let selectedKey = key; - let index = 1; - while (result[selectedKey] != null && index < 100) { - selectedKey = `${key} (${index++})`; - } - result[selectedKey] = childTree; - return result; - }, - {} - ), + let selectedKey = key; + let index = 1; + while (result[selectedKey] != null && index < 100) { + selectedKey = `${key} (${index++})`; + } + result[selectedKey] = childTree; + return result; + }, {}), }; } @@ -240,7 +239,9 @@ export function buildDirtyTree(node: Node): { return { dirtyTree: {}, paths: [] }; } - const childrenDirtyTree = Array.from(node.children(), (c) => buildDirtyTree(c)).filter((c) => c.paths.length > 0); + const childrenDirtyTree = Array.from(node instanceof Group ? node.children() : [], (c) => buildDirtyTree(c)).filter( + (c) => c.paths.length > 0 + ); const name = Group.is(node) ? node.name ?? node.id : node.id; const paths = childrenDirtyTree.length ? childrenDirtyTree.flatMap((c) => c.paths).map((p) => `${name}.${p}`) diff --git a/packages/ag-charts-community/src/scene/selection.test.ts b/packages/ag-charts-community/src/scene/selection.test.ts index 87bfcfabba1..7a8ebb2ca7c 100644 --- a/packages/ag-charts-community/src/scene/selection.test.ts +++ b/packages/ag-charts-community/src/scene/selection.test.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from '@jest/globals'; -import { Node } from './node'; +import { Group } from './group'; import { Selection } from './selection'; -class TestNode extends Node {} +class TestNode extends Group {} const expectSelectionToMatchData = (selection: Selection, data: Array) => { expect(selection.nodes()).toHaveLength(data.length); diff --git a/packages/ag-charts-community/src/scene/selection.ts b/packages/ag-charts-community/src/scene/selection.ts index c5df571f1ef..6a089bdc84b 100644 --- a/packages/ag-charts-community/src/scene/selection.ts +++ b/packages/ag-charts-community/src/scene/selection.ts @@ -1,4 +1,5 @@ import { Debug } from '../util/debug'; +import { Group } from './group'; import { Node } from './node'; type ValidId = string | number; @@ -8,7 +9,7 @@ type NodeConstructorOrFactory = NodeConstructor { static select( - parent: Node, + parent: Group, classOrFactory: NodeConstructorOrFactory, garbageCollection: boolean = true ) { @@ -21,8 +22,10 @@ export class Selection { if (predicate(node)) { results.push(node); } - for (const child of node.children()) { - traverse(child); + if (node instanceof Group) { + for (const child of node.children()) { + traverse(child); + } } }; traverse(parent); @@ -47,7 +50,7 @@ export class Selection { private readonly debug = Debug.create(true, 'scene', 'scene:selections'); constructor( - private readonly parentNode: Node, + private readonly parentNode: Group, classOrFactory: NodeConstructorOrFactory, private readonly autoCleanup: boolean = true ) { diff --git a/packages/ag-charts-enterprise/src/features/crosshair/crosshair.ts b/packages/ag-charts-enterprise/src/features/crosshair/crosshair.ts index 863367fb6d7..d33be274e19 100644 --- a/packages/ag-charts-enterprise/src/features/crosshair/crosshair.ts +++ b/packages/ag-charts-enterprise/src/features/crosshair/crosshair.ts @@ -125,11 +125,7 @@ export class Crosshair extends _ModuleSupport.BaseModuleInstance implements _Mod } private updateSelections(data: string[]) { - this.lineGroupSelection.update( - data, - (group) => group.append(new Line()), - (key: string) => key - ); + this.lineGroupSelection.update(data, undefined, (key: string) => key); } private updateLabels(keys: string[]) { diff --git a/packages/ag-charts-enterprise/src/features/error-bar/errorBarNode.ts b/packages/ag-charts-enterprise/src/features/error-bar/errorBarNode.ts index 8b9340dd5bd..88599b0de98 100644 --- a/packages/ag-charts-enterprise/src/features/error-bar/errorBarNode.ts +++ b/packages/ag-charts-enterprise/src/features/error-bar/errorBarNode.ts @@ -54,7 +54,7 @@ export class ErrorBarNode extends _ModuleSupport.Group { // of 0. Therefore, we only need bounding boxes for number based ranges. private readonly bboxes: HierarchicalBBox; - protected override _datum?: ErrorBarNodeDatum = undefined; + protected override _datum: ErrorBarNodeDatum | undefined = undefined; public override get datum(): ErrorBarNodeDatum | undefined { return this._datum; } diff --git a/packages/ag-charts-enterprise/src/gradient-legend/axisTicks.ts b/packages/ag-charts-enterprise/src/gradient-legend/axisTicks.ts index 10848565afc..036eaa36261 100644 --- a/packages/ag-charts-enterprise/src/gradient-legend/axisTicks.ts +++ b/packages/ag-charts-enterprise/src/gradient-legend/axisTicks.ts @@ -44,7 +44,7 @@ export class AxisTicks { return this.position === 'top' || this.position === 'bottom'; } - attachAxis(axisNode: _ModuleSupport.Node) { + attachAxis(axisNode: _ModuleSupport.Group) { axisNode.appendChild(this.axisGroup); }