Skip to content

Commit 34bd29f

Browse files
chiefcllclaude
andauthored
fix(core): stable zIndex child sort for Chrome < 70 (#101)
Array.prototype.sort is unstable on Chrome < 70 (V8 quicksorts arrays longer than 10 elements), so equal-zIndex siblings could silently reshuffle paint order on embedded targets. Replace the native sort in sortChildren with a stable, allocation-free insertion sort — children arrays are small and nearly sorted, so it is O(n) in practice. Also removes the unused bucketSortByZIndex, which indexed buckets by zIndex - min: legal huge zIndex values created enormous sparse arrays, and below-min or fractional values leaked as object properties. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent cd165f2 commit 34bd29f

3 files changed

Lines changed: 106 additions & 41 deletions

File tree

src/core/CoreNode.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import type { IAnimationController } from '../common/IAnimationController.js';
4646
import { createAnimation } from './animations/CoreAnimation.js';
4747
import type { CoreShaderNode } from './renderers/CoreShaderNode.js';
4848
import { AutosizeMode, Autosizer } from './Autosizer.js';
49-
import { removeChild } from './lib/collectionUtils.js';
49+
import { removeChild, sortByZIndexStable } from './lib/collectionUtils.js';
5050

5151
export enum CoreNodeRenderState {
5252
Init = 0,
@@ -63,11 +63,6 @@ const NO_CLIPPING_RECT: RectWithValid = {
6363
valid: false,
6464
};
6565

66-
// Hoisted so `sortChildren` doesn't allocate a fresh comparator closure on
67-
// every z-index reorder.
68-
const compareZIndex = (a: CoreNode, b: CoreNode): number =>
69-
a.props.zIndex - b.props.zIndex;
70-
7166
const CoreNodeRenderStateMap: Map<CoreNodeRenderState, string> = new Map();
7267
CoreNodeRenderStateMap.set(CoreNodeRenderState.Init, 'init');
7368
CoreNodeRenderStateMap.set(CoreNodeRenderState.OutOfBounds, 'outOfBounds');
@@ -2207,7 +2202,7 @@ export class CoreNode extends EventEmitter {
22072202
}
22082203

22092204
sortChildren() {
2210-
this.children.sort(compareZIndex);
2205+
sortByZIndexStable(this.children);
22112206
this.stage.requestRenderListUpdate();
22122207
}
22132208

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
import { describe, it, expect } from 'vitest';
2+
import type { CoreNode } from '../CoreNode.js';
3+
import { sortByZIndexStable } from './collectionUtils.js';
4+
5+
const makeNode = (zIndex: number, tag: number): CoreNode =>
6+
({ props: { zIndex }, tag } as unknown as CoreNode);
7+
8+
const tags = (nodes: CoreNode[]): number[] => {
9+
const out: number[] = [];
10+
for (let i = 0; i < nodes.length; i++) {
11+
out.push((nodes[i] as unknown as { tag: number }).tag);
12+
}
13+
return out;
14+
};
15+
16+
describe('sortByZIndexStable', () => {
17+
it('should sort nodes ascending by zIndex', () => {
18+
const nodes = [makeNode(3, 0), makeNode(1, 1), makeNode(2, 2)];
19+
20+
sortByZIndexStable(nodes);
21+
22+
expect(tags(nodes)).toEqual([1, 2, 0]);
23+
});
24+
25+
it('should preserve insertion order for equal zIndex values', () => {
26+
const nodes = [
27+
makeNode(1, 0),
28+
makeNode(0, 1),
29+
makeNode(1, 2),
30+
makeNode(0, 3),
31+
makeNode(1, 4),
32+
];
33+
34+
sortByZIndexStable(nodes);
35+
36+
expect(tags(nodes)).toEqual([1, 3, 0, 2, 4]);
37+
});
38+
39+
it('should be stable for arrays longer than 10 elements', () => {
40+
// V8 on Chrome < 70 switched to an unstable quicksort above 10 elements;
41+
// this guards the case the native sort got wrong.
42+
const nodes: CoreNode[] = [];
43+
for (let i = 0; i < 20; i++) {
44+
nodes.push(makeNode(i % 2, i));
45+
}
46+
47+
sortByZIndexStable(nodes);
48+
49+
const expected: number[] = [];
50+
for (let i = 0; i < 20; i += 2) {
51+
expected.push(i);
52+
}
53+
for (let i = 1; i < 20; i += 2) {
54+
expected.push(i);
55+
}
56+
expect(tags(nodes)).toEqual(expected);
57+
});
58+
59+
it('should handle negative and fractional zIndex values', () => {
60+
const nodes = [
61+
makeNode(0.5, 0),
62+
makeNode(-2, 1),
63+
makeNode(1000000000, 2),
64+
makeNode(0, 3),
65+
makeNode(-2, 4),
66+
];
67+
68+
sortByZIndexStable(nodes);
69+
70+
expect(tags(nodes)).toEqual([1, 4, 3, 0, 2]);
71+
});
72+
73+
it('should leave an already sorted array unchanged', () => {
74+
const nodes = [makeNode(0, 0), makeNode(0, 1), makeNode(1, 2)];
75+
76+
sortByZIndexStable(nodes);
77+
78+
expect(tags(nodes)).toEqual([0, 1, 2]);
79+
});
80+
81+
it('should handle empty and single-element arrays', () => {
82+
const empty: CoreNode[] = [];
83+
const single = [makeNode(5, 0)];
84+
85+
sortByZIndexStable(empty);
86+
sortByZIndexStable(single);
87+
88+
expect(empty.length).toBe(0);
89+
expect(tags(single)).toEqual([0]);
90+
});
91+
});

src/core/lib/collectionUtils.ts

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,23 @@
11
import type { CoreNode } from '../CoreNode.js';
22

3-
//Bucket sort implementation for sorting CoreNode arrays by zIndex
4-
export const bucketSortByZIndex = (nodes: CoreNode[], min: number): void => {
5-
const buckets: CoreNode[][] = [];
6-
const bucketIndices: number[] = [];
7-
//distribute nodes into buckets
8-
for (let i = 0; i < nodes.length; i++) {
3+
// Stable in-place sort by zIndex. Array.prototype.sort is not stable on
4+
// Chrome < 70 (V8 quicksorts arrays longer than 10), and equal-zIndex
5+
// siblings must keep insertion order or paint order silently reshuffles.
6+
// Children arrays are small and nearly sorted (kept sorted; a re-sort
7+
// usually follows a single zIndex change), so insertion sort is O(n) in
8+
// practice and allocation-free.
9+
export const sortByZIndexStable = (nodes: CoreNode[]): void => {
10+
const len = nodes.length;
11+
for (let i = 1; i < len; i++) {
912
const node = nodes[i]!;
10-
const index = node.props.zIndex - min;
11-
//create bucket if it doesn't exist
12-
if (buckets[index] === undefined) {
13-
buckets[index] = [];
14-
bucketIndices.push(index);
15-
}
16-
buckets[index]!.push(node);
17-
}
18-
19-
//sort each bucket using insertion sort
20-
for (let i = 1; i < bucketIndices.length; i++) {
21-
const key = bucketIndices[i]!;
13+
const z = node.props.zIndex;
2214
let j = i - 1;
23-
while (j >= 0 && bucketIndices[j]! > key) {
24-
bucketIndices[j + 1] = bucketIndices[j]!;
15+
while (j >= 0 && nodes[j]!.props.zIndex > z) {
16+
nodes[j + 1] = nodes[j]!;
2517
j--;
2618
}
27-
bucketIndices[j + 1] = key;
19+
nodes[j + 1] = node;
2820
}
29-
30-
//flatten buckets
31-
let idx = 0;
32-
for (let i = 0; i < bucketIndices.length; i++) {
33-
const bucket = buckets[bucketIndices[i]!]!;
34-
for (let j = 0; j < bucket.length; j++) {
35-
nodes[idx++] = bucket[j]!;
36-
}
37-
}
38-
39-
//clean up
40-
buckets.length = 0;
41-
bucketIndices.length = 0;
4221
};
4322

4423
export const incrementalRepositionByZIndex = (

0 commit comments

Comments
 (0)