Skip to content

Commit 5ad49e9

Browse files
authored
Handle edge cases from add()ing elements twice (#122)
1 parent eebabe1 commit 5ad49e9

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

src/graphics/index.js

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,9 @@ class GraphicsManager extends Manager {
466466
elem.stop();
467467
}
468468
elem.alive = false;
469+
// mark the element as having invalidated sort, so in the case that it's
470+
// add()ed later, a re-sort will happen and trigger an update in the pool size
471+
elem._sortInvalidated = true;
469472
if (elem._hasAccessibleDOMElement) {
470473
const focusButtonID = HIDDEN_KEYBOARD_NAVIGATION_DOM_ELEMENT_ID(elem._id);
471474
document.getElementById(focusButtonID)?.remove();
@@ -679,7 +682,6 @@ class GraphicsManager extends Manager {
679682
let elem;
680683
let sortPool;
681684
const context = this.getContext();
682-
let numberRemovedElementsFound = 0;
683685
for (let i = 0; i < this.elementPoolSize; i++) {
684686
elem = this.elementPool[i];
685687

@@ -691,15 +693,21 @@ class GraphicsManager extends Manager {
691693
elem.draw(context);
692694
} else {
693695
sortPool = true;
694-
numberRemovedElementsFound++;
695696
}
696697
}
697698
// sort all dead elements to the end of the pool
698699
// and all elements with lower layer before elements
699700
// with higher layer
700701
if (sortPool) {
701-
this.elementPoolSize -= numberRemovedElementsFound;
702702
this.elementPool.sort((a, b) => b.alive - a.alive || a.layer - b.layer);
703+
let lastAliveElementIndex = -1;
704+
for (let i = this.elementPool.length - 1; i >= 0; i--) {
705+
if (this.elementPool[i].alive) {
706+
lastAliveElementIndex = i;
707+
break;
708+
}
709+
}
710+
this.elementPoolSize = lastAliveElementIndex + 1;
703711
}
704712
}
705713

test/graphics.test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,32 @@ describe('Graphics', () => {
276276
expect(g.elementPool[1].alive).toBeTrue();
277277
expect(g.elementPool[2].alive).toBeFalse();
278278
});
279+
it('Properly handles duplicated elements', () => {
280+
const g = new Graphics({ shouldUpdate: false });
281+
const a = new Circle(1);
282+
const b = new Circle(1);
283+
const c = new Circle(1);
284+
g.add(a);
285+
g.add(b);
286+
g.add(c);
287+
g.add(a);
288+
expect(g.elementPool).toEqual([a, b, c, a]);
289+
expect(g.elementPoolSize).toEqual(4);
290+
g.remove(a);
291+
expect(g.elementPool).toEqual([a, b, c, a]);
292+
expect(g.elementPoolSize).toEqual(4);
293+
g.redraw();
294+
expect(g.elementPool).toEqual([b, c, a, a]);
295+
expect(g.elementPoolSize).toEqual(2);
296+
g.add(a);
297+
g.redraw();
298+
expect(g.elementPool).toEqual([b, c, a, a]);
299+
expect(g.elementPoolSize).toEqual(3);
300+
a.layer = -1;
301+
g.redraw();
302+
expect(g.elementPool).toEqual([a, a, b, c]);
303+
expect(g.elementPoolSize).toEqual(4);
304+
});
279305
});
280306
describe('setBackgroundColor', () => {
281307
it('Causes drawBackground to be invoked in redraw()', () => {

0 commit comments

Comments
 (0)