feat(spine): add Spine runtime for fix/shaderlab#3063
feat(spine): add Spine runtime for fix/shaderlab#3063cptbtptpbcptdtptp wants to merge 2 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 |
- Add tintBlack to the material cache key so renderers differing only in tintBlack no longer share one material and clobber its macro each frame; delete cache entries by the key a material was registered under, so destroy tolerates user-set materials and earlier-cached settings - Destroy the old vertex/index buffers on resize and on renderer destroy instead of leaving them to resourceManager.gc() - Guard _cloneTo for renderers never bound to a resource - Remove the unreachable separate-slot machinery (per-renderer config that landed on the runtime-singleton generator with no public entry point) and the unused SpineAnimationUpdateFlags enum - Use the parsed atlas path when loading via item.urls (item.url is undefined on that path) - Key the Trilinear filter mapping off the atlas min filter (mag filters are never MipMap*), in both backends - Document that registering "json" overrides the default JSON loader Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
第二轮增量审查(HEAD e91f86e8b → 995aff8d0,author=cptbtptpbcptdtptp)。新增一个 commit 995aff8d0 fix(spine): harden material cache, buffers, loaders and filter mapping——一批 8 项加固修复,逐条核对全部正确、注释准确、correctness 类修复均配反向证伪测试。无 P0/P1,无新 P2/P3。首轮的 [P2] CI 覆盖 / [P3] stacked 依赖两项状态未变(见文末),非新问题、不重提。
逐项核对(新 commit 995aff8d0)
均为真修复,非空操作,注释与代码一致:
-
material cache key 补
tintBlack(SpineAnimationRenderer.ts:314) —tintBlack切换 per-material 宏,旧 key 只含texture/blendMode/pma,仅 tintBlack 不同的两个 renderer 会共享同一 material 并每帧互相 clobber 宏。补_${tintBlack?1:0}后隔离。反向测试_getMaterial does not share ... across tintBlack variantsrevert 即红。✓ -
_clearMaterialCache改按_cacheKey删除(:329-340+SpineMaterial._cacheKey) — 旧实现从 renderer 当前状态重算 key 删除,若 register 后premultipliedAlpha变过就会漏删 stale 条目;且把materials[i]无脑as SpineMaterial取shaderData.getTexture(...).instanceId,遇到用户经setMaterial(public API)塞进来的普通 Material / null 洞会 NPE。新实现instanceof SpineMaterial守卫 + 按注册时的确切_cacheKey删除。反向测试destroy tolerates user-set materials ...revert 即抛。✓_cacheKey是「该 material 注册于哪个 key」的元数据,@internal归属正确。 -
resize / destroy 主动 destroy 旧 buffer(
_createAndBindBuffer/_onDestroy) — 已逐链核 refCount:primitive._addReferCount(1)(:146)使绑定的 buffer refCount=1;setVertexBufferBinding/setIndexBufferBinding在 primitive refCount>0 时对旧 buffer_addReferCount(-1)→0、新 buffer +1;_onDestroy里primitive._addReferCount(-1)经 Primitive 重写的_addReferCount级联把 buffer 归零。两处?.destroy()时 refCount 已到 0,ReferResource.destroy(force=false)真正释放而非return false。注释「destroy() is refCount-guarded / 已释放引用」属实非 aspirational,确实免去等gc()。首帧oldXxx为undefined、?.no-op。✓ -
_cloneTo空 skeleton 守卫(:229) — 手动加、未绑定 resource 的 renderer_skeleton/_state为 null,旧代码runtime.createSkeleton(this._skeleton.data)NPE。守卫if (!this._skeleton || !this._state) return;放在getSpineRuntime()之前,故即便无 runtime 注册也不抛。_setSkeleton/_setState成对赋值(_applyDefaultConfig里if (skeleton && state)),||守卫安全。反向测试cloning ... no skeleton does not throwrevert 即红。✓ -
删除 separate-slot 死代码(两 generator 各 −41 行 + 删
SpineAnimationUpdateFlagsenum) — 已核实这是真死代码而非丢功能:SpineGenerator由Spine42Runtime._generator持有,runtime 经SpineRuntimeRegistry的模块级单例_runtime共享 → generator 是进程单例,_separateSlots/_separateSlotTextureMap本是 per-renderer 语义却落在单例上,且addSeparateSlot/addSeparateSlotTexture无任何 public 入口(interface / index / 测试 / e2e / examples 全 tree grep 零消费者)。#3057 忠实带进的这段在新单例架构下已 broken-by-design,删除是诚实收口。展开循环if(blendModeChanged||textureChanged){flush} count+=finalIndicesLength与旧非 separate 路径逐字等价。✓ -
SpineAtlasLoader.ts:67atlasPath 取解析值 — 旧const atlasPath = item.url在item.urls路径下item.url为 undefined;真值来自上方const { atlasPath } = spineAtlasAsset(由_groupAssetsByExtension/_assignAssetPathsFromUrl填充),diff 只是删掉那行遮蔽用的错误声明。✓ -
SpineTexture.setFiltersTrilinear 改由 min filter 决定(两后端) — 旧else if (magFilter === MipMapLinearLinear)是死分支:mag filter 在 GL 里只能是Nearest/Linear,永不为 MipMap*,故旧码永远落 Bilinear、mipmap 图集拿不到三线性。新逻辑Nearest→Point / Linear→Bilinear / else(MipMap*)→Trilinear正确。注释「mag 永非 MipMap*」「mipmaps 在构造器生成」属实(4.2 ctor 有image.generateMipmaps())。新增SpineTexture.test.ts三分支双后端反向证伪。✓ -
SpineLoader.ts文档「json」覆盖默认 JSONLoader — 纯注释,描述与@resourceLoader("Spine", ["json",...])last-registration-wins 行为一致。✓
问题
无新问题。
首轮遗留(状态未变,非重提,仅记录)
- [P2] 全套 CI 未运行 — target 仍为
fix/shaderlab,ci.yml触发白名单仍只main/dev/*,gh pr checks现仍仅labeler通过。本 commit 改的 material/buffer/loader 逻辑有 vitest 覆盖(作者已跑vitest tests/src/spine),但渲染路径仍无 e2e 对图。同首轮结论,随fix/shaderlab→dev/2.0合并时由 CI 补齐。 - [P3] 仍为 #3057 的 stacked 重建 — 状态未变,仅记录。
简化建议
无。这一批修复本身就是净简化(删 82 行死代码 + 消除两处 latent bug),代码干净。
Rebuilds #3057 for fix/shaderlab, carrying over the Spine packages, runtime backends, tests, examples, and e2e assets while excluding unrelated dev/2.0 changes.
Target-specific adjustments:
Validation: