feat(spine): render SpineAnimationRenderer directly inside UICanvas#3068
feat(spine): render SpineAnimationRenderer directly inside UICanvas#3068cptbtptpbcptdtptp wants to merge 8 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SpineAnimationRenderer now works in both spaces with no separate component: in world space it renders through the camera pipeline as before; placed under a root UICanvas it is hosted by the canvas — collected and ordered with the other UI elements (all render modes incl. overlay), faded by UIGroup alpha, clipped by RectMask2D, and optionally hit-testable against its skeleton bounds. resource.instantiate() therefore works in both spaces. Core gains a canvas-hosting protocol so any renderer package can opt in without depending on the ui package: - IUIElement/IUIGroupAble/IUIHostedRenderer/IUIGroup interfaces + the RootCanvasModifyFlags/GroupModifyFlags/EntityUIModifyFlags enums move to core (ui re-exports them); IUICanvas widened with the hosting surface - UIElementUtils: the root-canvas/group registration + parent-chain listener machinery, extracted from ui's Utils (ui delegates); hierarchy version counter and Entity._updateUIHierarchyVersion move from the ui Entity mixin into core ui package: - UICanvas._walk collects any Renderer implementing IUIHostedRenderer and wires its canvas/group/rect-mask state exactly like a UIRenderer; the canvas applies rect-clip shader state for hosted renderers (UIRenderer keeps doing its own via the shared Utils helper) - rect-clip state computation extracted to Utils for both consumer kinds spine package: - SpineAnimationRenderer implements IUIHostedRenderer: registration switches between componentsManager renderers and canvas hosting on enable/reparent (whole-parent-chain listeners re-home it live), dual-path _render (canvas elements vs pipeline push), group alpha folds into vertex colors through the new ISpineRenderTarget.globalAlpha (multiplied before premultiplication so PMA light/dark colors follow; both 3.8/4.2 generators), and the RENDERER_UI_RECT_CLIP shader macro toggles with hosting - spine shader: macro-gated rect clip deriving world position from renderer_ModelMat (spine vertices are entity-local, unlike pre-transformed UI chunk vertices), PMA-aware clip fade, hard clip on the rect factor (additive slots legitimately emit zero-alpha fragments), and overlay-pass sRGB output correction e2e: serve spine packages from source in the case dev server — their ES5 dist subclasses of native @esotericsoftware/spine-core classes throw "Class constructor cannot be invoked without 'new'", so spine cases could not run at all; existing spineboy/tintBlack baselines pass unchanged. tests: 14-case hosted-behavior suite (incl. live re-homing on reparent); e2e: spine-ui-canvas and spine-ui-rect-mask with baselines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
75c9348 to
432cd27
Compare
_setIsRootCanvas(false) marked the demoted canvas itself dirty once per element instead of the element (present since galacean#2375). The bulk clear below only empties the canvas's element list, leaving each element's _rootCanvas pointer, _indexInRootCanvas and dirty flag stale — and the new root's walk re-assignment is gated on that dirty flag, so re-homed elements kept pushing render elements into the demoted canvas's dead queue and disappeared. Canvas-hosted renderers rely on the same gate, so demotion re-homing now works for them too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Added a standalone fix commit (dd965d4): |
GuoLei1990
left a comment
There was a problem hiding this comment.
审查 @ dd965d450(第三轮,增量)— 独立修复:fix(ui): reset element canvas state when their root canvas is demoted
总结
第三轮相较上轮 432cd2708 是线性追加单 commit dd965d450(compare 返 ahead:1 / behind:0,无 force-push),仅 2 文件:UICanvas.ts +4/-1、UICanvas.test.ts +38/-2。修复 _setIsRootCanvas(false) 降级路径里一处自 #2375(8703a2dc4)就存在的真 bug——把子元素状态重置误传成 canvas 自身(setRootCanvasDirty(this) → setRootCanvasDirty(element))。逐链核对无 P0/P1/P2/P3,方向正确、落点在根因端、回归测试真反向证伪。上轮两条未闭环 [P3](ISpineRenderTarget.ts:8 注释失真 / vite.config.js:60 死别名)本 commit 未触及,状态不变、不重提。
逐链核对(对 dd965d450 tree 实读 + git blame/git show origin/dev/2.0 校验,非信 commit message)
- 确是 pre-existing bug 非本 PR 引入:
git log -S'setRootCanvasDirty(this)'指向8703a2dc4 Support GUI (#2375);git show origin/dev/2.0:.../UICanvas.ts:650同样是setRootCanvasDirty(this)——mainline 也带此 bug,作者「present since #2375 (dev/2.0 has the same line)」准确。非对称是铁证:同段setGroupDirty(<IGroupAble>element)正确传element,唯独setRootCanvasDirty(this)传this,是复制/笔误,修复恢复了对称。 - 修复落点在输入端(root cause),非输出端擦屁股:bug 语义是「canvas 降级时漏重置子元素的托管状态」。核
UIElementUtils.setRootCanvasDirty(element)——①element._isRootCanvasDirty=true、②_registerRootCanvas(element,null)把element._rootCanvas置 null +_indexInRootCanvas=-1、③ fire_onRootCanvasModify(All)。旧setRootCanvasDirty(this)只动 canvas 自身 dirty flag,子element._rootCanvas仍悬挂指向已降级 canvas、_isRootCanvasDirty恒 false。 - 新根的重新收养被 dirty flag 门控——这是 bug 显形的机理:
_walk:534renderable._isRootCanvasDirty && Utils.setRootCanvas(renderable, this)。旧代码下降级元素_isRootCanvasDirty恒 false → 新根 walk 时跳过setRootCanvas→ 元素永远迁不到新根、继续往已降级 canvas 的 dead queue 推 render element → 从画面消失。修复令_isRootCanvasDirty=true才使新根:534收养成立。注释「the walk's re-assignment is gated on the dirty flag」与:534实码一致(非 aspirational)。 - hosted renderer(spine)依赖同一 gate 坐实:
_walk:530判据是instanceof UIRenderer || _isUIHostedRenderer,二者共用:534同一 dirty-gated 重分配。故降级下 spine 会中同款孤儿 bug,此 fix 对UIRenderer与 hosted renderer 统一收口修复。作者「hosted renderers rely on the same gate, hence fixing it here」准确,scope 收敛得当。 - 迭代内 forEach × deleteByIndex 无索引损坏:
_registerRootCanvas(element,null)会对正被forEach迭代的同一_disorderedElements调deleteByIndex(element._indexInRootCanvas)。DisorderedArray.forEach先_startLoop()使_loopCounter>0,deleteByIndex走 loop-safe 分支(仅置 null +_blankCount++,不压缩/不改.length、返回 undefined 故replaced && ...跳过)——不扰乱迭代下标。循环后disorderedElements.length=0 + garbageCollection()整体清空,in-loop 的 null 槽本就作废。安全。 - 回归测试是真反向证伪的链路测试:新
it("re-homes elements...")从公开入口innerCanvas._getRenderers()(触发 walk 收集)→ 祖先加 canvas 降级 inner → 断言image._isRootCanvasDirty===true+image._rootCanvas===null(旧代码此二者恒 false/悬挂→fail)→outerCanvas._getRenderers()walk 收养 →image._getRootCanvas()===outerCanvas(旧代码收养被跳过→fail)。revert 修复行则前后两组断言皆红,作者「fails on the unfixed line, verified」成立。outerEntity.destroy()收尾、子树隔离于共享rootCanvas,无状态泄漏。
问题
无。本 commit 无 P0/P1/P2/P3。
记录(不阻塞,状态延续上轮)
- 上轮两条 [P3](
ISpineRenderTarget.ts:8注释仍写「UI-space renderer」双组件失真 /e2e/.dev/vite.config.js:60死别名@galacean/engine-spine-ui)本 commit 未触及,仍开放但不阻塞。 - CI 白名单结构约束不变:target=
fix/shaderlab不在ci.yml的pull_request: branches,gh pr checks 3068仅 labeler 过。本 fix 是纯逻辑单行改动 + 单测,UICanvas.test.ts在 codecov/unit 线可覆盖,但受 shaderlab 合并粒度约束当前不跑;建议合并前本地跑UICanvas.test.ts或于fix/shaderlab→dev/2.0时由完整 CI 补验。
简化建议
无。单行修复已是最本质解——回到输入端补全子元素状态声明,而非在输出端(_walk / _getRenderers)打补丁,正是「修复落点在根因层」的范例。测试恰好从公开链路反向证伪,不戳私有制造虚假状态。代码干净。
🤖 Generated with Claude Code
Entity stays free of UI members: _uiHierarchyVersion/_updateUIHierarchyVersion return to the ui package's ApplyMixins extension. Hosted renderers call the mixin method optionally — a canvas implies the ui package is loaded, and without it there is nothing to notify. The version counter stays in core's UIElementUtils, which both sides read. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
审查 @ a169dcca7(第四轮,增量)— refactor(ui): keep the entity hierarchy-version mixin in the ui package
总结
第四轮相较上轮 dd965d450 是线性追加单 commit a169dcca7(compare 返 ahead:1 / behind:0,无 force-push),仅 3 文件:core/Entity.ts -14、ui/index.ts +13、spine/SpineAnimationRenderer.ts +6/-8。本 commit 回退了我第二轮点赞的那个决定——把第二轮从 ui 包 mixin 挪进 core Entity 的 _uiHierarchyVersion 字段 + _updateUIHierarchyVersion 方法,重新收回 ui 包的 EntityExtension mixin,spine 侧改用 (entity as any)._updateUIHierarchyVersion?.() 可选链调用。
据实修正我第二轮的评价:第二轮我称"收进 core Entity=消除 monkey-patch=净改进"。Git 历史透镜证明这个判断偏了——dev/2.0 mainline 本来就有 EntityExtension mixin(ui/index.ts 上 _uiHierarchyVersion=0 + ApplyMixins(Entity,[EntityExtension]) + declare module 增强,与本 commit 新增块逐字符相同),第二轮的"移入 core"其实是偏离既有 mainline 架构。本 commit 把 PR 拉回 mainline 模式,Entity.ts 相对 mainline 的 diff 归零、core 保持干净。逐链核对无 P0/P1/P2/P3,方向正确。
逐链核对(对 a169dcca7 tree 实读 + git show dev/2.0 校验,非信 commit message)
- 字段归属回到正确层次(对照元数据归属框架):
_uiHierarchyVersion语义 100% 由 ui 包UICanvas定义("root canvas 检测 subtree 变化重建 element list"),一个孤立 Entity(ui 包未加载)持有它无意义——这正是"全员税"形态,应放消费者(ui 包)而非 core。第二轮的 core 放置让所有 5000+ entity(含无 UI 场景)都背这个 UI-only 字段,是 anti-pattern。本 commit 修正到 ui 包 mixin 是更本质的 IoC 边界。 - core
Entity.ts已零残留:tip 上grep _uiHierarchyVersion|_updateUIHierarchyVersion于packages/core/src/Entity.ts返回 0 命中,字段+方法干净删除。 EntityExtensionmixin 是 mainline 忠实还原:dev/2.0:ui/index.ts的EntityExtension块与本 commit 新增块diff判定 IDENTICAL(含declare module "@galacean/engine"对Entity接口的_uiHierarchyVersion/_updateUIHierarchyVersion增强)。ui 包内UICanvas.ts:429entity._uiHierarchyVersion因增强可见故类型安全(无需@ts-ignore)。- spine 侧
(entity as any)._updateUIHierarchyVersion?.()的as any+?.均正确且必要:spine 只依赖@galacean/engine(core)看不到 ui 包类型增强,as any诚实。?.精确镜像运行时不变量——mixin 存在 ⟺ ui 包已加载 ⟺ 可能存在UICanvas。 - 四个调用点全部被 rootCanvas/hosted 门控,
?.不会静默吞掉真实调用::271-275_onEnableInScene门if (rootCanvas);:294-295_onDisableInScene门if (this._hostedByUICanvas);:592-597_refreshHosting门if (hosted)——三者的 rootCanvas 来自UIElementUtils.searchRootCanvasInParents,只有_isRootCanvas===true的组件(仅UICanvas设置,ui 包专属)才命中 → 分支可达 ⟺ ui 包已加载 ⟺ mixin 已装,?.永不需短路,方法必在。:400-407_rootCanvasListener的case Child:是唯一?.真正 load-bearing 的路径:非托管 spine 也经_onEnableInSceneelse 分支setRootCanvas(this,null,entity)注册全链 listener,普通EntityModifyFlags.Child在无 ui 包时也会 fire → 此时 mixin 不存在,?.正确 no-op(无 canvas 可通知,正是注释所述)。
- 类字段
_uiHierarchyVersion=0不被ApplyMixins(仅拷 prototype)安装到实例——但这是 mainline 既有已测行为,非本 PR 引入:useDefineForClassFields(esnext 默认 true)下该字段是EntityExtension构造器里的实例初始化,而EntityExtension从不被实例化、ApplyMixins只拷prototype属性(方法拷到、字段拷不到)→ 真实 Entity 上首次 stamp 前读到undefined而非0。但_updateUIHierarchyVersion用!==比较对undefined安全,UICanvas._hierarchyVersion初值-1(-1 !== undefined与-1 !== 0同样触发首次 rebuild 后稳定),行为等价。且此 mixin 早已在dev/2.0以同款 class-field 形态发布——既有行为,不是回归,非 finding。
问题
无。本 commit 无 P0/P1/P2/P3。
记录(不阻塞,状态延续上轮)
- 上轮两条 [P3](
ISpineRenderTarget.ts:8注释仍写「UI-space renderer」双组件失真 /e2e/.dev/vite.config.js:60死别名@galacean/engine-spine-ui)本 commit 未触及(delta 仅 3 文件均不含它们),仍开放但不阻塞。 - CI 白名单结构约束不变:target=
fix/shaderlab不在ci.yml的pull_request: branches,gh pr checks 3068仅 labeler 过。本次纯 refactor(字段归属搬迁 + spine 调用点收敛),无逻辑改动,UICanvas.test.ts/spine 单测在 codecov/unit 线可覆盖但受 shaderlab 合并粒度约束当前不跑;建议合并前本地跑,或于fix/shaderlab→dev/2.0时由完整 CI 补验。
简化建议
无。本 commit 本身就是简化:撤回第二轮的 core 侵入,回到 mainline 既有的 ui 包 mixin 边界,Entity.ts 相对 mainline diff 归零、core public surface 不再背 UI-only 概念。spine 用 (entity as any).method?.() 对未加载 ui 包的场景优雅降级,与它经 searchRootCanvasInParents 只在 canvas 存在时才 reach UICanvas 的既有惯例一致。代码干净。
🤖 Generated with Claude Code
The ui-local IElement/IGroupAble/IGraphics chain was a structural twin of the core protocol left over from the extraction; ui components now implement the core chain directly and the local interfaces are deleted: - IUIHostedRenderer renamed IUIRenderer — UIRenderer is its reference implementation (implements it with the _isUIRenderer marker), spine another - UICanvas implements IUIElement; UIGroup/UIInteractive implement IUIGroupAble; raycastPadding stays a UIRenderer member (canvas contract does not consume it) - the canvas walk gate collapses to the single marker check and CanvasRenderable simplifies to Renderer & IUIRenderer - rect-clip state is now applied by the canvas for every renderable in one place instead of UIRenderer self-applying and the canvas covering others Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
审查 @ 68a440934(第五轮,增量)— refactor(ui): unify the element contracts on the core IUIElement chain
总结
第五轮相较上轮 a169dcca7 是线性追加单 commit 68a440934(compare 返 ahead:1 / behind:0,无 force-push),10 文件。本 commit 删掉 ui 包里那套 IElement/IGroupAble/IGraphics 本地接口链(三文件共 -42 行),让 ui 组件直接实现 core 的 IUIElement 接口链——是「PR 整体重写把托管契约上移 core」之后遗留的结构性孪生副本的最终收口。逐链核对无 P0/P1/P2/P3,方向正确、是纯净的「大道至简」(删一层重复接口 + 两处特判塌缩成通用路径),行为等价。
四处改动:① IUIHostedRenderer 更名 IUIRenderer、marker _isUIHostedRenderer→_isUIRenderer,UIRenderer 补上 _isUIRenderer=true 成为该接口的引用实现,spine 是另一个;② UICanvas implements IUIElement、UIGroup/UIInteractive implements IUIGroupAble;③ walk 采集门从 instanceof UIRenderer || _isUIHostedRenderer 塌缩成单一 marker 检查 _isUIRenderer,CanvasRenderable 从联合类型 UIRenderer | (Renderer & IUIHostedRenderer) 简化为 Renderer & IUIRenderer;④ rect-clip 状态从「UIRenderer 自涂 + canvas 只涂其它」统一成「canvas 为每个 renderable 在一处统一涂」。
逐链核对(对 68a440934 tree 实读 + git show dev/2.0/git blame 校验,非信 commit message)
- walk 门塁缩到单 marker 是等价泛化,非行为变更:
_walk:524现为(component as IUIRenderer)._isUIRenderer。核UIRenderer.ts:52已补@ignoreClone _isUIRenderer = true(类字段初始化,构造期跑);Image/Text均extends UIRenderer继承此字段无 override(git grep extends UIRenderer仅这俩 +Transition泛型形参非子类);spine:145直设_isUIRenderer=true。故「所有 UI 渲染器 + spine」全带 marker,旧instanceof UIRenderer主门与新 marker 门等价。对无此字段的组件读到undefined(falsy)正确落入下方else if链(UIInteractive/RectMask2D/UIGroup),安全。 @ignoreClonemarker 克隆后仍在:_isUIRenderer=true是类字段初始化器,克隆走addComponent构造新实例→构造期必设 true,@ignoreClone只跳过字段拷贝不跳过构造 init,故 cloned UIRenderer/spine 保留 marker(与前轮核过的 spine marker 同模式)。marker 恒 true 用@ignoreClone也正确(常量不该被拷)。- rect-clip 统一收口无双涂、序不变:
UIRenderer._prepareRender删掉了自涂行Utils.updateRectMaskClipState(this)(旧:189);canvas:341现无条件Utils.updateRectMaskClipState(renderer)再renderer._prepareRender(context)。旧 UIRenderer 是在_prepareRender内部(_render前)自涂,新是 canvas 在调_prepareRender之前涂——两者都在_render前落定 rect-clip 状态,序等价;_prepareRender只_update/_render+union macro,不触碰 rect-clip 状态,无覆盖。updateRectMaskClipState是从target._rectMasks纯重算(无 mask 走 reset),幂等,即便重复调也安全。spine 侧无变化(本就走 canvas 涂的!(instanceof UIRenderer)分支,现走统一路径)。 IRectMaskTarget契约两类消费者都满足:spine 有全部 5 个 rect-clip 字段(:172-184)+shaderData(Renderer基类),UIRenderer 经IUIRenderer声明同样满足。canvas:341对每个 renderable 传入均合法。CanvasRenderable类型简化是收窄非放宽:UIRenderer | (Renderer & IUIRenderer)塌缩为Renderer & IUIRenderer——因UIRenderer现 IS-ARenderer & IUIRenderer,联合的左支被右支包含,是严格简化。:525的 cast 仍成立(marker + Renderer 基类)。消费者全在 UICanvas 内 + 一处 re-export,无外部破坏。- 删的三接口零残留、旧世界引用随架构退场:
git ls-tree确认packages/ui/src/interface/三文件全删;全 treegit grep零_isUIHostedRenderer/IUIHostedRenderer/IGraphics/裸IGroupAble/interface/IElement|IGraphics|IGroupAbleimport 残留;tests/e2e 零旧名引用(测试走公开路径不戳 marker,正确)。 - Git 历史透镜——无矛盾:walk 门是修改行,跑
git log -S/git show dev/2.0——dev/2.0mainline 门是instanceof UIRenderer(前托管时代单检查),432cd2708(本 PR 二轮)追加|| _isUIHostedRenderer兼容 spine,本 commit 塌缩成单 marker。链条自洽:无 fix/regression 被还原,塌缩反而消掉了「UIRenderer明明可带 marker 却还留instanceof特判」的冗余,是正确演进方向。
问题
无。本 commit 无 P0/P1/P2/P3。
记录(不阻塞,状态延续上轮)
- 两条 [P3](
ISpineRenderTarget.ts:8@remarks仍写「world-spaceSpineAnimationRendererand the UI-space renderer」双组件失真、重写后已无独立 UI-space renderer /e2e/.dev/vite.config.js:60死别名@galacean/engine-spine-ui指向已删包)本 commit 未触及(delta 10 文件均不含它们),仍开放但不阻塞。 - CI 白名单结构约束不变:target=
fix/shaderlab不在ci.yml的pull_request: branches,gh pr checks 3068仅 labeler 过。本次纯类型收口 refactor(接口链统一 + 两处特判塌缩),逻辑等价,UICanvas.test.ts/spine 单测在 codecov/unit 线可覆盖但受 shaderlab 合并粒度约束当前不跑;建议合并前本地跑,或于fix/shaderlab→dev/2.0时由完整 CI 补验。
简化建议
无——本 commit 本身就是最好的简化:删掉整套 ui 本地接口孪生副本(IElement/IGroupAble/IGraphics),walk 门从「instanceof + marker 双特判」塌缩成单一 marker 检查,rect-clip 从「self-apply + canvas-cover-others 两处」收口成「canvas 一处统一涂」,CanvasRenderable 联合类型收窄为单一交叉类型。新增能力(spine 入 canvas)走到第五轮反而净删了一层接口和两处特判,正是「大道至简」的范例。代码干净。
🤖 Generated with Claude Code
core now exposes only the declarative UI contract — the IUIElement/ IUIGroupAble/IUIRenderer chain, IUICanvas/IUIGroup and the flag enums that appear in its signatures — with zero behavior or state: - UIElementUtils is dissolved: the registration/listener machinery returns to ui's Utils and the hierarchy counter to UICanvas._hierarchyCounter - the ui Entity mixin's _updateUIHierarchyVersion now defaults to the current counter, so hosted renderers stamp with an optional no-arg call instead of reaching into ui internals - SpineAnimationRenderer implements its side of the protocol itself against the typed core interfaces (canvas/group ancestor search via the _isRootCanvas/_isUIGroup markers, parent-chain listeners over core Entity APIs, deregistration through IUICanvas/IUIGroup._disorderedElements) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
审查 @ 63ea8728(第六轮,增量)— refactor(ui): keep the element bookkeeping in the ui package
总结
第六轮相较上轮 68a440934 是线性追加单 commit 63ea8728(compare 返 ahead:1 / behind:0,无 force-push),9 文件、+295/-219。本 commit 是一次边界再收缩:删掉 core 里的 UIElementUtils(-172 行整文件),让 core 只留纯声明契约(IUIElement/IUIGroupAble/IUIRenderer/IUICanvas/IUIGroup 接口 + 三个 flag 枚举,零行为零状态),把注册/监听记账机器搬回 ui 包的 Utils、hierarchy counter 搬回 UICanvas._hierarchyCounter,spine 则自己实现一份同款记账(对着 core 的 typed 接口 + _isRootCanvas/_isUIGroup marker)。
Git 历史透镜——这是回归 mainline 架构:git show origin/dev/2.0 证明 dev/2.0 mainline 本来就是「记账机器在 ui Utils、counter 在 UICanvas._hierarchyCounter」(core 侧 UIUtils.ts 只做 overlay 渲染,与 element 记账无关)。本轮 ui Utils.ts 的记账段(setRootCanvas/_registerRootCanvas/_registerListener…)与 mainline Utils.ts 逐行相同(仅类型名 IElement→IUIElement 随第五轮接口统一而变),UICanvas._hierarchyCounter:number=1 与 mainline :41 逐字一致。所以第二~五轮把机器推进 core(UIElementUtils)是偏离 mainline,本轮拉回。方向正确、行为等价,逐链核对无 P0/P1/P2/P3 阻塞项,仅一条 [P2] 设计权衡(不阻塞)。
逐链核对(对 63ea8728 tree 实读 + git show origin/dev/2.0 校验,非信 commit message)
- core 变纯声明契约、
UIElementUtils零残留:core/src/ui/index.ts删掉export { UIElementUtils };git ls-tree确认整文件删除;逐个核对 9 个改动文件 +tests/src/spine/tests/src/ui全部单测grep UIElementUtils零命中;GitHub code-search 全仓 0 命中;dev/2.0mainline 本就无此符号——故它是本 PR 自己前几轮引入、生命周期完全自洽,本轮删除完整无孤儿消费者,build-safe。 - ui
Utils记账段 = mainline 忠实还原:与origin/dev/2.0:Utils.ts逐行 diff,setRootCanvasDirty/setRootCanvas/cleanRootCanvas/searchRootCanvasInParents/setGroupDirty/setGroup/_registerRootCanvas/_registerGroup/_registerListener/_unRegisterListener全部 IDENTICAL(modulo 类型名 + 枚举从本地 import 改为从@galacean/engineimport)。notify 语义保留(_onRootCanvasModify?.(All)/_onGroupModify(All)仍在),UIRenderer/Image/Text侧行为不变。 UICanvas._hierarchyCounter与 counter 递增/消费点全部对齐 mainline::43定义=1、:430++UICanvas._hierarchyCounter、:645_updateUIHierarchyVersion(UICanvas._hierarchyCounter)、_hierarchyVersion初值-1——与 mainline:41/:441/:641逐字一致。_updateUIHierarchyVersion(version = UICanvas._hierarchyCounter)默认参数收口正确且作用域安全:spine 从?.(UIElementUtils._hierarchyCounter)改为裸?.()。default 表达式在 ui 包EntityExtension的作用域求值(ui/index.ts新增import { UICanvas } from "./component"),故UICanvas._hierarchyCounter从 ui 闭包解析,spine 永不需看见UICanvas——正是这个默认参数把UICanvas引用从 spine 调用点移进 ui mixin 默认值,干净。递归this.parent?._updateUIHierarchyVersion(version)显式透传捕获的version(非逐级重读 counter),一次传播内无漂移;对undefined用!==比较安全(既有已测行为)。签名改version?: number向后兼容——显式传参的既有调用点(UICanvas/UIRenderer/RectMask2D)行为不变。- spine 自实现的记账是「有意特化的副本」非盲拷:
_registerRootCanvas/_registerListeners/_unRegisterListeners/_searchRootCanvasInParents/_searchGroupInParents与 uiUtils私有版逐字节等价(仅static+element参数 ↔ 实例this)。两处特化差异均正确:①_setRootCanvasDirty丢掉旧element._onRootCanvasModify?.(All)——spine 未实现_onRootCanvasModify(接口里 optional),optional-chaining 对 spine 本就是 no-op;②_registerGroup/_setGroupDirty丢掉_onGroupModify(All)——spine_onGroupModify(): void {}是空 no-op(:412),调它也无副作用。故 spine 副本删掉的正是对 spine 而言的死调用,行为等价,且_searchRootCanvasInParents用 marker_isRootCanvas===true(不引UICanvas类)正是 IoC seam 的目的。 _refreshHosting去托管路径等价:新_registerRootCanvas(null)+_isRootCanvasDirty=false+_registerListeners(this.entity,null,…)三操作,与旧UIElementUtils.setRootCanvas(this,null,this.entity)(_isRootCanvasDirty=false+_registerRootCanvas(elem,null)+toEntity=null?.parent??null=null+_registerListener(this.entity,null,…))逐操作对应,前两操作互不依赖、换序无影响。安全。- 接口契约齐备:
IUICanvas暴露 spine reach 的sortOrder/_isRootCanvas/_sortDistance/_renderElements/_disorderedElements{add,deleteByIndex}全在(:13-19);_isUIGroup在IUIGroup(IUIElement.ts:54);_onRootCanvasModify?optional(:45)、_onGroupModifyrequired(:72)——与 spine 实现一致。 - listener fall-through(
case Parent:→case Child:):本轮仅把UIElementUtils.setRootCanvasDirty(this)→this._setRootCanvasDirty()、UIElementUtils._hierarchyCounter→?.(),控制流不变,是第二/四轮已 clear 的既有模式,非新问题。
问题
[P2] SpineAnimationRenderer.ts:660-736 与 ui/Utils.ts:100-243 的注册/监听记账机器现在是「双源真相」(~120 行近逐字重复) — 本轮删掉 core UIElementUtils 单一真相源后,_registerRootCanvas/_registerGroup/_registerListeners/_unRegisterListeners/_searchRootCanvasInParents/_searchGroupInParents 在 spine 与 ui Utils 各存一份、逻辑逐字节等价(仅特化删掉 spine 的两处 no-op notify)。这是一个真实的权衡,不是缺陷,故不阻塞:作者的取向(core = 纯声明契约,行为/状态各自归消费者)是自洽的、也确实让 core 更干净;被删的 UIElementUtils 恰是「单源真相」的替代方案,且它本就只操作 IUIElement/IUICanvas 接口、零 ui 依赖,所以单源版在依赖上也是干净的——两条路都成立。代价点在于:将来每个想被 canvas 托管的非-ui renderer(如未来 particle-in-UI)都要再抄一遍这 ~120 行,且两份副本已在 _onGroupModify 这个细节上分叉——一处对 registration 逻辑的修复(如历史上 deleteByIndex 索引交换那类 bug)必须在两处同步,漏一处即静默 bug。若倾向单源,可保留被删的 UIElementUtils(操作接口、零 ui 依赖)作为共享实现,ui Utils 与 spine 都 delegate;若坚持 core 纯声明,则建议在两份副本各加一行注释互指「另一处是同款实现,改这里记得改那里」,把双源的维护契约显式化。倾向哪边由团队定,我只标出这条债的存在与合并方向。
记录(不阻塞,状态延续上轮)
- 两条 [P3] 仍开放、本 commit 未触及(delta 9 文件均不含它们):①
ISpineRenderTarget.ts:8@remarks仍写「world-spaceSpineAnimationRendererand the UI-space renderer」双组件失真(重写后已无独立 UI-space renderer,SpineAnimationRenderer是 world+UI 唯一实现者);②e2e/.dev/vite.config.js:60死别名@galacean/engine-spine-ui指向已删的packages/spine-ui。均无害不阻塞。 - CI 白名单结构约束不变:target=
fix/shaderlab不在ci.yml的pull_request: branches,gh pr checks 3068仅 labeler 过。本轮纯边界搬迁 refactor(core→ui/spine),逻辑等价,UICanvas.test.ts/SpineAnimationRendererUI.test.ts在 codecov/unit 线可覆盖但受 shaderlab 合并粒度约束当前不跑;建议合并前本地跑,或于fix/shaderlab→dev/2.0时由完整 CI 补验。
简化建议
本轮本身就是一次简化方向的收口:core 从「接口 + 行为 + 状态」瘦成「纯接口契约」,Entity 相对 mainline 归零、ui Utils/UICanvas._hierarchyCounter 回到 mainline 既有形态。唯一可再收的就是上面 [P2] 的双源——但那是一个「DRY vs core 纯净度」的取舍,不是无脑冗余。除此之外代码干净。
🤖 Generated with Claude Code
_registerListener truncated the listening array without unregistering the dropped tail, leaving stale listeners on ex-ancestor entities. Releasing them exposed that nested-canvas demotion relied on such a leaked listener to hear the same CanvasEnableInScene dispatch again: the enabling canvas claims root status only after its dispatch returns, so the demote cascade searched for it in vain and promoted nested canvases to root instead. Hand the enabling canvas down the cascade as the successor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hosting flips (renderer registration + macro) were reachable only from the Parent branch of the renderer's own listener, so every other transition left a split brain: a canvas enabled on an ancestor double-rendered (and crashed the overlay pass), a demoted root canvas left the renderer in no render list even after re-enabling, an enable-frame reparent orphaned it, and a world-to-world reparent left new ancestors unheard. _refreshHosting now settles registration, canvas bookkeeping and listener scope in one place; Parent events resolve eagerly, canvas enable/demote signals defer to a pre-update settle (their dispatch happens before the canvas claims root status). The listener truncation also releases dropped tail entities, and hosted raycasts respect RectMask2D clipping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- delete the vite alias for the abandoned spine-ui package - reword the ISpineRenderTarget doc to the single-component design - remove the orphaned UIRenderer._tempRect scratch vector - type _disorderedElements as DisorderedArray<T> instead of hand-written structural stubs, restoring type checking on the swap-delete index fixup Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
审查 @ 3c6c88c0(第七轮,增量)— 三 commit:hosting 状态机真修复 + 死残留清理
总结
第七轮相较上轮 63ea8728 是线性追加三 commit(compare 返 ahead:3 / behind:0,无 force-push):83d7bf6b fix(ui): keep element listeners in sync when the listening chain shrinks、27058581 fix(spine): re-evaluate hosting on every transition signal、3c6c88c0 refactor: drop dual-design leftovers and type element containers。前两个是真 behavioral bug 修复(补全 round-2 重写引入的 hosting 状态机在多种 transition 下的 split-brain 漏洞),第三个是清理——正好闭环我历史两条 [P3]。逐链核对无 P0/P1/P2/P3 新问题,方向正确、是「大道至简」(删掉 _setHostedByUICanvas 三参方法 + _render 两段重复循环合并成一段)。所有 correctness 类改动均配反向证伪的链路测试。
性质校准(对照前几轮):round 3-6 是边界/命名收敛(记账搬迁、接口统一),本轮不同——是真修 round-2 重写留下的、之前没被覆盖的状态机 bug。四类 split-brain 是真 red(详见测试反向证伪)。这不是「在错方向上继续打补丁」,而是完成 round-2 引入的状态机,且是净简化(删方法 + 去重)。
逐链核对(对 3c6c88c0 tree 实读 + git show fix/shaderlab 校验,非信 commit message)
27058581收口 hosting 到单一_refreshHosting+_settleHosting——真去重、非加复杂度:旧 hosting-flip 只从_rootCanvasListener的Parent分支可达,_onEnableInScene/_getRootCanvas/_setHostedByUICanvas(hosted, cm, switchRegistration)(三参 boolean-mode 签名)各自维护一段。本 commit 全部收敛到_refreshHosting(一处结算 registration + canvas 记账 + listener scope + macro),_setHostedByUICanvas彻底删除(tree 零残留,已 grep 核实)。_render从 hosted/world 两段近逐字重复的 for-loop 塌缩成一段共享循环(用renderElements ? push : pushRenderElement收尾)。净删代码。_settleHosting的 divergence check 是 edge-detector 而非冗余状态:this._isRootCanvasDirty || !!this._rootCanvas !== this._hostedByUICanvas——canvas walk(UICanvas._walk:531)经 IUIElement 契约(只知_rootCanvas)给 spine 赋_rootCanvas却不动_hostedByUICanvas(camera-pipeline registration 的开关),这是 spine 双服务 world+UI 的固有需求(UIRenderer恒 UI、无此二态)。_hostedByUICanvas滞后于_rootCanvas、在 settle 时对账,正是「flip 只发生一次、不每帧重跑 add/removeRenderer」的边沿检测机制。非双源真相冗余。- update→render 时序坐实 settle 在 canvas walk 之前:
Engine._render(:587callRendererOnUpdate→ spineupdate()→_settleHosting)先于:611camera.render()(canvas walk_getRenderers→_walk)。hosted spine 仍在_onUpdateRenderers(_refreshHosting只removeRenderer出_renderers、不出 on-update 列表),故update()/_settleHosting每帧仍跑。settle 里 hosted 分支entity._updateUIHierarchyVersion?.()打脏使同帧后段camera.render()的 walk 重收该 spine。同帧收敛,无掉帧。 - 「canvas 在祖先启用」双渲染 + overlay crash 修复机理坐实:旧代码
CanvasEnableInScene不触发_refreshHosting→spine 留_hostedByUICanvas=false(world)却又被 canvas walk 收进_rootCanvas→camera pipeline + canvas 双推→overlay crash。新代码_rootCanvasListener的CanvasEnableInScenecase 只_setRootCanvasDirty+_setGroupDirty(deferred,因 enabling canvas dispatch 时尚未 claim root),下帧_settleHosting时 canvas 已 claim root→正确 hosted。_render:320的else if (this._hostedByUICanvas) return(hosted 但 rootCanvas=null 的 mid-transition)是双渲染的第二道防线,跳过而非落入 camera 分支。 - demote cascade 的
successor机制坐实:UICanvas._setIsRootCanvas(false, successor)对非-canvas 元素(含 spine)Utils.setRootCanvasDirty(element)(置_isRootCanvasDirty=true+_rootCanvas=null+_indexInRootCanvas=-1),_onRootCanvasModify?.()对 spine optional-chain no-op(未实现);对嵌套 canvas 用searchRootCanvasInParents(element) ?? successor补上「搜索看不到尚未 claim root 的 enabling canvas」的窗口。下帧 spine_settleHosting见 dirty→_refreshHosting重解到新根。forEach × deleteByIndex走 loop-safe 分支(round 3 已核),无索引损坏。 Parentcase 去掉冗余_setRootCanvasDirty/_setGroupDirty是正确的:旧 Parent 分支_refreshHosting()后又_setRootCanvasDirty()+_setGroupDirty()——会 RE-dirty_refreshHosting刚 eager 解好的状态。新 Parent 只_refreshHosting()(内部已_isRootCanvasDirty=false+ hosted 时_setGroupDirty/ world 时_cleanGroup)。与 commit message「Parent events resolve eagerly」一致。83d7bf6blistener tail-release 修真泄漏:_registerListeners链缩短时旧代码listeningEntities.length = count只截断数组、不 unregister 被丢的 tail 实体→泄漏的 listener 在 ex-ancestor 移动时 fire 到已移出/已销毁的 component。补[count, oldLength)unregister 循环。Utils._registerListener与 spine_registerListeners两处同步补(Entity._unRegisterModifyListener用?.removeListener对已销毁实体安全)。- raycast rect-mask 裁剪:spine
_raycast补_isRaycastVisibleByRectMask(hitPoint)(与UIRenderer:356同款逻辑:遍历_rectMasks,enabled+active 的 mask_containsWorldPoint全过才可命中),修「hosted spine 命中被 mask 裁掉的区域」。 3c6c88c0清理三项全正确:① 删死 vite alias@galacean/engine-spine-ui(闭环我历史 [P3]);②ISpineRenderTarget.ts:8注释改回单实现者「Implemented bySpineAnimationRendererfor both world-space and UI-hosted rendering.」(闭环我历史 [P3]);③ 删UIRenderer._tempRect(tip 上UIRenderer零引用,rect-mask 用的是Utils._tempRect,真孤儿死码);④IUICanvas._disorderedElements/IUIGroup._disorderedElements从手写结构 stub 改DisorderedArray<IUIElement>/<IUIGroupAble>——是严格更精确类型,恢复了 swap-delete fixup(replaced._indexInRootCanvas = ...)的类型检查(旧 stubdeleteByIndex(): any运行时对但不类型检查)。
测试(反向证伪校验)
新增 7 个链路测试,全部从公开入口(createChild/addComponent/enabled/addChild/destroy + engine.update())触发、断言 worldRendererCount/canvas._renderElements/_raycast/modifyListenerCount 公开输出:
- 四类 split-brain 各一例(canvas 在祖先启用 re-home / overlay 在 world spine 上启用不 throw / root canvas disable→world fallback→re-enable 回归 / enable 帧 reparent 出去转 world / world→world reparent 进 canvas)——每例 revert 修复即 red。
- listener 泄漏例:
modifyListenerCount(a/b)===0(无修复恒 1)+ 销毁后祖先移动不 throw。 - rect-mask 裁剪例:
clippedRay命中为 false(无 guard 恒 true)+ mask disable 后恢复命中。
反向证伪成立,测试粒度正确(不戳私有制造虚假状态)。
记录(不阻塞,状态延续上轮)
- 上轮两条 [P3] 均已闭环(见上「清理」①②),本轮无残留 [P3]。
- round-6 [P2] 双源真相仍开放(不重提,仅状态更新):
83d7bf6b的 tail-release 修复正是我 round-6 预言的代价——同一 registration 逻辑修复必须在ui/Utils._registerListener与spine/SpineAnimationRenderer._registerListeners两处同步落地,本次两处逐字节同补印证了这条债真实存在、正在被偿付。是 round-6 已标 trade-off 的延续,非新问题。 - CI 白名单结构约束不变:target=
fix/shaderlab不在ci.yml的pull_request: branches,gh pr checks 3068仅 labeler 过。本轮含 hosting 状态机 + 8 个新测的渲染/结构改动却零 CI 覆盖,建议合并前本地跑SpineAnimationRendererUI.test.ts/UICanvas.test.ts,或于fix/shaderlab→dev/2.0时由完整 CI 补验。
简化建议
本轮本身就是简化的范例:hosting 逻辑从「四处散落 + 三参 boolean-mode 方法 + 两段重复 render loop」收敛成「单一 _refreshHosting 结算点 + _settleHosting 边沿检测 + 一段共享 render loop」,_setHostedByUICanvas 整体删除。新增能力(修四类 transition bug)反而净删了代码。唯一可再收的仍是 round-6 的双源 [P2]——但那是「DRY vs core 纯净度」的团队取舍,本轮的 tail-release 双补只是让这条债更显性,不改变其 trade-off 性质。除此之外代码干净。
🤖 Generated with Claude Code
Summary
SpineAnimationRenderernow works in both spaces — no separate UI component. In world space it renders through the camera pipeline exactly as before; placed under a rootUICanvasit is hosted by the canvas: collected and ordered with the other UI elements (WorldSpace / ScreenSpaceCamera / overlay), faded byUIGroupalpha, clipped byRectMask2D, and optionally hit-testable (raycastEnabled, skeleton world bounds). Moving the entity in/out of a canvas re-homes the renderer live.resource.instantiate()therefore works in both spaces unchanged.Core: a canvas-hosting protocol (groundwork for hosting any renderer)
The hosting contract moves into core so renderer packages can participate without depending on the ui package:
IUIElement/IUIGroupAble/IUIRenderer/IUIGroupinterfaces and theRootCanvasModifyFlags/GroupModifyFlags/EntityUIModifyFlagsenums live in core next to the existingIUICanvas(ui re-exports the enums, so no ui-facing breakage);IUICanvaswidened with the hosting surface. The ui components implement the core chain directly —UICanvas→IUIElement,UIGroup/UIInteractive→IUIGroupAble,UIRenderer→IUIRenderer(its reference implementation) — and the former ui-local twin chain (IElement/IGroupAble/IGraphics) is deletedUtils, the hierarchy counter onUICanvas, andEntity._updateUIHierarchyVersionremains a ui-package Entity mixin (now defaulting to the current counter). Hosted renderers implement their side of the protocol themselves against the typed core interfaces (ancestor search via the_isRootCanvas/_isUIGroupmarkers, parent-chain listeners over core Entity APIs, deregistration through_disorderedElements) and stamp the hierarchy version with an optional no-arg mixin call — a canvas implies the ui package is loadedRendererimplementingIUIRenderergets collected byUICanvas._walk(single marker check) and wired exactly like aUIRenderer; the canvas applies rect-clip shader state for every renderable in one place. Spine is the pilot; particles/trail/etc. can adopt the same contract later.Spine specifics
ComponentsManager's renderer list (no double render). One idempotent_refreshHostingsettles registration, canvas bookkeeping and listener scope;Parentevents resolve eagerly (synchronous re-homing), while canvas enable/demote signals defer to a pre-update settle because their dispatch happens before the canvas claims/releases root status — resolving eagerly there would race. Covered per transition by unit testsISpineRenderTarget.globalAlpha: both 3.8/4.2 generators multiply it into the final vertex alpha before premultiplication, so PMA light/dark colors follow automatically (2-line change per generator)RENDERER_UI_RECT_CLIPmacro (toggled with hosting) derives the rect-clip world position fromrenderer_ModelMat— spine vertices are entity-local, unlike pre-transformed UI chunk vertices; clip fade is PMA-aware and hard clip discards on the rect factor, since additive slots legitimately emit zero-alpha fragments; plusENGINE_SHOULD_SRGB_CORRECToutput correction for overlay parityHosting-transition hardening (review round)
A deep review of the first cut found that the hosting flip was reachable only from the
Parentbranch of the renderer's own listener — every other transition left a split brain (collected by the canvas walk while_renderstill took the world branch, or registered nowhere). All six confirmed and fixed, each pinned by a regression test:EntityUIModifyFlags.CanvasEnableInScenewas ignored: enabling aUICanvason an ancestor of a world-mode spine double-rendered it — and crashed the overlay pass every frame (OverlayCamerahas no_renderPipeline)_registerListenerstruncated a shrinking listening chain without unregistering the dropped tail — stale listeners on ex-ancestors later invoked the destroyed component (TypeError) or resurrected a destroyed renderer into the render list. ui'sUtils._registerListenerhad the byte-identical pre-existing hole; both fixed_raycastignored_rectMasks: pixels clipped away by aRectMask2Dwere still hittable and blocked UI beneath (now mirrorsUIRenderer._isRaycastVisibleByRectMask)Fixing (4) exposed a latent ui-package protocol flaw: nested-canvas demotion converged only because a leaked listener happened to sit at a low index of the backward-iterating dispatch. The demote cascade now receives the enabling canvas as an explicit
successor(_setIsRootCanvas(false, successor)) instead of searching for a canvas that has not claimed root status yet — regression-tested with a two-level nested canvas chain.e2e harness fix (pre-existing breakage)
Spine e2e cases could not run at all on this branch: package dist is downleveled to ES5, and
SpineTexture(ES5 class inspine-core-4.2dist) extends the external@esotericsoftware/spine-corenative class →Class constructor Texture cannot be invoked without 'new'. The case dev server now aliases the spine packages to source (scoped to spine only). The existingspineboy/tintBlackbaselines pass unchanged under this fix.Tests
tests/src/spine/SpineAnimationRendererUI.test.ts): canvas collection (world-space + overlay incl.subShader), pipeline-registration exclusivity, live re-homing on reparent (world ↔ canvas), group alpha vertex assertions (straight + PMA + dynamic change), alpha=0 element skip, rect-clip macro toggling with hosting, RectMask wiring, bounds-based canvas raycast, skeleton-derived culling bounds, clone under canvas, destroy while hosted — plus the transition family: canvas enabled on an ancestor at runtime (world-space + overlay no-throw), root canvas disable → world fallback → re-enable, enable-frame reparent, ancestor-subtree move under a canvas, listener release on chain shrink (with leak-repro no-throw), RectMask-clipped raycast miss.tests/src/ui/UICanvas.test.tsadds nested-canvas re-rooting on ancestor canvas enablespine-ui-canvas— world vs canvas-hosted render identically side by side, third one faded byUIGroup;spine-ui-rect-mask— masked vs unmasked. All 4 Spine cases green; the two new baselines pass pixel-identically against the screenshots produced by the previous two-component implementation, confirming render-path equivalencePre-existing failures on a clean
fix/shaderlabcheckout, unrelated to this PR (verified via stash baseline runs):Text > get bounds(tests/src/ui/Text.test.ts); local-only vitest browser-mode infra issues in thecore(websocketWS_ERR_UNSUPPORTED_MESSAGE_LENGTH) andshader-lab(collection error) suites.Out of scope
Maskinteraction with spine is untested (RectMask2D is the supported clipping path)UITransform.sizedoes not scale or clip the skeleton (it renders at the entity transform scale); hit-testing uses skeleton bounds instead of the rect🤖 Generated with Claude Code