Skip to content

feat(particle): add orbital radial velocity over lifetime#3049

Open
hhhhkrx wants to merge 30 commits into
galacean:dev/2.0from
hhhhkrx:feat/particle-vol-orbital-radial
Open

feat(particle): add orbital radial velocity over lifetime#3049
hhhhkrx wants to merge 30 commits into
galacean:dev/2.0from
hhhhkrx:feat/particle-vol-orbital-radial

Conversation

@hhhhkrx

@hhhhkrx hhhhkrx commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds orbital and radial support to the Particle Velocity over Lifetime module, including transform-feedback simulation for position-dependent velocity terms.

Details

  • Adds orbitalX, orbitalY, orbitalZ, radial, and offset properties to VelocityOverLifetimeModule.
  • Enables transform feedback when orbital or radial velocity is active, because these terms depend on the particle's current position.
  • Uploads constant and curve shader data for orbital/radial modes and keeps the Velocity over Lifetime macro split so linear-only paths remain guarded separately.
  • Applies orbital/radial in the transform-feedback shader by integrating base velocity first, then rotating/growing the relative position around the configured offset.
  • Expands particle bounds conservatively when orbital/radial terms are active.
  • Adds unit coverage for property defaults, active detection, TF switching, shader data upload, curve/constant switching, clone behavior, and disabled-module behavior.
  • Adds an e2e case with fixed random seed and Unity-like Velocity over Lifetime settings.

Validation

  • vitest tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts --run
  • node packages/shader-compiler/bundler/cli.js packages/shader/src/Shaders packages/shader/compiledShaders --clean --emit-index --emit-sources
  • BUILD_TYPE=MODULE NODE_ENV=release rollup -c
  • BUILD_TYPE=UMD NODE_ENV=release rollup -c
  • Local Playwright visual check for particleRenderer-velocity-orbital-constant completed without page errors.

Summary by CodeRabbit

  • New Features
    • Added orbital (X/Y/Z) and radial velocity support to particle velocity-over-lifetime, including an orbit-center offset.
    • Added WebGL2 transform-feedback rendering for both constant and curve-based orbital/radial modes.
  • Bug Fixes
    • Improved transformed-bounds calculations for orbital/radial motion to better account for the orbit-center offset.
  • Tests
    • Expanded unit tests covering activation, transform-feedback requirements, shader macro/uniform selection, and cloning behavior.
    • Added an E2E particle case (velocityOrbitalConstant) with screenshot verification and updated E2E configuration.

@hhhhkrx hhhhkrx requested a review from GuoLei1990 June 24, 2026 03:43
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds orbital and radial particle velocity support, updates transform-feedback and bounds handling, and extends unit and E2E coverage for the new particle behavior.

Changes

Orbital and radial velocity over lifetime

Layer / File(s) Summary
Module state and shader upload
packages/core/src/particle/modules/VelocityOverLifetimeModule.ts
Adds orbital and radial properties, default initialization, transform-feedback dirty handling, and shader uploads for orbital/radial modes, uniforms, and macros.
Generator transform feedback and bounds
packages/core/src/particle/ParticleGenerator.ts
Updates transform-feedback gating, random velocity setup, and transformed bounds expansion for orbital offset and radial reach.
Module tests for orbital and radial behavior
tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts
Covers defaults, active-state detection, offset copying, transform-feedback gating, shader mode selection, cloning, and dirty-bounds behavior after offset changes.
E2E orbital constant case and config
e2e/case/particleRenderer-velocity-orbital-constant.ts, e2e/config.ts
Adds a WebGL2 E2E particle case and registers it in the E2E config.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Suggested labels

enhancement, shader

Suggested reviewers

  • GuoLei1990
  • cptbtptpbcptdtptp

Poem

🐇 I hop जहां the particles twirl,
Orbiting softly in a glowing whirl.
Radial breezes guide their sway,
And a screenshot seals the display.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding orbital and radial velocity support to particle velocity over lifetime.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@hhhhkrx hhhhkrx self-assigned this Jun 24, 2026
@hhhhkrx hhhhkrx added effect Effect related functions particle labels Jun 24, 2026
@hhhhkrx hhhhkrx added this to Particle Jun 24, 2026
@hhhhkrx hhhhkrx added this to the 2.0 milestone Jun 24, 2026
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.59375% with 84 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.48%. Comparing base (c7f89db) to head (829b679).
⚠️ Report is 1 commits behind head on dev/2.0.

Files with missing lines Patch % Lines
...case/particleRenderer-velocity-orbital-constant.ts 0.00% 64 Missing and 1 partial ⚠️
packages/core/src/particle/ParticleGenerator.ts 91.34% 9 Missing ⚠️
e2e/config.ts 0.00% 6 Missing ⚠️
...ore/src/particle/modules/ParticleCompositeCurve.ts 91.48% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           dev/2.0    #3049      +/-   ##
===========================================
+ Coverage    79.36%   79.48%   +0.11%     
===========================================
  Files          903      904       +1     
  Lines       100628   101100     +472     
  Branches     11256    11353      +97     
===========================================
+ Hits         79866    80360     +494     
+ Misses       20578    20556      -22     
  Partials       184      184              
Flag Coverage Δ
unittests 79.48% <83.59%> (+0.11%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@augmentcode

augmentcode Bot commented Jun 24, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: Extends the Particle VelocityOverLifetimeModule with orbital (XYZ) and radial velocity terms that depend on particle position, and wires these terms into the WebGL2 transform-feedback simulation path.

Changes:

  • Adds orbitalX, orbitalY, orbitalZ, radial, and offset properties to velocity-over-lifetime.
  • Introduces shader macros/properties and uploads constant/curve data for orbital and radial modes.
  • Updates ParticleGenerator._setTransformFeedback() to enable TF when orbital/radial terms are active (WebGL2-only).
  • Applies conservative particle bounds expansion when orbital/radial are active to avoid culling.
  • Adds a new e2e case (particleRenderer-velocity-orbital-constant) and registers it in e2e/config.ts.
  • Adds unit tests covering defaults, active detection, TF switching, shader data upload, and clone behavior.

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 augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

/**
* The center of orbit for orbital/radial velocity, in the system's local space.
*/
get offset(): Vector3 {

@augmentcode augmentcode Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Fix This in Augment

🤖 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;

@augmentcode augmentcode Bot Jun 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts (1)

162-165: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Prefer 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

📥 Commits

Reviewing files that changed from the base of the PR and between 39b4b08 and bc6f703.

⛔ Files ignored due to path filters (5)
  • e2e/fixtures/originImage/Particle_particleRenderer-velocity-orbital-constant.jpg is excluded by !**/*.jpg
  • packages/shader/src/ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl is excluded by !**/*.glsl
  • packages/shader/src/ShaderLibrary/Particle/ParticleFeedback.glsl is excluded by !**/*.glsl
  • packages/shader/src/ShaderLibrary/Particle/ParticleVert.glsl is excluded by !**/*.glsl
  • packages/shader/src/Shaders/Effect/ParticleFeedback.shader is excluded by !**/*.shader
📒 Files selected for processing (5)
  • e2e/case/particleRenderer-velocity-orbital-constant.ts
  • e2e/config.ts
  • packages/core/src/particle/ParticleGenerator.ts
  • packages/core/src/particle/modules/VelocityOverLifetimeModule.ts
  • tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts

Comment thread packages/core/src/particle/modules/VelocityOverLifetimeModule.ts
Comment thread packages/core/src/particle/ParticleGenerator.ts Outdated
Comment thread tests/src/core/particle/VelocityOverLifetimeOrbital.test.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +206 to +208
get offset(): Vector3 {
return this._offset;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Invalidate bounds when offset is mutated in place

When callers use the usual mutable-vector pattern (velocityOverLifetime.offset.set(...) or assigning components), this getter returns the live Vector3 but no _onValueChanged handler is registered, so the setter and _onGeneratorParamsChanged() are bypassed. The shader can still read the changed offset, but the particle bounds stay based on the old orbit center and can cull live particles; other particle vector properties such as shape transforms hook _onValueChanged for this reason.

Useful? React with 👍 / 👎.

Comment on lines +1716 to +1719
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include world-space offsets in orbital bounds

When orbital velocity is combined with world-space VOL/FOL, the feedback shader converts those world-space deltas into simulation space before rotating around the offset, but this reach calculation happens before worldOffsetMin/Max are merged later. For example, an X-only world-space velocity can be swept into Z by the orbital rotation while the bounds only receive the X offset afterward, so particles can be incorrectly culled; fold the accumulated world offsets into the pre-orbit radius before computing reach.

Useful? React with 👍 / 👎.

GuoLei1990

This comment was marked as outdated.

@hhhhkrx hhhhkrx force-pushed the feat/particle-vol-orbital-radial branch 2 times, most recently from 0abcaf9 to 4d2f689 Compare June 24, 2026 04:06
@hhhhkrx hhhhkrx force-pushed the feat/particle-vol-orbital-radial branch from 4d2f689 to f65694a Compare June 24, 2026 04:07

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between bc6f703 and 0abcaf9.

⛔ Files ignored due to path filters (5)
  • e2e/fixtures/originImage/Particle_particleRenderer-velocity-orbital-constant.jpg is excluded by !**/*.jpg
  • packages/shader/src/ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl is excluded by !**/*.glsl
  • packages/shader/src/ShaderLibrary/Particle/ParticleFeedback.glsl is excluded by !**/*.glsl
  • packages/shader/src/ShaderLibrary/Particle/ParticleVert.glsl is excluded by !**/*.glsl
  • packages/shader/src/Shaders/Effect/ParticleFeedback.shader is excluded by !**/*.shader
📒 Files selected for processing (5)
  • e2e/case/particleRenderer-velocity-orbital-constant.ts
  • e2e/config.ts
  • packages/core/src/particle/ParticleGenerator.ts
  • packages/core/src/particle/modules/VelocityOverLifetimeModule.ts
  • tests/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

Comment thread packages/core/src/particle/ParticleGenerator.ts Outdated
Comment thread packages/core/src/particle/ParticleGenerator.ts Outdated
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

hhhhkrx added 2 commits June 30, 2026 11:37
…into feat/particle-vol-orbital-radial

# Conflicts:
#	packages/core/src/particle/modules/ParticleCompositeCurve.ts
GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

@GuoLei1990 GuoLei1990 force-pushed the feat/particle-vol-orbital-radial branch from 855d990 to 0530fd1 Compare June 30, 2026 06:52
GuoLei1990

This comment was marked as outdated.

@GuoLei1990

Copy link
Copy Markdown
Member

补一轮针对 orbital/radial 模拟逻辑正确性的审查(在已 approve 的功能主体之上,按"逻辑正确性 → 实现合理性 → 包围盒"三步过了一遍)。核心数学是对的,下面三条按严重度,P1/P2-bounds 同根。

[P1] gravity / FOL / drag / noise 被 orbital 一起旋转了

ParticleFeedback.shader 当前把当前帧的 baseVelocity * dt 在旋转之前就并入 position,于是被 rotationByEuler 转了:

// :315
vec3 position = attr.a_FeedbackPosition - previousLinearOffset + baseVelocity * dt;
...
rel = position - renderer_VOLOffset;          // :321  rel 里含了 baseVelocity*dt
rel = rotationByEuler(rel, getVOLOrbital(...) * dt);   // :334  ← 被转

baseVelocity = localVelocity(含 gravity + FOL + drag) + noise。问题:orbital 是"绕 offset 公转",gravity 是独立的世界力(恒定方向),当前帧的重力下落增量不应被这一帧的公转扫着转。

最有力的依据是实现自相矛盾:代码已经用 previousLinearOffset(减) / currentLinearOffset(加) 把 linear VOL 精心摘出旋转外——等于确立了"独立累加的线性位移不该被 orbital 旋转"这条原则。但同性质的 gravity/FOL/drag/noise 没享受同等待遇。是"做了一半"的不对称。

影响模拟本身(轨迹),量级 O(dt²)/帧、系统性累积,gravity+orbital 都强时可见。

修复(两行移动):把 baseVelocity * dt 从旋转前挪到旋转后,和 currentLinearOffset 并列:

// :315  删掉 + baseVelocity * dt
vec3 position = attr.a_FeedbackPosition - previousLinearOffset;

// ...orbital/radial 块 :318-341 完全不变...

// :343  加上 baseVelocity * dt
position += baseVelocity * dt + currentLinearOffset;

#else 非 orbital 分支(:344-352)、v_FeedbackVelocity(:355)、linear-offset 记账(:285-313) 都不动。

注意这是只摘当前帧增量,不是"gravity 永不转"——历史累积位移(含过去帧的 gravity)已是轨道的一部分,应该跟着公转。noise 一并摘出也对(computeNoiseVelocity 在固定空间采样,转它会 double-count)。

⚠️ 此修复会改变渲染输出velocityOrbitalConstant e2e(startSpeed=5baseVelocity≠0,orbital+radial+offset 放大累积)基线图会变,需重新生成并人眼确认新轨道。e2e 变红是真信号、非 flaky。

[P2] 包围盒 reach 偏小会 pop —— 与 P1 同根

ParticleGenerator._calculateTransformedBounds 的 orbital reach(:1709) 基于的 min/max 只含 local 位移;world-space VOL/FOL 在 min.add(worldOffsetMin)(:1727) 之后才合、gravity 在 _addGravityToBounds(:787,orbital 之后) 才加。但当前 shader 里 orbital 转的 position 含 gravity/world(即 P1)→ CPU 估的旋转半径偏小 → 剔除盒罩不住 → 边缘 pop。

包围盒可以偏大(粗),但这里偏小、漏覆盖真实粒子,是参考系不一致。

修复:包围盒代码本就按"orbital 只转 local 位移"写的(reach 在 worldOffset/gravity 合入之前算),它没写错。修 P1 后(shader orbital 只转 local base)两者重新对齐,此条基本自愈、包围盒无需改动。若不修 P1,则需把 worldOffset + gravity 幅度并入 reach(别挪 orbital 块到 transform 之后——盒子变世界空间而 offset 是局部空间会参考系错配)。

[P2] 死文件 ParticleFeedback.glsl 加了编译不到的逻辑

ShaderLibrary/Particle/ParticleFeedback.glsl 自 ShaderLab 迁移(#2961)起不被任何 .shader #include(只在 index.ts 空注册)。编译产物 compiledShaders/Effect/ParticleFeedback.shaderc 含 live 的 getVOLOrbital、不含该文件独有的 computeVOLPositionOffsetTF → 本 PR 往该死文件加的 ~164 行 orbital TF 数学 GPU 永远看不到。既有技术债(dev/2.0 上两份就在),但本 PR 放大了它。建议合并后 cleanup:删死文件 + index.ts 注册,或让 .shader 真正 include 它。非阻塞。


已确认正确、勿动:多轴 orbital 轴序(rotationByEuler = YXZ,对,只是缺一行注释);radial↔orbital 顺序相反但数学严格相等(旋转保模长);linear VOL 排除旋转 + 逐帧递推自洽;radial 中心奇点 relLen>1e-5 守卫;previousLinearOffset/currentLinearOffset 机制本身(position-based 排除 linear 的最简正确实现)。

收敛点:P1 一处两行修复同时正确 shader 模拟和包围盒一致性,建议两者绑定一起定(含重生成 e2e 基线)。除 P1/P2-bounds 外无新阻塞。

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

GuoLei1990

This comment was marked as outdated.

@GuoLei1990

Copy link
Copy Markdown
Member

复核了 65cc91bc9 / 4e2d79f14 / 73524570b 三个 commit,逐帧推演 + 对抗验证过。修法方向对,而且比我上一条建议的"整体挪 baseVelocity"更彻底、更正确——结论汇总如下。

✅ 死文件已删(4e2d79f14)

ParticleFeedback.glsl 401 行 + index.ts 注册删除,正确。

✅ orbital 速度重切分(65cc91bc9)—— 比单纯"挪 gravity 出旋转"更深刻

新模型把速度按物理来源切分,决定转不转:

orbitVelocity    = startVelocity + linearVelocity        // 粒子自身运动 → 进旋转
externalVelocity = baseVelocity - startVelocity          // gravity+FOL+drag+noise → 旋转外

这比"把 baseVelocity 整体挪到旋转后"更准确,因为它修掉的不只是"gravity 被转",还有一个更隐蔽的不对称:旧版 startSpeed 被转、linear VOL 却被 previousLinearOffset 排除在旋转外——而这两者都是粒子自身的速度贡献,没有理由区别对待。旧版那个"排除 linear VOL"其实是解析积分(computeVelocityPositionOffset cumulative)的副产物(解析绝对偏移没法穿过逐帧增量旋转,只能 subtract-before/re-add-after),不是物理决定。

新模型的划分是干净的第一性原理:粒子自身运动(startSpeed + linear VOL)随轨道公转;外部场(gravity/FOL/drag/noise)在固定仿真空间施加、不随公转。旧版恰好两个边界都反了(转了 gravity、没转 linear VOL)。删 previousLinearOffset/currentLinearOffset 递推是"把增量持久化进 position"的正确配对,帧递推自洽,无 double-count / drift。

精度上:常量模式(默认 + 当前唯一 e2e)bit-identical;仅曲线模式 linear VOL 从解析积分降为逐帧欧拉,有界 O(dt) 误差——但 orbital 旋转本就是欧拉,统一欧拉是更一致的精度预算,可接受。

三点 follow-up(非阻塞)

1. drag 对 startSpeed 的衰减被挪出了旋转桶(建议确认是否有意)
baseVelocity 携带的是 drag 衰减后的 startSpeed(LVL drag 在 :256/258 写回 localVelocity = draggedTotal - volAsLocal),但 externalVelocity = baseVelocity - startVelocityInSimulationSpace(:302) 减的是 a_DirectionTime*a_StartSpeed(:294) 这个未经 drag的原始 startSpeed。净效果:orbitVelocity满额未衰减的 startSpeed 注入旋转桶,drag 对 startSpeed 的衰减量被留在了 externalVelocity(旋转外)。总量守恒,但"drag 衰减 startSpeed"这件事的作用位置从旋转内迁到了旋转外。大概率二阶,是这个干净划分里唯一的泄漏点,值得一句注释或一个有意决定。

2. 曲线模式 linear VOL 的欧拉化建议补一行注释,说明此处刻意用瞬时速度×dt(非 cumulative),免得后人"好心修回" computeVelocityPositionOffset。注意该 helper 在 render pass ParticleVert.glsl(:119/183) 仍 live,不能全局删。

3. e2e 覆盖建议补 orbital-curve + 至少一个 radial 模式——本次 rework 最新的两个行为(被转的 linear VOL、曲线欧拉漂移)恰好都在没有基线图的模式里,常量 case 覆盖不到。


之前我提的"gravity 不该被旋转"方向对,但你这版看得更深(startSpeed/linearVOL 对称 + 删整套递推),比我建议的"整体挪 baseVelocity"更好。LGTM,三点 follow-up 不阻塞。

@GuoLei1990

Copy link
Copy Markdown
Member

把当前 shader(orbital/radial 位置积分)逐项核过、确认第一性原理正确后,回头按"包围盒 = shader 位置公式的区间算术像"重新推了一遍包围盒。下面是结论 + 可落地实现。两个 bug(一个 orbital 专属、一个 noise 通用),外加一个 shader 随机相关性问题。

第一性原理:orbital 包围盒就是绕 offset 的球

shader 里 orbital 是绕 offset保模长旋转,一生累积到任意角。所以一个粒子的可达集 = 以 offset 为心、半径 r = |pos − offset| 的球面;所有粒子的并 = 半径 R = max|pos − offset| 的实心球。球的紧 AABB 就是立方体 [offset − R, offset + R](现有代码这一步是对的)。

关键a_FeedbackPosition 累积了一生所有位移(shape + 全部 VOL/FOL + gravity + noise),它们每一项都在 base 里、都被 orbital 转。所以 R 必须包含每一项:

R = sqrt(dx² + dy² + dz²) + radialReach
dx/dy/dz = (local 项 shape+localVOL+localFOL 到 offset 的距离) + W   // W = world 项的幅度
W_a      = |worldVOL|_a·L + |worldFOL|_a·½L² + |gravity·gravMod|_a·½L² + |noise|_a·L   (L=maxLifetime)
radialReach = |radial|_max·L

[P1] 包围盒 reach 漏了 world VOL/FOL + gravity + noise —— orbital 时偏小、粒子被剔除 pop

_calculateTransformedBoundsParticleGenerator.ts:1706-1709)算 reach 时,min/max 只含 shape + localVOL + localFOL。而 world VOL/FOL(:1727 才 min.add(worldOffsetMin))、noise(:1742)、gravity(_addGravityToBounds 在 :787 整个函数之后)全在 orbital 块之后才折进盒子。但 shader 里这些项都在被旋转的 base 里 → 现有 reach 用的盒子比真实小 → 球偏小 → orbital 把粒子转到盒外 → 剔除盒罩不住,边缘 pop

数值验证(积分 shader 逐帧公式,对比真实 max|pos−offset| vs 现有 R):

配置 真实 现有 R 正确 R
gravity (0,−9.8,0) 5s + orbital 49 0.01 122.5
worldVOL=(10,0,0) + entity 转90° + orbital 19.95 0 30
noise strength=2 + orbital 8.23 0.01 10.4

只要 gravity/worldVOL/worldFOL/noise 任一非零 + orbital active,现有包围盒就非保守。

两个角色必须分开(这是数学,不是取舍)

  • 进 R(球半径):world 项用幅度 |v|。因为 R 量的是距离(标量),旋转保模长 → world 向量转到任何方向,对"离 offset 距离"的贡献就是 |v|,无需逆矩阵。
  • 进盒子的非 orbital 部分:world 项仍用有方向区间、transform 后加(单边 worldVOL=[0,+20] 若按幅度对称成 [−20,+20] 会过度膨胀,且会被 rotateMat 错转)。

实现(4 步,纯粹让 R 满足定义):

  1. orbital reach(:1706-1708)算 dx/dy/dz 时各轴加上 world/gravity/noise 的幅度 W_a(在 orbital 块前先算好这个幅度盒)。
  2. orbital-active 路径:跳过后面 :1727-1728 / :1742-1743 / _addGravityToBounds 的有方向加法(已在 R 里,再加是 double-count)。
  3. 非 orbital 路径:有方向加法照旧(给出紧盒子)。
  4. gravity 现在在 _calculateTransformedBounds 之外加(:787),orbital-active 时要把它的幅度引进函数内的 R(或把 gravity 折叠进来统一处理)。

[P2] noise 包围盒疑似欠界:少乘 maxLifetime(请确认 noise 语义)

:1729-1743 注释写 "max displacement = |strength_max|",noise 加的是 |noise.strengthX._getMax()|(无 maxLifetime)。但 shader 里 computeNoiseVelocity(函数名带 Velocity,ParticleFeedback.shader:281)是 baseVelocity += computeNoiseVelocity(...)速度积分(*dt),NoiseModule.glsl:92 返回 ±strength 作为速度。若 noise 是速度扰动,则累积一生的位移上界是 |strength|·maxLifetime,包围盒注释/代码当成"位移=strength"就偏小,且不限 orbital,所有 noise 粒子今天都欠界

但这取决于 noise 的设计语义:如果 noise 设计上就是"有界位移扰动"(位移幅度 = strength),则 shader 当速度积分才是问题、包围盒反而对。证据偏向"noise 是速度、包围盒漏乘"(函数名 + baseVelocity += 用法 + 仅 TF 路径),但那条注释像是早期非速度实现的遗留。请作者确认 noise 语义:若是速度,包围盒 noise 幅度应 × maxLifetime

[P2] shader 随机通道复用:3 个随机覆盖 7 个独立因子

ParticleGenerator.ts:1022-1024 只写 3 个随机(_velocityRand ×3 → a_Random1.yzw),但 shader 里 linear VOL(a_Random1.yzw)、orbital(a_Random1.yzw)、radial(a_Random1.y共用它们。一个落在 max-orbital-X 的粒子被强制 max-linear-X 且 max-radial —— linear/orbital/radial 的随机区间完全相关。每个粒子内部仍自洽(非单粒子物理错),但"随机范围多样性"这个维度塌缩了。建议加一个独立随机属性,让 orbital/radial 与 linear 独立取随机。非阻塞。

已确认正确、勿动

shader 侧第一性原理审计 18 项绝大多数 CORRECT:orbital 有限旋转、轴序 YXZ、radial 方向/奇点、gravity(×modifier + world-sim 空间往返抵消)、FOL、drag/dampen、4-way 空间变换、四元数、瞬时曲线求值(orbital=角速度、radial=线速度,瞬时×dt 正确)、render↔feedback 的 cross(ω,rel) 对应、73524570b 删 currentLinearOffset。包围盒的"球→立方体"AABB、radialReachabs()(向内 radial 不增 reach,正向 abs 安全)也都对。


收口:包围盒第一性原理 = "R 必须等于离 offset 最大距离,含所有被旋转的累积位移"。现有 R 漏 world/gravity/noise(P1,orbital 时 pop)+ noise 通用欠界(P2)。修法就是让 R 满足定义(world 项进 R 用幅度、进盒子用方向,两角色分开)。随机通道复用(P2)是 shader 侧独立项。都附了精确落点,不阻塞合并但 P1 会在常见组合下 pop,建议处理。

@GuoLei1990 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

总结

本轮 PR tip 从我上次审查的 73524570b 线性推进一个 commit 到 fcf37172bgh api compare 73524570b...fcf37172b 返回 status: ahead, ahead_by: 1, behind_by: 0fcf37172b 父 = 73524570b4396ce8c3fed6dc264595c0486a0266,确认非 force-push 回退)。唯一改动是 fix: expand orbital particle bounds——ParticleGenerator.ts(+85/-20)+ ParticleBoundingBox.test.ts(+59/-2)。这条 commit 闭环了我历轮反复记的非阻塞 follow-up [P2]「orbital reach 未并入 world-space VOL/FOL + gravity 位移」。逐路径核对:bounds 数学正确、保守安全、新增测试反向证伪有效。无 P0/P1,仅两条 P3。--comment LGTM。

✅ [原 P2] orbital reach 未并入 world-space VOL/FOL + gravity 位移 —— 已闭环(逐路径核对正确)

fcf37172b 把 orbital reach 从只含「box-to-offset 距离 + radialReach」扩展为 + worldReach + noiseReach + gravityReach

const worldReach = this._getRangeReach(worldOffsetMin, worldOffsetMax);
const noiseReach = this._getVectorReach(noiseBoundsExtents);
const gravityReach = this._getGravityBoundsReach(maxLifetime);
const reach = Math.sqrt(dx*dx + dy*dy + dz*dz) + worldReach + noiseReach + gravityReach + radialReach;

并把 orbital 路径下的 gravity/noise/worldOffset 加法从下游(_addGravityToBounds + 末尾 noise/worldOffset 加法)改为折进 reach,用 _useOrbitalBounds() 双端门控保证只算一次

逐路径核对(不只信 commit message,核 shader 模型对齐 + 数学正确 + 不双算 + 别名安全 + 反向证伪)

  1. 数学是正确的保守上界 —— orbit 把 rel = position − offsetoffset 旋转,position 累积了所有线性贡献(startVel/linearVOL/FOL/noise/gravity,其中 externalVelocity 经 a_FeedbackPosition 烘进每帧再被旋转)。旋转保 |rel|,故每个线性贡献至多给 |position| 加自身模长。把各贡献模长当标量半径相加(三角不等式)是合法的保守上界 ✅。dx/dy/dz 用的是已积分的 local box 角点(含 local VOL/FOL/startpos),world 贡献/noise/gravity/radial 再叠加,无遗漏(可能轻度 over-cover,但保守安全)。

  2. gravity 只算一次(无双算) —— orbitalActive_useOrbitalBounds()=true_updateBoundsSimulationLocal(:787)/_updateBoundsSimulationWorld(:823) 跳过 _addGravityToBounds,gravity 改由 _getGravityBoundsReach(:1724) 折进 reach。radial-only / 非 orbital 路径仍走旧的 _addGravityToBounds + 末尾 noise/worldOffset 加法(:1736-1751 else if/!orbitalActive 分支),行为保持。两端门控读同一 VOL 状态、一致 ✅。

  3. temp-vector 别名安全 —— 新增 _tempVector32: Vector3(:51)绑定 noiseBoundsExtents_getGravityBoundsReach 内部也写 _tempVector32(:1789 gravityBoundsExtents),但只在 orbitalActive 分支调用(:1724),且 noiseReach 已在前一行(:1723)snapshot 进 number;orbitalActive 路径下 noiseBoundsExtents 此后不再被读(:1747-1749 在 !orbitalActive 内)。非 orbital 路径不调 _getGravityBoundsReach_tempVector32 保留 noise 值供 :1748 读。两路径均安全 ✅。_tempVector20_getCurveMagnitudeFromZero/radial/_getGravityBoundsReach 间复用,但各自写后立即消费进 number,无交错污染 ✅。

  4. 新增测试反向证伪有效 —— VelocityOverLifetime orbital bounds include gravity reach(gravityModifier=1, orbitalY=1, startSpeed=0)断言 ±125.074。我手算核对:maxLifetime=5、gravity=(0,−9.81,0)、coefficient=0.5·25=12.5 → gravityReach=|−9.81·12.5|=122.625;box-to-offset sqrt(3·1.414²)≈2.449reach=2.449+122.625=125.074 ✅ 逐位匹配。若 gravity 不折进 reach(旧行为),box 只会是 ±2.449 → 测试 fail。是真回归网

  5. noise 计算同步修正(顺带 latent fix) —— noise extent 从 Math.abs(strengthX._getMax()) 改为 _getCurveMagnitudeFromZero(strengthX) = max(|min0|,|max0|),对负向 noise 摆动更正确(旧 |_getMax()| 对跨零曲线如 [−5,2] 会算成 2 漏掉 −5 摆幅);且补了 × maxLifetime(noise 在 shader 里是 velocity 贡献 baseVelocity += computeNoiseVelocity :281,积分到位移 ≈ strength·lifetime)。新增 Noise 测试(strength=2 → ±11.414 = box 1.414 + 2·5)反向证伪 × maxLifetime:旧行为(无 ×lifetime)会得 ±3.414 → fail ✅。

CI

tip fcf37172b 全绿:build(3 OS)/e2e(4/4 分片)/codecov(project + patch 均 success)/lint/labeler。本轮 codecov/patch 转绿(历轮聚合假红消解)= 新增两条 bounds 测试覆盖了本 commit 的新代码行。

注释合规

删除的 noise 注释(// Noise module impact: noise output is normalized...)是复述代码的注释,删除符合规范;数学已落进命名清晰的 helper(_getNoiseBoundsExtents/_getGravityBoundsReach/_getRangeReach/_getVectorReach/_getCurveMagnitudeFromZero),无 dead code(逐个 grep 调用点确认全部 live)。fix: expand orbital particle bounds commit message 准确(见 P3-1 的小瑕疵)。无新增违规。

问题(均非阻塞 P3)

[P3] ParticleGenerator.ts — noise bounds × maxLifetime 是对非 orbital 路径的行为变化,commit message 未提及

_getNoiseBoundsExtents 把 noise extent 补了 × maxLifetime 并修正了负向摆动,这同时改变了非 orbital 粒子的 noise bounds(旧 |strength| → 新 |strength|·maxLifetime)。这是正确的 latent-bug fix(noise 是 velocity,应积分到位移),且 bounds 变大是保守安全的(不会误剔除)。但 commit message 只说「expand orbital bounds」,未提这条对所有 noise 粒子的修正——属可追溯性瑕疵。建议 commit body 补一句,或在 PR 描述里点明。不阻塞。

[P3] ParticleGenerator.ts:1724 — orbital gravity reach 改为随 TransformVolume dirty 缓存,运行时改 scene.physics.gravity 不再即时反映

旧非 orbital 路径每帧无条件跑 _addGravityToBoundsscene.physics.gravity;新 orbital 路径把 gravity reach 折进 _calculateTransformedBounds(受 TransformVolume dirty 门控)。gravityModifier 改会经 _onGeneratorParamsChanged 置 TransformVolume dirty(已核),但 scene.physics.gravity 向量本身改不会 dirty 任何粒子 bounds。故 orbital 粒子存活期间若运行时增大 scene gravity 且无任何粒子参数变更,缓存的 reach 会偏小 → 极端窄场景下可能 culling pop。注:这非回归(orbital 是本 PR 新特性,无「before」)、触发条件极窄、且 bounds 是 culling 优化非正确性。与历轮「reach 保守性偏紧不崩」同性质。不阻塞,列作 follow-up。

剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)

  • [P2] stretched-billboard vert 路径无测试覆盖(上轮 P2)—— 本 commit 是 bounds,未触及 vert。
  • [P3] offset 类型选型(静态 Vector3 vs 曲线)—— 产品需求权衡。
  • [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
  • [P3] ParticleGenerator._setTransformFeedback 删掉的 why 注释建议保留。

已闭环(保持,不重复)

  • ✅ [本轮] orbital reach 未并入 world-space VOL/FOL + gravity —— fcf37172b 折进 worldReach+noiseReach+gravityReach,逐路径核对 + 反向证伪测试,已闭环。
  • ✅ feedback↔vert rel desync —— 73524570b 删幻影 currentLinearOffset
  • ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 —— 65cc91bc externalVelocity 摘成轨道后平移。
  • ParticleFeedback.glsl 孤儿死代码 —— 4e2d79f1 删整个文件。
  • ✅ offset 原地分量改不脏 bounds —— a5a081e34
  • ✅ TwoConstants/TwoCurves 静默 max-only —— 28f8b29dc
  • ✅ orbital 三轴混搭假抽象 —— 3277c2e57
  • ✅ orbital/radial property 缺 Max 命名 —— 5a09858e

本 commit 精准闭环了我历轮反复记的非阻塞 follow-up [P2]:把 world-space VOL/FOL + noise + gravity 折进 orbital reach。逐路径核对 shader 模型对齐、数学是正确保守上界、gravity 不双算、temp-vector 别名安全、两条新测试逐位反向证伪(手算 125.074/11.414 匹配)、CI 全绿含 codecov/patch。顺带修正了 noise bounds 漏 × maxLifetime + 负向摆动的 latent bug。无 P0/P1,仅两条 P3(noise 变化未在 commit message 提 + orbital gravity 缓存的窄 staleness 边缘)。LGTM。

@GuoLei1990

Copy link
Copy Markdown
Member

在确认 orbital/radial 模拟与包围盒都正确后,扫了一遍粒子 shader 找可简化处。下面两条是行为完全等价(bit-identical)的去重,不动 e2e 输出;外加一条数值验证时发现的预存包围盒缺口(非本 PR 引入)。

[简化] TF shader 里 getVOLOrbital/getVOLRadial 是 module 函数的逐字副本,可删

ParticleFeedback.shader 自己定义了 getVOLOrbital/getVOLRadial(约 :111-154),但它 #includeVelocityOverLifetime.glsl(:48)里已经有 evaluateVOLOrbital/evaluateVOLRadial——两者函数体逐字相同(同 #ifdef 阶梯、同 renderer_VOLOrbital*/Radial* uniform、同 a_Random1.yzw/.y、同 mix 操作数顺序)。已 diff 确认仅函数名 get vs evaluate 之差,正文一字不差。

改法:删 TF 的 getVOLOrbital/getVOLRadial(~44 行),调用点改用 module 的:

// :316  getVOLRadial → evaluateVOLRadial
rel += (rel / relLen) * evaluateVOLRadial(attr, normalizedAge) * dt;
// :321  getVOLOrbital → evaluateVOLOrbital
rel = rotationByEuler(rel, evaluateVOLOrbital(attr, normalizedAge) * dt);

bit-identical(同一 TU、同 #if 门、目标函数已编译进来)。收益:删 ~44 行,消除"orbital/radial 曲线求值的第二个真相源"——TF 那份副本必须和 module 手动保持同步,是漂移隐患。e2e 只需冒烟验证输出不变(不用重生成基线)。

[简化] render pass quaternionConjugate(worldRotation) 内联两次,可 hoist

ParticleVert.glslquaternionConjugate(worldRotation) 在 :192 和 :209 各内联一次。worldRotation 解析后不再变,提成一个 invWorldRotation 变量(与 TF shader 既有命名一致)即可,bit-identical(纯符号翻转 + CSE,编译器多半已做,收益是可读性 + 单一来源)。

[注意] 没采纳"跨 TF↔render 抽共享 helper"

一度想把两边都有的 rel(到 orbit center 的相对位置)空间变换抽成共享 helper,但否决了:虽然 rel 那一行文本相同,TF 是位置积分rotationByEuler(rel, ω·dt) 有限旋转),render 是速度公式cross(ω, rel) 瞬时切向)——两个 pass 周围的 orbital 数学故意不同。抽共享 helper 会诱导后人误以为周围逻辑也一致。所以只做上面两条 pass 内部的等价去重,不碰跨 pass 边界。

[预存缺口,非本 PR] 非 orbital 路径 + Local-sim + entity 旋转:noise/worldOffset 盒子未旋转 → 偏小

数值验证(8000 配置 fuzzer)发现:orbital-active 路径全部保守(你这次 reach 修复用幅度折进球,旋转不变,验证 PASS)。但非 orbital 分支里,noise(±noiseBoundsExtents)和 worldOffset 盒子在 out.transform(rotateMat) 之后轴对齐世界盒加(:1742-1750)。而 Local-sim 下它们是 local 位移、应被 entity 旋转。复现:noise s=2、entity 转 1 弧度 → 真实 world max.y 18.53,包围盒只到 ±13.41,缺 5.12(= |R|·extents 的 off-axis 超出)。

git diff 确认这个未旋转的加法修复前就存在——你只是把它重构进 _getNoiseBoundsExtents + 门控 !orbitalActive没引入也没恶化,orbital 路径免疫。属预存缺口、独立 follow-up:把这两个 extents 在世界加之前先旋转(镜像 orbital 路径用幅度的处理)。非阻塞。

附带极微项:所有位移项用连续积分(0.5·g·L²)vs shader 的半隐式欧拉(·(1+1/n)),亚秒生命周期下略欠界(L=0.3s 缺 0.025),可忽略,要严格可加个 ceil 余量。


小结:两条 bit-identical 去重(删 TF 重复的 VOL 函数 ~44 行、hoist invWorldRotation)可安全合入、不动基线。跨 pass 抽 helper 不做(两 pass 数学故意不同)。非 orbital noise/worldOffset 未旋转是预存缺口、独立 follow-up。

@GuoLei1990 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

总结

本轮 PR tip 从我上次审查的 fcf37172b 线性推进一个 commit 到 1395f995dgh api compare fcf37172b...1395f995d 返回 status: ahead, ahead_by: 1, behind_by: 01395f995d 父 = fcf37172b5e7f757ef68466993fb67c940aba507,确认非 force-push 回退)。唯一改动是 refactor: reuse particle orbital evaluators——ParticleVert.glsl(+3/-2)+ ParticleFeedback.shader(+2/-47)。纯 shader 去重 + CSE,零行为变化。逐路径核对:删除的是真重复、函数逐字等价、CSE 安全、e2e 反向证伪。无 P0/P1,无新问题。--comment LGTM。

✅ 改动 1 —— ParticleFeedback.shadergetVOLOrbital/getVOLRadial 改用共享 evaluateVOLOrbital/evaluateVOLRadial(逐路径核对真去重)

1395f995d 删掉 ParticleFeedback.shader 里本地的 getVOLOrbital(45 行)+ getVOLRadial,两个调用点改用 evaluateVOLOrbital/evaluateVOLRadial

逐路径核对(不只信 commit message,核共享函数在 scope + 逐字等价 + 宏门一致 + 零残留 + e2e 反向证伪)

  1. 共享函数已在 scope —— ParticleFeedback.shader 的 include 列表里有 #include "ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl"(在 Attributes/Varyings 之后),该 module 文件已定义 evaluateVOLOrbital(Attributes, float) / evaluateVOLRadial(Attributes, float)(包在 _VOL_MODULE_ENABLED 内)。解析无误。

  2. 删除的本地版与共享版逐字等价 —— 对照 module 里的 evaluateVOLOrbital/evaluateVOLRadial 与 patch 里删掉的 getVOLOrbital/getVOLRadial:常量/曲线两分支、RENDERER_VOL_ORBITAL_IS_RANDOM_TWO / RENDERER_VOL_RADIAL_IS_RANDOM_TWOmix 随机插值、uniform 名(renderer_VOLOrbitalMaxConst / renderer_VOLOrbitalMaxCurveX/Y/Z / renderer_VOLRadialMaxConst / renderer_VOLRadialMaxCurve 等)、随机分量(orbital 用 a_Random1.yzw、radial 用 a_Random1.y全部逐字相同。删的是真复制品,非语义有别的另一份实现。

  3. 宏门一致 —— evaluateVOLRadial 调用点在 #if defined(RENDERER_VOL_RADIAL_CONSTANT_MODE) || defined(RENDERER_VOL_RADIAL_CURVE_MODE) 内,与 module 里 evaluateVOLRadial 定义的 guard 逐字相同;evaluateVOLOrbital 同理。两调用点又嵌在外层 RENDERER_VOL_ORBITAL_*/RADIAL_* 块内,与 module _VOL_MODULE_ENABLED 覆盖一致。a_Random1 字段在 struct 里的存在 guard(含 defined(RENDERER_VOL_ORBITAL_IS_RANDOM_TWO)/...RADIAL...)与函数内对它的访问 guard 也一致,无越界访问。

  4. ParticleVert.glsl 早已用共享 evaluateVOL* —— vert 端在 :208/:213 本就调用 evaluateVOLRadial/evaluateVOLOrbital(共享 module 版),证明这两个是已确立的 canonical 共享函数,feedback shader 是唯一还留着私有副本的地方。本 commit 把 feedback shader 收口到同一份,方向正确(消除双源真相)。

  5. 零残留 —— grep getVOLOrbital|getVOLRadial packages/shader/src 全 NONE,PR-tip blob 也确认两函数定义已删、两调用点已转换。是真删除无悬挂。

✅ 改动 2 —— ParticleVert.glsl invWorldRotation CSE(逐路径核对行为等价)

quaternionConjugate(worldRotation) 提成局部 vec4 invWorldRotation(:179),两处内联调用(:193 _VOL_LINEAR_MODULE_ENABLED 分支、:210 orbital/radial rel 重建分支)改用该局部。

逐项验证

  1. 值逐字等价 —— invWorldRotationquaternionConjugate(worldRotation) 的 hoisting,worldRotation:171-175 已定,hoisting 后两处复用同一值,纯 CSE 无语义变化。
  2. 无残留内联 —— grep quaternionConjugate|invWorldRotation 仅 3 处:定义(:179) + 两处复用(:193/:210),无遗漏的内联 quaternionConjugate(worldRotation),完整且一致。
  3. 与 feedback shader 既有写法对齐 —— ParticleFeedback.shader 早已在 main 顶部 hoisting vec4 invWorldRotation = quaternionConjugate(worldRotation); 并全程复用,本 commit 让 vert shader 采用同一惯例,是一致性改进。
  4. invWorldRotation 无 VOL 时声明但未用是良性 —— TF 开但无任何 VOL 模块时 invWorldRotation 未被读,属 GLSL 良性 unused-local(编译警告非错误,lint/build 全绿),且与 feedback shader 同一模式,非缺陷。

CI

tip 1395f995d 全绿:build(3 OS)/e2e(4/4 分片)/codecov(project + patch 均 success)/lint/labeler 全 success。e2e 4/4 绿是决定性反向证伪——若删除的 getVOLOrbital/getVOLRadial 与共享 evaluateVOL* 不逐字等价,velocityOrbitalConstant 基线图会偏移 → e2e 红。实测全绿 ⇒ 去重行为端到端不变。

注释合规

本 commit 删的是两个函数定义(含其上的 // Orbital angular velocity... / // Radial velocity... 单行注释,随函数删除合理)。无新增注释。refactor: reuse particle orbital evaluators commit message 准确。无新增违规。

剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)

  • [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 的 vert 改动是行为保持的 CSE,未改 rel 重建语义,故该 P2 性质不变(既未新破坏也未新覆盖)。
  • [P3] offset 类型选型(静态 Vector3 vs 曲线)—— 产品需求权衡。
  • [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
  • [P3] ParticleGenerator._setTransformFeedback 删掉的 why 注释建议保留。

已闭环(保持,不重复)

  • ✅ orbital reach 未并入 world-space VOL/FOL + gravity —— fcf37172b
  • ✅ feedback↔vert rel desync —— 73524570b
  • ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 —— 65cc91bc
  • ParticleFeedback.glsl 孤儿死代码 —— 4e2d79f1
  • ✅ offset 原地分量改不脏 bounds —— a5a081e34
  • ✅ TwoConstants/TwoCurves 静默 max-only —— 28f8b29dc
  • ✅ orbital 三轴混搭假抽象 —— 3277c2e57
  • ✅ orbital/radial property 缺 Max 命名 —— 5a09858e

本 commit 是一次干净的 shader 去重:删 ParticleFeedback.shader 里与共享 module evaluateVOLOrbital/evaluateVOLRadial 逐字重复的私有 getVOLOrbital/getVOLRadial(消除双源真相,feedback shader 收口到 vert shader 早已使用的同一份),外加 ParticleVert.glsl 一处 invWorldRotation CSE(与 feedback shader 既有惯例对齐)。逐路径核对函数逐字等价、宏门一致、零残留、CSE 安全、e2e 4/4 反向证伪行为不变。无 P0/P1/新问题。LGTM。

@GuoLei1990 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

总结

本轮 PR tip 从我上次审查的 1395f995d 线性推进一个 commit 到 d5f09e951gh api compare 1395f995d...d5f09e951 返回 status: ahead, ahead_by: 1, behind_by: 0d5f09e951 父 = 1395f995d87192cb37e30b734a342e1867dce3e5,确认非 force-push 回退)。唯一改动是 refactor(particle): use dedicated evaluator for visual linear velocity——VelocityOverLifetime.glsl(+26/-0)+ ParticleVert.glsl(+1/-2)。纯 shader 重构 + 死路径剥离,零行为变化。逐路径核对:新增 evaluator 与旧 out-param 逐字等价、call-site 收口正确、非死代码、e2e 反向证伪。无 P0/P1,无新问题。--comment LGTM。

✅ 改动 1 —— VelocityOverLifetime.glsl 新增 evaluateVOLVelocity(逐路径核对与旧 out-param 逐字等价)

d5f09e951 新增 vec3 evaluateVOLVelocity(Attributes, in float normalizedAge)——只计算 linear VOL 的瞬时速度(constant/curve 两模式 + TwoConstants/TwoCurves 的 mix 随机插值),定义在 _VOL_LINEAR_MODULE_ENABLED 块内(紧邻既有 computeVelocityPositionOffset 之前)。

逐路径核对(不只信 commit message,核与旧 out-param 逐字等价 + 宏门一致 + 曲线函数等价 + e2e 反向证伪)

  1. CONSTANT_MODE 逐字等价 —— 新版 velocity = renderer_VOLMaxConst;RANDOM_TWO → mix(renderer_VOLMinConst, velocity, a_Random1.yzw)。与 computeVelocityPositionOffsetcurrentVelocity out-param 路径(currentVelocity = renderer_VOLMaxConst + 同样的 mix(min, currentVelocity, a_Random1.yzw)逐字相同——同 uniform、同 a_Random1.yzw、同 min/max 顺序。✅

  2. CURVE_MODE 等价(关键:evaluateParticleCurveevaluateParticleCurveCumulative 的 out-param) —— 新版用 evaluateParticleCurve(renderer_VOLMaxGradientX/Y/Z, normalizedAge);旧版用 evaluateParticleCurveCumulative(...Max..., normalizedAge, currentVelocity.x/y/z)out-param(第 3 参)。我对照了 ParticleCommon.glsl:44/60 两函数实现:

    • evaluateParticleCurve 在 bracketing 段返回 mix(lastKey.y, key.y, (normalizedAge − lastTime)/(time − lastTime))
    • evaluateParticleCurveCumulative 在同一 bracketing 段把 out-param 设为 currentValue = mix(lastValue, key.y, offsetTime/(time − lastTime)),其中 offsetTime = normalizedAge − lastTime
    • 两者表达式逐字相同(同 lastKey.y/key.y、同 age 比值)。cumulative 函数额外把积分累进返回值 cumulativeValue——而 vert 端从不消费返回值(旧代码把 computeVelocityPositionOffset 当语句调用、只取 out-param)。故 evaluateVOLVelocity 的 CURVE 分支输出与旧 currentVelocity out-param 逐字等价。✅
    • RANDOM_TWO 路径同样逐字镜像(minVelocity 从 min 曲线取值、velocity = mix(minVelocity, velocity, a_Random1.yzw))。✅
  3. 宏门一致 —— evaluateVOLVelocity 内部分支用 RENDERER_VOL_CONSTANT_MODE/RENDERER_VOL_CURVE_MODE/RENDERER_VOL_IS_RANDOM_TWO,与 computeVelocityPositionOffset 逐字相同;函数本身包在 _VOL_LINEAR_MODULE_ENABLED 内,与 call-site(vert :183#ifdef _VOL_LINEAR_MODULE_ENABLED 内)覆盖一致,scope 解析无误。a_Random1.yzw 的访问 guard(RANDOM_TWO)与 struct 字段存在 guard 一致,无越界。✅

✅ 改动 2 —— ParticleVert.glsl call-site 收口到 evaluateVOLVelocity(逐路径核对行为等价 + 非死代码)

// 前(借 computeVelocityPositionOffset 的 out-param 只取速度,浪费 cumulative 积分计算)
vec3 instantVOLVelocity;
computeVelocityPositionOffset(attr, normalizedAge, age, instantVOLVelocity);
// 后(专用 evaluator,只算瞬时速度)
vec3 instantVOLVelocity = evaluateVOLVelocity(attr, normalizedAge);

逐项验证

  1. instantVOLVelocity 消费点逐字未变 —— :184-193localVelocity += instantVOLVelocity / worldVelocity += instantVOLVelocity / currentLinearVelocity 计算都是未改动 context,前后逐字节相同。✅

  2. age 不再传是正确的 —— 旧 computeVelocityPositionOffsetage 只用于 velocityPosition 积分(feedback 路径根本不消费 position offset,位置已烘进 a_FeedbackPosition),新 evaluator 只算瞬时速度故不需要 ageagecomputeParticleCenter 其它处仍被使用,无悬挂。✅

  3. computeVelocityPositionOffset 非死代码 —— git grep PR-tip:该函数仍在 ParticleVert.glsl:119非 feedback 的 computeParticlePosition CPU-sim 路径)被调用,那里同时消费 cumulative 返回值(velocityPositionOffsetlocalPositionOffset out-param(lifeVelocitylocalVelocity),两个输出都用。所以这次重构是干净的职责分离:只要瞬时速度的 feedback 消费者用新 evaluateVOLVelocity要 position offset 的 CPU-sim 消费者保留 computeVelocityPositionOffset。无死代码、无双源真相。✅

  4. 顺带的微 perf 收益 —— feedback 路径的 curve 分支现在调 evaluateParticleCurve(单次插值查找)而非 evaluateParticleCurveCumulative(插值查找 + 梯形积分累进循环),剥掉了 feedback 渲染路径里被浪费的积分计算。是净改进非中性。

CI

tip d5f09e951 全绿:build(3 OS)/e2e(4/4 分片全 success)/codecov(project + patch 均 success)/lint/labeler。e2e velocityOrbitalConstant 是反向证伪——该 case 设 linearVOL constant (1,10,1) + orbital + radial(Local + TF),走的正是本次改的 call-site(:183evaluateVOLVelocity constant 分支 → currentLinearVelocityvisualSimulationVelocity)。若新 evaluator 的 constant 输出与旧 out-param 偏移 → 基线图偏移 → e2e 红。实测全绿 ⇒ constant 路径行为端到端不变(curve 路径走逐字等价证明,该 case 未覆盖 curve 分支)。

注释合规

本 commit 无新增/删除注释。新增 evaluateVOLVelocity 无 doc 注释——与同文件既有 shader 函数 evaluateVOLOrbital/evaluateVOLRadial/computeVelocityPositionOffset 惯例一致(该 .glsl 里这些函数均无 doc 注释)。refactor(particle): use dedicated evaluator for visual linear velocity commit message 准确。无新增违规。

剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)

  • [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 是行为保持的 evaluator 收口,未改 rel/视觉速度重建语义,P2 性质不变。
  • [P3] offset 类型选型(静态 Vector3 vs 曲线)—— 产品需求权衡。
  • [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
  • [P3] ParticleGenerator._setTransformFeedback 删掉的 why 注释建议保留。

已闭环(保持,不重复)

  • getVOLOrbital/getVOLRadial feedback shader 去重 —— 1395f995d
  • ✅ orbital reach 未并入 world VOL/FOL + gravity —— fcf37172b
  • ✅ feedback↔vert rel desync —— 73524570b
  • ✅ gravity/FOL/drag/noise 被 orbital 一起旋转 —— 65cc91bc
  • ParticleFeedback.glsl 孤儿死代码 —— 4e2d79f1
  • ✅ offset 原地分量改不脏 bounds —— a5a081e34
  • ✅ TwoConstants/TwoCurves 静默 max-only —— 28f8b29dc
  • ✅ orbital 三轴混搭假抽象 —— 3277c2e57
  • ✅ orbital/radial property 缺 Max 命名 —— 5a09858e

本 commit 是一次干净的职责分离重构:feedback 渲染路径原本借 computeVelocityPositionOffset 的 out-param 只取瞬时速度(浪费 cumulative 积分),现改用专用 evaluateVOLVelocity。逐路径核对新 evaluator 与旧 out-param 逐字等价(constant 直接相同;curve 经 evaluateParticleCurveevaluateParticleCurveCumulative out-param 逐字证明)、宏门一致、computeVelocityPositionOffset 仍在 CPU-sim 路径活用(非死代码)、call-site 消费点未变、e2e 4/4 反向证伪行为不变,顺带剥掉 feedback 路径的浪费积分计算。无 P0/P1/新问题。LGTM。

@cptbtptpbcptdtptp cptbtptpbcptdtptp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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 undefined

Consequences, all verified by running the engine in chromium:

  1. _isRadialActive()true → on WebGL2 the generator flips into transform feedback and _clearActiveParticles() kills every live particle — for a curve that evaluates to 0 everywhere.
  2. _updateShaderData then hits radial.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();
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Nit] Unrelated letconst lint fix that slipped into this feature PR — harmless, but ideally kept out of the diff.

@GuoLei1990 GuoLei1990 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

总结

本轮 PR tip 从我上次审查的 d5f09e951 线性推进一个 commit 到 8bcfc647cgh api compare d5f09e951...8bcfc647c 返回 status: ahead, ahead_by: 1, behind_by: 08bcfc647c 父 = 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:

  1. 修 P0 级 TypeError 崩溃(_updateShaderData 每帧路径) —— set mode 只设 this._mode不惰性创建 _curveMax(已读 :31-34 确认)。故 new ParticleCompositeCurve(0)(默认 constant,_curveMax=undefined)→ mode = CurvecurveMax 仍是 undefined——这是真实的编辑器/用户路径。

    • 修复前_getKeyMinMax(undefined?.keys, count=0) 返回 out.set(undefined, undefined)_isZero() = undefined===0 && undefined===0 = false_isRadialActive() = !false = true → 进入 _updateShaderDataradialActive 块(VelocityOverLifetimeModule.ts:391-393)→ radial.curveMax._getTypeArray() 命中 undefined._getTypeArray() = TypeError 崩溃。orbital 同理(:342-355 orbitalX.curveMax._getTypeArray())。
    • 修复后out.set(0,0)_isZero() = true_isRadialActive()/_isOrbitalActive() = falseradialActive/orbitalActive 块被跳过 → 不触碰 undefined._getTypeArray() → 无崩溃。语义正确:未创建曲线对象的 Curve 模式曲线在 shader 里求值为 0,判为 inactive 是对的。✅
  2. 修 NaN-bounds latent bug(含 base dev/2.0 就存在的) —— _getMinMax_getExtremeValueFromZeroParticleGenerator.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 同样返回 undefinedorigin/dev/2.0:ParticleCompositeCurve.ts:271-295),_getExtremeValueFromZero 同样 Math.min(0, undefined)——这条 NaN-bounds 是 base 就存在的 latent bug,本 commit 一并修掉。??0 让空曲线贡献 0 extent,保守安全。✅

  3. 只改空曲线路径,authored 曲线 byte 不变 —— ?? 0 仅在 min/maxundefined(无 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 反向证伪)

  1. byte 等价 —— 删除的 getVOLVelocity 与 module evaluateVOLVelocityVelocityOverLifetime.glsl:36-60)逐行对照:CONSTANT 分支 renderer_VOLMaxConst + mix(renderer_VOLMinConst, ·, a_Random1.yzw)、CURVE 分支 evaluateParticleCurve(renderer_VOLMaxGradientX/Y/Z) + mix(minVelocity, ·, a_Random1.yzw)——逐字相同(仅局部变量名 velvelocityminVelminVelocity)。✅

  2. 共享函数在 scope —— feedback shader #include "ShaderLibrary/Particle/Module/VelocityOverLifetime.glsl":48)。evaluateVOLVelocity 定义在 module 的 #ifdef _VOL_LINEAR_MODULE_ENABLED(module :13 开、:96 闭)内,call-site vec3 vol = evaluateVOLVelocity(...) 也在 #ifdef _VOL_LINEAR_MODULE_ENABLED(feedback :111-119)内——定义门与调用门逐字一致。✅

  3. 旧版无守卫 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 两分支必有一路赋值,无未初始化读。✅

  4. 零残留 —— git grep getVOLVelocity 8bcfc647c -- packages/shader/** = NONE。真删除无悬挂。✅

✅ 改动 3 —— _tempVector33 消除 gravity/noise 共享 scratch 的 latent aliasing

_getGravityBoundsReachgravityBoundsExtents_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 inactivevol.radial.mode = CurvecurveMax === undefined)→ 断言 _isZero() === true / _isRadialActive() === false / _needTransformFeedback() === false / _updateShaderData(...) to.not.throw()修复前反向证伪_getKeyMinMax 返回 undefined_isZero() = false_isRadialActive() = true → 断言 fail,且 _updateShaderData 会命中 undefined._getTypeArray() 抛 TypeErrorto.not.throw 直接 fail)。故这是真回归网——同时守住 _isZero 语义翻转 + 崩溃路径。orbital 三轴 Curve 分支同样覆盖。

剩余开放项(均非阻塞,历轮已记,源码本轮未触及,不重复展开)

  • [P2] stretched-billboard vert 路径无测试覆盖 —— 本 commit 未触及 vert rel/视觉速度重建。
  • [P3] offset 类型选型(静态 Vector3 vs 曲线)—— 产品需求权衡。
  • [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
  • [P3] ParticleGenerator._setTransformFeedback 删掉的 why 注释建议保留。

已闭环(保持,不重复)

  • evaluateVOLVelocity linear VOL evaluator —— d5f09e951
  • getVOLOrbital/getVOLRadial feedback shader 去重 —— 1395f995d
  • ✅ orbital reach 未并入 world VOL/FOL + gravity —— fcf37172b
  • ✅ feedback↔vert rel desync —— 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 cptbtptpbcptdtptp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[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
#endif

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

总结

本轮 PR tip 从我上次审查的 8bcfc647c 线性推进一个 commit 到 829b679dgh api compare 8bcfc647c...829b679d 返回 status: ahead, ahead_by: 1, behind_by: 0829b679d 父 = 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 反向证伪)

  1. _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 逐字相同布尔、相同操作数。✅
  2. 3 个使用点各自逐字等价 —— renderer_VOLOffset 声明(VelocityOverLifetime.glsl:103)、ParticleVert.glsl:197visualSimulationVelocity 重建入口)、ParticleFeedback.shader:210linearVelocity 初始化入口)三处 BASE 都是同一 4 项谓词,NEW 全改成 #ifdef _VOL_ORBITAL_RADIAL_MODULE_ENABLED,展开即原谓词。✅

  3. 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. 收窄范围正确——只收 4 项组合谓词,未误伤 2 项选择谓词 —— 全 shader tree 扫描:完整 4 项 inline 谓词现仅剩 VelocityOverLifetime.glsl:8 一处(即宏定义站),3 处旧副本全清。同时 ParticleVert.glsl:213/220ParticleFeedback.shader:239/246VelocityOverLifetime.glsl:137/1622 项 orbital-only(ORBITAL_CONST||ORBITAL_CURVE)/ radial-only(RADIAL_CONST||RADIAL_CURVE 选择谓词未动——这些是在 orbital vs radial 子块间做选择,与「orbital 或 radial 任一 active」的组合门语义不同,用 4 项宏替换它们会是错误的过度收口。作者正确区分了这两类谓词。✅

  5. 零残留 —— 全 packages/shader/src/** .glsl/.shader 逐文件扫描,旧 4 项组合谓词零残留(仅剩宏定义站),无悬挂副本。✅

  6. e2e 实跑反向证伪 —— tip 829b679d 上 e2e 4/4 全绿。这是决定性证据:velocityOrbitalConstant case 覆盖 orbital+radial+linear VOL 全模式,若宏收口误改了任一变体的编译门(漏掉某 mode、错配 renderer_VOLOffset uniform 声明门、或 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 谓词改成宏,未触及 vert rel/视觉速度重建逻辑,P2 性质不变。
  • [P3] offset 类型选型(静态 Vector3 vs 曲线)—— 产品需求权衡。
  • [P2] orbital 多轴 Euler 逐帧积分轴序未文档化 —— 单轴无影响。
  • [P3] ParticleGenerator._setTransformFeedback 删掉的 why 注释建议保留。

已闭环(保持,不重复)

  • ??0 空曲线崩溃 + NaN-bounds —— 8bcfc647c
  • _tempVector33 aliasing 硬化 —— 8bcfc647c
  • evaluateVOLVelocity linear VOL evaluator —— d5f09e951
  • getVOLVelocity/getVOLOrbital/getVOLRadial feedback shader 去重 —— 8bcfc647c/1395f995d
  • ✅ orbital reach 未并入 world VOL/FOL + gravity —— fcf37172b
  • ✅ feedback↔vert rel desync —— 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。

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

Labels

effect Effect related functions particle

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants