Found while fixing #5600. Splitting it out because the fix changes Android rendering in a way that needs its own investigation (see below).
The defect
AnimationManager.updateAnimations() polls the head of the queue:
if (c.isInProgress()) { c.updateAnimationState(); } // -> updateState()
else { c.completeAnimation(); anims.remove(c); }
completeAnimation() deliberately skips updateState(). That is right for an animation that already ran, but wrong for one whose isInProgress() is false from the moment it is queued: that animation was never stepped, so its entire payload is still sitting in updateState(). Core has three such animations, and all three are silently dead today:
Container.removeComponentImpl — the deferred removal taken while another animation is in flight. The component is detached from the layout and its parent nulled, but it is never taken out of components, so it keeps painting.
Container.addComponentImpl — the matching deferred insertion. cmp.setParent(this) is set optimistically and the component never enters components, so cnt.add(x) during an animation is a silent no-op.
RefreshThemeCallback — used by Container.wrapInLayeredPane(), which Form.getLayeredPane() relies on to re-root the content pane.
So: adding or removing a component while any animation is running does not do what it says.
Reproduction
Two tests, both red on master (they were on the #5600 branch at 90261fb, reverted in f169ae5):
form.getAnimationManager().addAnimation(new BlockingAnimation(3));
victim.remove();
drain(form);
assertEquals(0, cnt.getComponentCount()); // fails: 1
form.getAnimationManager().addAnimation(new BlockingAnimation(3));
cnt.add(added);
drain(form);
assertEquals(1, cnt.getComponentCount()); // fails: 0
The fix, and why it is not merged yet
completeAnimation() should give a never-stepped animation exactly one updateState() before completing. A CompoundAnimation needs to apply every pending child rather than one aggregate update — a never-started sequentialAnimation() has already had isInProgress() walk its cursor past the end, so the single update lands on the last child.
With that in place, the Android ValidatorLightweightPicker screenshot test fails deterministically on all three JDK legs. The diff is the content pane composited twice — actual = reference² / 255 across y 97..302 full width, with the title area and the opaque popup unchanged. Text drawn twice is a regression, so rebaselining would bury one.
What has been ruled out:
- Not a hierarchy difference. A headless replay of the picker scenario produces an identical component tree with and without the fix.
- Not the paint queue's ancestor dedup.
PaintSurface.repaint() does drop a component whose ancestor is queued but kept a descendant queued when the ancestor arrived later, so the subtree painted twice in one flush (pinned by a counting test). Fixing that made no difference to the Android output — the new screenshot was byte-identical to the previous one.
- Not reproducible headlessly. Instrumenting the never-stepped branch and replaying the picker scenario — even with the animation queue deliberately held busy — never reaches it off-device. Across the whole
core-unittests suite only Container$2 (deferred insert, 12 hits) and Container$3 (deferred remove, 22 hits) reach it.
Next step is most likely a bisect on CI: complete the deferred removal but not the insertion, and see which one moves the screenshot.
Related
The paint-queue asymmetry above is a real bug in its own right and worth fixing separately: a child queued before its parent paints twice in one flush, so anything translucent in it composites twice.
Found while fixing #5600. Splitting it out because the fix changes Android rendering in a way that needs its own investigation (see below).
The defect
AnimationManager.updateAnimations()polls the head of the queue:completeAnimation()deliberately skipsupdateState(). That is right for an animation that already ran, but wrong for one whoseisInProgress()is false from the moment it is queued: that animation was never stepped, so its entire payload is still sitting inupdateState(). Core has three such animations, and all three are silently dead today:Container.removeComponentImpl— the deferred removal taken while another animation is in flight. The component is detached from the layout and its parent nulled, but it is never taken out ofcomponents, so it keeps painting.Container.addComponentImpl— the matching deferred insertion.cmp.setParent(this)is set optimistically and the component never enterscomponents, socnt.add(x)during an animation is a silent no-op.RefreshThemeCallback— used byContainer.wrapInLayeredPane(), whichForm.getLayeredPane()relies on to re-root the content pane.So: adding or removing a component while any animation is running does not do what it says.
Reproduction
Two tests, both red on master (they were on the #5600 branch at
90261fb, reverted inf169ae5):The fix, and why it is not merged yet
completeAnimation()should give a never-stepped animation exactly oneupdateState()before completing. ACompoundAnimationneeds to apply every pending child rather than one aggregate update — a never-startedsequentialAnimation()has already hadisInProgress()walk its cursor past the end, so the single update lands on the last child.With that in place, the Android
ValidatorLightweightPickerscreenshot test fails deterministically on all three JDK legs. The diff is the content pane composited twice —actual = reference² / 255across y 97..302 full width, with the title area and the opaque popup unchanged. Text drawn twice is a regression, so rebaselining would bury one.What has been ruled out:
PaintSurface.repaint()does drop a component whose ancestor is queued but kept a descendant queued when the ancestor arrived later, so the subtree painted twice in one flush (pinned by a counting test). Fixing that made no difference to the Android output — the new screenshot was byte-identical to the previous one.core-unittestssuite onlyContainer$2(deferred insert, 12 hits) andContainer$3(deferred remove, 22 hits) reach it.Next step is most likely a bisect on CI: complete the deferred removal but not the insertion, and see which one moves the screenshot.
Related
The paint-queue asymmetry above is a real bug in its own right and worth fixing separately: a child queued before its parent paints twice in one flush, so anything translucent in it composites twice.