feat(particle): add orbital radial velocity over lifetime#3049
feat(particle): add orbital radial velocity over lifetime#3049hhhhkrx wants to merge 30 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds orbital and radial particle velocity support, updates transform-feedback and bounds handling, and extends unit and E2E coverage for the new particle behavior. ChangesOrbital and radial velocity over lifetime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #3049 +/- ##
===========================================
+ Coverage 79.36% 79.48% +0.11%
===========================================
Files 903 904 +1
Lines 100628 101100 +472
Branches 11256 11353 +97
===========================================
+ Hits 79866 80360 +494
+ Misses 20578 20556 -22
Partials 184 184
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🤖 Augment PR SummarySummary: Extends the Particle Changes:
Technical Notes: Orbital/radial are implemented only on the transform-feedback path (WebGL2), while keeping the existing linear-only velocity path guarded separately to avoid unnecessary TF. 🤖 Was this summary useful? React with 👍 or 👎 |
| /** | ||
| * The center of orbit for orbital/radial velocity, in the system's local space. | ||
| */ | ||
| get offset(): Vector3 { |
There was a problem hiding this comment.
packages/core/src/particle/modules/VelocityOverLifetimeModule.ts:206 (get offset()): since this returns the internal Vector3, in-place mutations like vol.offset.set(...) won’t trigger _onGeneratorParamsChanged(), so the shader may keep a stale offset until something else dirties the generator.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| private _isCurveMode(curve: ParticleCompositeCurve): boolean { | ||
| return curve.mode === ParticleCurveMode.Curve || curve.mode === ParticleCurveMode.TwoCurves; |
There was a problem hiding this comment.
packages/core/src/particle/modules/VelocityOverLifetimeModule.ts:413 (_isCurveMode): treating ParticleCurveMode.TwoCurves as “curve mode” here looks inconsistent with the lack of any random-min upload/macro for orbital/radial, so TwoCurves (and TwoConstants) likely behave as “max only” rather than true random range.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts (1)
162-165: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer tolerance checks for shader float assertions.
These assertions compare decimal shader values with exact equality. Since shader data commonly round-trips through float32, this can cause intermittent failures on different environments.
Suggested fix
- expect(orbital.x).to.eq(0.77); - expect(orbital.y).to.eq(1.02); - expect(orbital.z).to.eq(0.94); + expect(orbital.x).to.be.closeTo(0.77, 1e-6); + expect(orbital.y).to.be.closeTo(1.02, 1e-6); + expect(orbital.z).to.be.closeTo(0.94, 1e-6); @@ - expect(orbital.x).to.eq(0.77); - expect(orbital.y).to.eq(1.02); - expect(orbital.z).to.eq(0.94); + expect(orbital.x).to.be.closeTo(0.77, 1e-6); + expect(orbital.y).to.be.closeTo(1.02, 1e-6); + expect(orbital.z).to.be.closeTo(0.94, 1e-6);Also applies to: 186-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts` around lines 162 - 165, The assertions in the VelocityOverLifetimeOrbital.test.ts file use exact equality checks for floating point shader values (orbital.x, orbital.y, orbital.z, and the particleRenderer.shaderData.getFloat() call), which can fail intermittently due to float32 precision variations across environments. Replace the exact equality assertions with tolerance-based assertions that allow for small floating point differences. Apply this change to all the assertions mentioned, including those at lines 186-188, using a small epsilon value appropriate for float32 comparisons.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/particle/modules/VelocityOverLifetimeModule.ts`:
- Around line 206-216: The offset getter in VelocityOverLifetimeModule returns
the internal mutable Vector3 object directly, allowing callers to mutate it in
place via direct property assignment (e.g., velocityOverLifetime.offset.x = 2)
which bypasses the setter and never triggers _onGeneratorParamsChanged(). To fix
this, the offset Vector3 returned from the getter must be wrapped in a proxy or
use another mechanism to intercept property mutations on x, y, and z properties,
ensuring that any direct modification to those properties still triggers the
_onGeneratorParamsChanged() call to keep orbital/radial bounds and uniforms in
sync.
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 1708-1720: The orbital reach calculation in the
velocityOverLifetime._isOrbitalActive() block does not account for world-space
velocity or force offsets (worldOffsetMin and worldOffsetMax), which can cause
particle bounds to miss the swept direction and cull visible particles. Include
the world-space offset values in the reach calculation by factoring the distance
from the orbital offset to the world-space offset bounds into the reach value
before applying min.set and max.set operations. This ensures the orbital reach
encompasses both the rotational movement around renderer_VOLOffset and any
world-space offset translation.
In `@tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts`:
- Around line 25-44: The WebGLEngine instance is created in the beforeAll hook
but never disposed, causing GPU/WebGL resource leaks. Add an afterAll hook after
the existing beforeAll hook that calls the appropriate disposal method on the
engine instance (typically engine.destroy() or a similar cleanup method) to
properly release resources and prevent destabilization of subsequent test files.
This ensures the engine lifecycle is properly managed with matching setup and
teardown.
---
Nitpick comments:
In `@tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts`:
- Around line 162-165: The assertions in the VelocityOverLifetimeOrbital.test.ts
file use exact equality checks for floating point shader values (orbital.x,
orbital.y, orbital.z, and the particleRenderer.shaderData.getFloat() call),
which can fail intermittently due to float32 precision variations across
environments. Replace the exact equality assertions with tolerance-based
assertions that allow for small floating point differences. Apply this change to
all the assertions mentioned, including those at lines 186-188, using a small
epsilon value appropriate for float32 comparisons.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e0c28792-c579-40d2-a7e7-a3961a027c0a
⛔ Files ignored due to path filters (5)
e2e/fixtures/originImage/Particle_particleRenderer-velocity-orbital-constant.jpgis excluded by!**/*.jpgpackages/shader/src/ShaderLibrary/Particle/Module/VelocityOverLifetime.glslis excluded by!**/*.glslpackages/shader/src/ShaderLibrary/Particle/ParticleFeedback.glslis excluded by!**/*.glslpackages/shader/src/ShaderLibrary/Particle/ParticleVert.glslis excluded by!**/*.glslpackages/shader/src/Shaders/Effect/ParticleFeedback.shaderis excluded by!**/*.shader
📒 Files selected for processing (5)
e2e/case/particleRenderer-velocity-orbital-constant.tse2e/config.tspackages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/modules/VelocityOverLifetimeModule.tstests/src/core/particle/VelocityOverLifetimeOrbital.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc6f7030d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| get offset(): Vector3 { | ||
| return this._offset; | ||
| } |
There was a problem hiding this comment.
Invalidate bounds when offset is mutated in place
When callers use the usual mutable-vector pattern (velocityOverLifetime.offset.set(...) or assigning components), this getter returns the live Vector3 but no _onValueChanged handler is registered, so the setter and _onGeneratorParamsChanged() are bypassed. The shader can still read the changed offset, but the particle bounds stay based on the old orbit center and can cull live particles; other particle vector properties such as shape transforms hook _onValueChanged for this reason.
Useful? React with 👍 / 👎.
| const dx = Math.max(Math.abs(min.x - offset.x), Math.abs(max.x - offset.x)); | ||
| const dy = Math.max(Math.abs(min.y - offset.y), Math.abs(max.y - offset.y)); | ||
| const dz = Math.max(Math.abs(min.z - offset.z), Math.abs(max.z - offset.z)); | ||
| const reach = Math.sqrt(dx * dx + dy * dy + dz * dz) + radialReach; |
There was a problem hiding this comment.
Include world-space offsets in orbital bounds
When orbital velocity is combined with world-space VOL/FOL, the feedback shader converts those world-space deltas into simulation space before rotating around the offset, but this reach calculation happens before worldOffsetMin/Max are merged later. For example, an X-only world-space velocity can be swept into Z by the orbital rotation while the bounds only receive the X offset afterward, so particles can be incorrectly culled; fold the accumulated world offsets into the pre-orbit radius before computing reach.
Useful? React with 👍 / 👎.
0abcaf9 to
4d2f689
Compare
4d2f689 to
f65694a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 714-716: After calling _migrateActiveParticlesToFeedback() in the
conditional block where previousUseTransformFeedback is false, the GPU instance
buffer still contains the old non-TF compacted layout and needs to be
synchronized with the new feedback slot organization. Add a call to mark the
active particle range as dirty or perform an in-place instance buffer upload
immediately after _migrateActiveParticlesToFeedback() completes to ensure
instance attributes match the reorganized feedback slots.
- Around line 1913-1918: The orbital reach calculation in the _isOrbitalActive()
block computes the bounding sphere for orbital particles without accounting for
gravity displacement that gets applied before orbital rotation. This causes
visible particles to be culled when they move outside the calculated sphere
after gravity is applied. Modify the reach calculation (where Math.sqrt(dx * dx
+ dy * dy + dz * dz) + radialReach is computed) to include the maximum possible
gravity displacement in the distance calculation, ensuring the bounding sphere
encompasses particles after gravity displacement but before orbital velocity
rotation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2229cd18-f79f-4d3e-a62c-59447d39414c
⛔ Files ignored due to path filters (5)
e2e/fixtures/originImage/Particle_particleRenderer-velocity-orbital-constant.jpgis excluded by!**/*.jpgpackages/shader/src/ShaderLibrary/Particle/Module/VelocityOverLifetime.glslis excluded by!**/*.glslpackages/shader/src/ShaderLibrary/Particle/ParticleFeedback.glslis excluded by!**/*.glslpackages/shader/src/ShaderLibrary/Particle/ParticleVert.glslis excluded by!**/*.glslpackages/shader/src/Shaders/Effect/ParticleFeedback.shaderis excluded by!**/*.shader
📒 Files selected for processing (5)
e2e/case/particleRenderer-velocity-orbital-constant.tse2e/config.tspackages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/modules/VelocityOverLifetimeModule.tstests/src/core/particle/VelocityOverLifetimeOrbital.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- e2e/config.ts
- e2e/case/particleRenderer-velocity-orbital-constant.ts
- tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts
- packages/core/src/particle/modules/VelocityOverLifetimeModule.ts
…into feat/particle-vol-orbital-radial # Conflicts: # packages/core/src/particle/modules/ParticleCompositeCurve.ts
855d990 to
0530fd1
Compare
|
补一轮针对 orbital/radial 模拟逻辑正确性的审查(在已 approve 的功能主体之上,按"逻辑正确性 → 实现合理性 → 包围盒"三步过了一遍)。核心数学是对的,下面三条按严重度,P1/P2-bounds 同根。 [P1] gravity / FOL / drag / noise 被 orbital 一起旋转了
// :315
vec3 position = attr.a_FeedbackPosition - previousLinearOffset + baseVelocity * dt;
...
rel = position - renderer_VOLOffset; // :321 rel 里含了 baseVelocity*dt
rel = rotationByEuler(rel, getVOLOrbital(...) * dt); // :334 ← 被转
最有力的依据是实现自相矛盾:代码已经用 影响模拟本身(轨迹),量级 O(dt²)/帧、系统性累积,gravity+orbital 都强时可见。 修复(两行移动):把 // :315 删掉 + baseVelocity * dt
vec3 position = attr.a_FeedbackPosition - previousLinearOffset;
// ...orbital/radial 块 :318-341 完全不变...
// :343 加上 baseVelocity * dt
position += baseVelocity * dt + currentLinearOffset;
注意这是只摘当前帧增量,不是"gravity 永不转"——历史累积位移(含过去帧的 gravity)已是轨道的一部分,应该跟着公转。noise 一并摘出也对(
[P2] 包围盒 reach 偏小会 pop —— 与 P1 同根
包围盒可以偏大(粗),但这里偏小、漏覆盖真实粒子,是参考系不一致。 修复:包围盒代码本就按"orbital 只转 local 位移"写的(reach 在 worldOffset/gravity 合入之前算),它没写错。修 P1 后(shader orbital 只转 local base)两者重新对齐,此条基本自愈、包围盒无需改动。若不修 P1,则需把 worldOffset + gravity 幅度并入 reach(别挪 orbital 块到 transform 之后——盒子变世界空间而 offset 是局部空间会参考系错配)。 [P2] 死文件 ParticleFeedback.glsl 加了编译不到的逻辑
已确认正确、勿动:多轴 orbital 轴序( 收敛点:P1 一处两行修复同时正确 shader 模拟和包围盒一致性,建议两者绑定一起定(含重生成 e2e 基线)。除 P1/P2-bounds 外无新阻塞。 |
|
复核了 ✅ 死文件已删(4e2d79f14)
✅ orbital 速度重切分(65cc91bc9)—— 比单纯"挪 gravity 出旋转"更深刻新模型把速度按物理来源切分,决定转不转: orbitVelocity = startVelocity + linearVelocity // 粒子自身运动 → 进旋转
externalVelocity = baseVelocity - startVelocity // gravity+FOL+drag+noise → 旋转外这比"把 baseVelocity 整体挪到旋转后"更准确,因为它修掉的不只是"gravity 被转",还有一个更隐蔽的不对称:旧版 新模型的划分是干净的第一性原理:粒子自身运动(startSpeed + linear VOL)随轨道公转;外部场(gravity/FOL/drag/noise)在固定仿真空间施加、不随公转。旧版恰好两个边界都反了(转了 gravity、没转 linear VOL)。删 精度上:常量模式(默认 + 当前唯一 e2e)bit-identical;仅曲线模式 linear VOL 从解析积分降为逐帧欧拉,有界 O(dt) 误差——但 orbital 旋转本就是欧拉,统一欧拉是更一致的精度预算,可接受。 三点 follow-up(非阻塞)1. drag 对 startSpeed 的衰减被挪出了旋转桶(建议确认是否有意) 2. 曲线模式 linear VOL 的欧拉化建议补一行注释,说明此处刻意用瞬时速度×dt(非 cumulative),免得后人"好心修回" 3. e2e 覆盖建议补 orbital-curve + 至少一个 radial 模式——本次 rework 最新的两个行为(被转的 linear VOL、曲线欧拉漂移)恰好都在没有基线图的模式里,常量 case 覆盖不到。 之前我提的"gravity 不该被旋转"方向对,但你这版看得更深(startSpeed/linearVOL 对称 + 删整套递推),比我建议的"整体挪 baseVelocity"更好。LGTM,三点 follow-up 不阻塞。 |
|
把当前 shader(orbital/radial 位置积分)逐项核过、确认第一性原理正确后,回头按"包围盒 = shader 位置公式的区间算术像"重新推了一遍包围盒。下面是结论 + 可落地实现。两个 bug(一个 orbital 专属、一个 noise 通用),外加一个 shader 随机相关性问题。 第一性原理:orbital 包围盒就是绕 offset 的球shader 里 orbital 是绕 关键: [P1] 包围盒
|
| 配置 | 真实 | 现有 R | 正确 R |
|---|---|---|---|
| gravity (0,−9.8,0) 5s + orbital | 49 | 0.01 | 122.5 |
| worldVOL=(10,0,0) + entity 转90° + orbital | 19.95 | 0 | 30 |
| noise strength=2 + orbital | 8.23 | 0.01 | 10.4 |
只要 gravity/worldVOL/worldFOL/noise 任一非零 + orbital active,现有包围盒就非保守。
两个角色必须分开(这是数学,不是取舍):
- 进 R(球半径):world 项用幅度
|v|。因为 R 量的是距离(标量),旋转保模长 → world 向量转到任何方向,对"离 offset 距离"的贡献就是|v|,无需逆矩阵。 - 进盒子的非 orbital 部分:world 项仍用有方向区间、transform 后加(单边 worldVOL=[0,+20] 若按幅度对称成 [−20,+20] 会过度膨胀,且会被
rotateMat错转)。
实现(4 步,纯粹让 R 满足定义):
- orbital reach(:1706-1708)算
dx/dy/dz时各轴加上 world/gravity/noise 的幅度W_a(在 orbital 块前先算好这个幅度盒)。 - orbital-active 路径:跳过后面 :1727-1728 / :1742-1743 /
_addGravityToBounds的有方向加法(已在 R 里,再加是 double-count)。 - 非 orbital 路径:有方向加法照旧(给出紧盒子)。
- gravity 现在在
_calculateTransformedBounds之外加(:787),orbital-active 时要把它的幅度引进函数内的 R(或把 gravity 折叠进来统一处理)。
[P2] noise 包围盒疑似欠界:少乘 maxLifetime(请确认 noise 语义)
:1729-1743 注释写 "max displacement = |strength_max|",noise 加的是 |noise.strengthX._getMax()|(无 maxLifetime)。但 shader 里 computeNoiseVelocity(函数名带 Velocity,ParticleFeedback.shader:281)是 baseVelocity += computeNoiseVelocity(...) 当速度积分(*dt),NoiseModule.glsl:92 返回 ±strength 作为速度。若 noise 是速度扰动,则累积一生的位移上界是 |strength|·maxLifetime,包围盒注释/代码当成"位移=strength"就偏小,且不限 orbital,所有 noise 粒子今天都欠界。
但这取决于 noise 的设计语义:如果 noise 设计上就是"有界位移扰动"(位移幅度 = strength),则 shader 当速度积分才是问题、包围盒反而对。证据偏向"noise 是速度、包围盒漏乘"(函数名 + baseVelocity += 用法 + 仅 TF 路径),但那条注释像是早期非速度实现的遗留。请作者确认 noise 语义:若是速度,包围盒 noise 幅度应 × maxLifetime。
[P2] shader 随机通道复用:3 个随机覆盖 7 个独立因子
ParticleGenerator.ts:1022-1024 只写 3 个随机(_velocityRand ×3 → a_Random1.yzw),但 shader 里 linear VOL(a_Random1.yzw)、orbital(a_Random1.yzw)、radial(a_Random1.y)共用它们。一个落在 max-orbital-X 的粒子被强制 max-linear-X 且 max-radial —— linear/orbital/radial 的随机区间完全相关。每个粒子内部仍自洽(非单粒子物理错),但"随机范围多样性"这个维度塌缩了。建议加一个独立随机属性,让 orbital/radial 与 linear 独立取随机。非阻塞。
已确认正确、勿动
shader 侧第一性原理审计 18 项绝大多数 CORRECT:orbital 有限旋转、轴序 YXZ、radial 方向/奇点、gravity(×modifier + world-sim 空间往返抵消)、FOL、drag/dampen、4-way 空间变换、四元数、瞬时曲线求值(orbital=角速度、radial=线速度,瞬时×dt 正确)、render↔feedback 的 cross(ω,rel) 对应、73524570b 删 currentLinearOffset。包围盒的"球→立方体"AABB、radialReach 用 abs()(向内 radial 不增 reach,正向 abs 安全)也都对。
收口:包围盒第一性原理 = "R 必须等于离 offset 最大距离,含所有被旋转的累积位移"。现有 R 漏 world/gravity/noise(P1,orbital 时 pop)+ noise 通用欠界(P2)。修法就是让 R 满足定义(world 项进 R 用幅度、进盒子用方向,两角色分开)。随机通道复用(P2)是 shader 侧独立项。都附了精确落点,不阻塞合并但 P1 会在常见组合下 pop,建议处理。
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 73524570b 线性推进一个 commit 到 fcf37172b(gh api compare 73524570b...fcf37172b 返回 status: ahead, ahead_by: 1, behind_by: 0;fcf37172b 父 = 73524570b4396ce8c3fed6dc264595c0486a0266,确认非 force-push 回退)。唯一改动是 fix: expand orbital particle bounds——ParticleGenerator.ts(+85/-20)+ ParticleBoundingBox.test.ts(+59/-2)。这条 commit 闭环了我历轮反复记的非阻塞 follow-up [P2]「orbital reach 未并入 world-space VOL/FOL + gravity 位移」。逐路径核对:bounds 数学正确、保守安全、新增测试反向证伪有效。无 P0/P1,仅两条 P3。--comment LGTM。
✅ [原 P2] orbital reach 未并入 world-space VOL/FOL + gravity 位移 —— 已闭环(逐路径核对正确)
fcf37172b 把 orbital reach 从只含「box-to-offset 距离 + radialReach」扩展为 + worldReach + noiseReach + gravityReach:
const worldReach = this._getRangeReach(worldOffsetMin, worldOffsetMax);
const noiseReach = this._getVectorReach(noiseBoundsExtents);
const gravityReach = this._getGravityBoundsReach(maxLifetime);
const reach = Math.sqrt(dx*dx + dy*dy + dz*dz) + worldReach + noiseReach + gravityReach + radialReach;并把 orbital 路径下的 gravity/noise/worldOffset 加法从下游(_addGravityToBounds + 末尾 noise/worldOffset 加法)改为折进 reach,用 _useOrbitalBounds() 双端门控保证只算一次。
逐路径核对(不只信 commit message,核 shader 模型对齐 + 数学正确 + 不双算 + 别名安全 + 反向证伪):
-
数学是正确的保守上界 —— orbit 把
rel = position − offset绕offset旋转,position 累积了所有线性贡献(startVel/linearVOL/FOL/noise/gravity,其中 externalVelocity 经a_FeedbackPosition烘进每帧再被旋转)。旋转保|rel|,故每个线性贡献至多给|position|加自身模长。把各贡献模长当标量半径相加(三角不等式)是合法的保守上界 ✅。dx/dy/dz用的是已积分的 local box 角点(含 local VOL/FOL/startpos),world 贡献/noise/gravity/radial 再叠加,无遗漏(可能轻度 over-cover,但保守安全)。 -
gravity 只算一次(无双算) ——
orbitalActive时_useOrbitalBounds()=true→_updateBoundsSimulationLocal(:787)/_updateBoundsSimulationWorld(:823) 跳过_addGravityToBounds,gravity 改由_getGravityBoundsReach(:1724) 折进 reach。radial-only / 非 orbital 路径仍走旧的_addGravityToBounds+ 末尾 noise/worldOffset 加法(:1736-1751else if/!orbitalActive分支),行为保持。两端门控读同一 VOL 状态、一致 ✅。 -
temp-vector 别名安全 —— 新增
_tempVector32: Vector3(:51)绑定noiseBoundsExtents。_getGravityBoundsReach内部也写_tempVector32(:1789 gravityBoundsExtents),但只在 orbitalActive 分支调用(:1724),且noiseReach已在前一行(:1723)snapshot 进 number;orbitalActive 路径下noiseBoundsExtents此后不再被读(:1747-1749 在!orbitalActive内)。非 orbital 路径不调_getGravityBoundsReach,_tempVector32保留 noise 值供 :1748 读。两路径均安全 ✅。_tempVector20在_getCurveMagnitudeFromZero/radial/_getGravityBoundsReach间复用,但各自写后立即消费进 number,无交错污染 ✅。 -
新增测试反向证伪有效 ——
VelocityOverLifetime orbital bounds include gravity reach(gravityModifier=1, orbitalY=1, startSpeed=0)断言±125.074。我手算核对:maxLifetime=5、gravity=(0,−9.81,0)、coefficient=0.5·25=12.5 → gravityReach=|−9.81·12.5|=122.625;box-to-offsetsqrt(3·1.414²)≈2.449;reach=2.449+122.625=125.074✅ 逐位匹配。若 gravity 不折进 reach(旧行为),box 只会是±2.449→ 测试 fail。是真回归网。 -
noise 计算同步修正(顺带 latent fix) —— noise extent 从
Math.abs(strengthX._getMax())改为_getCurveMagnitudeFromZero(strengthX) = max(|min0|,|max0|),对负向 noise 摆动更正确(旧|_getMax()|对跨零曲线如[−5,2]会算成 2 漏掉 −5 摆幅);且补了× maxLifetime(noise 在 shader 里是 velocity 贡献baseVelocity += computeNoiseVelocity:281,积分到位移 ≈ strength·lifetime)。新增Noise测试(strength=2 →±11.414= box 1.414 + 2·5)反向证伪× maxLifetime:旧行为(无 ×lifetime)会得±3.414→ fail ✅。
CI
tip fcf37172b 全绿:build(3 OS)/e2e(4/4 分片)/codecov(project + patch 均 success)/lint/labeler。本轮 codecov/patch 转绿(历轮聚合假红消解)= 新增两条 bounds 测试覆盖了本 commit 的新代码行。
注释合规
删除的 noise 注释(// Noise module impact: noise output is normalized...)是复述代码的注释,删除符合规范;数学已落进命名清晰的 helper(_getNoiseBoundsExtents/_getGravityBoundsReach/_getRangeReach/_getVectorReach/_getCurveMagnitudeFromZero),无 dead code(逐个 grep 调用点确认全部 live)。fix: expand orbital particle bounds commit message 准确(见 P3-1 的小瑕疵)。无新增违规。
问题(均非阻塞 P3)
[P3] ParticleGenerator.ts — noise bounds × maxLifetime 是对非 orbital 路径的行为变化,commit message 未提及
_getNoiseBoundsExtents 把 noise extent 补了 × maxLifetime 并修正了负向摆动,这同时改变了非 orbital 粒子的 noise bounds(旧 |strength| → 新 |strength|·maxLifetime)。这是正确的 latent-bug fix(noise 是 velocity,应积分到位移),且 bounds 变大是保守安全的(不会误剔除)。但 commit message 只说「expand orbital bounds」,未提这条对所有 noise 粒子的修正——属可追溯性瑕疵。建议 commit body 补一句,或在 PR 描述里点明。不阻塞。
[P3] ParticleGenerator.ts:1724 — orbital gravity reach 改为随 TransformVolume dirty 缓存,运行时改 scene.physics.gravity 不再即时反映
旧非 orbital 路径每帧无条件跑 _addGravityToBounds 读 scene.physics.gravity;新 orbital 路径把 gravity reach 折进 _calculateTransformedBounds(受 TransformVolume dirty 门控)。gravityModifier 改会经 _onGeneratorParamsChanged 置 TransformVolume dirty(已核),但 scene.physics.gravity 向量本身改不会 dirty 任何粒子 bounds。故 orbital 粒子存活期间若运行时增大 scene gravity 且无任何粒子参数变更,缓存的 reach 会偏小 → 极端窄场景下可能 culling pop。注:这非回归(orbital 是本 PR 新特性,无「before」)、触发条件极窄、且 bounds 是 culling 优化非正确性。与历轮「reach 保守性偏紧不崩」同性质。不阻塞,列作 follow-up。
剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)
- [P2] stretched-billboard vert 路径无测试覆盖(上轮 P2)—— 本 commit 是 bounds,未触及 vert。
- [P3]
offset类型选型(静态Vector3vs 曲线)—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环(保持,不重复)
- ✅ [本轮] orbital
reach未并入 world-space VOL/FOL + gravity ——fcf37172b折进worldReach+noiseReach+gravityReach,逐路径核对 + 反向证伪测试,已闭环。 - ✅ feedback↔vert
reldesync ——73524570b删幻影currentLinearOffset。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bcexternalVelocity 摘成轨道后平移。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1删整个文件。 - ✅ offset 原地分量改不脏 bounds ——
a5a081e34。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max命名 ——5a09858e。
本 commit 精准闭环了我历轮反复记的非阻塞 follow-up [P2]:把 world-space VOL/FOL + noise + gravity 折进 orbital reach。逐路径核对 shader 模型对齐、数学是正确保守上界、gravity 不双算、temp-vector 别名安全、两条新测试逐位反向证伪(手算 125.074/11.414 匹配)、CI 全绿含 codecov/patch。顺带修正了 noise bounds 漏 × maxLifetime + 负向摆动的 latent bug。无 P0/P1,仅两条 P3(noise 变化未在 commit message 提 + orbital gravity 缓存的窄 staleness 边缘)。LGTM。
|
在确认 orbital/radial 模拟与包围盒都正确后,扫了一遍粒子 shader 找可简化处。下面两条是行为完全等价(bit-identical)的去重,不动 e2e 输出;外加一条数值验证时发现的预存包围盒缺口(非本 PR 引入)。 [简化] TF shader 里
|
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 fcf37172b 线性推进一个 commit 到 1395f995d(gh api compare fcf37172b...1395f995d 返回 status: ahead, ahead_by: 1, behind_by: 0;1395f995d 父 = fcf37172b5e7f757ef68466993fb67c940aba507,确认非 force-push 回退)。唯一改动是 refactor: reuse particle orbital evaluators——ParticleVert.glsl(+3/-2)+ ParticleFeedback.shader(+2/-47)。纯 shader 去重 + CSE,零行为变化。逐路径核对:删除的是真重复、函数逐字等价、CSE 安全、e2e 反向证伪。无 P0/P1,无新问题。--comment LGTM。
✅ 改动 1 —— ParticleFeedback.shader 删 getVOLOrbital/getVOLRadial 改用共享 evaluateVOLOrbital/evaluateVOLRadial(逐路径核对真去重)
1395f995d 删掉 ParticleFeedback.shader 里本地的 getVOLOrbital(45 行)+ getVOLRadial,两个调用点改用 evaluateVOLOrbital/evaluateVOLRadial。
逐路径核对(不只信 commit message,核共享函数在 scope + 逐字等价 + 宏门一致 + 零残留 + e2e 反向证伪):
-
共享函数已在 scope ——
ParticleFeedback.shader的 include 列表里有#include "ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl"(在Attributes/Varyings之后),该 module 文件已定义evaluateVOLOrbital(Attributes, float)/evaluateVOLRadial(Attributes, float)(包在_VOL_MODULE_ENABLED内)。解析无误。 -
删除的本地版与共享版逐字等价 —— 对照 module 里的
evaluateVOLOrbital/evaluateVOLRadial与 patch 里删掉的getVOLOrbital/getVOLRadial:常量/曲线两分支、RENDERER_VOL_ORBITAL_IS_RANDOM_TWO/RENDERER_VOL_RADIAL_IS_RANDOM_TWO的mix随机插值、uniform 名(renderer_VOLOrbitalMaxConst/renderer_VOLOrbitalMaxCurveX/Y/Z/renderer_VOLRadialMaxConst/renderer_VOLRadialMaxCurve等)、随机分量(orbital 用a_Random1.yzw、radial 用a_Random1.y)全部逐字相同。删的是真复制品,非语义有别的另一份实现。 -
宏门一致 ——
evaluateVOLRadial调用点在#if defined(RENDERER_VOL_RADIAL_CONSTANT_MODE) || defined(RENDERER_VOL_RADIAL_CURVE_MODE)内,与 module 里evaluateVOLRadial定义的 guard 逐字相同;evaluateVOLOrbital同理。两调用点又嵌在外层RENDERER_VOL_ORBITAL_*/RADIAL_*块内,与 module_VOL_MODULE_ENABLED覆盖一致。a_Random1字段在 struct 里的存在 guard(含defined(RENDERER_VOL_ORBITAL_IS_RANDOM_TWO)/...RADIAL...)与函数内对它的访问 guard 也一致,无越界访问。 -
ParticleVert.glsl早已用共享evaluateVOL*—— vert 端在:208/:213本就调用evaluateVOLRadial/evaluateVOLOrbital(共享 module 版),证明这两个是已确立的 canonical 共享函数,feedback shader 是唯一还留着私有副本的地方。本 commit 把 feedback shader 收口到同一份,方向正确(消除双源真相)。 -
零残留 ——
grep getVOLOrbital|getVOLRadial packages/shader/src全 NONE,PR-tip blob 也确认两函数定义已删、两调用点已转换。是真删除无悬挂。
✅ 改动 2 —— ParticleVert.glsl invWorldRotation CSE(逐路径核对行为等价)
把 quaternionConjugate(worldRotation) 提成局部 vec4 invWorldRotation(:179),两处内联调用(:193 _VOL_LINEAR_MODULE_ENABLED 分支、:210 orbital/radial rel 重建分支)改用该局部。
逐项验证:
- 值逐字等价 ——
invWorldRotation是quaternionConjugate(worldRotation)的 hoisting,worldRotation在:171-175已定,hoisting 后两处复用同一值,纯 CSE 无语义变化。 - 无残留内联 ——
grep quaternionConjugate|invWorldRotation仅 3 处:定义(:179) + 两处复用(:193/:210),无遗漏的内联quaternionConjugate(worldRotation),完整且一致。 - 与 feedback shader 既有写法对齐 ——
ParticleFeedback.shader早已在main顶部 hoistingvec4 invWorldRotation = quaternionConjugate(worldRotation);并全程复用,本 commit 让 vert shader 采用同一惯例,是一致性改进。 invWorldRotation无 VOL 时声明但未用是良性 —— TF 开但无任何 VOL 模块时invWorldRotation未被读,属 GLSL 良性 unused-local(编译警告非错误,lint/build 全绿),且与 feedback shader 同一模式,非缺陷。
CI
tip 1395f995d 全绿:build(3 OS)/e2e(4/4 分片)/codecov(project + patch 均 success)/lint/labeler 全 success。e2e 4/4 绿是决定性反向证伪——若删除的 getVOLOrbital/getVOLRadial 与共享 evaluateVOL* 不逐字等价,velocityOrbitalConstant 基线图会偏移 → e2e 红。实测全绿 ⇒ 去重行为端到端不变。
注释合规
本 commit 删的是两个函数定义(含其上的 // Orbital angular velocity... / // Radial velocity... 单行注释,随函数删除合理)。无新增注释。refactor: reuse particle orbital evaluators commit message 准确。无新增违规。
剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 的 vert 改动是行为保持的 CSE,未改
rel重建语义,故该 P2 性质不变(既未新破坏也未新覆盖)。 - [P3]
offset类型选型(静态Vector3vs 曲线)—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环(保持,不重复)
- ✅ orbital
reach未并入 world-space VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ offset 原地分量改不脏 bounds ——
a5a081e34。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max命名 ——5a09858e。
本 commit 是一次干净的 shader 去重:删 ParticleFeedback.shader 里与共享 module evaluateVOLOrbital/evaluateVOLRadial 逐字重复的私有 getVOLOrbital/getVOLRadial(消除双源真相,feedback shader 收口到 vert shader 早已使用的同一份),外加 ParticleVert.glsl 一处 invWorldRotation CSE(与 feedback shader 既有惯例对齐)。逐路径核对函数逐字等价、宏门一致、零残留、CSE 安全、e2e 4/4 反向证伪行为不变。无 P0/P1/新问题。LGTM。
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 1395f995d 线性推进一个 commit 到 d5f09e951(gh api compare 1395f995d...d5f09e951 返回 status: ahead, ahead_by: 1, behind_by: 0;d5f09e951 父 = 1395f995d87192cb37e30b734a342e1867dce3e5,确认非 force-push 回退)。唯一改动是 refactor(particle): use dedicated evaluator for visual linear velocity——VelocityOverLifetime.glsl(+26/-0)+ ParticleVert.glsl(+1/-2)。纯 shader 重构 + 死路径剥离,零行为变化。逐路径核对:新增 evaluator 与旧 out-param 逐字等价、call-site 收口正确、非死代码、e2e 反向证伪。无 P0/P1,无新问题。--comment LGTM。
✅ 改动 1 —— VelocityOverLifetime.glsl 新增 evaluateVOLVelocity(逐路径核对与旧 out-param 逐字等价)
d5f09e951 新增 vec3 evaluateVOLVelocity(Attributes, in float normalizedAge)——只计算 linear VOL 的瞬时速度(constant/curve 两模式 + TwoConstants/TwoCurves 的 mix 随机插值),定义在 _VOL_LINEAR_MODULE_ENABLED 块内(紧邻既有 computeVelocityPositionOffset 之前)。
逐路径核对(不只信 commit message,核与旧 out-param 逐字等价 + 宏门一致 + 曲线函数等价 + e2e 反向证伪):
-
CONSTANT_MODE 逐字等价 —— 新版
velocity = renderer_VOLMaxConst;RANDOM_TWO →mix(renderer_VOLMinConst, velocity, a_Random1.yzw)。与computeVelocityPositionOffset的currentVelocityout-param 路径(currentVelocity = renderer_VOLMaxConst+ 同样的mix(min, currentVelocity, a_Random1.yzw))逐字相同——同 uniform、同a_Random1.yzw、同 min/max 顺序。✅ -
CURVE_MODE 等价(关键:
evaluateParticleCurve≡evaluateParticleCurveCumulative的 out-param) —— 新版用evaluateParticleCurve(renderer_VOLMaxGradientX/Y/Z, normalizedAge);旧版用evaluateParticleCurveCumulative(...Max..., normalizedAge, currentVelocity.x/y/z)的 out-param(第 3 参)。我对照了ParticleCommon.glsl:44/60两函数实现:evaluateParticleCurve在 bracketing 段返回mix(lastKey.y, key.y, (normalizedAge − lastTime)/(time − lastTime))。evaluateParticleCurveCumulative在同一 bracketing 段把 out-param 设为currentValue = mix(lastValue, key.y, offsetTime/(time − lastTime)),其中offsetTime = normalizedAge − lastTime。- 两者表达式逐字相同(同
lastKey.y/key.y、同age比值)。cumulative 函数额外把积分累进返回值cumulativeValue——而 vert 端从不消费返回值(旧代码把computeVelocityPositionOffset当语句调用、只取 out-param)。故evaluateVOLVelocity的 CURVE 分支输出与旧currentVelocityout-param 逐字等价。✅ - RANDOM_TWO 路径同样逐字镜像(
minVelocity从 min 曲线取值、velocity = mix(minVelocity, velocity, a_Random1.yzw))。✅
-
宏门一致 ——
evaluateVOLVelocity内部分支用RENDERER_VOL_CONSTANT_MODE/RENDERER_VOL_CURVE_MODE/RENDERER_VOL_IS_RANDOM_TWO,与computeVelocityPositionOffset逐字相同;函数本身包在_VOL_LINEAR_MODULE_ENABLED内,与 call-site(vert:183在#ifdef _VOL_LINEAR_MODULE_ENABLED内)覆盖一致,scope 解析无误。a_Random1.yzw的访问 guard(RANDOM_TWO)与 struct 字段存在 guard 一致,无越界。✅
✅ 改动 2 —— ParticleVert.glsl call-site 收口到 evaluateVOLVelocity(逐路径核对行为等价 + 非死代码)
// 前(借 computeVelocityPositionOffset 的 out-param 只取速度,浪费 cumulative 积分计算)
vec3 instantVOLVelocity;
computeVelocityPositionOffset(attr, normalizedAge, age, instantVOLVelocity);
// 后(专用 evaluator,只算瞬时速度)
vec3 instantVOLVelocity = evaluateVOLVelocity(attr, normalizedAge);逐项验证:
-
instantVOLVelocity消费点逐字未变 ——:184-193的localVelocity += instantVOLVelocity/worldVelocity += instantVOLVelocity/currentLinearVelocity计算都是未改动 context,前后逐字节相同。✅ -
age不再传是正确的 —— 旧computeVelocityPositionOffset的age只用于velocityPosition积分(feedback 路径根本不消费 position offset,位置已烘进a_FeedbackPosition),新 evaluator 只算瞬时速度故不需要age。age在computeParticleCenter其它处仍被使用,无悬挂。✅ -
computeVelocityPositionOffset非死代码 ——git grepPR-tip:该函数仍在ParticleVert.glsl:119(非 feedback 的computeParticlePositionCPU-sim 路径)被调用,那里同时消费 cumulative 返回值(velocityPositionOffset→localPositionOffset)和 out-param(lifeVelocity→localVelocity),两个输出都用。所以这次重构是干净的职责分离:只要瞬时速度的 feedback 消费者用新evaluateVOLVelocity,要 position offset 的 CPU-sim 消费者保留computeVelocityPositionOffset。无死代码、无双源真相。✅ -
顺带的微 perf 收益 —— feedback 路径的 curve 分支现在调
evaluateParticleCurve(单次插值查找)而非evaluateParticleCurveCumulative(插值查找 + 梯形积分累进循环),剥掉了 feedback 渲染路径里被浪费的积分计算。是净改进非中性。
CI
tip d5f09e951 全绿:build(3 OS)/e2e(4/4 分片全 success)/codecov(project + patch 均 success)/lint/labeler。e2e velocityOrbitalConstant 是反向证伪——该 case 设 linearVOL constant (1,10,1) + orbital + radial(Local + TF),走的正是本次改的 call-site(:183 → evaluateVOLVelocity constant 分支 → currentLinearVelocity → visualSimulationVelocity)。若新 evaluator 的 constant 输出与旧 out-param 偏移 → 基线图偏移 → e2e 红。实测全绿 ⇒ constant 路径行为端到端不变(curve 路径走逐字等价证明,该 case 未覆盖 curve 分支)。
注释合规
本 commit 无新增/删除注释。新增 evaluateVOLVelocity 无 doc 注释——与同文件既有 shader 函数 evaluateVOLOrbital/evaluateVOLRadial/computeVelocityPositionOffset 惯例一致(该 .glsl 里这些函数均无 doc 注释)。refactor(particle): use dedicated evaluator for visual linear velocity commit message 准确。无新增违规。
剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 是行为保持的 evaluator 收口,未改
rel/视觉速度重建语义,P2 性质不变。 - [P3]
offset类型选型(静态Vector3vs 曲线)—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环(保持,不重复)
- ✅
getVOLOrbital/getVOLRadialfeedback shader 去重 ——1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ offset 原地分量改不脏 bounds ——
a5a081e34。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max命名 ——5a09858e。
本 commit 是一次干净的职责分离重构:feedback 渲染路径原本借 computeVelocityPositionOffset 的 out-param 只取瞬时速度(浪费 cumulative 积分),现改用专用 evaluateVOLVelocity。逐路径核对新 evaluator 与旧 out-param 逐字等价(constant 直接相同;curve 经 evaluateParticleCurve ≡ evaluateParticleCurveCumulative out-param 逐字证明)、宏门一致、computeVelocityPositionOffset 仍在 CPU-sim 路径活用(非死代码)、call-site 消费点未变、e2e 4/4 反向证伪行为不变,顺带剥掉 feedback 路径的浪费积分计算。无 P0/P1/新问题。LGTM。
cptbtptpbcptdtptp
left a comment
There was a problem hiding this comment.
Reviewed the orbital/radial implementation end to end (TS + GLSL + TF simulation + bounds math). Overall the architecture looks solid: exact rotationByEuler integration in the TF shader instead of a linear ω×r step, conservative bounds composition is self-consistent (gravity/noise/world reach are folded into the orbital reach, and the outer _addGravityToBounds skip correctly avoids double-counting), the macro split (_VOL_LINEAR_MODULE_ENABLED) checks out at every usage site, clone wiring survives entity.clone(), and the deleted ParticleFeedback.glsl was confirmed dead code (never #included).
Found 1 confirmed bug (empirically reproduced with a passing vitest repro in real chromium WebGL2 — see the inline comment on ParticleCompositeCurve.ts), 1 latent-hazard pattern, and a handful of dedup/doc suggestions. Details inline.
| } | ||
| } | ||
| return min; | ||
| out.set(min, max); |
There was a problem hiding this comment.
[Bug — confirmed with a running repro] For a Curve/TwoCurves-mode composite whose curve is unassigned or has zero keys, min/max stay undefined here, so _isZero() returns false (undefined === 0) and the curve is classified as active.
Repro (single line, natural "switch mode first, author curve later" order):
vol.enabled = true;
vol.radial.mode = ParticleCurveMode.Curve; // curveMax is still undefinedConsequences, all verified by running the engine in chromium:
_isRadialActive()→true→ on WebGL2 the generator flips into transform feedback and_clearActiveParticles()kills every live particle — for a curve that evaluates to 0 everywhere._updateShaderDatathen hitsradial.curveMax._getTypeArray()→ TypeError thrown every frame (this crash path doesn't even need WebGL2). Same crash for all-three-orbital-axes Curve mode.
Suggested fix — treat an unauthored curve as zero, matching the ?? 0 convention already used in _evaluateCumulative:
out.set(min ?? 0, max ?? 0);which fixes _isZero, _getMax, and the NaN-prone _getExtremeValueFromZero consumers in one place.
vitest repro (4/4 assertions pass on this branch, chromium WebGL2)
it("mechanism: curve mode with unassigned curveMax is classified non-zero", () => {
const vol = particleRenderer.generator.velocityOverLifetime;
vol.enabled = true;
expect((vol.radial as any)._isZero()).to.eq(true);
vol.radial.mode = ParticleCurveMode.Curve;
expect(vol.radial.curveMax).to.eq(undefined);
expect((vol.radial as any)._isZero()).to.eq(false); // BUG
expect(vol._isRadialActive()).to.eq(true); // BUG
});
it("consequence A: transform feedback flips on and live particles are cleared", () => {
expect((particleRenderer.generator as any)._useTransformFeedback).to.eq(isWebGL2);
});
it("consequence B: _updateShaderData throws TypeError on undefined curveMax", () => {
expect(() => {
particleRenderer.generator._updateShaderData(particleRenderer.shaderData);
}).to.throw(TypeError);
});
it("control: assigning Constant mode afterwards recovers", () => {
const vol = particleRenderer.generator.velocityOverLifetime;
vol.radial.mode = ParticleCurveMode.Constant;
expect(vol._isRadialActive()).to.eq(false);
expect(() => {
particleRenderer.generator._updateShaderData(particleRenderer.shaderData);
}).to.not.throw();
});There was a problem hiding this comment.
Verified fixed in 8bcfc64 ✅ — rebuilt this branch and ran the suite in chromium WebGL2: the new "unauthored orbital/radial curves stay inactive" regression test covers the exact repro chain (no spurious TF flip, no TypeError), 30/30 passing. Thanks for also adopting the ?? 0 form — it fixes _getMax/_getExtremeValueFromZero for empty curves in the same stroke.
| const maxGravityEffect = modifierMinMax.y * coefficient; | ||
| const { x, y, z } = this._renderer.scene.physics.gravity; | ||
|
|
||
| const gravityBoundsExtents = ParticleGenerator._tempVector32; |
There was a problem hiding this comment.
[Latent hazard] _getGravityBoundsReach grabs ParticleGenerator._tempVector32 as its own scratch — but the caller _calculateTransformedBounds has that same static aliased as noiseBoundsExtents (L1632), writes it at L1707, and reads it at L1723 and L1748.
It happens to be safe today only by accident of ordering/branch shape: the L1723 read executes one line before this function clobbers the vector at L1724, and the L1748 read sits in the mutually-exclusive !orbitalActive branch. The two adjacent lines at L1723-1724 look freely reorderable; swapping them (or supporting orbital+noise in one branch later) silently makes noiseReach read gravity data. No comment or assertion documents the constraint.
Suggestion: give this helper its own temp (_tempVector33) or take an out: Vector3 parameter.
There was a problem hiding this comment.
Verified fixed in 8bcfc64 ✅ — _getGravityBoundsReach now uses the dedicated _tempVector33 (confirmed single user via grep), so the noiseBoundsExtents aliasing hazard is gone. Bounds tests (16/16, incl. the orbital gravity-reach case) pass on the updated branch.
| #ifdef RENDERER_VOL_ORBITAL_CONSTANT_MODE | ||
| vec3 orbital = renderer_VOLOrbitalMaxConst; | ||
| #ifdef RENDERER_VOL_ORBITAL_IS_RANDOM_TWO | ||
| orbital = mix(renderer_VOLOrbitalMinConst, orbital, attributes.a_Random1.yzw); |
There was a problem hiding this comment.
[Worth a comment in code or PR description] Orbital (and radial, via .y) random-two mode reuses the exact random channel a_Random1.yzw that linear VOL's random-two mode consumes. When a user enables random mode on both linear and orbital/radial simultaneously, each particle's lerp factors are identical — particles that draw a fast linear velocity always also orbit fastest (Unity draws these independently per property).
I traced the instance layout: all 42 float slots are currently assigned, so independent draws would require growing the per-particle layout — the sharing looks like a reasonable trade-off (SOL and Noise already share a_Random0.z), but it's worth documenting the intent here so it doesn't read as an oversight.
There was a problem hiding this comment.
Resolved in 8bcfc64 ✅ — the comment documenting the shared random channel (and the instance-layout rationale) is exactly what was needed here.
| vec3 getVOLVelocity(Attributes attributes, float normalizedAge) { | ||
| vec3 vel = vec3(0.0); | ||
| #ifdef _VOL_MODULE_ENABLED | ||
| #ifdef _VOL_LINEAR_MODULE_ENABLED |
There was a problem hiding this comment.
[Dedup] getVOLVelocity is now body-for-body identical to the new evaluateVOLVelocity in VelocityOverLifetime.glsl (which this file already #includes at L48, and the single call site at L142 sits inside the same _VOL_LINEAR_MODULE_ENABLED guard). The local copy can be deleted and the call site switched to evaluateVOLVelocity(attr, normalizedAge) — one less pair of functions to keep in sync on the next VOL change.
There was a problem hiding this comment.
Verified fixed in 8bcfc64 ✅ — local getVOLVelocity removed and the call site switched to the shared evaluateVOLVelocity; shader precompile passes on the updated branch.
| const orbitalActive = this._isOrbitalActive(); | ||
| const radialActive = this._isRadialActive(); | ||
|
|
||
| if (orbitalActive) { |
There was a problem hiding this comment.
[Dedup] This orbital 3-axis curve/constant detection + upload block is a structural copy of the linear velocityX/Y/Z block above in the same method (~45 lines, only property/macro names differ). LimitVelocityOverLifetimeModule already factors the same shape into _uploadScalarSpeed/_uploadSeparateAxisSpeeds returning { modeMacro, randomMacro } — the same extraction here would keep the two copies from drifting.
|
|
||
| vec3 rel; | ||
| if (renderer_SimulationSpace == 0) { | ||
| rel = attr.a_FeedbackPosition - renderer_VOLOffset; |
There was a problem hiding this comment.
[Dedup] This rel computation (position relative to orbit center, with the local/world simulation-space branching) is duplicated near-identically in ParticleFeedback.shader (L261-266). The two copies live in files that don't share the function, so a future fix to the offset space conversion (sign/rotation) has to land in both, risking silent divergence between the rendered stretched-billboard velocity and the simulated position. Both files already include VelocityOverLifetime.glsl — a small shared helper taking the position as a parameter would remove the risk.
There was a problem hiding this comment.
Following up with a concrete, verified shape for this — a small helper in VelocityOverLifetime.glsl (next to evaluateVOLOrbital/evaluateVOLRadial, inside the orbital/radial guard):
// Position relative to the orbit center, in the system's local space.
vec3 computeVOLRelativePosition(vec3 position, vec3 simulationWorldPosition, vec4 invWorldRotation) {
if (renderer_SimulationSpace == 0) {
return position - renderer_VOLOffset;
} else {
return rotationByQuaternions(position - simulationWorldPosition, invWorldRotation) - renderer_VOLOffset;
}
}Call sites collapse to:
// ParticleVert.glsl
vec3 rel = computeVOLRelativePosition(attr.a_FeedbackPosition, attr.a_SimulationWorldPosition, invWorldRotation);
// ParticleFeedback.shader
vec3 rel = computeVOLRelativePosition(position, attr.a_SimulationWorldPosition, invWorldRotation);renderer_SimulationSpace is declared before the #include in both host shaders (ParticleVert.glsl L37, ParticleFeedback.shader L18), so the helper resolves fine. Verified locally: shader precompile passes and all particle unit tests (162/162) stay green with this applied.
| radialReach = Math.max(Math.abs(velMinMaxX.x), Math.abs(velMinMaxX.y)) * maxLifetime; | ||
| } | ||
| if (orbitalActive) { | ||
| const dx = Math.max(Math.abs(min.x - offset.x), Math.abs(max.x - offset.x)); |
There was a problem hiding this comment.
[Dedup] These three lines + the Math.sqrt(dx*dx + dy*dy + dz*dz) below re-implement exactly what the new _getRangeReach(min, max) helper (L1798, added in this same PR and called two lines later for worldReach) already does — just with an offset shift. Shifting min/max by offset into spare temps and calling _getRangeReach would drop the duplicate.
| // Switching TF mode invalidates all active particle state: feedback buffers and instance | ||
| // buffer layout are incompatible between the two paths. Clear rather than show a one-frame | ||
| // jump; new particles will fill in naturally from the next emit cycle. | ||
| this._clearActiveParticles(); |
There was a problem hiding this comment.
[Docs] The comment that used to sit above this line ("Switching TF mode invalidates all active particle state: feedback buffers and instance buffer layout are incompatible between the two paths. Clear rather than show a one-frame jump...") was deleted, but it's still accurate and explains a non-obvious invariant — without it, a future reader could "optimize away" this clear and reintroduce the buffer-incompatibility bug it prevents. Please restore it.
|
|
||
| const loaders = ResourceManager._loaders; | ||
| for (let key in loaders) { | ||
| for (const key in loaders) { |
There was a problem hiding this comment.
[Nit] Unrelated let → const lint fix that slipped into this feature PR — harmless, but ideally kept out of the diff.
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 d5f09e951 线性推进一个 commit 到 8bcfc647c(gh api compare d5f09e951...8bcfc647c 返回 status: ahead, ahead_by: 1, behind_by: 0;8bcfc647c 父 = d5f09e951071f84be6fa583c8c43000cc0712ee0,确认非 force-push 回退)。唯一改动是 fix: handle empty particle curves,5 文件:ParticleCompositeCurve.ts(+1/-1)+ ParticleGenerator.ts(+2/-1)+ VelocityOverLifetime.glsl(+1/-0)+ ParticleFeedback.shader(+1/-30)+ 测试(+20)。三类改动:① ??0 修一个真实的 P0 级崩溃 + NaN-bounds latent bug(含 base dev/2.0 就存在的)② feedback shader 删本地 getVOLVelocity 收口到共享 evaluateVOLVelocity(纯去重,byte 等价)③ _tempVector33 消除一处 latent aliasing 隐患。逐路径核对全部正确,新测试反向证伪有效,CI 全绿。无 P0/P1,无新问题。--comment LGTM。
✅ 改动 1 —— ParticleCompositeCurve._getKeyMinMax 空曲线 undefined → 0(逐路径核对 = 真崩溃 + NaN-bounds 双修复)
out.set(min, max) → out.set(min ?? 0, max ?? 0)。看似只是"处理空曲线",实为一个实质行为变化:Curve/TwoCurves 模式但 curveMax/curveMin 对象未创建(undefined)的曲线,从"被判为 active"翻转为"判为 inactive"。逐路径核对这是两个真实 bug 的正确修复,非 cosmetic:
-
修 P0 级 TypeError 崩溃(
_updateShaderData每帧路径) ——set mode只设this._mode,不惰性创建_curveMax(已读:31-34确认)。故new ParticleCompositeCurve(0)(默认 constant,_curveMax=undefined)→mode = Curve后curveMax仍是undefined——这是真实的编辑器/用户路径。- 修复前:
_getKeyMinMax(undefined?.keys, count=0)返回out.set(undefined, undefined)→_isZero()=undefined===0 && undefined===0=false→_isRadialActive()=!false=true→ 进入_updateShaderData的radialActive块(VelocityOverLifetimeModule.ts:391-393)→radial.curveMax._getTypeArray()命中undefined._getTypeArray()= TypeError 崩溃。orbital 同理(:342-355orbitalX.curveMax._getTypeArray())。 - 修复后:
out.set(0,0)→_isZero()=true→_isRadialActive()/_isOrbitalActive()=false→radialActive/orbitalActive块被跳过 → 不触碰undefined._getTypeArray()→ 无崩溃。语义正确:未创建曲线对象的 Curve 模式曲线在 shader 里求值为 0,判为 inactive 是对的。✅
- 修复前:
-
修 NaN-bounds latent bug(含 base dev/2.0 就存在的) ——
_getMinMax经_getExtremeValueFromZero(ParticleGenerator.ts:1852-1855)喂 bounds:curve._getMinMax(out); out.x = Math.min(0, out.x); // 修复前 Math.min(0, undefined) = NaN out.y = Math.max(0, out.y); // 修复前 Math.max(0, undefined) = NaN
修复前,任何 Curve/TwoCurves 模式但 curve 对象未创建的 size/velocity/force 曲线喂 bounds 会产出 NaN → 污染整个包围盒。我核了 base
dev/2.0:旧_getMaxKeyValue/_getMinKeyValue空 keys 同样返回undefined(origin/dev/2.0:ParticleCompositeCurve.ts:271-295),_getExtremeValueFromZero同样Math.min(0, undefined)——这条 NaN-bounds 是 base 就存在的 latent bug,本 commit 一并修掉。??0让空曲线贡献0extent,保守安全。✅ -
只改空曲线路径,authored 曲线 byte 不变 ——
?? 0仅在min/max为undefined(无 keys)时生效。有 keys 的曲线(全零zeroCurve或非零)走min = max = keys[0].value循环,?? 0不触发,与修复前逐字节相同。无误伤 active 曲线。✅
✅ 改动 2 —— ParticleFeedback.shader 删本地 getVOLVelocity 改用共享 evaluateVOLVelocity(逐路径核对真去重)
删掉 feedback shader 里本地的 getVOLVelocity(30 行),调用点改用 d5f09e951 新增的共享 module evaluateVOLVelocity——与前几轮 getVOLOrbital/getVOLRadial 收口是同一模式(feedback shader 是该 VOL 速度 evaluator 最后的私有副本,ParticleVert.glsl 早已用共享版)。
逐路径核对(不只信 commit message,核 byte 等价 + 共享函数在 scope + 宏门一致 + 零残留 + e2e 反向证伪):
-
byte 等价 —— 删除的
getVOLVelocity与 moduleevaluateVOLVelocity(VelocityOverLifetime.glsl:36-60)逐行对照:CONSTANT 分支renderer_VOLMaxConst+mix(renderer_VOLMinConst, ·, a_Random1.yzw)、CURVE 分支evaluateParticleCurve(renderer_VOLMaxGradientX/Y/Z)+mix(minVelocity, ·, a_Random1.yzw)——逐字相同(仅局部变量名vel→velocity、minVel→minVelocity)。✅ -
共享函数在 scope —— feedback shader
#include "ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl"(:48)。evaluateVOLVelocity定义在 module 的#ifdef _VOL_LINEAR_MODULE_ENABLED(module:13开、:96闭)内,call-sitevec3 vol = evaluateVOLVelocity(...)也在#ifdef _VOL_LINEAR_MODULE_ENABLED(feedback:111-119)内——定义门与调用门逐字一致。✅ -
旧版无守卫 vs 新版有守卫——安全 —— 旧
getVOLVelocity无#if守卫(vec3 vel = vec3(0.0)初始化);新evaluateVOLVelocity仅在_VOL_LINEAR_MODULE_ENABLED下定义。因唯一 call-site 同样被_VOL_LINEAR_MODULE_ENABLED守卫,函数不存在时永不被调用;且_VOL_LINEAR_MODULE_ENABLED蕴含 CONSTANT 或 CURVE 二选一(module:4),故velocity两分支必有一路赋值,无未初始化读。✅ -
零残留 ——
git grep getVOLVelocity 8bcfc647c -- packages/shader/**= NONE。真删除无悬挂。✅
✅ 改动 3 —— _tempVector33 消除 gravity/noise 共享 scratch 的 latent aliasing
_getGravityBoundsReach 里 gravityBoundsExtents 从 _tempVector32 改用新增的 _tempVector33。_tempVector32 在 _calculateTransformedBounds(:1633)绑定 noiseBoundsExtents。
逐路径核对(这条正是我 13:33 那轮"temp-vector 别名安全"里评估过的 latent 隐患):
- orbital 分支:
noiseReach = _getVectorReach(noiseBoundsExtents)在_getGravityBoundsReach调用前一行已 snapshot 进 number(:1720-1722),故此前_tempVector32复用未污染noiseReach——我 13:33 判"latent 但当前未触发"是对的。 !orbitalActive分支读noiseBoundsExtents(:1748-1749),但它与 orbital 分支互斥,_getGravityBoundsReach只在 orbitalActive 时调用——读 noise 的分支永不在 gravity reach 计算后运行。- 故这是正确的防御性硬化(消除一处未来重构易踩的共享 scratch 耦合),非当前可观测 bug 的修复。零行为变化、CI 全绿。✅
CI
tip 8bcfc647c 全绿:build(3 OS)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。
注释合规
新增一行 // Orbital/radial random modes share linear VOL random channels to keep the particle instance layout unchanged.——单行 //、无末尾句号 ✓,是准确的 why 注释(记录 orbital 用 a_Random1.yzw/radial 用 a_Random1.y 刻意复用 linear VOL 随机槽以保持实例布局不变的设计约定,非复述代码),已对照实际 a_Random1 用法核实准确。删除的 30 行是函数体,无注释违规。fix: handle empty particle curves commit message 准确。无新增违规。
新增测试反向证伪有效
velocity over lifetime unauthored orbital/radial curves stay inactive:vol.radial.mode = Curve(curveMax === undefined)→ 断言 _isZero() === true / _isRadialActive() === false / _needTransformFeedback() === false / _updateShaderData(...) to.not.throw()。修复前反向证伪:_getKeyMinMax 返回 undefined → _isZero() = false → _isRadialActive() = true → 断言 fail,且 _updateShaderData 会命中 undefined._getTypeArray() 抛 TypeError(to.not.throw 直接 fail)。故这是真回归网——同时守住 _isZero 语义翻转 + 崩溃路径。orbital 三轴 Curve 分支同样覆盖。
剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 未触及 vert
rel/视觉速度重建。 - [P3]
offset类型选型(静态Vector3vs 曲线)—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环(保持,不重复)
- ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅
getVOLOrbital/getVOLRadialfeedback shader 去重 ——1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ offset 原地分量改不脏 bounds ——
a5a081e34。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max命名 ——5a09858e。
本 commit 是一次干净的 bug 修复 + 去重:??0 同时修掉一个 P0 级崩溃(未创建曲线对象的 Curve 模式曲线因 _isActive 误判为 true 而在 _updateShaderData 命中 undefined._getTypeArray() 抛 TypeError)+ 一个 NaN-bounds latent bug(Math.min(0, undefined)=NaN,含 base dev/2.0 就存在的),语义翻转正确(空曲线 → inactive);feedback shader 收口到共享 evaluateVOLVelocity(byte 等价、宏门一致、零残留);_tempVector33 消除一处 latent aliasing 隐患(防御性硬化)。逐路径核对全部正确,新测试反向证伪同时守住语义翻转 + 崩溃路径,CI 4/4 全绿。无 P0/P1/新问题。LGTM。
cptbtptpbcptdtptp
left a comment
There was a problem hiding this comment.
One more small structural suggestion on the shader side (verified: the change below compiles through the shader precompiler and passes all 13 particle test files, 162/162, on this branch).
| #define _VOL_LINEAR_MODULE_ENABLED | ||
| #endif | ||
|
|
||
| #if defined(_VOL_LINEAR_MODULE_ENABLED) || defined(RENDERER_VOL_ORBITAL_CONSTANT_MODE) || defined(RENDERER_VOL_ORBITAL_CURVE_MODE) || defined(RENDERER_VOL_RADIAL_CONSTANT_MODE) || defined(RENDERER_VOL_RADIAL_CURVE_MODE) |
There was a problem hiding this comment.
[Dedup] The 4-condition orbital/radial guard defined(RENDERER_VOL_ORBITAL_CONSTANT_MODE) || ... || defined(RENDERER_VOL_RADIAL_CURVE_MODE) is repeated 4 times across 3 files (twice here, ParticleVert.glsl L197, ParticleFeedback.shader L210). Following the _VOL_LINEAR_MODULE_ENABLED convention this PR already established, a composite macro collapses them all and removes the risk of one site drifting when a new mode macro is added later:
#if defined(RENDERER_VOL_ORBITAL_CONSTANT_MODE) || defined(RENDERER_VOL_ORBITAL_CURVE_MODE) || defined(RENDERER_VOL_RADIAL_CONSTANT_MODE) || defined(RENDERER_VOL_RADIAL_CURVE_MODE)
#define _VOL_ORBITAL_RADIAL_MODULE_ENABLED
#endif
#if defined(_VOL_LINEAR_MODULE_ENABLED) || defined(_VOL_ORBITAL_RADIAL_MODULE_ENABLED)
#define _VOL_MODULE_ENABLED
#endifThe other three sites then become plain #ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED. Both consumers include this file before use, so visibility is fine. I applied exactly this locally: shader precompile passes and all particle unit tests (162/162) stay green.
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 8bcfc647c 线性推进一个 commit 到 829b679d(gh api compare 8bcfc647c...829b679d 返回 status: ahead, ahead_by: 1, behind_by: 0;829b679d 父 = 8bcfc647c11b7a3149835e77e4d11b67a4a4a740,确认非 force-push 回退)。唯一改动是 refactor: share orbital radial vol shader guard,3 个 shader 文件、净 +6/-2:VelocityOverLifetime.glsl(+6/-2)+ ParticleVert.glsl(+1/-1)+ ParticleFeedback.shader(+1/-1)。纯宏守卫收口,零行为变化——把重复 3 次的 4 项 #if defined(...ORBITAL...) || ...RADIAL... inline 谓词提取成单个 _VOL_ORBITAL_RADIAL_MODULE_ENABLED 宏(在 VelocityOverLifetime.glsl 定义一次),另两个文件改引用它。逐路径核对:宏展开逐字等价、定义先于所有使用点(include-order 契约保持)、消费范围收窄正确(只收 4 项组合谓词,2 项 orbital-only/radial-only 选择谓词未误伤)、零残留、e2e 反向证伪。无 P0/P1,无新问题。--comment LGTM。
✅ 本轮唯一改动 —— orbital/radial 组合守卫收口到 _VOL_ORBITAL_RADIAL_MODULE_ENABLED(逐路径核对宏展开逐字等价 + include-order 契约 + 收窄范围正确 + 零残留 + e2e 反向证伪)
829b679d 把在 3 个文件里逐字重复的组合谓词 defined(RENDERER_VOL_ORBITAL_CONSTANT_MODE) || defined(RENDERER_VOL_ORBITAL_CURVE_MODE) || defined(RENDERER_VOL_RADIAL_CONSTANT_MODE) || defined(RENDERER_VOL_RADIAL_CURVE_MODE) 提取成一个宏,在 VelocityOverLifetime.glsl:8-10 定义,另两处改 #ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED。这是消除双源真相的合理收口——组合谓词原本 3 副本,改一处漏一处会 desync,现单一 source of truth,与既有 _VOL_LINEAR_MODULE_ENABLED(同一模块文件里 RENDERER_VOL_CONSTANT_MODE || RENDERER_VOL_CURVE_MODE 的收口宏)完全同构对称。
逐路径核对(不只信 commit message,核宏展开等价 + _VOL_MODULE_ENABLED gate 不变 + include-order 契约 + 收窄边界 + 零残留 + e2e 反向证伪):
-
_VOL_MODULE_ENABLED派生逻辑逐字等价 ——- BASE(
8bcfc647c):_VOL_MODULE_ENABLED = _VOL_LINEAR_MODULE_ENABLED || ORBITAL_CONST || ORBITAL_CURVE || RADIAL_CONST || RADIAL_CURVE(单行 5 项 OR)。 - NEW:先
_VOL_ORBITAL_RADIAL_MODULE_ENABLED = ORBITAL_CONST || ORBITAL_CURVE || RADIAL_CONST || RADIAL_CURVE,再_VOL_MODULE_ENABLED = _VOL_LINEAR_MODULE_ENABLED || _VOL_ORBITAL_RADIAL_MODULE_ENABLED。 - 代入展开 =
_VOL_LINEAR_MODULE_ENABLED || (ORBITAL_CONST || ORBITAL_CURVE || RADIAL_CONST || RADIAL_CURVE)——||结合律下与 BASE 逐字相同布尔、相同操作数。✅
- BASE(
-
3 个使用点各自逐字等价 ——
renderer_VOLOffset声明(VelocityOverLifetime.glsl:103)、ParticleVert.glsl:197(visualSimulationVelocity重建入口)、ParticleFeedback.shader:210(linearVelocity初始化入口)三处 BASE 都是同一 4 项谓词,NEW 全改成#ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED,展开即原谓词。✅ -
include-order 契约保持(跨文件宏依赖的关键) —— 新宏在
VelocityOverLifetime.glsl:8定义,被ParticleVert.glsl:197/ParticleFeedback.shader:210引用;两文件均在引用点之前#include ".../Module/VelocityOverLifetime.glsl"(vert:103先于:197、feedback:48先于:210)。且这两文件早已在同一模块 include 之后使用_VOL_LINEAR_MODULE_ENABLED(vert:117/:182、feedback:112/:212)——即新宏沿用了已确立的同一 include-order 契约,不是新引入的脆弱依赖。✅ -
收窄范围正确——只收 4 项组合谓词,未误伤 2 项选择谓词 —— 全 shader tree 扫描:完整 4 项 inline 谓词现仅剩
VelocityOverLifetime.glsl:8一处(即宏定义站),3 处旧副本全清。同时ParticleVert.glsl:213/220、ParticleFeedback.shader:239/246、VelocityOverLifetime.glsl:137/162的 2 项 orbital-only(ORBITAL_CONST||ORBITAL_CURVE)/ radial-only(RADIAL_CONST||RADIAL_CURVE) 选择谓词未动——这些是在 orbital vs radial 子块间做选择,与「orbital 或 radial 任一 active」的组合门语义不同,用 4 项宏替换它们会是错误的过度收口。作者正确区分了这两类谓词。✅ -
零残留 —— 全
packages/shader/src/**.glsl/.shader逐文件扫描,旧 4 项组合谓词零残留(仅剩宏定义站),无悬挂副本。✅ -
e2e 实跑反向证伪 —— tip
829b679d上 e2e 4/4 全绿。这是决定性证据:velocityOrbitalConstantcase 覆盖 orbital+radial+linear VOL 全模式,若宏收口误改了任一变体的编译门(漏掉某 mode、错配renderer_VOLOffsetuniform 声明门、或 include-order 契约破裂使宏在使用点未定义)→ 编译出的 shader 变体不同 → 基线图偏移 → e2e 红。实测 4/4 全绿 ⇒ 守卫收口产出的 shader 变体端到端逐字节等价。✅
CI
tip 829b679d 全绿:build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。
注释合规
本 commit 零注释改动——VelocityOverLifetime.glsl:102 的 // Orbital / Radial (transform-feedback only). Center of orbit in system-local space. 是被移动宏所在块的未改动既有注释,非本次新增/修改。新增的是宏定义(#if/#define/#endif),无注释。refactor: share orbital radial vol shader guard commit message 准确。无新增违规。
剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 只把
ParticleVert.glsl:197的守卫从 inline 谓词改成宏,未触及 vertrel/视觉速度重建逻辑,P2 性质不变。 - [P3]
offset类型选型(静态Vector3vs 曲线)—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环(保持,不重复)
- ✅
??0空曲线崩溃 + NaN-bounds ——8bcfc647c。 - ✅
_tempVector33aliasing 硬化 ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅
getVOLVelocity/getVOLOrbital/getVOLRadialfeedback shader 去重 ——8bcfc647c/1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ offset 原地分量改不脏 bounds ——
a5a081e34。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max命名 ——5a09858e。
本 commit 是一次干净的宏守卫收口:把 3 个 shader 文件里逐字重复的 4 项 orbital/radial 组合谓词提取成单个 _VOL_ORBITAL_RADIAL_MODULE_ENABLED 宏(消除双源真相,与既有 _VOL_LINEAR_MODULE_ENABLED 同构对称)。逐路径核对 _VOL_MODULE_ENABLED 派生逐字等价(|| 结合律)、3 处使用点等价、include-order 契约沿用既有 _VOL_LINEAR_MODULE_ENABLED 同一模式、收窄范围正确(未误伤 2 项 orbital-only/radial-only 选择谓词)、零残留、e2e 4/4 反向证伪行为不变。无 P0/P1/新问题。LGTM。
Summary
Adds orbital and radial support to the Particle Velocity over Lifetime module, including transform-feedback simulation for position-dependent velocity terms.
Details
orbitalX,orbitalY,orbitalZ,radial, andoffsetproperties toVelocityOverLifetimeModule.Validation
vitest tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts --runnode packages/shader-compiler/bundler/cli.js packages/shader/src/Shaders packages/shader/compiledShaders --clean --emit-index --emit-sourcesBUILD_TYPE=MODULE NODE_ENV=release rollup -cBUILD_TYPE=UMD NODE_ENV=release rollup -cparticleRenderer-velocity-orbital-constantcompleted without page errors.Summary by CodeRabbit
offset.offset.velocityOrbitalConstant) with screenshot verification and updated E2E configuration.