feat(particle): add NoiseModule for simplex noise turbulence#2953
feat(particle): add NoiseModule for simplex noise turbulence#2953hhhhkrx wants to merge 24 commits intogalacean:dev/2.0from
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:
WalkthroughAdded a new NoiseModule to the particle system, exported its shader, integrated it into ParticleGenerator (property, shader-data updates, TF activation), and expanded world-space bounds computation to account for noise amplitude when enabled. Changes
Sequence DiagramsequenceDiagram
participant PG as ParticleGenerator
participant NM as NoiseModule
participant R as Renderer
participant SD as ShaderData
participant SH as Shader
PG->>NM: construct NoiseModule(this)
PG->>NM: set properties (strengthX/Y/Z, frequency, scrollSpeed, octaves, octaveMultiplier, octaveScale)
NM->>R: _onGeneratorParamsChanged()
PG->>SD: _updateShaderData(shaderData)
PG->>NM: NM._updateShaderData(SD)
alt Noise enabled
NM->>SD: write strengthVec, freq, scroll, octaveVec
NM->>SH: _enableMacro("RENDERER_NOISE_MODULE_ENABLED", true)
else Noise disabled
NM->>SH: _enableMacro("RENDERER_NOISE_MODULE_ENABLED", false)
end
PG->>PG: _calculateTransformedBounds()
PG->>PG: expand bounds by axis-aligned margin from noise amplitude
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Add GPU-computed simplex noise displacement to particles, referencing Unity's Noise Module. Supports per-axis strength, frequency, scroll speed, damping, and up to 3 octaves. Reuses existing noise_common and noise_simplex_3D shader libraries. No instance buffer changes needed.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2953 +/- ##
===========================================
- Coverage 77.73% 77.55% -0.18%
===========================================
Files 898 900 +2
Lines 98310 98748 +438
Branches 9806 9803 -3
===========================================
+ Hits 76417 76588 +171
- Misses 21728 21994 +266
- Partials 165 166 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/particle/modules/NoiseModule.ts`:
- Around line 146-150: The octaveMultiplier setter currently allows negative
values which can invert bounds; update set octaveMultiplier(value: number) to
validate and reject or clamp values to a safe non-negative range (e.g., clamp to
>= 0.0 or a chosen min like 0.0/1.0), only assign to this._octaveMultiplier and
call this._generator._renderer._onGeneratorParamsChanged() when the
validated/clamped value differs from the current value; ensure any invalid
inputs are normalized (or throw a clear error) so downstream bounds calculations
in ParticleGenerator no longer receive negative multipliers.
🪄 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: bd9486dd-e2cd-4d0b-91a3-d78d7fd9eb2b
⛔ Files ignored due to path filters (2)
packages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/noise_over_lifetime_module.glslis excluded by!**/*.glsl
📒 Files selected for processing (4)
packages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/index.tspackages/core/src/particle/modules/NoiseModule.tspackages/core/src/shaderlib/particle/index.ts
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ulation space - Force-enable Transform Feedback when noise is active (like LimitVelocityOverLifetime) - Sample noise at a_FeedbackPosition (current simulation-space position) instead of birth position - Apply worldRotation transform to noise offset in Local simulation space - Simplify GLSL function signature, compute normalizedAge internally for damping
- Divide accumulated noise by totalWeight so output stays in [-1,1] regardless of octave count, keeping strength as the authoritative control - Remove redundant amplitude/frequency init variables - Rename to octaveMultiplier/octaveScale/weight/totalWeight for clarity - Simplify bounds calculation (normalized noise max = strength)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/core/src/particle/modules/NoiseModule.ts (2)
186-214: Shader data update logic is correct; consider uniform packing optimization.The implementation properly reuses temp vectors to avoid per-frame allocations and correctly manages macro state.
Per the PR discussion, you could pack the uniforms into two
vec4s (e.g.,(strengthX, strengthY, strengthZ, frequency)and(scrollSpeed, octaves, octaveMultiplier, octaveScale)) to reduce uniform slot usage, but this is a minor optimization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/NoiseModule.ts` around lines 186 - 214, The current _updateShaderData correctly sets separate vector3/float uniforms but to reduce uniform slots pack them into two vec4s: first vec4 = (strengthX, strengthY, strengthZ, frequency) and second vec4 = (scrollSpeed, octaves, octaveMultiplier, octaveScale); update the temp vectors (replace _strengthVec and _octaveInfoVec uses) to setVector4 on new ShaderProperty IDs (e.g., NoiseModule._packed1Property, NoiseModule._packed2Property) and remove the separate shaderData.setVector3/ setFloat calls for strength/frequency/scrollSpeed/octaveInfo, keeping macro handling in _updateShaderData unchanged (_enabledModuleMacro, _dampingModuleMacro, _enableMacro).
40-80: Consider usingParticleCompositeCurvefor strength properties.Per the PR discussion,
strengthX/Y/Zshould useParticleCompositeCurve(MinMaxCurve equivalent) to match other modules and support curve-based strength variation over particle lifetime. This would also require@deepClonedecorator and on-change handling consistent with similar properties elsewhere.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/NoiseModule.ts` around lines 40 - 80, Replace the numeric strengthX/strengthY/strengthZ properties with ParticleCompositeCurve instances (instead of number) and annotate them with `@deepClone`; update their getters/setters so get strengthX/Y/Z returns a ParticleCompositeCurve and set strengthX/Y/Z accepts a ParticleCompositeCurve, assigning to this._strengthX/_strengthY/_strengthZ and calling this._generator._renderer._onGeneratorParamsChanged() when the property reference changes; additionally ensure the ParticleCompositeCurve instances wire their internal change callback (or subscribe to their on-change event as done in other modules) to call _generator._renderer._onGeneratorParamsChanged() when the curve contents mutate so curve edits trigger renderer updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/particle/modules/NoiseModule.ts`:
- Around line 89-94: The frequency setter for NoiseModule currently assigns any
value to this._frequency which can allow zero/near-zero and cause
division-by-zero in ParticleGenerator bounds (uses strength / frequency); modify
NoiseModule.set frequency to clamp the incoming value to a safe minimum (e.g.,
Math.max(value, 1e-6)) before assigning this._frequency and then call
this._generator._renderer._onGeneratorParamsChanged(); ensure you reference the
existing setter (set frequency), the _frequency field, and keep the downstream
notification call unchanged.
- Around line 160-164: The octaveScale setter should clamp incoming values to a
safe range (e.g., 1 to 4) before assigning to this._octaveScale to prevent
zero/negative scales; update the setter in NoiseModule (the set
octaveScale(value: number) method) to compute a clampedValue = Math.min(4,
Math.max(1, value)), only assign and call
this._generator._renderer._onGeneratorParamsChanged() when clampedValue !==
this._octaveScale, and preserve the existing behavior otherwise.
- Around line 171-180: The enabled setter in NoiseModule currently calls
this._generator._setTransformFeedback(value) unconditionally, which disables
transform feedback even when the other TF-dependent module
(LimitVelocityOverLifetimeModule) is still enabled; update the
NoiseModule.enabled setter to only call _setTransformFeedback(false) when both
this._enabled and the other module's enabled flag are false (e.g., check
this._generator._modules.limitVelocityOverLifetime?.enabled or similar), and
mirror the same symmetric change in LimitVelocityOverLifetimeModule to check
noise.enabled before turning TF off so that TF is only disabled when neither
module requires it.
---
Nitpick comments:
In `@packages/core/src/particle/modules/NoiseModule.ts`:
- Around line 186-214: The current _updateShaderData correctly sets separate
vector3/float uniforms but to reduce uniform slots pack them into two vec4s:
first vec4 = (strengthX, strengthY, strengthZ, frequency) and second vec4 =
(scrollSpeed, octaves, octaveMultiplier, octaveScale); update the temp vectors
(replace _strengthVec and _octaveInfoVec uses) to setVector4 on new
ShaderProperty IDs (e.g., NoiseModule._packed1Property,
NoiseModule._packed2Property) and remove the separate shaderData.setVector3/
setFloat calls for strength/frequency/scrollSpeed/octaveInfo, keeping macro
handling in _updateShaderData unchanged (_enabledModuleMacro,
_dampingModuleMacro, _enableMacro).
- Around line 40-80: Replace the numeric strengthX/strengthY/strengthZ
properties with ParticleCompositeCurve instances (instead of number) and
annotate them with `@deepClone`; update their getters/setters so get strengthX/Y/Z
returns a ParticleCompositeCurve and set strengthX/Y/Z accepts a
ParticleCompositeCurve, assigning to this._strengthX/_strengthY/_strengthZ and
calling this._generator._renderer._onGeneratorParamsChanged() when the property
reference changes; additionally ensure the ParticleCompositeCurve instances wire
their internal change callback (or subscribe to their on-change event as done in
other modules) to call _generator._renderer._onGeneratorParamsChanged() when the
curve contents mutate so curve edits trigger renderer updates.
🪄 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: 79899686-6b78-47da-9c84-0207d618d7c6
⛔ Files ignored due to path filters (2)
packages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/noise_over_lifetime_module.glslis excluded by!**/*.glsl
📒 Files selected for processing (2)
packages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/modules/NoiseModule.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/particle/ParticleGenerator.ts
- Remove damping property, _damping field, RENDERER_NOISE_DAMPING macro - Bake strength/frequency into uniform on CPU side for parameter orthogonality - frequency controls spatial detail only, strength controls amplitude only - Simplify GLSL by removing damping branch - Update bounds calculation to use strength/frequency
… scrollSpeed - Change strengthX/Y/Z and scrollSpeed from number to ParticleCompositeCurve - Add @deepClone decorator and _onCompositeCurveChange setter pattern - Initialize curves in constructor following ForceOverLifetimeModule pattern - Read .constantMax in _updateShaderData, use ._getMax() for bounds
- octaves → octaveCount - octaveMultiplier → octaveIntensityMultiplier - octaveScale → octaveFrequencyMultiplier - sampleNoise3D → sampleSimplexNoise3D
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/particle/modules/NoiseModule.ts (2)
153-160:⚠️ Potential issue | 🔴 CriticalDerive transform feedback from the combined module state.
Line 159 passes
valuestraight into_setTransformFeedback(), but TF is shared withLimitVelocityOverLifetimeModule, and_setTransformFeedback()clears all live particles every time it runs. Disabling noise can tear TF down while the other module still needs it, and enabling noise while TF is already active still nukes the current particle set.One way to avoid the destructive no-op transition
override set enabled(value: boolean) { if (value !== this._enabled) { if (value && !this._generator._renderer.engine._hardwareRenderer.isWebGL2) { return; } this._enabled = value; - this._generator._setTransformFeedback(value); + const needsTF = value || this._generator.limitVelocityOverLifetime.enabled; + if (needsTF !== this._generator._useTransformFeedback) { + this._generator._setTransformFeedback(needsTF); + } this._generator._renderer._onGeneratorParamsChanged(); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/NoiseModule.ts` around lines 153 - 160, The enabled setter in NoiseModule currently passes the raw value into this._generator._setTransformFeedback(), which tears down TF even when other modules (e.g., LimitVelocityOverLifetimeModule) still require it; instead compute the combined transform-feedback requirement and call _setTransformFeedback with that combined boolean. Concretely, in NoiseModule.enabled() use this._enabled = value, then compute shouldEnableTF = value || this._generator.getModule("LimitVelocityOverLifetimeModule")?.enabled (or another generator method that checks all modules), and call this._generator._setTransformFeedback(shouldEnableTF); keep calls to this._generator._renderer._onGeneratorParamsChanged() as-is. Ensure you reference the _setTransformFeedback method and LimitVelocityOverLifetimeModule's enabled flag when making the change.
85-89:⚠️ Potential issue | 🟠 MajorClamp
frequency,octaveMultiplier, andoctaveScalebefore storing them.
frequencyis divided into the baked strength vector at Line 179 and into transformed-bounds expansion inParticleGenerator.ts, Line 1397. Allowing0or negative values produces infinities or even shrinks the bounds.octaveMultiplierandoctaveScalealso still accept values outside their documented decay/increase semantics.Proposed clamp logic
set frequency(value: number) { - if (value !== this._frequency) { - this._frequency = value; + const nextValue = Number.isFinite(value) ? Math.max(value, 1e-6) : 1e-6; + if (nextValue !== this._frequency) { + this._frequency = nextValue; this._generator._renderer._onGeneratorParamsChanged(); } } set octaveMultiplier(value: number) { - if (value !== this._octaveMultiplier) { - this._octaveMultiplier = value; + const nextValue = Number.isFinite(value) ? Math.min(1, Math.max(0, value)) : 0; + if (nextValue !== this._octaveMultiplier) { + this._octaveMultiplier = nextValue; this._generator._renderer._onGeneratorParamsChanged(); } } set octaveScale(value: number) { - if (value !== this._octaveScale) { - this._octaveScale = value; + const nextValue = Number.isFinite(value) ? Math.min(4, Math.max(1, value)) : 1; + if (nextValue !== this._octaveScale) { + this._octaveScale = nextValue; this._generator._renderer._onGeneratorParamsChanged(); } }Also applies to: 128-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/NoiseModule.ts` around lines 85 - 89, Clamp the values for frequency, octaveMultiplier, and octaveScale inside NoiseModule setters before assigning to the backing fields to prevent zeros/negatives and out-of-range growth: in the frequency setter (where it assigns this._frequency and calls this._generator._renderer._onGeneratorParamsChanged()) enforce a minimum > 0 (e.g. tiny positive epsilon) and an optional sensible max; similarly clamp this._octaveMultiplier and this._octaveScale in their setters (use documented decay/increase bounds) prior to assignment and before invoking _onGeneratorParamsChanged() so downstream code in ParticleGenerator (e.g., baked strength vector and transformed-bounds expansion) never receives invalid values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 1393-1402: The current AABB expansion uses raw per-axis
noiseMaxX/Y/Z in world space but misses contributions when the emitter is
rotated; compute local noise extents extLocal = (abs(strengthX), abs(strengthY),
abs(strengthZ)) * (1.0 / noise.frequency), then rotate those extents into
world-space by applying the absolute-value of the emitter rotation matrix
(abs(R) * extLocal) to get worldExt = (worldExtX, worldExtY, worldExtZ), and
finally expand min and max by worldExt (min -= worldExt, max += worldExt)
instead of using noiseMaxX/Y/Z directly; locate this change in ParticleGenerator
where noise.enabled is checked and min/max are modified.
---
Duplicate comments:
In `@packages/core/src/particle/modules/NoiseModule.ts`:
- Around line 153-160: The enabled setter in NoiseModule currently passes the
raw value into this._generator._setTransformFeedback(), which tears down TF even
when other modules (e.g., LimitVelocityOverLifetimeModule) still require it;
instead compute the combined transform-feedback requirement and call
_setTransformFeedback with that combined boolean. Concretely, in
NoiseModule.enabled() use this._enabled = value, then compute shouldEnableTF =
value || this._generator.getModule("LimitVelocityOverLifetimeModule")?.enabled
(or another generator method that checks all modules), and call
this._generator._setTransformFeedback(shouldEnableTF); keep calls to
this._generator._renderer._onGeneratorParamsChanged() as-is. Ensure you
reference the _setTransformFeedback method and LimitVelocityOverLifetimeModule's
enabled flag when making the change.
- Around line 85-89: Clamp the values for frequency, octaveMultiplier, and
octaveScale inside NoiseModule setters before assigning to the backing fields to
prevent zeros/negatives and out-of-range growth: in the frequency setter (where
it assigns this._frequency and calls
this._generator._renderer._onGeneratorParamsChanged()) enforce a minimum > 0
(e.g. tiny positive epsilon) and an optional sensible max; similarly clamp
this._octaveMultiplier and this._octaveScale in their setters (use documented
decay/increase bounds) prior to assignment and before invoking
_onGeneratorParamsChanged() so downstream code in ParticleGenerator (e.g., baked
strength vector and transformed-bounds expansion) never receives invalid values.
🪄 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: 0b30aa56-5adc-4f68-815c-a26ff20fbdff
⛔ Files ignored due to path filters (1)
packages/core/src/shaderlib/particle/noise_over_lifetime_module.glslis excluded by!**/*.glsl
📒 Files selected for processing (2)
packages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/modules/NoiseModule.ts
- frequency: min 1e-6 to prevent division by zero - octaveIntensityMultiplier: clamp [0, 1] - octaveFrequencyMultiplier: clamp [1, 4]
renderer_NoiseParams = (strengthX/freq, strengthY/freq, strengthZ/freq, frequency) renderer_NoiseOctaveParams = (scrollSpeed, octaveCount, octaveIntensityMul, octaveFreqMul)
…r noise sampling Swapping coordinate axes guarantees independent sampling per axis without relying on arbitrary prime constants.
- Shorten particle lifetime to 0.3-0.6s - Increase simulation time to 2s - Set diffPercentage to 0 for strict comparison
…ty integration - Remove noise position offset in vertex shader, apply noise purely via Transform Feedback velocity integration for smooth turbulence accumulation - Add per-particle random offset (a_Random0.yzw * 50) in noise space to decorrelate particles emitted from the same position - Remove invFreq scaling — strength directly equals max displacement (matches Unity) - Remove (1-normalizedAge) damping — noise strength is constant over lifetime - Add separateAxes property to NoiseModule for per-axis strength control - Update bounds calculation to respect separateAxes - Update e2e test: use ConeShape, Local space, adjusted parameters
Use birth position + initial velocity * age instead of feedback position to break the position → noise → velocity → position feedback loop that causes particles to get stuck at high noise strength values.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Add Constant/TwoConstants/Curve/TwoCurves mode support for noise strength, following the same pattern as SizeOverLifetimeModule - Add RENDERER_NOISE_STRENGTH_CURVE, RENDERER_NOISE_STRENGTH_IS_RANDOM_TWO, RENDERER_NOISE_IS_SEPARATE shader macros and corresponding uniforms - Use analytical base position for noise sampling in world space mode to match world-aligned noise direction - Fix bounds calculation to use Math.abs for negative strength values
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… octave params - Fix TwoConstants having no effect: a_Random0.y was only populated by colorOverLifetime, defaulting to 0 and collapsing mix to min value - Write noise-dedicated random to a_Random0.z (offset 21) via _noiseRand - Derive noise spatial offset from a_Random0.z instead of a_Random0.yzw - Rename octaveIntensityMultiplier/octaveFrequencyMultiplier to octavePersistence/octaveLacunarity for clarity
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
粒子系统 NoiseModule，基于 simplex noise 的湍流位移，GPU 计算，纯 uniform 驱动无 instance buffer 变更。经过多轮迭代，大部分之前的问题已修复：参数 clamp、uniform 打包、ParticleCompositeCurve 曲线支持、归一化、API 命名（octavePersistence/octaveLacunarity）。整体实现质量高。
剩余一个 P0 问题。
已修复的问题（对照之前 review）
- ✅ 参数 clamp — frequency clamp 到 1e-6，octaveCount [1,3]，octavePersistence [0,1]，octaveLacunarity [1,4]
- ✅ Uniform 打包 — 从 4 个独立 slot 打包为 2 个 vec4
- ✅ strength 改为 ParticleCompositeCurve — 支持 Curve/TwoCurves/TwoConstants
- ✅ octave 归一化 — shader 中
noiseValue / totalWeight确保输出 [-1,1]，bounds 用|strength|正确 - ✅ API 命名 —
octaveMultiplier/octaveScale→octavePersistence/octaveLacunarity，标准术语 - ✅ e2e 测试 — 已补充
- ✅ 分析式采样位置 — 避免 feedback loop：用
birth + velocity * age而非a_FeedbackPosition - ✅ TwoConstants 随机 — 使用专用
a_Random0.z（offset 21），不再复用 colorOverLifetime 的 random
问题
[P0] NoiseModule.ts:471 & LimitVelocityOverLifetimeModule.ts:244 — 禁用任一模块会错误关闭 Transform Feedback，破坏另一个模块
两个模块的 enabled setter 都直接调用 _setTransformFeedback(value)。当 Noise 和 LimitVelocity 同时启用时：
- 用户关闭 Noise →
_setTransformFeedback(false) _useTransformFeedback = false+_clearActiveParticles()（所有粒子被清空）- LimitVelocityOverLifetimeModule 仍然 enabled，但 TF 管线已关闭 → 模块失效 + 视觉跳变
反过来也一样：关闭 LimitVelocity 会破坏 Noise。
_cloneTo 已经用了 OR 逻辑（target.limitVelocityOverLifetime.enabled || target.noise.enabled），但 enabled setter 没有对齐。
建议：将 TF 启停逻辑收敛到 _setTransformFeedback，让它查询所有依赖模块：
// ParticleGenerator
_setTransformFeedback(requested: boolean): void {
const needed = this.limitVelocityOverLifetime.enabled || this.noise.enabled;
if (needed === this._useTransformFeedback) return;
this._useTransformFeedback = needed;
// ... 现有的初始化/清理逻辑
}两个模块的 enabled setter 改为无脑调用 _setTransformFeedback(value)，由 Generator 统一决策。这样：
- 不需要模块之间互相知道对方的存在
- 新增 TF 依赖模块时只需改
_setTransformFeedback一处 _cloneTo的 OR 逻辑也可以简化为target._setTransformFeedback(true)
简化建议
无重大简化空间。代码组织干净，shader/CPU 职责分明，曲线/常量双模式处理与其他模块一致。
唯一可以考虑的小优化：sampleSimplexNoise3D 用固定 offset d = 100.0 做三轴去相关，如果日后 simplex noise 库提供 3D→vec3 的原生接口，可以替换掉这个 workaround，但目前完全可以接受。
…ach other _setTransformFeedback now queries all TF-dependent modules (noise, limitVelocityOverLifetime) internally, so disabling one module no longer incorrectly tears down TF for the other.
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
为粒子系统新增 NoiseModule，基于 GPU simplex noise 实现湍流位移。经过 20 轮 commit 迭代，代码质量很高：TF 路径下通过解析基础位置避免反馈环路、octave 归一化保证输出 [-1,1]、uniform 打包为 2 个 vec4、strength 支持全部 4 种 ParticleCompositeCurve 模式、参数全部有合理 clamp。
上轮 P0（TF 生命周期互踩）已修复：_setTransformFeedback() 改为无参，内部查询 limitVelocityOverLifetime.enabled || noise.enabled 统一决策，两个模块的 enabled setter 不再互相干扰。_cloneTo 也统一为 target._setTransformFeedback()。设计干净，新增 TF 依赖模块时只需改一处。✅
以下是当前最终代码的剩余问题。
问题
P2
-
[P2]
NoiseModule.ts:288—scrollSpeed声明为ParticleCompositeCurve但 shader 只读constantMaxnoiseOctaveParams.set( this._scrollSpeed.constantMax, // ← Curve/TwoCurves/TwoConstants 模式全部被忽略 ... );
用户如果设置
scrollSpeed.mode = ParticleCurveMode.Curve，行为不符合预期——shader 永远只取constantMax。两个选择：- 如果当前版本只支持 constant，将类型降级为
number，避免 API 误导（推荐，可删 ~15 行 curve setter/listener 代码） - 如果打算支持 curve，需在 shader 侧增加采样逻辑
[Unity] Unity Noise Module 的 scrollSpeed 也是 constant-only（
float，不是 MinMaxCurve），所以降级为number是与 Unity 对齐的。 - 如果当前版本只支持 constant，将类型降级为
-
[P2]
noise_over_lifetime_module.glsl:80-82— TwoConstants/TwoCurves 随机插值三轴全用a_Random0.z同一通道sx = mix(..., sx, a_Random0.z); sy = mix(..., sy, a_Random0.z); // 同一个 random sz = mix(..., sz, a_Random0.z); // 同一个 random
三轴强度随机完全正相关——每个粒子要么三轴都偏大，要么都偏小。这与 Unity 行为一致（Unity 也是 single random per particle for noise strength），但值得在注释中说明这是有意设计，否则后续维护者可能会误认为是 bug。
-
[P2]
NoiseModule.ts:231-250— separateAxes 模式下三轴 curve mode 必须一致的隐式约束const isRandomCurveMode = separateAxes ? strengthX.mode === TwoCurves && strengthY.mode === TwoCurves && strengthZ.mode === TwoCurves : ...;
如果用户设置 X 为
Curve、Y 为Constant，会 fall through 到 constant 路径，Y 读constantMax而 X 的 curve 数据被忽略——静默错误。这与 VOL 的假设一致（VOL 也要求三轴 mode 一致），所以不阻塞，但建议在separateAxes的 JSDoc 中注明"启用时三轴 mode 应保持一致"。 -
[P2]
particle_feedback_simulation.glsl:246—noiseBasePos在 Local 模式下不含重力noiseBasePos = a_ShapePositionStartLifeTime.xyz + a_DirectionTime.xyz * a_StartSpeed * age;
这是纯初速度的解析位置，不含重力产生的位移。当重力显著时（如
gravityModifier = -9.8），粒子实际位置与noiseBasePos偏差越来越大，noise 采样点与视觉位置脱节，noise 看起来会"飘"。可以考虑加入重力解析解（
+ 0.5 * gravity * age²），它不依赖反馈不会引入环路，且计算开销很小（gravity uniform 已经在 TF shader 中了）。不阻塞——这是精度优化而非正确性问题。
P3
-
[P3]
e2e/case/particleRenderer-noise.ts:91-96— 被注释掉的 colorOverLifetime 代码应清理// colorOverLifetime.enabled = true; // colorOverLifetime.color.mode = ParticleGradientMode.Gradient; // ...
测试代码中的注释块没有调试价值，应删除。
-
[P3]
e2e/config.ts:437—diffPercentage: 0对 noise 测试过于严格Noise 涉及大量浮点运算，不同 GPU 的精度差异可能导致像素级偏差。
diffPercentage: 0意味着像素级完全一致，可能导致 CI 在不同平台上不稳定。建议设一个小容差（如0.01）。 -
[P3]
noise_over_lifetime_module.glsl:7— 注释中的参数名过时// renderer_NoiseOctaveParams: x = scrollSpeed, y = octaveCount, z = octaveIntensityMul, w = octaveFreqMulAPI 已重命名为
octavePersistence/octaveLacunarity，注释还在用旧名octaveIntensityMul/octaveFreqMul。建议同步更新。
简化建议
无重大简化空间。代码组织干净，shader/CPU 职责分明，曲线/常量双模式处理与其他模块一致。迭代质量很高——20 个 commit 从原型到生产级，每一步都在收敛正确方向。
唯一可考虑的优化：scrollSpeed 类型从 ParticleCompositeCurve 降级为 number，可删约 15 行代码，API 更诚实，且与 Unity 对齐。
…erence Reduce per-particle noise offset from 50 to 5 so nearby particles sample similar noise regions, producing coherent field-like turbulence instead of independent per-particle jittering at higher frequencies.
…eCurve to number Shader only ever read constantMax, so Curve/TwoCurves/TwoConstants modes were silently ignored. Aligns with Unity's constant-only scrollSpeed API.
GuoLei1990
left a comment
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
为粒子系统新增 NoiseModule，基于 GPU simplex noise 实现湍流位移。经过多轮迭代，代码质量很高：TF 生命周期管理已收敛为 _setTransformFeedback() 无参统一决策、参数全部有合理 clamp、uniform 打包为 2 个 vec4、strength 支持全部 4 种 ParticleCompositeCurve 模式、octave 归一化保证输出 [-1,1]、解析基础位置避免反馈环路。整体方向正确，代码结构与现有模块一致。
已修复的问题（对照之前 review）
- ✅ TF 生命周期互踩（前轮 P0） —
_setTransformFeedback()改为无参，内部 OR 查询两个模块，_cloneTo也统一。设计干净。 - ✅ 参数 clamp — frequency → 1e-6，octaveCount [1,3]，octavePersistence [0,1]，octaveLacunarity [1,4]
- ✅ Uniform 打包 — 2 个 vec4
- ✅ strength → ParticleCompositeCurve — 支持 Constant/TwoConstants/Curve/TwoCurves
- ✅ API 命名 — octavePersistence / octaveLacunarity，标准术语
- ✅ e2e 测试 — 已补充
- ✅ Bounds 绝对值 — 使用
Math.abs(noise.strengthX._getMax()) - ✅ 专用 random 通道 —
a_Random0.z（offset 21）
问题
P1
-
[P1]
noise_over_lifetime_module.glsl:48-50—noiseRandOffset的三轴去相关不够独立，存在周期性碰撞风险vec3 noiseRandOffset = vec3(_nr, fract(_nr * 314.159), fract(_nr * 271.828)) * 5.0;
fract(x * C)对于特定的_nr值会产生碰撞：当_nr接近 0 或 1 时，三个分量都趋近 0，导致三轴采样位置几乎重叠，noise 退化为各轴相同的位移（视觉上粒子沿对角线移动而非湍流扩散）。更根本的问题是：
_nr是 [0,1) 的单个随机数，通过fract变换得到的 3 个值并非独立均匀分布。fract(x * 314.159)本质上是一个简单的 hash，对于均匀分布的输入，输出的分布质量取决于乘数的无理程度，但输入的精度有限（float 只有 ~23 bit mantissa），实际分布会有微弱的结构性偏差。建议：改用
a_Random0.z（已有值）加上a_Random0.w和a_Random1.x（如果这些通道空闲）作为三个独立随机源。如果通道不够用，至少选择更好的 hash 常数或使用位运算 hash（如fract(sin(_nr * 12345.6789) * 43758.5453)）来改善独立性。或者接受当前方案，但在注释中说明此限制。不过如果
a_Random0的其他通道都已被其他模块占用，当前方案作为工程折中可以接受——只是需要注释说明三轴并非完全独立。
P2
-
[P2]
NoiseModule.ts—scrollSpeed类型为number但 e2e 测试直接赋值0，应保持一致（已是 number，这条确认无问题）。但_scrollSpeed在构造函数中设置了两次private _scrollSpeed: number = 0; // 字段初始化 ... constructor(generator: ParticleGenerator) { super(generator); this.strengthX = new ParticleCompositeCurve(1); this.strengthY = new ParticleCompositeCurve(1); this.strengthZ = new ParticleCompositeCurve(1); this._scrollSpeed = 0; // 冗余赋值 }
构造函数中
this._scrollSpeed = 0是冗余的，字段初始化已经设为 0。删除即可。 -
[P2]
noise_over_lifetime_module.glsl:79-82— TwoConstants/TwoCurves 随机插值三轴全用a_Random0.z同一通道sx = mix(..., sx, a_Random0.z); sy = mix(..., sy, a_Random0.z); // 同一个 random sz = mix(..., sz, a_Random0.z); // 同一个 random
三轴强度随机完全正相关。这与 Unity 行为一致（Unity 也是 single random per particle for noise strength），但建议添加注释说明这是有意设计，避免后续维护者误认为 bug。
-
[P2]
NoiseModule.ts:231-250— separateAxes 模式下三轴 curve mode 必须一致的隐式约束如果用户设置 X 为
Curve、Y 为Constant，会 fall through 到 constant 路径，Y 读constantMax而 X 的 curve 数据被忽略——静默错误。这与 VOL 的行为一致（VOL 也有此假设），不阻塞，但建议在separateAxes的 JSDoc 中注明"启用时三轴 mode 应保持一致"。 -
[P2]
particle_feedback_simulation.glsl:246—noiseBasePos不含重力分量noiseBasePos = a_ShapePositionStartLifeTime.xyz + a_DirectionTime.xyz * a_StartSpeed * age;
纯初速度解析位置，不含
0.5 * gravity * age²。当重力显著时（如模拟喷泉下落），粒子实际位置与noiseBasePos偏差大，noise 效果看起来会"飘"。gravity 的解析解不依赖反馈，不会引入环路，计算开销极小（
renderer_Gravityuniform 已经在 TF shader 中），可以考虑加上：noiseBasePos += 0.5 * renderer_Gravity * a_Random0.x * age * age;
不阻塞——精度优化而非正确性问题。
-
[P2]
NoiseModule.ts:84—noise属性声明处有@deepClone，但赋值在构造函数中在
ParticleGenerator.ts中：@deepClone readonly noise: NoiseModule; ... // constructor: this.noise = new NoiseModule(this);
@deepClone装饰器在构造函数赋值模式下是否被框架正确处理？对比其他同模式的模块（forceOverLifetime、limitVelocityOverLifetime），它们在ParticleGenerator声明处是否也有@deepClone？如果没有，这里应该删掉以保持一致；如果有，则无问题。需确认框架 clone 机制对构造函数赋值 +@deepClone的行为。
P3
-
[P3]
e2e/case/particleRenderer-noise.ts:91-96— 被注释掉的 colorOverLifetime 代码应清理测试代码中的注释块没有价值，应删除。
-
[P3]
e2e/config.ts:132—diffPercentage: 0对 noise 测试过于严格Noise 涉及大量浮点运算，不同 GPU 精度差异可能导致像素级偏差。
diffPercentage: 0意味着像素级完全一致，可能导致 CI 在不同平台上不稳定。建议设一个小容差（如0.01）。 -
[P3]
noise_over_lifetime_module.glsl:7— 注释中的参数名已过时注释写的是
octaveIntensityMul/octaveFreqMul（如果从旧版本遗留），而 API 已重命名为octavePersistence/octaveLacunarity。请确认注释与实际 uniform 命名是否一致。当前代码注释写的是z = octavePersistence, w = octaveLacunarity，如果已同步则忽略此条。
简化建议
无重大简化空间。代码组织干净，shader/CPU 职责分明，曲线/常量双模式处理与其他模块一致。
唯一值得考虑的优化：noiseBasePos 加入重力解析解（上述 P2），一行代码即可让重力场景下 noise 采样更精准。
2 participants
Summary
NoiseModuleto the particle system, referencing Unity's Noise Modulenoise_common+noise_simplex_3Dshader librariesUsage
Test plan
npm run buildpassesSummary by CodeRabbit
New Features
Bug Fixes