From 3c010cbefb4970105871cdab4564c7fc347cfc1b Mon Sep 17 00:00:00 2001 From: cptbtptpbcptdtptp Date: Thu, 16 Jul 2026 19:47:54 +0800 Subject: [PATCH] fix(ui): correct root-canvas demotion state and listener bookkeeping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three coupled fixes ported from #3068 (fix/shaderlab): - The demote loop dirtied the canvas itself instead of each element (setRootCanvasDirty(this) -> (element)), so demoted elements kept stale _rootCanvas/_indexInRootCanvas and never re-homed: the new root's walk re-assignment is gated on the element's dirty flag. - _registerListener truncated a shrinking listening chain without unregistering the dropped tail, leaking listeners on ex-ancestors. - Releasing that leak exposed that nested-canvas demotion relied on it: UpdateFlagManager.dispatch iterates a backward snapshot, so a listener appended mid-dispatch is never invoked by that dispatch, and the enabling canvas claims root status only after its dispatch returns — the cascade's search could not find it and promoted nested canvases to root. The demote cascade now receives the enabling canvas as an explicit successor. Co-Authored-By: Claude Fable 5 --- packages/ui/src/Utils.ts | 6 +++ packages/ui/src/component/UICanvas.ts | 17 ++++--- tests/src/ui/UICanvas.test.ts | 65 ++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/packages/ui/src/Utils.ts b/packages/ui/src/Utils.ts index f47e0a8053..60c0418630 100644 --- a/packages/ui/src/Utils.ts +++ b/packages/ui/src/Utils.ts @@ -139,6 +139,12 @@ export class Utils { entity = entity.parent; count++; } + // A shorter chain drops tail entries — they must release the listener, or the bound + // element stays reachable from (and invocable by) entities it no longer tracks. + for (let i = count, n = listeningEntities.length; i < n; i++) { + // @ts-ignore + listeningEntities[i]._unRegisterModifyListener(listener); + } listeningEntities.length = count; } diff --git a/packages/ui/src/component/UICanvas.ts b/packages/ui/src/component/UICanvas.ts index a69cd846bf..28c85b3caa 100644 --- a/packages/ui/src/component/UICanvas.ts +++ b/packages/ui/src/component/UICanvas.ts @@ -412,7 +412,9 @@ export class UICanvas extends Component implements IElement { this._setIsRootCanvas(!rootCanvas); Utils.setRootCanvas(this, rootCanvas); } else if (flag === EntityUIModifyFlags.CanvasEnableInScene) { - this._setIsRootCanvas(false); + // The enabling canvas has not claimed root status at dispatch time, so searches + // inside the demote cascade cannot find it — hand it down as the successor. + this._setIsRootCanvas(false, param); Utils.setRootCanvas(this, param); } } else { @@ -632,7 +634,7 @@ export class UICanvas extends Component implements IElement { } } - private _setIsRootCanvas(isRootCanvas: boolean): void { + private _setIsRootCanvas(isRootCanvas: boolean, successor: UICanvas = null): void { if (this._isRootCanvas !== isRootCanvas) { this._isRootCanvas = isRootCanvas; this._updateCameraObserver(); @@ -643,11 +645,16 @@ export class UICanvas extends Component implements IElement { const { _disorderedElements: disorderedElements } = this; disorderedElements.forEach((element: IElement) => { if (element instanceof UICanvas) { - const rootCanvas = Utils.searchRootCanvasInParents(element); - element._setIsRootCanvas(!rootCanvas); + // `successor` covers the enable-driven demote, where the search cannot see the + // enabling canvas yet (it claims root status only after its dispatch returns). + const rootCanvas = Utils.searchRootCanvasInParents(element) ?? successor; + element._setIsRootCanvas(!rootCanvas, successor); Utils.setRootCanvas(element, rootCanvas); } else { - Utils.setRootCanvasDirty(this); + // Reset the element's own canvas state (stale _rootCanvas/_indexInRootCanvas and + // the dirty flag the new root's walk re-assignment is gated on) — the bulk + // clear below only empties this canvas's list. + Utils.setRootCanvasDirty(element); Utils.setGroupDirty(element); } }); diff --git a/tests/src/ui/UICanvas.test.ts b/tests/src/ui/UICanvas.test.ts index 5e534763c8..602d4c388a 100644 --- a/tests/src/ui/UICanvas.test.ts +++ b/tests/src/ui/UICanvas.test.ts @@ -1,7 +1,7 @@ import { Camera } from "@galacean/engine-core"; import { Vector2 } from "@galacean/engine-math"; import { WebGLEngine } from "@galacean/engine"; -import { CanvasRenderMode, ResolutionAdaptationMode, UICanvas, UITransform } from "@galacean/engine-ui"; +import { CanvasRenderMode, Image, ResolutionAdaptationMode, UICanvas, UITransform } from "@galacean/engine-ui"; import { describe, expect, it } from "vitest"; describe("UICanvas", async () => { @@ -351,4 +351,67 @@ describe("UICanvas", async () => { // @ts-ignore expect(rootCanvas._canDispatchEvent(camera2)).to.be.false; }); + it("re-homes elements to the new root canvas when theirs loses root status", () => { + const outerEntity = root.createChild("outer"); + const innerEntity = outerEntity.createChild("inner"); + const innerCanvas = innerEntity.addComponent(UICanvas); + const imageEntity = innerEntity.createChild("image"); + const image = imageEntity.addComponent(Image); + + // Collected by the inner canvas while it is the root. + // @ts-ignore + innerCanvas._getRenderers(); + // @ts-ignore + expect(image._getRootCanvas()).to.eq(innerCanvas); + // @ts-ignore + expect(innerCanvas._disorderedElements.length).to.be.greaterThan(0); + + // Enabling a canvas on an ancestor demotes the inner one. + const outerCanvas = outerEntity.addComponent(UICanvas); + // @ts-ignore + expect(innerCanvas._isRootCanvas).to.be.false; + // @ts-ignore + expect(outerCanvas._isRootCanvas).to.be.true; + // The element's canvas state must be reset (the walk's re-assignment is gated on the + // dirty flag), otherwise it keeps rendering into the demoted canvas's dead queue. + // @ts-ignore + expect(image._isRootCanvasDirty).to.be.true; + // @ts-ignore + expect(image._rootCanvas).to.be.null; + + // The new root adopts it on its next walk. + // @ts-ignore + outerCanvas._getRenderers(); + // @ts-ignore + expect(image._getRootCanvas()).to.eq(outerCanvas); + + outerEntity.destroy(); + }); + it("re-roots nested canvases to the enabling ancestor canvas", () => { + const outerEntity = root.createChild("outer"); + const midEntity = outerEntity.createChild("mid"); + const midCanvas = midEntity.addComponent(UICanvas); + const innerEntity = midEntity.createChild("inner"); + const innerCanvas = innerEntity.addComponent(UICanvas); + // @ts-ignore + expect(midCanvas._isRootCanvas).to.be.true; + // @ts-ignore + expect(innerCanvas._isRootCanvas).to.be.false; + + // The enabling canvas claims root status only after its enable dispatch returns, so the + // demote cascade must hand it down instead of searching for it. + const outerCanvas = outerEntity.addComponent(UICanvas); + // @ts-ignore + expect(outerCanvas._isRootCanvas).to.be.true; + // @ts-ignore + expect(midCanvas._isRootCanvas).to.be.false; + // @ts-ignore + expect(innerCanvas._isRootCanvas).to.be.false; + // @ts-ignore + expect(midCanvas._getRootCanvas()).to.eq(outerCanvas); + // @ts-ignore + expect(innerCanvas._getRootCanvas()).to.eq(outerCanvas); + + outerEntity.destroy(); + }); });