feat(particle): add orbital radial velocity over lifetime#3049
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 101104 +476
Branches 11256 11355 +99
===========================================
+ Hits 79866 80360 +494
+ Misses 20578 20560 -18
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 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| /** | ||
| * 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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.
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.
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 👍 / 👎.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
hhhhkrx
force-pushed
the
feat/particle-vol-orbital-radial
branch
2 times, most recently
from
0abcaf9 to
4d2f689
Compare
hhhhkrx
force-pushed
the
feat/particle-vol-orbital-radial
branch
from
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
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
cptbtptpbcptdtptp
left a comment
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
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
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
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。
|
Non-blocking performance / gating suggestion: Orbital/radial VOL is a WebGL2 + transform feedback feature, but some of its side effects are currently gated by slightly different predicates. It may be worth centralizing this into one predicate, for example
This would keep WebGL1 / non-TF cases from generating unused shader variants or doing per-frame shaderData/random work for a feature that cannot affect the result. Related small shader cleanup: in |
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 829b679d 线性推进一个 commit 到 fdb377b39（gh api compare 829b679d...fdb377b39 返回 status: ahead, ahead_by: 1, behind_by: 0；fdb377b39 父 = 829b679d43a071c5ecc22ef08f393276c63cceed，确认非 force-push 回退）。唯一改动是 perf: gate orbital radial vol runtime work，2 文件：VelocityOverLifetimeModule.ts（+10/-5）+ ParticleVert.glsl（+33/-34）。两类改动：① TS 侧用 _needTransformFeedback() gate 掉 orbital/radial 的 shader-data 上传 + 随机槽写入（WebGL1 / orbital·radial 均 inactive 时跳过，纯 perf，正确）② shader 侧把 orbital/radial visual velocity 重建包进 RENDERER_MODE_STRETCHED_BILLBOARD（只有 stretched billboard 才消费 visualLocalVelocity，其余模式白算——方向正确）。但改动 ② 在把旧的 #else 分支删掉、改依赖顶部默认初始化时，引入了一个 stretched-billboard 视觉速度丢失 linear VOL 贡献的正确性回退（P1），且恰好静默撤销了 141aacf35 fix(particle): align stretched billboard to orbital velocity 的 linear-only 分支。故 --request-changes。
问题
[P1] ParticleVert.glsl:177-241 — stretched-billboard + linear-VOL（无 orbital/radial）TF 路径下，visualLocalVelocity 丢失 linear VOL 速度贡献（静默撤销 141aacf35 的 fix）
改动 ② 把 orbital/radial visual velocity 重建包进 #ifdef RENDERER_MODE_STRETCHED_BILLBOARD 是对的（只有 stretched billboard 在 :277 消费 visualLocalVelocity/visualWorldVelocity），但同时删掉了旧的 #else 分支，改为完全依赖顶部默认初始化。旧代码（829b679d）：
// :176-177 init（在 _VOL_LINEAR_MODULE_ENABLED 修改 localVelocity 之前）
vec3 visualLocalVelocity = localVelocity; // = a_FeedbackVelocity
vec3 visualWorldVelocity = worldVelocity; // = vec3(0.0)
...
#ifdef _VOL_LINEAR_MODULE_ENABLED
localVelocity += instantVOLVelocity; // localVelocity 现在含 linear VOL
...
#endif
#ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED
... visualLocalVelocity = visualSimulationVelocity + orbitalRadialVelocity; // visualSimulationVelocity += currentLinearVelocity
#else
visualLocalVelocity = localVelocity; // ← 在 linear VOL 加完之后重读，含 linear VOL
visualWorldVelocity = worldVelocity;
#endif新代码删掉 #else，所以 stretched-billboard + linear-VOL + 无 orbital/radial 时，visualLocalVelocity 保留 :177 的初值 = a_FeedbackVelocity（在 linear VOL += instantVOLVelocity 之前捕获），linear VOL 的速度贡献被丢掉。
这是正确性回退，非等价重构，理由三条（均已读源码核实，非推断）：
-
a_FeedbackVelocity不含 linear VOL —— 不是双算，是真丢失。 feedback shader（ParticleFeedback.shader:105/268）里localVelocity = attr.a_FeedbackVelocity，linear VOL 只算进volLocal/volWorld用于位置积分，v_FeedbackVelocity = localVelocity原样透传上一帧的 feedback 速度，从不把 linear VOL 写进v_FeedbackVelocity。所以 vert 端必须把instantVOLVelocity加回来才是真实瞬时速度——旧#else的visualLocalVelocity = localVelocity（=a_FeedbackVelocity + instantVOLVelocity）正是干这个，新代码把它丢了。 -
保留的 orbital 分支自己也加 linear VOL —— 证明设计意图就是「stretch 速度含 linear VOL」。 orbital/radial 分支
:213明确visualSimulationVelocity += currentLinearVelocity。新代码在 orbital 路径honor 这个语义，却在 linear-only 路径丢了它，前后不自洽。 -
非 TF 路径（
:239-241）也含 VOL，作为参照。 非 TF 分支visualLocalVelocity = localVelocity是在computeParticlePosition（:239，inout 累积了 start+VOL+FOL）之后捕获的，含 VOL。新 TF linear-only 路径与它不一致。
触发条件（完全可达、且是常见配置）：WebGL2 + RENDERER_MODE_STRETCHED_BILLBOARD + linear VOL 启用 + 无 orbital/radial + TF 被 noise/limitVelocity/death-subEmitter 任一触发（ParticleGenerator._setTransformFeedback:684-690：isWebGL2 && (limit || noise || VOL._needTransformFeedback() || deathSubEmitter)——linear-only VOL 自己不开 TF，但 noise/limit 会开）。即：WebGL2 上「拖尾/火花」类常见效果 = stretched billboard + linear VOL + noise，stretch 朝向/长度会静默丢掉 linear VOL 分量。
为什么 e2e 4/4 绿漏过：particleRenderer-velocity-orbital-constant.ts 用默认 billboard（未设 renderMode，:277 消费点被编译掉）且有 orbital active（走 orbital 分支非 #else）——两头都覆盖不到这个 linear-only stretched-billboard 回退。这正是历轮 [P2]「stretched-billboard vert 路径无测试覆盖」放大的后果。
Git 历史透镜证据：git log -S "visualLocalVelocity = localVelocity" 指向 141aacf35 fix(particle): align stretched billboard to orbital velocity——该 fix: commit 正是故意引入这个 #else 分支（linear-only 也要把 stretch 速度对齐到真实速度）。本 commit 删掉它 = 静默撤销该 fix 的 linear-only 分支，无 commit body 说明依据。
修复：把 :177-178 的 visualLocalVelocity/visualWorldVelocity 初始化下移到 _VOL_LINEAR_MODULE_ENABLED 块（:193）之后，让默认值捕获含 linear VOL 的 localVelocity/worldVelocity（orbital 块随后照常 overwrite），或直接恢复 #else 分支。同时建议补一条 stretched billboard + linear VOL + noise（无 orbital/radial） 的 e2e 把这条契约 pin 住——这个回退能静默漏过的根因就是它无覆盖。
✅ 改动 1（TS gating）—— 逐路径核对正确
_updateShaderData：orbitalActive/radialActive 现在 = needTransformFeedback && _isOrbitalActive()/_isRadialActive()。因整块在 if (this.enabled) 内，_needTransformFeedback() 退化为 isWebGL2 && (_isOrbitalActive() || _isRadialActive())。逐路径核：
- WebGL2 + orbital/radial 任一 active：
needTransformFeedback=true→ 与旧行为逐字等价。✅ - WebGL1：
needTransformFeedback=false→ 跳过 orbital/radial 曲线数组上传 + 不设 orbital/radial 宏，且_enableMacro(…, null)正确 disable 旧宏。WebGL1 无 TF，orbital/radial 本就跑不了，这是正确的 perf 收窄。✅
_isRandomMode() 早返回 isLinearRandomMode（当 !_needTransformFeedback()）：唯一消费者 ParticleGenerator.ts:1028 门控随机槽 24/25/26（a_Random1.yzw）写入。核对边界——orbital 在 TwoConstants(0,0)（_isZero()=true → inactive 但 mode===TwoConstants）时，旧代码会因 isOrbitalRandomMode=true 写 3 个随机值、新代码不写：但此配置下 orbital 宏不设、shader 不消费这些槽，结果不变；_velocityRand 是 VOL 专属子种子（grep 确认无跨模块共享），不写只影响自己的未被消费的槽，无 RNG 跨模块污染。✅
✅ 改动 2 的 perf 意图 —— 正确
把 orbital/radial visual velocity 重建包进 RENDERER_MODE_STRETCHED_BILLBOARD 方向正确：grep 确认 visualLocalVelocity/visualWorldVelocity 唯一消费点在 :277（#ifdef RENDERER_MODE_STRETCHED_BILLBOARD 内），其余 billboard/mesh 模式该重建被 DCE 白算。只是删 #else 的落地方式引入了上面的 P1。
CI
tip fdb377b39 全绿：build(3 OS)/e2e(4/4 全 success)/codecov(project + patch)/lint/labeler。但 e2e 覆盖不到 P1 config（默认 billboard + orbital active，见上）。
注释合规
本 commit 零注释改动。TS 侧新增 3 行逻辑无注释。commit message perf: gate orbital radial vol runtime work 准确描述了 perf 意图，但未提改动 ② 顺带删除了 141aacf35 的 #else——属可追溯性缺口（P1 修复时应在 commit body 说明）。
剩余开放项（非阻塞，历轮已记，本轮未触及）
- [P3]
offset类型选型（静态Vector3vs 曲线）—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——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。
改动的 perf 意图（TS gating + shader 侧只在 stretched billboard 才算 visual velocity）方向都对，TS 侧逐路径核对等价/安全。但 shader 侧删 #else 改依赖顶部默认初始化时，因初始化在 linear VOL += 之前捕获，导致 stretched billboard + linear VOL + noise（无 orbital/radial）TF 路径的 stretch 速度丢失 linear VOL 分量（P1，静默撤销 141aacf35 fix 的 linear-only 分支，e2e 用默认 billboard + orbital active 漏过）。建议把 visualLocalVelocity/visualWorldVelocity 初始化下移到 linear VOL 块之后（或恢复 #else）+ 补 stretched-billboard linear-only e2e 后再合。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 fdb377b39 线性推进一个 commit 到 e911a1dc8（gh api compare fdb377b39...e911a1dc8 返回 status: ahead, ahead_by: 1, behind_by: 0；e911a1dc8 父 = fdb377b398eeabad012235a690d571a3f1f1fdda，确认非 force-push 回退）。唯一改动是 fix: preserve linear vol stretched velocity——ParticleVert.glsl 单文件 +3/-2，正是我上轮 [P1] 所指的那个文件、那个落点。逐路径核对：修复正确，且落点与我上轮建议完全一致。解除我历史 CHANGES_REQUESTED 门控，approve。
✅ [原 P1] stretched-billboard + linear-VOL（无 orbital/radial）丢失 linear VOL 速度贡献 —— 已修复（落点精准）
e911a1dc8 把 visualLocalVelocity/visualWorldVelocity 的初始化从 _VOL_LINEAR_MODULE_ENABLED 块之前（旧 :176-177）下移到该块之后（新 :192-193），正是我上轮建议的两个方案之一：
// 前（fdb377b39，init 在 linear VOL 修改 localVelocity 之前捕获）
localVelocity = attr.a_FeedbackVelocity;
worldVelocity = vec3(0.0);
vec3 visualLocalVelocity = localVelocity; // ← 捕获的是 a_FeedbackVelocity，不含 linear VOL
vec3 visualWorldVelocity = worldVelocity;
...
#ifdef _VOL_LINEAR_MODULE_ENABLED
localVelocity += instantVOLVelocity; // linear VOL 加进 localVelocity（但 visual 已在此之前捕获）
#endif
// 后（e911a1dc8，init 下移到 linear VOL 块之后）
#ifdef _VOL_LINEAR_MODULE_ENABLED
localVelocity += instantVOLVelocity; // localVelocity 现在含 linear VOL
...
#endif
vec3 visualLocalVelocity = localVelocity; // ← 捕获含 linear VOL 的值
vec3 visualWorldVelocity = worldVelocity;逐路径核对（不只信 commit message，核四条路径 + 作用域 + 编译门 + P1 载荷前提 + CI）：
-
[P1 修复路径] stretched + linear-VOL(VOLSpace==0) + 无 orbital/radial —— 新 init 在
localVelocity += instantVOLVelocity之后捕获 →visualLocalVelocity = a_FeedbackVelocity + instantVOLVelocity（含 linear VOL），无 orbital 块不 overwrite → linear VOL 分量被正确保留。上轮丢失的正是这条，已修。✅ -
[对称路径] stretched + linear-VOL(VOLSpace!=0, world) + 无 orbital/radial —— 新 init 捕获
worldVelocity = instantVOLVelocity→visualWorldVelocity含 world VOL，无 orbital 不 overwrite → world VOL 保留。✅ -
[orbital 路径不受影响] stretched + orbital/radial（任意 linear 状态） ——
:227-231无条件 overwritevisualLocalVelocity/visualWorldVelocity为visualSimulationVelocity + orbitalRadialVelocity，且visualSimulationVelocity独立于 init（来自a_FeedbackVelocity + currentLinearVelocity，非localVelocity），故新 init 值对 orbital 路径是死值，行为与 base 逐字节相同，无回归。✅ -
[非 TF 路径 bit-identical] ——
#else分支的visualLocalVelocity = localVelocity（:241-242）未动，逐字节不变。✅
作用域 / 编译门核对（GLSL 正确性关键）：
- 新 init（
:195-196）在_VOL_LINEAR_MODULE_ENABLED的#endif（:193）之后、#ifdef RENDERER_MODE_STRETCHED_BILLBOARD（:198）之前——即在 TF 分支顶层作用域声明，与#else分支的声明（:241-242）同一 brace 作用域深度。消费点:278在 TF/non-TF#if/#else/#endif（:243）闭合之后，两分支都声明了该变量，故所有宏组合下visualLocalVelocity恰好声明一次且在 scope。✅ _VOL_LINEAR_MODULE_ENABLED未定义时，localVelocity（:176无条件赋a_FeedbackVelocity）在:195已有定义，visualLocalVelocity = localVelocity读到有效值。✅- 全文件
#if/#endif= 36/36 平衡，region 嵌套良构（移动两行未破坏预处理平衡）。✅
P1 载荷前提复核（feedback shader 不把 linear VOL 写进 v_FeedbackVelocity） —— 我逐行核了 tip 的 ParticleFeedback.shader：localVelocity = a_FeedbackVelocity（:105）→ linear VOL 只算进 volLocal/volWorld（:113-119，不加进 localVelocity）→ localVelocity += folDeltaLocal + gravityLocal（:133，只加 FOL+gravity）→ LVL 块（:136-189）把 volAsLocal 临时加入求 damping 后又减出（:145 totalLocal = localVelocity + volAsLocal → :147 localVelocity = dampenedTotal - volAsLocal）→ v_FeedbackVelocity = localVelocity（:268）仍不含 linear VOL。故 vert 端必须自己把 instantVOLVelocity 加回——新 init 位置正是干这个。P1 的载荷前提成立，修复对症。✅
CI
tip e911a1dc8 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success，已等 1/4 queued 分片跑完转绿)/codecov(project + patch)/lint/labeler。注：e2e 用默认 billboard + orbital active，覆盖不到本次修的 linear-only stretched-billboard 路径（见下 P2），故 e2e 绿既不能证也不能否这条 fix——修复的正确性由上面逐路径 shader 分析（by construction）确立，非靠 e2e。
注释合规
本 commit 零注释改动（纯代码移动两行声明）。fix: preserve linear vol stretched velocity commit message 准确描述了修复，也把 P1 修复正确标为 fix:。无新增违规。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 仍未加测试（正是让上轮 P1 静默漏过的那道缺口）。本次修复经逐路径 shader 分析 by construction 正确，但这条 feedback↔vert / linear-VOL-stretch 契约仍未被任何测试 pin 住。不阻塞（历轮已反复记，硬去重不再展开），但建议合并前或紧接着补一条 stretched billboard + linear VOL + noise（无 orbital/radial） 的 e2e，把这条契约钉死——否则未来任一端改动仍可能静默 desync。
- [P3]
offset类型选型（静态Vector3vs 曲线）—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ [本轮] stretched-billboard + linear-VOL 丢失 linear VOL 速度贡献 ——
e911a1dc8init 下移到 linear VOL 块之后，逐路径核对 + 作用域/编译门/载荷前提全核，已闭环。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——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 精准修掉了我上轮的阻塞项 [P1]：把 visualLocalVelocity/visualWorldVelocity 初始化从 linear VOL 块之前下移到之后，使 stretched-billboard + linear-VOL（无 orbital/radial）路径的 stretch 速度正确含 linear VOL 分量。逐路径核对四条路径行为（P1 路径修复 / world-space 对称 / orbital 路径死值不回归 / 非 TF bit-identical）、作用域与编译门（两分支同 scope 声明恰一次、#if/#endif 36/36 平衡）、P1 载荷前提（feedback shader v_FeedbackVelocity 确不含 linear VOL）全部核实，落点与我上轮建议完全一致，CI 4/4 全绿。无 P0/P1/新问题。解除我历史 CHANGES_REQUESTED 门控，approve。唯一开放项是这条契约仍无测试守护（P2，不阻塞，历轮已记）。
cptbtptpbcptdtptp
left a comment
cptbtptpbcptdtptp
left a comment
There was a problem hiding this comment.
Follow-up review @ e911a1d
All findings from the previous rounds are verified fixed. This pass went after behavior semantics and the public API surface — no blocking bugs found. Things I verified and am NOT flagging: the TF integration decomposition (orbitVelocity vs externalVelocity) is algebraically equivalent to the old path in both simulation spaces; every macro combination that makes a shader read a_Random1 has a matching CPU write via _isRandomMode(); the orbital bounds fold (world/noise/gravity/radial reaches collapsed into a rotation-invariant scalar sphere) neither double-counts nor drops gravity in either bounds caller; module construction order makes the new optional chaining in _setTransformFeedback necessary and sufficient; clone keeps the _offset change listener because CloneManager deep-copies into the existing instance via copyFrom.
Inline, most-important first: 2 semantics decisions to confirm (world-space orbit center, LimitVelocity not constraining orbital/radial), 1 API naming window that closes at merge (offset → orbitalOffset), 2 behavior changes that deserve a line in the PR description (linear TwoCurves random fix, noise bounds ×lifetime), 1 editor-facing UX edge (zero-crossing clears particles), 1 test-placement note.
One non-inline note: the conservative orbital bounds are large by design — the new gravity+orbital test pins ±125 where directional gravity alone would be far tighter, since the scalar-reach sphere turns directional gravity into all-directions reach. Correct and safe, just worth keeping culling efficiency in mind if orbital becomes a common authoring pattern.
Also, the PR description's validation command references tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts, which no longer exists on this branch — please update it alongside the test-placement comment below.
| if (renderer_SimulationSpace == 0) { | ||
| rel = position - renderer_VOLOffset; | ||
| } else { | ||
| rel = rotationByQuaternions(position - attr.a_SimulationWorldPosition, invWorldRotation) - renderer_VOLOffset; |
There was a problem hiding this comment.
[Design — please confirm] In world simulation space the orbit center is attr.a_SimulationWorldPosition — the emitter's position at the particle's birth. With a moving emitter, each particle orbits its own birth-frame center (plus the birth-rotation-transformed offset), so the trail becomes a family of rings pinned to past emitter positions. This is internally consistent with how the engine already treats world-sim (noise's noiseBasePos uses the same anchor), but the PR aims for Unity-like behavior, and my understanding is Unity orbits the system's current center in world space (moving the emitter drags the orbits along) — worth verifying against a Unity side-by-side before this ships. Whichever semantics is intended, please state it in the offset jsdoc; "in the system's local space" currently doesn't tell a world-sim user which transform snapshot applies.
| baseVelocity += computeNoiseVelocity(attr, noiseBasePos, normalizedAge); | ||
| #endif | ||
|
|
||
| #ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED |
There was a problem hiding this comment.
[Behavior gap vs Unity — worth documenting] The LVL dampen/drag steps above run before this block and only see base + linear VOL velocity, so the orbital tangential velocity and radial velocity are never speed-limited. Unity's Limit Velocity over Lifetime clamps the total velocity, orbital included. The deviation is defensible (limiting a positional rotation is awkward in this integration scheme), but it should be a deliberate, documented choice — a short comment here and/or a @remarks line on the orbital properties would stop it reading as an oversight later.
| /** | ||
| * The center of orbit for orbital/radial velocity, in the system's local space. | ||
| */ | ||
| get offset(): Vector3 { |
There was a problem hiding this comment.
[Naming — window closes at merge] offset only affects orbital/radial and is meaningless for the linear velocityX/Y/Z on this same module, so the bare name reads ambiguous on a module called VelocityOverLifetime. Unity names this orbitalOffsetX/Y/Z; orbitalOffset here would be self-describing. Renaming after release is a breaking change, so this is the only cheap moment to decide. (Minor, same block: the trailing explicit _onGeneratorParamsChanged() in the setter is redundant — copyFrom already fires the _onValueChanged listener wired in the constructor, and the explicit call also fires on self-assignment where nothing changed. Safe to drop.)
| velocityOverLifetime.velocityY.mode === ParticleCurveMode.TwoConstants && | ||
| velocityOverLifetime.velocityZ.mode === ParticleCurveMode.TwoConstants | ||
| ) { | ||
| if (velocityOverLifetime.enabled && velocityOverLifetime._isRandomMode()) { |
There was a problem hiding this comment.
[Behavior change — please call out in PR description] The old condition only wrote a_Random1.yzw when all three linear curves were TwoConstants, while RENDERER_VOL_IS_RANDOM_TWO was already set for the all-TwoCurves case too — so linear TwoCurves random mode previously sampled zeros from the never-written slots and every particle rendered at the min curve. _isRandomMode() fixing that is correct, but existing content using linear TwoCurves VOL will visibly change after this PR (it becomes actually randomized). Deserves an explicit line in the description/changelog so the rendering diff is traceable to this fix rather than to the orbital feature.
| } else { | ||
| noiseMaxX = noiseMaxY = noiseMaxZ = this._getCurveMagnitudeFromZero(noise.strengthX); | ||
| } | ||
| out.set(noiseMaxX * maxLifetime, noiseMaxY * maxLifetime, noiseMaxZ * maxLifetime); |
There was a problem hiding this comment.
[Behavior change — please call out in PR description] Noise bounds used to be |strength_max| with no lifetime factor; since TF noise contributes velocity, × maxLifetime is the correct displacement upper bound and the old formula under-estimated. Right fix — but existing scenes with noise get much larger culling bounds after this PR (the new "Noise" bounds test pins ±11.414 where the old formula gave ±3.414 for the same setup). Same ask as the random-write fix: one line in the description declaring the bundled fix.
| this._onCompositeCurveChange(lastValue, value); | ||
| lastValue?._unRegisterOnValueChanged(this._onTransformFeedbackDirty); | ||
| value?._registerOnValueChanged(this._onTransformFeedbackDirty); | ||
| this._generator._setTransformFeedback(); |
There was a problem hiding this comment.
[UX edge — editor team should know] TF activation is value-based (_isZero() over the whole curve range), so animating or slider-dragging an orbital constant through exactly 0 toggles TF off and back on, and each toggle runs _clearActiveParticles() — all live particles vanish twice mid-drag. It's a narrow trigger (the value must land exactly on zero-range) and probably acceptable, but since editor sliders make it reachable, a @remarks note on the orbital/radial properties ("switching between zero and non-zero restarts the simulation") would set expectations.
| }).to.not.throw(); | ||
| }); | ||
|
|
||
| it("velocity over lifetime orbital/radial default values", function () { |
There was a problem hiding this comment.
[Tests] These ~210 lines are VelocityOverLifetimeModule tests (an earlier revision of this branch had them in a dedicated VelocityOverLifetimeOrbital.test.ts); landing them inside the LimitVelocityOverLifetime suite makes them hard to discover, and the suite-level beforeEach now resets VOL orbital state for LVL tests that never touch it. Suggest moving them to tests/src/core/particle/VelocityOverLifetime.test.ts (no VOL test file exists yet — arguably overdue). Nit while moving: the 0x7/0x4 magic numbers in the dirty-flag test would read better with a comment pointing at ParticleUpdateFlags.
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 e911a1dc8 线性推进一个 commit 到 2956d0b71（gh api compare e911a1dc8...2956d0b71 返回 status: ahead, ahead_by: 1, behind_by: 0；2956d0b71 父 = e911a1dc8bc7b7b491f36630679c24f75a6748ef，确认非 force-push 回退）。唯一改动是 refactor(particle): clarify orbital radial API，7 文件：VelocityOverLifetimeModule.ts（+14/-8）+ ParticleGenerator.ts（+10/-10）+ ParticleFeedback.shader（+2/-1）+ e2e case（+1/-1）+ ParticleBoundingBox.test.ts（+1/-1）+ 测试搬迁（LimitVelocityOverLifetime.test.ts -222 → 新 VelocityOverLifetime.test.ts +267）。四类改动：① API 重命名 offset → centerOffset（含 setter/getter + 全部 in-PR 调用点）② setter 去掉冗余的显式 _onGeneratorParamsChanged() ③ 三处 JSDoc + shader 注释澄清 ④ 把误放在 LVL 测试文件里的 orbital/radial 测试搬到专属 VOL 测试文件。逐路径核对全部正确，历史 a5a081e34 回归网完整保留并重命名，CI 4/4 全绿。无 P0/P1/新问题。--comment LGTM。
✅ 改动 1 —— offset → centerOffset 重命名（逐路径核对：pre-release API 无外部消费者 + 全 in-PR 调用点已迁移）
gh api .../VelocityOverLifetimeModule.ts?ref=dev/2.0 确认：offset/orbital*/radial/centerOffset getter 在 base dev/2.0 零存在——整个 orbital/radial VOL 特性是本 PR（feat/particle-vol-orbital-radial）净新增、从未发版。故 offset → centerOffset 是 pre-release API 清晰化，非 breaking change，无外部消费者可破坏。全部 in-PR 调用点（e2e case :93 + ParticleBoundingBox.test.ts:112 + 新测试）本 commit 同步改完，CI 4/4 e2e 全绿反向证伪重命名未漏调用点（漏则编译失败）。命名本身是净改进——centerOffset 显式表达「相对系统原点的中心偏移」，比裸 offset 少歧义。✅
✅ 改动 2 —— setter 去掉显式 _onGeneratorParamsChanged()（逐路径核对：冗余去重，非 a5a081e34 回退）
// 前（e911a1dc8）
set offset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
this._generator._renderer._onGeneratorParamsChanged(); // ← 本次删掉
}
// 后（2956d0b71）
set centerOffset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
}这是冗余去重，非正确性回退，逐路径核对（不只信 commit message，核 copyFrom 副作用 + 构造期 wiring + 幂等 dirty + git 历史透镜）：
-
copyFrom自己就 fire dirty ——Vector3.copyFrom（packages/math/src/Vector3.ts:556-562）末行this._onValueChanged?.()。构造期 wiringthis._offset._onValueChanged = () => this._generator._renderer._onGeneratorParamsChanged()（:266，本 PR 保留）把_offset的值变化路由到 dirty。故 reassignment 路径（centerOffset = new Vector3(...)，value !== offset→copyFrom→_onValueChanged→ dirty）仍然脏 bounds——经 callback，非经被删的显式调用。✅ -
被删的显式调用是双派发的第二源 —— 旧 setter 在 reassignment 时 dirty 两次（
copyFrom→callback + 显式调用）。_onGeneratorParamsChanged（ParticleRenderer.ts:306-308）是纯幂等|=三 dirty bit，故双派发无害、去掉一个无行为变化。唯一差异：旧 setter 在value === offset（同实例自赋）时仍显式 dirty，新 setter 不 dirty——但同实例自赋值本就未变，不 dirty 是正确的（旧的是在 no-op 上过度 fire）。✅ -
git 历史透镜——不是回退
a5a081e34—— 我核了a5a081e34 fix(particle): dirty bounds when velocity offset changes：它只加了两样东西——(a) 构造期 wiring_offset._onValueChanged = ...（本 PR 保留:266）+ (b) "offset component changes dirty bounds after clone" 测试（本 PR 保留并重命名 为 centerOffset）。a5a081e34从未触碰 setter 的显式调用（那行 pre-date 该 fix）。故删它不撤销a5a081e34——该 fix 的真实机制（constructor wiring + 回归测试）完整保留。无矛盾。✅ -
in-place 路径（历史 P1 关注点）仍被测试守护 —— 新测试
centerOffset component changes dirty bounds after clone（VelocityOverLifetime.test.ts:246）做centerOffset.x = 1（set x→_onValueChanged）+centerOffset.set(2,0,0)（.set→_onValueChanged）在 cloned module 上，断言_isContainDirtyFlag(GeneratorVolume) === true。这条走的正是_offset._onValueChanged构造期 wiring 路径（非被删的 setter 显式调用），revert wiring 即红——a5a081e34的回归网原封守住。✅
注：setter reassignment 的 dirty 行为无直接断言（clone preserves ... centerOffset 只断言 value 拷贝非 dirty），但如上 by construction 证明（copyFrom→_onValueChanged 是 math 包 invariant + 构造 wiring 未动），且 in-place 路径（历史 P1 真正踩过的坑）有测试。不阻塞。
✅ 改动 3 —— JSDoc + shader 注释澄清（逐条打开代码验非 aspirational）
三处新增/改写的行为描述，逐条对照代码核实全部准确，无 aspirational：
-
"Switching orbital/radial values between zero and non-zero restarts the simulation" —— 准确。orbital/radial 曲线值变化 →
_onTransformFeedbackDirty（:96，经_onOrbitalRadialChange:498注册）→_setTransformFeedback()。值跨零翻转 →_isOrbitalActive/_isRadialActive（读_isZero()）翻转 →_needTransformFeedback()结果翻转 →_setTransformFeedback（ParticleGenerator.ts:683）里needed !== _useTransformFeedback→ 调this._clearActiveParticles()重启仿真。✅ -
"LimitVelocityOverLifetime does not clamp orbital/radial motion"（JSDoc + shader 注释）—— 准确。shader（
ParticleFeedback.shader）里 LVL 块（RENDERER_LVL_MODULE_ENABLED）clamp 的是localVelocity（base+FOL/gravity）与volLocal/volWorld（linear VOL）；orbital/radial 块（_VOL_ORBITAL_RADIAL_MODULE_ENABLED，另起于:211）用未经 LVL clamp 的volLocal/volWorld重算linearVelocity，orbital 旋转（rotationByEuler）+ radial 生长对rel做位置积分，完全在 LVL 之外。故 orbital/radial motion 确实不被 LVL clamp。✅ -
centerOffset "in world simulation space, this local offset is transformed by the simulation transform captured when each particle is born" —— 准确。shader
:237（world 空间分支）renderer_VOLOffset经worldRotation+attr.a_SimulationWorldPosition（每粒子出生帧仿真变换，烘进 attribute）变换。✅
JSDoc 格式合规：多行 /** */ 块、@remarks block tag、句尾句号。setter 不重复 JSDoc（getter 覆盖 accessor pair，符合项目惯例）。
✅ 改动 4 —— 测试搬迁（逐条核对：搬对位置 + 回归网无丢失）
8 个 velocity over lifetime orbital/radial ... 测试从 LimitVelocityOverLifetime.test.ts（它们本就误放在这里）搬到新专属 VelocityOverLifetime.test.ts。对照 base vs tip 的 it() 标题清单：LVL 测试文件精确剩下 21 个真 LVL 测试（速度/dampen/drag/space 等），8 个 orbital/radial 全部搬走且都在新文件出现（含关键的 clone preserves orbital/radial/centerOffset + centerOffset component changes dirty bounds after clone）。搬迁同步改了 offset → centerOffset。是合理的测试归位（orbital/radial 属 VOL 概念，不属 LVL），非覆盖丢失。✅
CI
tip 2956d0b71 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡；本次重命名offset→centerOffset使「静态中心点」语义更清晰，部分收敛该讨论，但类型本身未变。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ [本轮相关] offset in-place 分量改脏 bounds ——
a5a081e34的构造期 wiring + 回归测试本 PR 完整保留并重命名为 centerOffset，本次删 setter 显式调用不撤销该 fix。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——8bcfc647c/1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max——5a09858e。
本 commit 是一次干净的 pre-release API 清晰化：offset→centerOffset 重命名（净新增特性，无外部消费者，全 in-PR 调用点已迁移）+ setter 去冗余显式 dirty（copyFrom→_onValueChanged 已覆盖，非 a5a081e34 回退——该 fix 的构造 wiring + 回归测试完整保留）+ 三处行为描述澄清（逐条对照 shader/TS 验准确非 aspirational）+ 误放的 orbital/radial 测试归位到专属 VOL 测试文件（回归网无丢失）。逐路径核对全部正确，git 历史透镜确认无矛盾，CI 4/4 全绿。无 P0/P1/新问题。LGTM。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 e911a1dc8 线性推进一个 commit 到 2956d0b71（gh api compare e911a1dc8...2956d0b71 返回 status: ahead, ahead_by: 1, behind_by: 0；2956d0b71 父 = e911a1dc8bc7b7b491f36630679c24f75a6748ef，确认非 force-push 回退）。唯一改动是 refactor(particle): clarify orbital radial API，7 文件：VelocityOverLifetimeModule.ts（+14/-8）+ ParticleGenerator.ts（+10/-10）+ ParticleFeedback.shader（+2/-1）+ e2e case（+1/-1）+ ParticleBoundingBox.test.ts（+1/-1）+ 测试搬迁（LimitVelocityOverLifetime.test.ts -222 → 新 VelocityOverLifetime.test.ts +267）。四类改动：① API 重命名 offset → centerOffset（含 setter/getter + 全部 in-PR 调用点）② setter 去掉冗余的显式 _onGeneratorParamsChanged() ③ 三处 JSDoc + shader 注释澄清 ④ 把误放在 LVL 测试文件里的 orbital/radial 测试搬到专属 VOL 测试文件。逐路径核对全部正确，历史 a5a081e34 回归网完整保留并重命名，CI 4/4 全绿。无 P0/P1/新问题。--comment LGTM。
✅ 改动 1 —— offset → centerOffset 重命名（逐路径核对：pre-release API 无外部消费者 + 全 in-PR 调用点已迁移）
gh api .../VelocityOverLifetimeModule.ts?ref=dev/2.0 确认：offset/orbital*/radial/centerOffset getter 在 base dev/2.0 零存在——整个 orbital/radial VOL 特性是本 PR（feat/particle-vol-orbital-radial）净新增、从未发版。故 offset → centerOffset 是 pre-release API 清晰化，非 breaking change，无外部消费者可破坏。全部 in-PR 调用点（e2e case :93 + ParticleBoundingBox.test.ts:112 + 新测试）本 commit 同步改完，CI 4/4 e2e 全绿反向证伪重命名未漏调用点（漏则编译失败）。命名本身是净改进——centerOffset 显式表达「相对系统原点的中心偏移」，比裸 offset 少歧义。✅
✅ 改动 2 —— setter 去掉显式 _onGeneratorParamsChanged()（逐路径核对：冗余去重，非 a5a081e34 回退）
// 前（e911a1dc8）
set offset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
this._generator._renderer._onGeneratorParamsChanged(); // ← 本次删掉
}
// 后（2956d0b71）
set centerOffset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
}这是冗余去重，非正确性回退，逐路径核对（不只信 commit message，核 copyFrom 副作用 + 构造期 wiring + 幂等 dirty + git 历史透镜）：
-
copyFrom自己就 fire dirty ——Vector3.copyFrom（packages/math/src/Vector3.ts:556-562）末行this._onValueChanged?.()。构造期 wiringthis._offset._onValueChanged = () => this._generator._renderer._onGeneratorParamsChanged()（:266，本 PR 保留）把_offset的值变化路由到 dirty。故 reassignment 路径（centerOffset = new Vector3(...)，value !== offset→copyFrom→_onValueChanged→ dirty）仍然脏 bounds——经 callback，非经被删的显式调用。✅ -
被删的显式调用是双派发的第二源 —— 旧 setter 在 reassignment 时 dirty 两次（
copyFrom→callback + 显式调用）。_onGeneratorParamsChanged（ParticleRenderer.ts:306-308）是纯幂等|=三 dirty bit，故双派发无害、去掉一个无行为变化。唯一差异：旧 setter 在value === offset（同实例自赋）时仍显式 dirty，新 setter 不 dirty——但同实例自赋值本就未变，不 dirty 是正确的（旧的是在 no-op 上过度 fire）。✅ -
git 历史透镜——不是回退
a5a081e34—— 我核了a5a081e34 fix(particle): dirty bounds when velocity offset changes：它只加了两样东西——(a) 构造期 wiring_offset._onValueChanged = ...（本 PR 保留:266）+ (b) "offset component changes dirty bounds after clone" 测试（本 PR 保留并重命名 为 centerOffset）。a5a081e34从未触碰 setter 的显式调用（那行 pre-date 该 fix）。故删它不撤销a5a081e34——该 fix 的真实机制（constructor wiring + 回归测试）完整保留。无矛盾。✅ -
in-place 路径（历史 P1 关注点）仍被测试守护 —— 新测试
centerOffset component changes dirty bounds after clone（VelocityOverLifetime.test.ts:246）做centerOffset.x = 1（set x→_onValueChanged）+centerOffset.set(2,0,0)（.set→_onValueChanged）在 cloned module 上，断言_isContainDirtyFlag(GeneratorVolume) === true。这条走的正是_offset._onValueChanged构造期 wiring 路径（非被删的 setter 显式调用），revert wiring 即红——a5a081e34的回归网原封守住。✅
注：setter reassignment 的 dirty 行为无直接断言（clone preserves ... centerOffset 只断言 value 拷贝非 dirty），但如上 by construction 证明（copyFrom→_onValueChanged 是 math 包 invariant + 构造 wiring 未动），且 in-place 路径（历史 P1 真正踩过的坑）有测试。不阻塞。
✅ 改动 3 —— JSDoc + shader 注释澄清（逐条打开代码验非 aspirational）
三处新增/改写的行为描述，逐条对照代码核实全部准确，无 aspirational：
-
"Switching orbital/radial values between zero and non-zero restarts the simulation" —— 准确。orbital/radial 曲线值变化 →
_onTransformFeedbackDirty（:96，经_onOrbitalRadialChange:498注册）→_setTransformFeedback()。值跨零翻转 →_isOrbitalActive/_isRadialActive（读_isZero()）翻转 →_needTransformFeedback()结果翻转 →_setTransformFeedback（ParticleGenerator.ts:683）里needed !== _useTransformFeedback→ 调this._clearActiveParticles()重启仿真。✅ -
"LimitVelocityOverLifetime does not clamp orbital/radial motion"（JSDoc + shader 注释）—— 准确。shader（
ParticleFeedback.shader）里 LVL 块（RENDERER_LVL_MODULE_ENABLED）clamp 的是localVelocity（base+FOL/gravity）与volLocal/volWorld（linear VOL）；orbital/radial 块（_VOL_ORBITAL_RADIAL_MODULE_ENABLED，另起于:211）用未经 LVL clamp 的volLocal/volWorld重算linearVelocity，orbital 旋转（rotationByEuler）+ radial 生长对rel做位置积分，完全在 LVL 之外。故 orbital/radial motion 确实不被 LVL clamp。✅ -
centerOffset "in world simulation space, this local offset is transformed by the simulation transform captured when each particle is born" —— 准确。shader
:237（world 空间分支）renderer_VOLOffset经worldRotation+attr.a_SimulationWorldPosition（每粒子出生帧仿真变换，烘进 attribute）变换。✅
JSDoc 格式合规：多行 /** */ 块、@remarks block tag、句尾句号。setter 不重复 JSDoc（getter 覆盖 accessor pair，符合项目惯例）。
✅ 改动 4 —— 测试搬迁（逐条核对：搬对位置 + 回归网无丢失）
8 个 velocity over lifetime orbital/radial ... 测试从 LimitVelocityOverLifetime.test.ts（它们本就误放在这里）搬到新专属 VelocityOverLifetime.test.ts。对照 base vs tip 的 it() 标题清单：LVL 测试文件精确剩下 21 个真 LVL 测试（速度/dampen/drag/space 等），8 个 orbital/radial 全部搬走且都在新文件出现（含关键的 clone preserves orbital/radial/centerOffset + centerOffset component changes dirty bounds after clone）。搬迁同步改了 offset → centerOffset。是合理的测试归位（orbital/radial 属 VOL 概念，不属 LVL），非覆盖丢失。✅
CI
tip 2956d0b71 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡；本次重命名offset→centerOffset使「静态中心点」语义更清晰，部分收敛该讨论，但类型本身未变。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ [本轮相关] offset in-place 分量改脏 bounds ——
a5a081e34的构造期 wiring + 回归测试本 PR 完整保留并重命名为 centerOffset，本次删 setter 显式调用不撤销该 fix。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——8bcfc647c/1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max——5a09858e。
本 commit 是一次干净的 pre-release API 清晰化：offset→centerOffset 重命名（净新增特性，无外部消费者，全 in-PR 调用点已迁移）+ setter 去冗余显式 dirty（copyFrom→_onValueChanged 已覆盖，非 a5a081e34 回退——该 fix 的构造 wiring + 回归测试完整保留）+ 三处行为描述澄清（逐条对照 shader/TS 验准确非 aspirational）+ 误放的 orbital/radial 测试归位到专属 VOL 测试文件（回归网无丢失）。逐路径核对全部正确，git 历史透镜确认无矛盾，CI 4/4 全绿。无 P0/P1/新问题。LGTM。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 e911a1dc8 线性推进一个 commit 到 2956d0b71（gh api compare e911a1dc8...2956d0b71 返回 status: ahead, ahead_by: 1, behind_by: 0；2956d0b71 父 = e911a1dc8bc7b7b491f36630679c24f75a6748ef，确认非 force-push 回退）。唯一改动是 refactor(particle): clarify orbital radial API，7 文件：VelocityOverLifetimeModule.ts（+14/-8）+ ParticleGenerator.ts（+10/-10）+ ParticleFeedback.shader（+2/-1）+ e2e case（+1/-1）+ ParticleBoundingBox.test.ts（+1/-1）+ 测试搬迁（LimitVelocityOverLifetime.test.ts -222 → 新 VelocityOverLifetime.test.ts +267）。四类改动：① API 重命名 offset → centerOffset（含 setter/getter + 全部 in-PR 调用点）② setter 去掉冗余的显式 _onGeneratorParamsChanged() ③ 三处 JSDoc + shader 注释澄清 ④ 把误放在 LVL 测试文件里的 orbital/radial 测试搬到专属 VOL 测试文件。逐路径核对全部正确，历史 a5a081e34 回归网完整保留并重命名，CI 4/4 全绿。无 P0/P1/新问题。--comment LGTM。
✅ 改动 1 —— offset → centerOffset 重命名（逐路径核对：pre-release API 无外部消费者 + 全 in-PR 调用点已迁移）
gh api .../VelocityOverLifetimeModule.ts?ref=dev/2.0 确认：offset/orbital*/radial/centerOffset getter 在 base dev/2.0 零存在——整个 orbital/radial VOL 特性是本 PR（feat/particle-vol-orbital-radial）净新增、从未发版。故 offset → centerOffset 是 pre-release API 清晰化，非 breaking change，无外部消费者可破坏。全部 in-PR 调用点（e2e case :93 + ParticleBoundingBox.test.ts:112 + 新测试）本 commit 同步改完，CI 4/4 e2e 全绿反向证伪重命名未漏调用点（漏则编译失败）。命名本身是净改进——centerOffset 显式表达「相对系统原点的中心偏移」，比裸 offset 少歧义。✅
✅ 改动 2 —— setter 去掉显式 _onGeneratorParamsChanged()（逐路径核对：冗余去重，非 a5a081e34 回退）
// 前（e911a1dc8）
set offset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
this._generator._renderer._onGeneratorParamsChanged(); // ← 本次删掉
}
// 后（2956d0b71）
set centerOffset(value: Vector3) {
const offset = this._offset;
if (value !== offset) { offset.copyFrom(value); }
}这是冗余去重，非正确性回退，逐路径核对（不只信 commit message，核 copyFrom 副作用 + 构造期 wiring + 幂等 dirty + git 历史透镜）：
-
copyFrom自己就 fire dirty ——Vector3.copyFrom（packages/math/src/Vector3.ts:556-562）末行this._onValueChanged?.()。构造期 wiringthis._offset._onValueChanged = () => this._generator._renderer._onGeneratorParamsChanged()（:266，本 PR 保留）把_offset的值变化路由到 dirty。故 reassignment 路径（centerOffset = new Vector3(...)，value !== offset→copyFrom→_onValueChanged→ dirty）仍然脏 bounds——经 callback，非经被删的显式调用。✅ -
被删的显式调用是双派发的第二源 —— 旧 setter 在 reassignment 时 dirty 两次（
copyFrom→callback + 显式调用）。_onGeneratorParamsChanged（ParticleRenderer.ts:306-308）是纯幂等|=三 dirty bit，故双派发无害、去掉一个无行为变化。唯一差异：旧 setter 在value === offset（同实例自赋）时仍显式 dirty，新 setter 不 dirty——但同实例自赋值本就未变，不 dirty 是正确的（旧的是在 no-op 上过度 fire）。✅ -
git 历史透镜——不是回退
a5a081e34—— 我核了a5a081e34 fix(particle): dirty bounds when velocity offset changes：它只加了两样东西——(a) 构造期 wiring_offset._onValueChanged = ...（本 PR 保留:266）+ (b) "offset component changes dirty bounds after clone" 测试（本 PR 保留并重命名 为 centerOffset）。a5a081e34从未触碰 setter 的显式调用（那行 pre-date 该 fix）。故删它不撤销a5a081e34——该 fix 的真实机制（constructor wiring + 回归测试）完整保留。无矛盾。✅ -
in-place 路径（历史 P1 关注点）仍被测试守护 —— 新测试
centerOffset component changes dirty bounds after clone（VelocityOverLifetime.test.ts:246）做centerOffset.x = 1（set x→_onValueChanged）+centerOffset.set(2,0,0)（.set→_onValueChanged）在 cloned module 上，断言_isContainDirtyFlag(GeneratorVolume) === true。这条走的正是_offset._onValueChanged构造期 wiring 路径（非被删的 setter 显式调用），revert wiring 即红——a5a081e34的回归网原封守住。✅
注：setter reassignment 的 dirty 行为无直接断言（clone preserves ... centerOffset 只断言 value 拷贝非 dirty），但如上 by construction 证明（copyFrom→_onValueChanged 是 math 包 invariant + 构造 wiring 未动），且 in-place 路径（历史 P1 真正踩过的坑）有测试。不阻塞。
✅ 改动 3 —— JSDoc + shader 注释澄清（逐条打开代码验非 aspirational）
三处新增/改写的行为描述，逐条对照代码核实全部准确，无 aspirational：
-
"Switching orbital/radial values between zero and non-zero restarts the simulation" —— 准确。orbital/radial 曲线值变化 →
_onTransformFeedbackDirty（:96，经_onOrbitalRadialChange:498注册）→_setTransformFeedback()。值跨零翻转 →_isOrbitalActive/_isRadialActive（读_isZero()）翻转 →_needTransformFeedback()结果翻转 →_setTransformFeedback（ParticleGenerator.ts:683）里needed !== _useTransformFeedback→ 调this._clearActiveParticles()重启仿真。✅ -
"LimitVelocityOverLifetime does not clamp orbital/radial motion"（JSDoc + shader 注释）—— 准确。shader（
ParticleFeedback.shader）里 LVL 块（RENDERER_LVL_MODULE_ENABLED）clamp 的是localVelocity（base+FOL/gravity）与volLocal/volWorld（linear VOL）；orbital/radial 块（_VOL_ORBITAL_RADIAL_MODULE_ENABLED，另起于:211）用未经 LVL clamp 的volLocal/volWorld重算linearVelocity，orbital 旋转（rotationByEuler）+ radial 生长对rel做位置积分，完全在 LVL 之外。故 orbital/radial motion 确实不被 LVL clamp。✅ -
centerOffset "in world simulation space, this local offset is transformed by the simulation transform captured when each particle is born" —— 准确。shader
:237（world 空间分支）renderer_VOLOffset经worldRotation+attr.a_SimulationWorldPosition（每粒子出生帧仿真变换，烘进 attribute）变换。✅
JSDoc 格式合规：多行 /** */ 块、@remarks block tag、句尾句号。setter 不重复 JSDoc（getter 覆盖 accessor pair，符合项目惯例）。
✅ 改动 4 —— 测试搬迁（逐条核对：搬对位置 + 回归网无丢失）
8 个 velocity over lifetime orbital/radial ... 测试从 LimitVelocityOverLifetime.test.ts（它们本就误放在这里）搬到新专属 VelocityOverLifetime.test.ts。对照 base vs tip 的 it() 标题清单：LVL 测试文件精确剩下 21 个真 LVL 测试（速度/dampen/drag/space 等），8 个 orbital/radial 全部搬走且都在新文件出现（含关键的 clone preserves orbital/radial/centerOffset + centerOffset component changes dirty bounds after clone）。搬迁同步改了 offset → centerOffset。是合理的测试归位（orbital/radial 属 VOL 概念，不属 LVL），非覆盖丢失。✅
CI
tip 2956d0b71 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡；本次重命名offset→centerOffset使「静态中心点」语义更清晰，部分收敛该讨论，但类型本身未变。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ [本轮相关] offset in-place 分量改脏 bounds ——
a5a081e34的构造期 wiring + 回归测试本 PR 完整保留并重命名为 centerOffset，本次删 setter 显式调用不撤销该 fix。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——8bcfc647c/1395f995d。 - ✅ orbital
reach未并入 world VOL/FOL + gravity ——fcf37172b。 - ✅ feedback↔vert
reldesync ——73524570b。 - ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 ——
65cc91bc。 - ✅
ParticleFeedback.glsl孤儿死代码 ——4e2d79f1。 - ✅ TwoConstants/TwoCurves 静默 max-only ——
28f8b29dc。 - ✅ orbital 三轴混搭假抽象 ——
3277c2e57。 - ✅ orbital/radial property 缺
Max——5a09858e。
本 commit 是一次干净的 pre-release API 清晰化：offset→centerOffset 重命名（净新增特性，无外部消费者，全 in-PR 调用点已迁移）+ setter 去冗余显式 dirty（copyFrom→_onValueChanged 已覆盖，非 a5a081e34 回退——该 fix 的构造 wiring + 回归测试完整保留）+ 三处行为描述澄清（逐条对照 shader/TS 验准确非 aspirational）+ 误放的 orbital/radial 测试归位到专属 VOL 测试文件（回归网无丢失）。逐路径核对全部正确，git 历史透镜确认无矛盾，CI 4/4 全绿。无 P0/P1/新问题。LGTM。
cptbtptpbcptdtptp
left a comment
cptbtptpbcptdtptp
left a comment
There was a problem hiding this comment.
Reviewed the three commits after 829b679. Module build/shader precompile and all 14 particle test files (162/162) pass. One uncovered runtime crash is reproducible on the current head, plus one correction to the new public API remarks.
| ); | ||
| shaderData.setFloatArray( | ||
| VelocityOverLifetimeModule._orbitalMaxCurveYProperty, | ||
| orbitalY.curveMax._getTypeArray() |
There was a problem hiding this comment.
[P1 — runtime crash, reproduced on current head] The unauthored-curve fix covers the all-zero case, but this upload path still assumes every companion axis owns a curve object once the orbital group is active. A natural partial-authoring state crashes:
vol.enabled = true;
vol.orbitalX = new ParticleCompositeCurve(
new ParticleCurve(new CurveKey(0, 1), new CurveKey(1, 2))
);
vol.orbitalY.mode = ParticleCurveMode.Curve; // curveMax is undefined
vol.orbitalZ.mode = ParticleCurveMode.Curve; // curveMax is undefined
generator._updateShaderData(shaderData); // TypeError: undefined._getTypeArray_isOrbitalActive() is true because X is non-zero, and all three modes select this curve branch, so Y throws here. The same issue applies to missing curveMin/curveMax in TwoCurves; an explicitly empty companion ParticleCurve also uploads an invalid all-zero key array whose shader evaluator has no matching segment after age 0.
The submitted regression test only makes all three axes unauthored, so the group remains inactive and never reaches this block. Please upload a valid zero curve (for example keys at 0 and 1 with value 0) for missing/empty companion axes, including both min and max arrays.
| /** | ||
| * Orbital velocity (radians/second) around the x axis of the system. | ||
| * @remarks Requires WebGL2 and transform feedback. Switching orbital/radial values between zero and non-zero | ||
| * restarts the simulation. LimitVelocityOverLifetime does not clamp orbital/radial motion. |
There was a problem hiding this comment.
[Docs — correction to my earlier slider note] This wording is broader than the implementation. _setTransformFeedback() clears particles only when the generator aggregate _useTransformFeedback value toggles. If orbitalX is already active, changing radial from zero to non-zero leaves TF enabled and returns before _clearActiveParticles(); likewise, if Noise/LVL already keeps TF enabled, activating the first orbital value does not restart anything.
Suggested wording: “Changing orbital/radial values may clear active particles when the change toggles the generator between transform-feedback and non-transform-feedback modes.” The same correction applies to the repeated remarks on Y, Z, and radial.
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 2956d0b71 线性推进一个 commit 到 682154818（gh api compare 2956d0b71...682154818 返回 status: ahead, ahead_by: 1, behind_by: 0；682154818 父 = 2956d0b71，确认非 force-push 回退）。唯一改动是 docs(particle): clarify transform feedback restart——VelocityOverLifetimeModule.ts 单文件 +11/-7，纯 JSDoc @remarks 改写，4 个 getter（orbitalX/Y/Z + radial）里把「Switching orbital/radial values between zero and non-zero restarts the simulation」改写为「Changing orbital/radial values may clear active particles when the change toggles the generator between transform-feedback and non-transform-feedback modes」。零产物/逻辑改动。逐条对照代码：这是一次准确性修正——把旧措辞的过度断言收紧到真实触发条件。CI 全绿。无 P0/P1/新问题。--comment LGTM。
✅ 本轮唯一改动 —— orbital/radial @remarks 改写（逐路径核对：把过度断言收紧到真实条件，是净改进非漂移）
我上轮把旧措辞标为「准确」，本轮把它改得更准确——逐路径 trace 机制核实新措辞是真实条件，旧措辞是 over-claim：
触发链路（端到端 trace）：
- orbital/radial 曲线值变化 →
_onValueChanged→_onTransformFeedbackDirty（VelocityOverLifetimeModule.ts:96，经_onOrbitalRadialChange:499-503注册/转发）→_setTransformFeedback()。 _setTransformFeedback（ParticleGenerator.ts:683-694）：needed = isWebGL2 && (limit || noise || VOL._needTransformFeedback() || deathSubEmitter)；:691if (needed === this._useTransformFeedback) return;早返回——TF 模式未翻转时直接 return，不 clear；只有needed !== _useTransformFeedback（TF 模式真翻转）才:694 _clearActiveParticles()。
新旧措辞对照：
- 旧措辞（"switching zero↔non-zero restarts"）是 over-claim —— 它断言 orbital/radial 跨零翻转必然 restart。但若 noise 或 limitVelocity 同时启用，
_useTransformFeedback恒true，orbital/radial 跨零翻转时needed === _useTransformFeedback（都是 true）→:691早返回 → 不 clear。故旧措辞在「其他 TF 源已开启」时是错的。 - 新措辞（"may clear ... when the change toggles TF/non-TF modes"）准确 ——
may+ 明确条件「toggles TF/non-TF mode」，恰好对应needed !== _useTransformFeedback这个真实触发闸门（跨零翻转但被 noise/limit 保持 TF 开启时不 toggle → 不 clear）。逐字命中代码语义。✅
这是由更优措辞 supersede 旧结论（我上轮判「旧措辞准确」是近似判断，新措辞严格更准确），非自我矛盾、非静默回退——docs 改写方向正确。
格式合规：4 处 JSDoc 均为多行 /** */ 块 + @remarks block tag + 句尾句号（motion.），符合项目 TSDoc 惯例；radial getter 保留 The center is given by \centerOffset`.` 引用（上轮已核准确，本轮未动）。4 处措辞完全一致，无漂移。
CI
tip 682154818 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。docs-only 改动零产物，CI 绿符合预期。
注释合规
本 commit 是 4 处 JSDoc @remarks 改写：多行块 ✓、@remarks block tag ✓、句尾句号 ✓、非 aspirational（逐条 trace _setTransformFeedback:691 早返回门 + _needTransformFeedback 核实新措辞与代码语义一致）✓、非复述代码（解释「何时会 clear active particles」这一 why，非复述实现）✓。docs(particle): clarify transform feedback restart commit message 准确。无新增违规。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 是纯 docs，未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ [本轮相关] orbital/radial restart 语义文档 ——
682154818把过度断言收紧到真实 TF-mode-toggle 条件，逐路径 trace_setTransformFeedback:691早返回门核实，已闭环。 - ✅
offset→centerOffset重命名 + setter 去冗余显式 dirty ——2956d0b71。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——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 是一次纯 docs 准确性修正：把 4 个 orbital/radial getter 的 @remarks 从「跨零翻转必然 restart」的过度断言收紧为「may clear active particles 当改动 toggle TF/non-TF 模式时」的真实条件。逐路径 trace _onOrbitalRadialChange→_setTransformFeedback→:691 needed===_useTransformFeedback 早返回门核实：只有 TF 模式真翻转（跨零翻转且无 noise/limit 保持 TF 开启）才 _clearActiveParticles，新措辞与此逐字对应，旧措辞在「其他 TF 源已开启」时是 over-claim。这是由更优措辞 supersede 我上轮的近似判断，非自我矛盾。格式合规、CI 4/4 全绿、零产物改动。无 P0/P1/新问题。LGTM。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 682154818 线性推进两个 commit 到 6309efa61（gh api compare 682154818...6309efa61 返回 status: ahead, ahead_by: 2, behind_by: 0；6309efa61 父 = 1c84911be，1c84911be 父 = 682154818，逐级确认非 force-push 回退）。两个都是纯 docs commit，仅动 2 文件、净 +4/-14：
1c84911be docs(particle): simplify orbital radial requirements——VelocityOverLifetimeModule.ts，把 4 个 getter（orbitalX/Y/Z+radial）的@remarks从三句话（Requires WebGL2 and transform feedback / 跨 TF-mode toggle 可能 clear active particles / LimitVelocityOverLifetime 不 clamp orbital/radial）收缩为一句Requires WebGL2.。6309efa61 docs(particle): describe orbital center semantics——VelocityOverLifetime.glsl，把renderer_VOLOffset上的注释从// Orbital / Radial (transform-feedback only). Center of orbit in system-local space.改写为// Center of orbital/radial motion in system-local space.。
零产物/逻辑改动，CI 全绿。无 P0/P1/新问题。仅一条 P3 观察（下）。--comment LGTM。
✅ 逐条核对 —— docs 简化，删除的是准确但冗长的内容（非 stale-doc，也非静默回退）
我先核实了被删的三段描述在当前代码里仍然为真（判断这是"删对的准确文档"还是"删掉的正确文档"）：
- "跨 TF-mode toggle 可能 clear active particles" —— 仍准确。orbital/radial setter →
_onOrbitalRadialChange→_onTransformFeedbackDirty→_setTransformFeedback（ParticleGenerator.ts:683-694），:691 if (needed === this._useTransformFeedback) return;早返回，只有 TF 模式真翻转才:694 _clearActiveParticles()。行为未变，删的是准确描述。 - "LimitVelocityOverLifetime does not clamp orbital/radial motion" —— 仍准确（上轮已核 shader 里 orbital/radial 块独立于 LVL clamp 块，本轮源码未动）。
- "and transform feedback" → 只留 "Requires WebGL2" —— 合理去实现细节：WebGL2 是用户可操作的前置条件，transform feedback 是内部机制。
所以这不是 stale-doc 清理，删的是准确的行为文档。但也不是静默回退——我上轮（682154818）夸的是这段措辞准确（把 over-claim "restarts" 收紧到真实条件），并未主张它必须保留；本轮 commit message 明说 simplify，是透明的编辑取舍，非撤销 fix:。删除准确但冗长的 JSDoc 是站得住的编辑决策，不构成矛盾。
格式合规：4 处 getter 保留多行 /** */ 块 + @remarks block tag + 主描述句尾句号；centerOffset 的 world-simulation-space 解释未动（上轮已核准确）。API 仍保留主描述 + 可操作前置条件（Requires WebGL2）。
shader 注释改写：新旧均准确，renderer_VOLOffset 在 _VOL_ORBITAL_RADIAL_MODULE_ENABLED（TF-only）guard 内，guard 名本身已表达模块门控，删掉冗余的 (transform-feedback only) 后仍解释了字段是什么（orbital/radial 运动中心 / system-local 空间）。非 aspirational、非复述代码。
问题（非阻塞）
[P3] VelocityOverLifetimeModule.ts:145/161/177/193 — 删掉的"TF-mode toggle 会 clear active particles"是真实的用户可观测 gotcha，建议在别处保留一处
被删的这句描述的是一个非平凡、用户可观测的副作用：运行时把 orbital/radial 从 0 调到非零（若恰好翻转 TF 模式）会让所有活跃粒子瞬间清空重启。这类"我拖动这个 slider 粒子怎么全没了/闪了一下"的意外，正是文档存在的意义。同理 "LVL 不 clamp orbital/radial" 是一条跨模块交互意外。
不阻塞——本轮把它从 4 个 getter 各重复一遍收敛掉，本身是合理的去重（避免 4 处同文漂移）。但建议这个 gotcha 别彻底消失：更自然的落点是 orbital/radial 的 setter（副作用发生处）或模块级 JSDoc 各写一处，而非在 4 个 getter 里重复。属编辑取舍，作者按产品文档策略定即可。
CI
tip 6309efa61 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。docs-only 零产物，绿符合预期。
注释合规
两个 commit 均为注释/JSDoc 改动：4 处 getter @remarks 保持多行块 + block tag + 句尾句号 ✓；shader 单行 // 注释无句尾句号 ✓、非 aspirational（逐条核实删的内容在代码里仍为真）✓、非复述代码 ✓。两条 commit message（simplify orbital radial requirements / describe orbital center semantics）准确。无新增违规。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本轮纯 docs，未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。
已闭环（保持，不重复）
- ✅ orbital/radial restart 语义文档 ——
682154818收紧到 TF-mode-toggle 条件；本轮1c84911be将其连同 LVL-clamp 说明一并简化为Requires WebGL2.（准确内容的编辑取舍，见 P3）。 - ✅
offset→centerOffset重命名 + setter 去冗余显式 dirty ——2956d0b71。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——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。
本轮两个纯 docs commit：① 把 4 个 orbital/radial getter 的 @remarks 从三句（WebGL2+TF / TF-mode-toggle 清空粒子 / LVL 不 clamp）简化为一句 Requires WebGL2.；② 把 shader renderer_VOLOffset 注释改写得更简洁。逐条核实被删内容在代码里仍为真（_setTransformFeedback:691 早返回门 + LVL 独立块），故删的是准确但冗长的文档——这是站得住的编辑取舍、非静默回退（我上轮夸的是措辞"准确"非"必须保留"，commit message 明说 simplify）。唯一 P3：删掉的"TF-mode toggle 清空活跃粒子"是真实用户 gotcha，建议在 setter 或模块级保留一处。格式合规、CI 4/4 全绿、零产物。无 P0/P1/新问题。LGTM。
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 PR tip 从我上次审查的 6309efa61 线性推进一个 commit 到 2d94f638（gh api compare 6309efa61...2d94f638 返回 status: ahead, ahead_by: 1, behind_by: 0；2d94f638 父 = 6309efa61，确认非 force-push 回退）。唯一改动是 docs(particle): simplify center offset description——VelocityOverLifetimeModule.ts 单文件 +1/-3，纯 JSDoc 改写：把 centerOffset getter 的文档从「主描述（… in the system's local space.）+ 三行 @remarks（world simulation space 下 offset 经每粒子出生帧仿真变换，故移动发射器的粒子绕各自出生帧中心公转）」收缩为一句主描述「The center offset of orbital/radial motion from the particle system origin.」。零产物/逻辑改动，CI 全绿。无 P0/P1/新问题。--comment LGTM。
✅ 逐条核对 —— 删的是准确但冗长的内容（编辑取舍，非 stale-doc，也非静默回退）
我先核实被删的 @remarks（birth-frame center transform 语义）在当前 shader 里仍然为真（判断是「删对的准确文档」还是「删掉的正确文档」）：
- world-space 分支（
ParticleFeedback.shader:236，renderer_SimulationSpace != 0）：rel = rotationByQuaternions(position − attr.a_SimulationWorldPosition, invWorldRotation) − renderer_VOLOffset，随后position = attr.a_SimulationWorldPosition + rotationByQuaternions(renderer_VOLOffset + rel, worldRotation)。a_SimulationWorldPosition+invWorldRotation正是每粒子出生帧的仿真变换（烘进 attribute），故 offset 确实在每粒子出生帧空间应用 → 移动发射器的粒子绕各自出生帧中心公转。被删的@remarks逐字命中此语义，删的是准确描述。 - 主描述改写（
… in the system's local space.→… from the particle system origin.）：renderer_VOLOffset=_offset（Vector3，3 个 local 分量，:414经setVector3直接上传），故「local space」限定词是准确的，新描述去掉了空间限定——是轻微精度损失但非错误（「offset from origin」隐含 local 语义），属合理去实现细节的编辑取舍。
这不是静默回退：我在 2956d0b71 轮夸的是这段 @remarks 措辞准确、非 aspirational，并未主张它必须保留；本轮 commit message 明说 simplify，是透明的编辑取舍。删除准确但冗长的 JSDoc 是站得住的编辑决策——与我 1c84911be 轮接受的 simplify orbital radial requirements 同一模式（那轮把三句 @remarks 简化为 Requires WebGL2.）。不构成自我矛盾。
格式合规：仍是多行 /** */ 块 + 主描述句尾句号（origin.）✓，符合项目 TSDoc 惯例；setter 不重复 JSDoc（getter 覆盖 accessor pair，符合项目惯例）✓。
CI
tip 2d94f638 全绿：build(3 OS: macos/ubuntu/windows)/e2e(4/4 全 success)/codecov(project + patch 均 success)/lint/labeler。docs-only 零产物，绿符合预期。
注释合规
本 commit 是单处 getter JSDoc 收缩：多行块 ✓、句尾句号 ✓、非 aspirational（逐路径核实被删的 birth-frame 语义在 ParticleFeedback.shader:236 world-space 分支仍为真）✓、非复述代码（描述「centerOffset 是什么」而非复述实现）✓。docs(particle): simplify center offset description commit message 准确。无新增违规。
剩余开放项（均非阻塞，历轮已记，本轮未触及，不重复展开）
- [P2] stretched-billboard vert 路径无测试覆盖 —— 本轮纯 docs，未触及 vert
rel/视觉速度重建逻辑。 - [P3]
centerOffset类型选型（静态Vector3vs 曲线）—— 产品需求权衡。 - [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
- [P3]
ParticleGenerator._setTransformFeedback删掉的 why 注释建议保留。 - [P3] 删掉的行为 gotcha（TF-mode toggle 清空活跃粒子 / birth-frame 公转中心）建议在别处保留一处 —— 本轮的 birth-frame 语义与
1c84911be轮的 TF-toggle gotcha 属同类「删了准确的用户可观测行为文档」，硬去重不重复展开；属编辑取舍，作者按产品文档策略定。
已闭环（保持，不重复）
- ✅ centerOffset birth-frame 语义文档 ——
2d94f638将其连同 local-space 限定一并简化为一句主描述（准确内容的编辑取舍，见剩余开放项 P3）。 - ✅ orbital/radial restart 语义文档 ——
682154818收紧到 TF-mode-toggle 条件；1c84911be简化为Requires WebGL2.。 - ✅
offset→centerOffset重命名 + setter 去冗余显式 dirty ——2956d0b71。 - ✅ stretched-billboard + linear-VOL 丢速度 ——
e911a1dc8。 - ✅
_VOL_ORBITAL_RADIAL_MODULE_ENABLEDguard 收口 ——829b679d。 - ✅
??0空曲线崩溃 + NaN-bounds /_tempVector33aliasing ——8bcfc647c。 - ✅
evaluateVOLVelocitylinear VOL evaluator ——d5f09e951。 - ✅ feedback shader
getVOL*去重 ——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。
本轮一个纯 docs commit：把 centerOffset getter 从「主描述 + 三行 birth-frame @remarks」收缩为一句主描述。逐路径核实被删的 birth-frame 公转语义在 ParticleFeedback.shader:236 world-space 分支仍为真、local-space 限定词准确，故删的是准确但冗长的文档——这是站得住的编辑取舍、非静默回退（我上轮夸的是措辞「准确」非「必须保留」，commit message 明说 simplify，与 1c84911be 同一模式）。格式合规、CI 4/4 全绿、零产物。无 P0/P1/新问题。LGTM。
…fault Upstream galacean#3049 guaranteed orbital curve/offset cloning with @deepClone; the merge dropped those decorators as redundant under type-driven Deep. Pin the equivalence with identity + value assertions so a regression in the type default surfaces here instead of in particle rendering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
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.