Sprites sometimes continue to render after exit

PAIR-code / megaplot

Apache License 2.0

19 stars 5 forks source link

Sprites sometimes continue to render after exit #77

Closed jimbojw closed 1 year ago

jimbojw commented 1 year ago

Under some circumstances, Sprites that should have been removed after calling TextSelection's bind() method remain when they should have been removed from the Scene.

To reproduce:

Run the text thrashing demo: npm run text-demo.
Lower the maxBatchTimeMs value down to 1.
Lower the corpusPercentage value down to 1.
Zoom out and look down.

Expected: Only white text remains. Removed sprites (red) should have disappeared.

Actual: Red, removed sprites persist.

Screenshot is from work-in-progress PR #75

jimbojw commented 1 year ago

Update: This is a problem with the underlying Sprite implementation, not TextSelection specifically. The following screenshot shows two sprites remaining from an earlier state which should have been removed (near the top-left and in the middle region).

The bug went unnoticed for a long time because the demos fade sprites out on exit. The leftover sprites' exit handlers are being run, so they would become transparent, even though they remained in the scene. Changing the exit callback to keep opacity revealed the bug.

One clue is that the bug seems to happen more frequently the lower that maxBatchTimeMs is set. For debugging testing, we should crank this all the way down to the lowest possible values.

jimbojw commented 1 year ago

Local changes:

Set maxBatchTimeMs to 0.
Set steps between checks to 1.
Set sprites to fade in from green (full opacity).
Set sprites to fade out to black (full opacity).

Expected: Only four sprites remain (note count set to 2).

Actual: Earlier sprites are still going through their enter/update/exit lifecycle. Even stranger, only every other sprite is actually being removed after its exit code has run, leading to a checkerboard effect, but even this is inconsistent.

This setup seems to be sufficient to determine when the issue has been fixed, however there's too much going on for debugging purposes. Next step is to develop a unit test that exhibits the problem at the Sprite level of the API, since the debugging demo operates at the Selection level.