fix(physics): make mesh rebuilds atomic and scale PhysX defaults#3042
fix(physics): make mesh rebuilds atomic and scale PhysX defaults#3042luzhuang wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. 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 |
dbe20e6 to
0ad6942
Compare
GuoLei1990
left a comment
There was a problem hiding this comment.
首轮 review @ 0ad694262(stacked PR 3/3,--comment)
⚠️ HEAD 校准:gh一度缓存的headRefOid=dbe20e696(含_pendingNativeShapeCreation/_onPhysicsUpdate每帧重试机制)是旧 rebase 版;当前真实分支 tip 是0ad694262(commit 2 "keep mesh recooking transactional"),已整体删掉重试机制、改成事务式。本轮按真 tip0ad694262审。
总结
一组 mesh collider / 物理材质 / 默认值的修复,6 个子项:clone 重建 native shape、recooking 事务化、cook-null 视为终止失败(不再每帧重试)、PhysX tolerancesScale 选项 + scaled 默认 contactOffset/sleepThreshold、未显式设置时不覆盖 PhysX 默认、physics-lite 材质写入降级为一次性提示。整体方向正确,事务式重写比旧的"每帧重试"版本干净得多(cook-null 在 PhysX 侧基本是几何确定性失败,每帧重 cook 是热路径开销 + 每帧刷错 —— 删掉是对的,与描述一致)。
重点发现
[P2] PhysicsMaterial 构造期 friction/bounce combine 参数顺序——本 PR 修了一个真实潜伏 bug,但 PR 描述未提及。
createPhysicsMaterial 接口签名是 (staticFriction, dynamicFriction, bounciness, frictionCombine, bounceCombine)(design/IPhysics.ts:76-82,frictionCombine 在前)。dev/2.0 与本 PR base 上,PhysicsMaterial 构造器传的是 (..., this._bounceCombine, this._frictionCombine) —— 顺序反了:PhysX 侧 setFrictionCombineMode(bounceCombine) / setRestitutionCombineMode(frictionCombine),构造时把摩擦与弹性的 combine 模式装反。本 PR 改成 (this._frictionCombine, this._bounceCombine),修对了。
- 之所以一直没暴露:两者默认都是
Average(PhysicsMaterial.ts:13-14),只有用户把 frictionCombine / bounceCombine 设成不同值时才显形,且只在构造期(单独 setter 路径一直是对的)。 - 这是个货真价实的正确性修复,值得在 PR 描述里单列一条(当前 6 条 summary 没有这一项)。混在大 PR 里又不提,影响 changelog / 回滚判断。建议补进描述,或单独成一句 commit message 点明。
[P2] LitePhysicsMaterial 用 console.log 报"不支持",与 physics-lite 既有惯例不一致。
新代码把 5 个 setter 从 throw 改成一次性 console.log("Physics-lite don't support physics material...")(LitePhysicsMaterial.ts:66)。改成非抛出是对的(否则 clone _syncNative 重写会崩),但报告通道选错:同包 LiteDynamicCollider 对同类"不支持,请用 PhysX"用的是 Logger.error(...)(LiteDynamicCollider.ts:27/34/42…)。console.log 既不符合 Logger 惯例、严重度也偏低("不支持某能力"应当是 warn/error 级,不是 log 级)。建议改成 Logger.warn(保留一次性去重),与既有 lite 不支持提示对齐。参见 cr 原则"不支持应尽早清晰报错,不静默"。
核对通过项
- 事务式 recooking 正确(MeshColliderShape.ts
set mesh/set cookingFlags、PhysXMeshColliderShape_cookMesh/setMeshData/_updateGeometry):先 cook 新 geometry,setGeometry包 try/catch,失败则清新资源 + 回滚_mesh/_positions/_indices+ refCount,成功才 release 旧资源。失败不丢已工作的 mesh,公开mesh/cookingFlags状态也回滚。无悬挂 native。 - 重试机制已彻底移除、无悬挂引用:
_onPhysicsUpdate(base no-op + override)、Collider._onUpdate的 per-shape 循环、_pendingNativeShapeCreation字段全删干净(grep 物理目录零残留),_createNativeShape/_updateNativeShapeData改返 bool。cook-null 直接return false,终止、不每帧重 cook。 - clone 重建路径正确:
PhysicsMaterial._nativeMaterial/MeshColliderShape._nativeShape标@ignoreClone,靠_cloneTo钩子重建——material_cloneTo调_syncNative()把 5 个属性重写到克隆 native(且_syncNative用对了setFrictionCombine/setBounceCombine顺序),mesh_cloneTo用已 deepClone 的_positions重 cook。_cloneTo由 cloner 同步直调(与 #3036 判例一致),clone 测试(sphere 落在克隆地面)从公开clone()入口守住这条链。 - scaled 默认值公式 + 边界校验:
contactOffset = 0.02 * length、sleepThreshold = 5e-5 * speed²(PhysXPhysics.ts:354-356),与 PhysX 自身 tolerancesScale 缩放约定一致(接触偏移线性于长度单位、休眠能量阈值二次于速度);_assertPositiveFinite在系统边界(用户传入 options)对 length/speed 校验 + 抛错,符合"边界防御"。构造器重载(typeof === "object"检测 options)向后兼容,干净。 _contractOffset → _contactOffset拼写修复跨 3 处(PhysXColliderShape 字段+setter、PhysXCharacterController 读取)一致修齐,无遗漏。- 可选默认字段下沉(
ColliderShape._contactOffset/DynamicCollider._sleepThreshold改number | undefined,getter?? getDefault?.() ?? 硬编码、setter 仅显式设值时 sync native):让 PhysX 按 world scale 管默认而不强制覆盖用户值,符合"shared asset + override 透传"心法。
测试
does not retry non-convex mesh creation on non-kinematic dynamic colliders 名字带旧"retry"措辞(机制已删),但测试本身仍有效:验证终止失败只 console.error 一次、随后 3 tick 不再刷错——守住"终止失败不每帧重报"。可顺手把名字里的 "retry" 去掉以免误导,非阻塞。tolerancesScale drives PhysX default...(length=2→0.04 / speed=20→0.02)公式断言正确。material clone 测试从公开 clone 驱动、跑 40 tick 看克隆体按摩擦回弹,守住 material 同步。
结论
无 P0/P1,两个 [P2](material 参数顺序修复未在描述点明 + lite 用 console.log 不合惯例)均不阻塞,--comment。 stacked PR,建议随 #3025 → #3041 → 本 PR 栈顺序合并。补一下 PR 描述里漏的 combine 顺序修复 + lite 改 Logger.warn 即可。
fbffe68 to
e6cac5d
Compare
Problem
This PR fixes several ownership and default-value problems in the physics shape lifecycle:
MeshColliderShapeandColliderboth participated in native shape attachment. Replacing or clearing a mesh could therefore leave duplicate attachments, stale native geometry, or a "ghost" collider aftermesh = null.setMeshDatatransaction. That is not a valid PhysX transaction boundary: PhysX 4'sPxShape::setGeometryreports validation through the error callback, returns no success value, and cannot change the geometry type between convex and triangle mesh. A failed update could not be rolled back reliably.tolerancesScalewas read lazily from a caller-owned mutable object, while Core eagerly overwrote backend defaults with fixed contact-offset and sleep-threshold values.PhysicsMaterialpassed friction and bounce combine modes to the native factory in the wrong order.0.1), losing both the configured value and the backend-scaled default.Solution
Atomic mesh-shape rebuild
Collider: it removes the previous native shape, attaches the candidate, destroys the previous shape, and emits one shape-change notification.meshnow performs the same owned replacement withnull, so the old native shape is detached and destroyed immediately.setMeshDatamutation API and the second attachment-state flag. Convex/triangle transitions are represented by rebuilding the native shape, which matches the PhysX API contract.uploadData(true), and retain/releaseModelMeshreferences explicitly when cloning.Scaled PhysX defaults
tolerancesScale.lengthandtolerancesScale.speedin the PhysX backend constructor, so later caller mutations cannot change initialized physics behavior.PxPhysicsandPxCookingParamsfrom the same scale and release the temporary Embind wrapper after the native objects have copied it.0.02 * lengthand sleep-threshold default as5e-5 * speed^2.PxControllerDesc, because Core permits0while PhysX rejects a descriptor contact offset that is not strictly positive.Material ownership
PhysicsMaterial, preserving existing assignment semantics, and explicitly destroy the constructor-created temporary material.IPhysics.createPhysicsMaterialorder.Scope
This PR is based directly on
dev/2.0and contains one commit with only the mesh-shape lifecycle, scaled-default, and material-ownership changes described above. It does not include #3025 or #3041.Related work remains separate:
DynamicCollider.applyForceAtPosition.Verification
pnpm -F @galacean/engine-design run b:typespnpm -F @galacean/engine-core run b:typespnpm -F @galacean/engine-physics-physx run b:typespnpm b:moduledev/2.0: 5 files, 101 tests passedmesh = null, and post-upload convex switchingtolerancesScalereadback and scaled Core defaultscontactOffset = 0git diff --check