Skip to content

feat(spine): add Spine runtime for fix/shaderlab#3063

Open
cptbtptpbcptdtptp wants to merge 2 commits into
fix/shaderlabfrom
codex/spine-fix-shaderlab
Open

feat(spine): add Spine runtime for fix/shaderlab#3063
cptbtptpbcptdtptp wants to merge 2 commits into
fix/shaderlabfrom
codex/spine-fix-shaderlab

Conversation

@cptbtptpbcptdtptp

Copy link
Copy Markdown
Collaborator

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:

  • registers the 2D/Spine shader through the current ShaderPool path
  • maps Spine texture loading to AssetType.Texture2D
  • pins only the required Spine workspace/package lock entries

Validation:

  • pnpm run b:types
  • pnpm run b:module
  • pnpm exec vitest tests/src/spine --run

@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)
  • ^(dev/)?\d+.\d+$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2827820d-cb94-471d-883b-492a55aed0ba

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/spine-fix-shaderlab

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jul 8, 2026
GuoLei1990

This comment was marked as outdated.

- 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 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

总结

第二轮增量审查(HEAD e91f86e8b995aff8d0,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

均为真修复,非空操作,注释与代码一致:

  1. material cache key 补 tintBlackSpineAnimationRenderer.ts:314tintBlack 切换 per-material 宏,旧 key 只含 texture/blendMode/pma,仅 tintBlack 不同的两个 renderer 会共享同一 material 并每帧互相 clobber 宏。补 _${tintBlack?1:0} 后隔离。反向测试 _getMaterial does not share ... across tintBlack variants revert 即红。✓

  2. _clearMaterialCache 改按 _cacheKey 删除(:329-340 + SpineMaterial._cacheKey — 旧实现从 renderer 当前状态重算 key 删除,若 register 后 premultipliedAlpha 变过就会漏删 stale 条目;且把 materials[i] 无脑 as SpineMaterialshaderData.getTexture(...).instanceId,遇到用户经 setMaterial(public API)塞进来的普通 Material / null 洞会 NPE。新实现 instanceof SpineMaterial 守卫 + 按注册时的确切 _cacheKey 删除。反向测试 destroy tolerates user-set materials ... revert 即抛。✓ _cacheKey 是「该 material 注册于哪个 key」的元数据,@internal 归属正确。

  3. 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;_onDestroyprimitive._addReferCount(-1) 经 Primitive 重写的 _addReferCount 级联把 buffer 归零。两处 ?.destroy() 时 refCount 已到 0,ReferResource.destroy(force=false) 真正释放而非 return false。注释「destroy() is refCount-guarded / 已释放引用」属实非 aspirational,确实免去等 gc()。首帧 oldXxxundefined?. no-op。✓

  4. _cloneTo 空 skeleton 守卫(:229 — 手动加、未绑定 resource 的 renderer _skeleton/_state 为 null,旧代码 runtime.createSkeleton(this._skeleton.data) NPE。守卫 if (!this._skeleton || !this._state) return; 放在 getSpineRuntime() 之前,故即便无 runtime 注册也不抛。_setSkeleton/_setState 成对赋值(_applyDefaultConfigif (skeleton && state)),|| 守卫安全。反向测试 cloning ... no skeleton does not throw revert 即红。✓

  5. 删除 separate-slot 死代码(两 generator 各 −41 行 + 删 SpineAnimationUpdateFlags enum) — 已核实这是真死代码而非丢功能:SpineGeneratorSpine42Runtime._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 路径逐字等价。✓

  6. SpineAtlasLoader.ts:67 atlasPath 取解析值 — 旧 const atlasPath = item.urlitem.urls 路径下 item.url 为 undefined;真值来自上方 const { atlasPath } = spineAtlasAsset(由 _groupAssetsByExtension/_assignAssetPathsFromUrl 填充),diff 只是删掉那行遮蔽用的错误声明。✓

  7. SpineTexture.setFilters Trilinear 改由 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 三分支双后端反向证伪。✓

  8. SpineLoader.ts 文档「json」覆盖默认 JSONLoader — 纯注释,描述与 @resourceLoader("Spine", ["json",...]) last-registration-wins 行为一致。✓

问题

无新问题。

首轮遗留(状态未变,非重提,仅记录)

  • [P2] 全套 CI 未运行 — target 仍为 fix/shaderlabci.yml 触发白名单仍只 main/dev/*gh pr checks 现仍仅 labeler 通过。本 commit 改的 material/buffer/loader 逻辑有 vitest 覆盖(作者已跑 vitest tests/src/spine),但渲染路径仍无 e2e 对图。同首轮结论,随 fix/shaderlabdev/2.0 合并时由 CI 补齐。
  • [P3] 仍为 #3057 的 stacked 重建 — 状态未变,仅记录。

简化建议

无。这一批修复本身就是净简化(删 82 行死代码 + 消除两处 latent bug),代码干净。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants