particle support limit velocity over lifetime#2925
Open
hhhhkrx wants to merge 44 commits intogalacean:dev/2.0from
Open
particle support limit velocity over lifetime#2925hhhhkrx wants to merge 44 commits intogalacean:dev/2.0from
hhhhkrx wants to merge 44 commits intogalacean:dev/2.0from
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds GPU transform‑feedback particle simulation, a new LimitVelocityOverLifetime particle module, WebGL2 transform‑feedback platform and primitive implementations, shader compilation/path updates, buffer/layout additions, generator/renderer integration, unit and e2e tests, and related RHI/shader surface exports. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant ParticleRenderer
participant ParticleGenerator
participant TFSimulator
participant ShaderProgram
participant WebGLContext
App->>ParticleRenderer: update(deltaTime)
ParticleRenderer->>ParticleGenerator: _update(shaderData, deltaTime)
alt Transform Feedback enabled
ParticleGenerator->>TFSimulator: update(shaderData, particleCount, firstActive, firstFree, deltaTime)
TFSimulator->>ShaderProgram: compile/retrieve(program with feedback varyings)
TFSimulator->>WebGLContext: bindTransformFeedback()
WebGLContext->>TFSimulator: beginTransformFeedback(POINTS)
TFSimulator->>WebGLContext: drawArrays(POINTS, firstActive, count)
WebGLContext-->>TFSimulator: captured feedback (position, velocity)
TFSimulator->>WebGLContext: endTransformFeedback()
TFSimulator->>TFSimulator: swap read/write buffers
TFSimulator-->>ParticleGenerator: provide readBinding for render
end
ParticleRenderer->>WebGLContext: render using instance buffers (may use TF read binding)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #2925 +/- ##
===========================================
+ Coverage 77.29% 78.31% +1.01%
===========================================
Files 884 895 +11
Lines 96638 98083 +1445
Branches 9509 9646 +137
===========================================
+ Hits 74697 76809 +2112
+ Misses 21777 21110 -667
Partials 164 164
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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/case/particleRenderer-limitVelocity.ts`:
- Around line 27-65: The test never uses the imported E2E helpers so the
screenshot harness may capture before particles/textures finish; at the end of
createScalarLimitParticle add calls to updateForE2E(engine, 500) and
initScreenshot(engine, camera) so the engine has time to settle and the camera
frame is registered before the reference screenshot is taken — locate the calls
inside the async particle setup sequence (referencing createScalarLimitParticle,
engine and camera) and append updateForE2E and initScreenshot accordingly.
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 254-269: _isRandomMode currently only considers
_limitX/_limitY/_limitZ and thus returns false when drag uses
TwoConstants/TwoCurves, causing ParticleGenerator to skip per-particle randoms
(Random2.w) and produce deterministic drag; update _isRandomMode (and the
equivalent checks in the ranges 274-323 and 427-448) to include the module's
drag random modes as well (i.e., check the drag-related curve/mode fields used
by _uploadDrag for TwoConstants and TwoCurves) so the module-wide "needs
per-particle random" decision returns true whenever drag is in a random mode and
ParticleGenerator will allocate Random2.w accordingly. Ensure you reference and
update the same logic in the _isRandomMode helper(s) and harmonize it with
_uploadDrag's accepted modes so behavior is consistent.
- Around line 360-425: The function _uploadSeparateAxisLimits currently coerces
mixed per-axis modes to the constant path; update it to handle each axis
individually by checking limitX.mode, limitY.mode, limitZ.mode separately and
uploading the correct representation per axis: use
LimitVelocityOverLifetimeModule._limitXMaxCurveProperty/_limitXMinCurveProperty
(and Y/Z equivalents) when an axis is Curve/TwoCurves, and write the
corresponding component into
LimitVelocityOverLifetimeModule._limitMaxConstVecProperty/_limitMinConstVecProperty
for Constant/TwoConstants axes; set per-axis random flags and mode macros
(introduce or reuse per-axis macros if needed instead of the single
_limitCurveModeMacro/_limitConstantModeMacro) so the shader can distinguish
which axes are curves vs constants, and ensure _uploadSeparateAxisLimits returns
the appropriate macros (modeMacro/randomMacro) reflecting per-axis state rather
than forcing all-constant fallback.
In `@tests/src/core/particle/LimitVelocityOverLifetime.test.ts`:
- Around line 216-239: The tests override the global performance.now in multiple
places (e.g., inside the "enabling module triggers shader update without error"
case that manipulates engine._vSyncCount and engine._time._lastSystemTime and
calls engine.update()), but never restore it; capture the original
performance.now before replacing it and ensure you restore it in a finally block
or an afterEach hook so the global clock is returned to its original function
after each smoke test; update all occurrences that set performance.now
(including the other similar test blocks) to follow this pattern so later specs
are not affected by the mocked clock.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e9b43c5e-3212-4921-8132-1ec2c353b844
⛔ Files ignored due to path filters (3)
packages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/force_over_lifetime_module.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/limit_velocity_over_lifetime_module.glslis excluded by!**/*.glsl
📒 Files selected for processing (8)
e2e/case/particleRenderer-limitVelocity.tse2e/config.tspackages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/enums/ParticleRandomSubSeeds.tspackages/core/src/particle/index.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.tspackages/core/src/shaderlib/particle/index.tstests/src/core/particle/LimitVelocityOverLifetime.test.ts
Comment on lines
+216
to
+239
| it("enabling module triggers shader update without error", function () { | ||
| const lvl = particleRenderer.generator.limitVelocityOverLifetime; | ||
| lvl.enabled = true; | ||
| lvl.limit = new ParticleCompositeCurve(5); | ||
| lvl.dampen = 0.8; | ||
| lvl.drag = new ParticleCompositeCurve(0.5); | ||
|
|
||
| // Should not throw when updating shader data | ||
| particleRenderer.generator.play(); | ||
| expect(() => { | ||
| //@ts-ignore | ||
| engine._vSyncCount = Infinity; | ||
| //@ts-ignore | ||
| engine._time._lastSystemTime = 0; | ||
| let times = 0; | ||
| performance.now = function () { | ||
| times++; | ||
| return times * 100; | ||
| }; | ||
| for (let i = 0; i < 10; ++i) { | ||
| engine.update(); | ||
| } | ||
| }).to.not.throw(); | ||
| }); |
There was a problem hiding this comment.
Restore performance.now after these smoke tests.
Line 231, Line 257, and Line 282 overwrite the global clock and never restore it. That leaks into later specs and makes the browser suite order-dependent. Save the original function and put it back in finally or afterEach.
Suggested pattern
+ const originalNow = performance.now;
expect(() => {
- performance.now = function () {
- times++;
- return times * 100;
- };
- for (let i = 0; i < 10; ++i) {
- engine.update();
- }
+ try {
+ performance.now = function () {
+ times++;
+ return times * 100;
+ };
+ for (let i = 0; i < 10; ++i) {
+ engine.update();
+ }
+ } finally {
+ performance.now = originalNow;
+ }
}).to.not.throw();Also applies to: 241-265, 267-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/src/core/particle/LimitVelocityOverLifetime.test.ts` around lines 216 -
239, The tests override the global performance.now in multiple places (e.g.,
inside the "enabling module triggers shader update without error" case that
manipulates engine._vSyncCount and engine._time._lastSystemTime and calls
engine.update()), but never restore it; capture the original performance.now
before replacing it and ensure you restore it in a finally block or an afterEach
hook so the global clock is returned to its original function after each smoke
test; update all occurrences that set performance.now (including the other
similar test blocks) to follow this pattern so later specs are not affected by
the mocked clock.
- Implement per-frame TF-based particle simulation matching Unity's ClampVelocityModule (dampen, drag, execution order, world space handling) - Add TransformFeedbackPrimitive for reusable ping-pong buffer/VAO management - Add TransformFeedback/GLTransformFeedback platform abstraction layer - Remove inaccurate 8-step approximation fallback (WebGL1 ignores module) - Optimize TF pass to only process alive particles via bindBufferRange - Optimize instance buffer upload with Discard mode for TF Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use VertexElement instead of custom TFAttributeDescriptor - Auto-detect VAO rebuild via _lastProgramId (no manual markVAODirty) - Extract attribute descriptors as static readonly arrays - Remove unnecessary comments and simplify code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GuoLei1990
force-pushed
the
feat/limitParticle
branch
from
272bf29 to
2193eb7
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
e2e/case/particleRenderer-limitVelocity.ts (1)
27-27:⚠️ Potential issue | 🟡 MinorHook this case into the screenshot harness.
initScreenshotandupdateForE2Eare still unused, so the capture can happen before the texture load and first burst settle. Please callupdateForE2E(engine, 500)andinitScreenshot(engine, camera)after the particle setup completes, even if that means threadingcameraintocreateScalarLimitParticle().Also applies to: 63-65, 68-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/case/particleRenderer-limitVelocity.ts` at line 27, After the particle setup completes, hook the test into the screenshot harness by calling updateForE2E(engine, 500) and then initScreenshot(engine, camera); if camera isn't available where you build the particle case, thread the camera through (e.g. add a camera parameter to createScalarLimitParticle or return the camera from the setup) so you can call initScreenshot(engine, camera) immediately after setup; apply the same change to the other similar cases referenced (the blocks that import initScreenshot/updateForE2E).packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (2)
256-270:⚠️ Potential issue | 🟠 MajorTreat randomized drag as a random-mode configuration.
_uploadDrag()supportsTwoConstantsandTwoCurves, but_isRandomMode()still only checks the limit curves. In separate-axes mode it also requires every axis to be randomized. The module-level random check should flip on whenever any limit axis or drag uses a randomized mode; otherwise the generator skips the per-particle random this shader path needs and drag becomes deterministic.Also applies to: 429-448
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 256 - 270, The _isRandomMode() function currently only inspects _limitX/_limitY/_limitZ modes (and requires all axes randomized when _separateAxes is true), but it must also treat randomized drag as random-mode so the per-particle random shader path is enabled; update _isRandomMode() to return true if any of _limitX/_limitY/_limitZ is in ParticleCurveMode.TwoConstants or TwoCurves OR if the drag configuration inspected by _uploadDrag() is in TwoConstants/TwoCurves (and when _separateAxes is true, consider each axis ORed rather than requiring all three), ensuring the method references the same drag mode checks used by _uploadDrag() so randomized drag flips the random-mode flag; apply the same fix to the other _isRandomMode() copy around the 429-448 area.
362-424:⚠️ Potential issue | 🟠 MajorDon't silently fall back to constants for mixed per-axis modes.
Anything other than “all curves” or “all constants” drops into the constant upload branch. A setup like
limitX = Curve,limitY = Constant,limitZ = TwoConstantsloses the authored curve/random data instead of matching the per-axis configuration. Either reject mixed setups or upload each axis independently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 362 - 424, In _uploadSeparateAxisLimits detect and handle each axis mode individually instead of falling back to the “all curves” vs “all constants” branches: inspect limitX.limitY.limitZ modes and for each axis upload the appropriate data (for curves use _limit{X|Y|Z}MaxCurveProperty and _limit{X|Y|Z}MinCurveProperty when TwoCurves, for constants use _limitMaxConstVecProperty/_limitMinConstVecProperty but only set the corresponding component), set modeMacro to reflect per-axis curve/constant usage (or compute a composite shader macro logic) and set randomMacro only if any axis is random (TwoCurves/TwoConstants) while preserving each axis’s authored values; modify _uploadSeparateAxisLimits, the uses of LimitVelocityOverLifetimeModule._limitXMaxCurveProperty/_limitXMinCurveProperty/_limitY.../_limitZ... and the constant vector packing logic to write per-component values rather than globally switching branches.
🤖 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/graphic/TransformFeedbackPrimitive.ts`:
- Around line 89-105: updateVAOs currently skips rebuilding VAOs when only
program.id matches _lastProgramId, causing stale VAO bindings if the underlying
buffers are reallocated; update the cache key logic in updateVAOs (and related
fields) to include the current transform buffers (this._readBuffer and
this._writeBuffer) so the VAOs are recreated when either buffer changes: compare
buffer identity (e.g., buffer.id or the buffer object references) against stored
lastReadBuffer/lastWriteBuffer (or store a composite _lastKey) and call
_deleteVAOs()/_createVAO(...) whenever program.id or either buffer has changed,
then update those stored buffer ids/refs along with _lastProgramId.
- Around line 58-84: resize() must reset the ping-pong state so an odd number of
prior swap() calls doesn't leave _useA false and cause read/write to the same
buffer; after recreating and assigning _readBuffer and _writeBuffer in
TransformFeedbackPrimitive.resize(), set the ping-pong flag (this._useA) to a
known side (e.g., true) and ensure the current render binding
(_renderBufferBinding) points at the new read buffer so subsequent draw()/swap()
behavior is consistent with the new buffers.
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 625-651: When enabling TF at runtime in _setTFMode, resizing the
transform feedback buffers is insufficient because existing live particles'
state must be copied into TF storage; update _setTFMode so after
creating/resizing this._transformFeedback (ParticleTransformFeedbackSimulator)
you backfill the TF buffers from the current CPU/GPU particle state (e.g. read
the geometry/attribute buffers or the particle system's CPU arrays for position,
velocity, life, etc.) into the transform feedback buffers before enabling the TF
macro (renderer.shaderData.enableMacro(ParticleGenerator._tfModeMacro)),
ensuring the first TF update sees correct data; keep the call to
_reorganizeGeometryBuffers() but perform the backfill immediately after resize
and before any TF-driven update.
- Around line 1023-1036: The world-space TF init currently uses identity
rotation; instead read the captured a_SimulationWorldRotation from the instance
buffer (stored at offset+30) when main.simulationSpace !==
ParticleSimulationSpace.Local and pass that quaternion into _initTFParticle so
the emitted shape position is rotated the same way as the non-TF path; update
the call sites that currently set qx/qy/qz/qw to identity to extract the
quaternion from the instance buffer (or forward it) and ensure _initTFParticle
consumes that quaternion rather than assuming identity, keeping the existing
local-space branch that uses transform.worldRotationQuaternion.
In `@packages/core/src/shader/ShaderProgram.ts`:
- Around line 267-274: The Transform Feedback setup in ShaderProgram currently
casts gl to WebGL2RenderingContext without validation; update the block in the
method that handles transformFeedbackVaryings to first assert that the context
is WebGL2 (e.g., using a type guard like "gl instanceof WebGL2RenderingContext"
or checking an existing "isWebGL2" flag) and early-return if it's not WebGL2,
then call (<WebGL2RenderingContext>gl).transformFeedbackVaryings only when that
guard passes to avoid unsafe casts and runtime errors for future callers.
In `@packages/rhi-webgl/src/GLTransformFeedback.ts`:
- Around line 13-17: The GLTransformFeedback constructor calls
createTransformFeedback() on rhi.gl without ensuring it's a WebGL2 context,
causing a TypeError for WebGL1; update the GLTransformFeedback constructor to
check that rhi.gl is an instance of WebGL2RenderingContext (or that
rhi.supportsTransformFeedback/WebGL2 flag is true) before calling
createTransformFeedback(), and if not supported either set _glTransformFeedback
to null/undefined and mark the instance as unsupported or throw a clear error;
make sure callers via WebGLGraphicDevice.createPlatformTransformFeedback(),
TransformFeedback, and TransformFeedbackPrimitive handle the unsupported case
consistently.
---
Duplicate comments:
In `@e2e/case/particleRenderer-limitVelocity.ts`:
- Line 27: After the particle setup completes, hook the test into the screenshot
harness by calling updateForE2E(engine, 500) and then initScreenshot(engine,
camera); if camera isn't available where you build the particle case, thread the
camera through (e.g. add a camera parameter to createScalarLimitParticle or
return the camera from the setup) so you can call initScreenshot(engine, camera)
immediately after setup; apply the same change to the other similar cases
referenced (the blocks that import initScreenshot/updateForE2E).
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 256-270: The _isRandomMode() function currently only inspects
_limitX/_limitY/_limitZ modes (and requires all axes randomized when
_separateAxes is true), but it must also treat randomized drag as random-mode so
the per-particle random shader path is enabled; update _isRandomMode() to return
true if any of _limitX/_limitY/_limitZ is in ParticleCurveMode.TwoConstants or
TwoCurves OR if the drag configuration inspected by _uploadDrag() is in
TwoConstants/TwoCurves (and when _separateAxes is true, consider each axis ORed
rather than requiring all three), ensuring the method references the same drag
mode checks used by _uploadDrag() so randomized drag flips the random-mode flag;
apply the same fix to the other _isRandomMode() copy around the 429-448 area.
- Around line 362-424: In _uploadSeparateAxisLimits detect and handle each axis
mode individually instead of falling back to the “all curves” vs “all constants”
branches: inspect limitX.limitY.limitZ modes and for each axis upload the
appropriate data (for curves use _limit{X|Y|Z}MaxCurveProperty and
_limit{X|Y|Z}MinCurveProperty when TwoCurves, for constants use
_limitMaxConstVecProperty/_limitMinConstVecProperty but only set the
corresponding component), set modeMacro to reflect per-axis curve/constant usage
(or compute a composite shader macro logic) and set randomMacro only if any axis
is random (TwoCurves/TwoConstants) while preserving each axis’s authored values;
modify _uploadSeparateAxisLimits, the uses of
LimitVelocityOverLifetimeModule._limitXMaxCurveProperty/_limitXMinCurveProperty/_limitY.../_limitZ...
and the constant vector packing logic to write per-component values rather than
globally switching branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 135872d8-619c-4b54-83ae-4a092ee18b8f
⛔ Files ignored due to path filters (4)
packages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/limit_velocity_over_lifetime_module.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/particle_common.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/particle_transform_feedback_update.glslis excluded by!**/*.glsl
📒 Files selected for processing (16)
e2e/case/particleRenderer-limitVelocity.tspackages/core/src/graphic/Buffer.tspackages/core/src/graphic/TransformFeedback.tspackages/core/src/graphic/TransformFeedbackPrimitive.tspackages/core/src/graphic/enums/BufferBindFlag.tspackages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/ParticleRenderer.tspackages/core/src/particle/ParticleTransformFeedbackSimulator.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.tspackages/core/src/renderingHardwareInterface/IPlatformTransformFeedback.tspackages/core/src/renderingHardwareInterface/index.tspackages/core/src/shader/ShaderProgram.tspackages/core/src/shaderlib/particle/index.tspackages/rhi-webgl/src/GLBuffer.tspackages/rhi-webgl/src/GLTransformFeedback.tspackages/rhi-webgl/src/WebGLGraphicDevice.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/graphic/enums/BufferBindFlag.ts
Comment on lines
+267
to
+274
| // Set Transform Feedback varyings before linking (WebGL2 only) | ||
| if (transformFeedbackVaryings?.length) { | ||
| (<WebGL2RenderingContext>gl).transformFeedbackVaryings( | ||
| program, | ||
| transformFeedbackVaryings, | ||
| (<WebGL2RenderingContext>gl).INTERLEAVED_ATTRIBS | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all ShaderProgram callsites that pass a 4th argument are WebGL2-gated.
rg -nP --type=ts -C3 'new\s+ShaderProgram\s*\([^,]+,\s*[^,]+,\s*[^,]+,\s*[^)]+\)'
rg -nP --type=ts -C3 '\bisWebGL2\b|transformFeedbackVaryings'Repository: galacean/engine
Length of output: 44452
Consider adding WebGL2 validation in the Transform Feedback varyings setup.
The code at lines 268–273 unconditionally casts gl to WebGL2RenderingContext. While the only known call site in ParticleTransformFeedbackSimulator.ts:168 is already guarded with if (!engine._hardwareRenderer.isWebGL2) return;, the function itself lacks internal protection. A future caller could pass transformFeedbackVaryings without the guard, causing a runtime error. Adding a defensive check (e.g., type guard or early return) would improve safety and prevent potential bugs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/shader/ShaderProgram.ts` around lines 267 - 274, The
Transform Feedback setup in ShaderProgram currently casts gl to
WebGL2RenderingContext without validation; update the block in the method that
handles transformFeedbackVaryings to first assert that the context is WebGL2
(e.g., using a type guard like "gl instanceof WebGL2RenderingContext" or
checking an existing "isWebGL2" flag) and early-return if it's not WebGL2, then
call (<WebGL2RenderingContext>gl).transformFeedbackVaryings only when that guard
passes to avoid unsafe casts and runtime errors for future callers.
Comment on lines
+13
to
+17
| constructor(rhi: WebGLGraphicDevice) { | ||
| const gl = <WebGL2RenderingContext>rhi.gl; | ||
| this._gl = gl; | ||
| this._glTransformFeedback = gl.createTransformFeedback(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "WebGL2-only transform feedback API usage:"
rg -n -C2 'createTransformFeedback|bindTransformFeedback|beginTransformFeedback|endTransformFeedback' packages/rhi-webgl/src
echo
echo "Creation path into GLTransformFeedback:"
rg -n -C2 'createPlatformTransformFeedback|new GLTransformFeedback|new TransformFeedback\(' packagesRepository: galacean/engine
Length of output: 3528
🏁 Script executed:
# Check WebGLGraphicDevice.createPlatformTransformFeedback and surrounding context
sed -n '265,280p' packages/rhi-webgl/src/WebGLGraphicDevice.ts
# Check TransformFeedback constructor and surrounding context
sed -n '10,25p' packages/core/src/graphic/TransformFeedback.ts
# Check TransformFeedbackPrimitive constructor and surrounding context
sed -n '40,60p' packages/core/src/graphic/TransformFeedbackPrimitive.ts
# Check if isWebGL2 is used elsewhere for feature gating
rg -n 'isWebGL2' packagesRepository: galacean/engine
Length of output: 13563
Guard this constructor against WebGL1 contexts.
rhi.gl can still be a WebGLRenderingContext, and createTransformFeedback() is WebGL2-only. On WebGL1 this becomes a TypeError immediately when instantiated.
The creation path through WebGLGraphicDevice.createPlatformTransformFeedback() and the public TransformFeedback and TransformFeedbackPrimitive constructors have no upstream WebGL2 checks, so the guard must be placed here.
Suggested fix
constructor(rhi: WebGLGraphicDevice) {
+ if (!rhi.isWebGL2) {
+ throw new Error("Transform Feedback requires a WebGL2 context.");
+ }
const gl = <WebGL2RenderingContext>rhi.gl;
this._gl = gl;
this._glTransformFeedback = gl.createTransformFeedback();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rhi-webgl/src/GLTransformFeedback.ts` around lines 13 - 17, The
GLTransformFeedback constructor calls createTransformFeedback() on rhi.gl
without ensuring it's a WebGL2 context, causing a TypeError for WebGL1; update
the GLTransformFeedback constructor to check that rhi.gl is an instance of
WebGL2RenderingContext (or that rhi.supportsTransformFeedback/WebGL2 flag is
true) before calling createTransformFeedback(), and if not supported either set
_glTransformFeedback to null/undefined and mark the instance as unsupported or
throw a clear error; make sure callers via
WebGLGraphicDevice.createPlatformTransformFeedback(), TransformFeedback, and
TransformFeedbackPrimitive handle the unsupported case consistently.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (2)
256-265:⚠️ Potential issue | 🟠 Major
_isRandomMode()doesn't account for drag's random modes.The method only checks
_limitX,_limitY,_limitZmodes but ignores_drag. When drag usesTwoConstantsorTwoCurveswhile limits use non-random modes, the particle generator won't allocate per-particle random values for drag interpolation, causing all particles to use the same drag factor.Suggested fix to include drag mode check
_isRandomMode(): boolean { + const dragRandom = this._drag.mode === ParticleCurveMode.TwoConstants || + this._drag.mode === ParticleCurveMode.TwoCurves; if (this._separateAxes) { return ( (this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves) && (this._limitY.mode === ParticleCurveMode.TwoConstants || this._limitY.mode === ParticleCurveMode.TwoCurves) && (this._limitZ.mode === ParticleCurveMode.TwoConstants || this._limitZ.mode === ParticleCurveMode.TwoCurves) - ); + ) || dragRandom; } - return this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves; + return this._limitX.mode === ParticleCurveMode.TwoConstants || + this._limitX.mode === ParticleCurveMode.TwoCurves || + dragRandom; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 256 - 265, The _isRandomMode() method currently checks only _limitX/_limitY/_limitZ modes and misses _drag; update _isRandomMode in LimitVelocityOverLifetimeModule to include a check that _drag.mode is also TwoConstants or TwoCurves (when deciding random mode), i.e., when _separateAxes is true require both all three limit axes and _drag be random-mode, and when _separateAxes is false require either _limitX or _drag to be TwoConstants/TwoCurves so per-particle random values are allocated for drag interpolation.
356-421:⚠️ Potential issue | 🟠 MajorMixed per-axis modes silently fall back to constant upload.
When
separateAxesis enabled with mixed modes (e.g.,limitX = Curve,limitY = Constant,limitZ = Curve), lines 368-373 require all axes to be curves, so the code falls through to the constant path (line 402+). This silently discards curve data for axes configured with curves.Consider either:
- Supporting per-axis mode representation in the shader
- Rejecting/warning on mixed-mode configurations
- Documenting this as expected behavior
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 356 - 421, In _uploadSeparateAxisLimits detect mixed per-axis modes (using limitX/limitY/limitZ and ParticleCurveMode) and avoid silently falling into the constant branch: either (A) implement per-axis uploads by setting per-axis curve/constant shader data (call shaderData.setFloatArray for curve axes using limitX.curveMax/_curveMin and shaderData.setVector3 for constant axes) and introduce per-axis mode macros (e.g., add/check new macros like _limitXCurveMacro/_limitYCurveMacro/_limitZCurveMacro and still set _limitIsRandomMacro where applicable), or (B) if you prefer not to support mixed modes, throw/log a clear warning/error from _uploadSeparateAxisLimits when modes differ instead of uploading constants only; update the function _uploadSeparateAxisLimits, references to LimitVelocityOverLifetimeModule._limitCurveModeMacro/_limitConstantModeMacro/_limitIsRandomMacro, and the shaderData.setFloatArray/setVector3 calls accordingly so curve data is not discarded silently.
🧹 Nitpick comments (5)
e2e/case/particleRenderer-limitVelocity.ts (1)
136-147: Remove commented-out code.Lines 136 and 143-146 contain commented-out alternative configurations. These should either be removed or, if they serve as documentation for alternative test scenarios, converted to separate test functions.
Suggested cleanup
limitVelocityOverLifetime.limitZ = new ParticleCompositeCurve(0); - // limitVelocityOverLifetime.limit = new ParticleCompositeCurve(1); limitVelocityOverLifetime.space = ParticleSimulationSpace.World; limitVelocityOverLifetime.dampen = 0.25; limitVelocityOverLifetime.drag = new ParticleCompositeCurve(0.0); limitVelocityOverLifetime.multiplyDragByParticleSize = true; limitVelocityOverLifetime.multiplyDragByParticleVelocity = true; - - // limitVelocityOverLifetime.enabled = true; - // limitVelocityOverLifetime.separateAxes = false; - // limitVelocityOverLifetime.limit = new ParticleCompositeCurve(0); - // limitVelocityOverLifetime.dampen = 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/case/particleRenderer-limitVelocity.ts` around lines 136 - 147, Remove the commented-out alternative configurations around the limitVelocityOverLifetime block: delete the commented lines referencing ParticleCompositeCurve, enabled, separateAxes, and dampen (the comments at and around the limitVelocityOverLifetime assignment), or if those variants are useful as documented alternatives, extract them into separate test functions instead of leaving them commented; locate the limitVelocityOverLifetime usage and ParticleCompositeCurve / ParticleSimulationSpace references in this file to remove or refactor the commented alternatives accordingly.packages/core/src/graphic/TransformFeedbackPrimitive.ts (2)
154-160: Consider reusingVertexBufferBindingobjects to reduce allocations.
swap()creates a newVertexBufferBindinginstance on every call. Since TF updates happen every frame for active particle systems, this causes per-frame allocations. Consider maintaining two pre-allocated binding objects and swapping between them.Suggested optimization
+ private _renderBufferBindingA: VertexBufferBinding; + private _renderBufferBindingB: VertexBufferBinding; + private _useBindingA = true; resize(vertexCount: number): void { // ... existing code ... this._readBuffer = readBuffer; this._writeBuffer = writeBuffer; - this._renderBufferBinding = new VertexBufferBinding(this._readBuffer, this._byteStride); + this._renderBufferBindingA = new VertexBufferBinding(readBuffer, this._byteStride); + this._renderBufferBindingB = new VertexBufferBinding(writeBuffer, this._byteStride); + this._renderBufferBinding = this._renderBufferBindingA; + this._useBindingA = true; // ... } swap(): void { const temp = this._readBuffer; this._readBuffer = this._writeBuffer; this._writeBuffer = temp; this._currentVAO = this._currentVAO === this._vaoA ? this._vaoB : this._vaoA; - this._renderBufferBinding = new VertexBufferBinding(this._readBuffer, this._byteStride); + this._useBindingA = !this._useBindingA; + this._renderBufferBinding = this._useBindingA ? this._renderBufferBindingA : this._renderBufferBindingB; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/graphic/TransformFeedbackPrimitive.ts` around lines 154 - 160, The swap() method in TransformFeedbackPrimitive currently allocates a new VertexBufferBinding each call (this._renderBufferBinding = new VertexBufferBinding(...)); instead, pre-allocate two VertexBufferBinding instances (e.g., this._bindingA / this._bindingB) as members alongside _vaoA/_vaoB and initialize them for the corresponding buffers and _byteStride, then in swap() simply toggle which binding is active (assign this._renderBufferBinding = this._bindingA or this._bindingB) and update their internal buffer reference/stride if necessary; update any initialization code that creates _renderBufferBinding to use the pre-allocated pair and ensure _renderBufferBinding is swapped when _readBuffer/_writeBuffer and _currentVAO are swapped.
61-86: Consider resetting_currentVAOreference inresize()to avoid stale reference.After
resize()sets_vaoDirty = true, the old VAOs will be deleted inrebuildVAOsIfNeeded(). However, ifbindVAO()were called beforerebuildVAOsIfNeeded(),_currentVAOwould reference a deleted VAO. While the current usage pattern appears safe (rebuild is always called before bind), explicitly nullifying_currentVAOinresize()would make the code more defensive.Suggested defensive fix
this._vertexCount = vertexCount; this._initialized = true; this._vaoDirty = true; + this._currentVAO = null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/graphic/TransformFeedbackPrimitive.ts` around lines 61 - 86, The resize() method should clear the cached VAO reference to avoid holding a pointer to a VAO that will be destroyed; add a line to set this._currentVAO = null (or undefined consistent with class) when resizing—ideally right after setting this._vaoDirty = true—so that rebuildVAOsIfNeeded() can safely recreate VAOs and bindVAO() cannot reference a deleted object.packages/core/src/particle/ParticleTransformFeedbackSimulator.ts (2)
127-127: Replace magic number with GL constant for clarity.
POINTS = 0x0000is the raw WebGL constant value. Consider usinggl.POINTSfor better readability and maintainability.Suggested fix
+ const gl = this._engine._hardwareRenderer.gl; // Bind VAO and execute TF for alive particles this._primitive.bindVAO(); - const POINTS = 0x0000; if (firstActive < firstFree) { - this._primitive.draw(rhi, POINTS, firstActive, firstFree - firstActive); + this._primitive.draw(rhi, gl.POINTS, firstActive, firstFree - firstActive); } else { - this._primitive.draw(rhi, POINTS, firstActive, particleCount - firstActive); + this._primitive.draw(rhi, gl.POINTS, firstActive, particleCount - firstActive); if (firstFree > 0) { - this._primitive.draw(rhi, 0, firstFree); + this._primitive.draw(rhi, gl.POINTS, 0, firstFree); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts` at line 127, Replace the magic numeric literal used for POINTS with the WebGL constant from the existing GL context: change the declaration that sets POINTS = 0x0000 to use the in-scope WebGLRenderingContext constant (e.g., POINTS = gl.POINTS or this.gl.POINTS depending on how the context is referenced in ParticleTransformFeedbackSimulator) so the code uses the named constant instead of the raw hex value.
109-113: Per-frameVertexBufferBindingallocation in update loop.
VertexBufferBindingis instantiated on everyupdate()call (line 110). Since this runs every frame for active particle systems, it creates unnecessary GC pressure. Consider caching and reusing the binding when the instance buffer hasn't changed.Suggested optimization
+ private _cachedInstanceBuffer: Buffer; + private _cachedInstanceBinding: VertexBufferBinding; update( instanceBuffer: Buffer, // ... ): void { // ... - const instanceBinding = new VertexBufferBinding(instanceBuffer, ParticleBufferUtils.instanceVertexStride); + if (this._cachedInstanceBuffer !== instanceBuffer) { + this._cachedInstanceBuffer = instanceBuffer; + this._cachedInstanceBinding = new VertexBufferBinding(instanceBuffer, ParticleBufferUtils.instanceVertexStride); + } + const instanceBinding = this._cachedInstanceBinding; this._primitive.rebuildVAOsIfNeeded(this._tfProgram, ParticleTransformFeedbackSimulator._tfElements, [ { binding: instanceBinding, elements: ParticleTransformFeedbackSimulator._instanceElements } ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts` around lines 109 - 113, The code currently allocates a new VertexBufferBinding each frame inside update(), causing GC churn; cache and reuse the binding instead: add a class field (e.g., this._instanceBinding) and in update() only create a new VertexBufferBinding when the instanceBuffer reference changes (compare instanceBuffer to the cached binding's buffer or null), otherwise reuse the cached binding when calling this._primitive.rebuildVAOsIfNeeded(this._tfProgram, ParticleTransformFeedbackSimulator._tfElements, [{ binding: this._instanceBinding, elements: ParticleTransformFeedbackSimulator._instanceElements }]); ensure the cache is cleared/updated whenever the instanceBuffer is replaced or disposed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 256-265: The _isRandomMode() method currently checks only
_limitX/_limitY/_limitZ modes and misses _drag; update _isRandomMode in
LimitVelocityOverLifetimeModule to include a check that _drag.mode is also
TwoConstants or TwoCurves (when deciding random mode), i.e., when _separateAxes
is true require both all three limit axes and _drag be random-mode, and when
_separateAxes is false require either _limitX or _drag to be
TwoConstants/TwoCurves so per-particle random values are allocated for drag
interpolation.
- Around line 356-421: In _uploadSeparateAxisLimits detect mixed per-axis modes
(using limitX/limitY/limitZ and ParticleCurveMode) and avoid silently falling
into the constant branch: either (A) implement per-axis uploads by setting
per-axis curve/constant shader data (call shaderData.setFloatArray for curve
axes using limitX.curveMax/_curveMin and shaderData.setVector3 for constant
axes) and introduce per-axis mode macros (e.g., add/check new macros like
_limitXCurveMacro/_limitYCurveMacro/_limitZCurveMacro and still set
_limitIsRandomMacro where applicable), or (B) if you prefer not to support mixed
modes, throw/log a clear warning/error from _uploadSeparateAxisLimits when modes
differ instead of uploading constants only; update the function
_uploadSeparateAxisLimits, references to
LimitVelocityOverLifetimeModule._limitCurveModeMacro/_limitConstantModeMacro/_limitIsRandomMacro,
and the shaderData.setFloatArray/setVector3 calls accordingly so curve data is
not discarded silently.
---
Nitpick comments:
In `@e2e/case/particleRenderer-limitVelocity.ts`:
- Around line 136-147: Remove the commented-out alternative configurations
around the limitVelocityOverLifetime block: delete the commented lines
referencing ParticleCompositeCurve, enabled, separateAxes, and dampen (the
comments at and around the limitVelocityOverLifetime assignment), or if those
variants are useful as documented alternatives, extract them into separate test
functions instead of leaving them commented; locate the
limitVelocityOverLifetime usage and ParticleCompositeCurve /
ParticleSimulationSpace references in this file to remove or refactor the
commented alternatives accordingly.
In `@packages/core/src/graphic/TransformFeedbackPrimitive.ts`:
- Around line 154-160: The swap() method in TransformFeedbackPrimitive currently
allocates a new VertexBufferBinding each call (this._renderBufferBinding = new
VertexBufferBinding(...)); instead, pre-allocate two VertexBufferBinding
instances (e.g., this._bindingA / this._bindingB) as members alongside
_vaoA/_vaoB and initialize them for the corresponding buffers and _byteStride,
then in swap() simply toggle which binding is active (assign
this._renderBufferBinding = this._bindingA or this._bindingB) and update their
internal buffer reference/stride if necessary; update any initialization code
that creates _renderBufferBinding to use the pre-allocated pair and ensure
_renderBufferBinding is swapped when _readBuffer/_writeBuffer and _currentVAO
are swapped.
- Around line 61-86: The resize() method should clear the cached VAO reference
to avoid holding a pointer to a VAO that will be destroyed; add a line to set
this._currentVAO = null (or undefined consistent with class) when
resizing—ideally right after setting this._vaoDirty = true—so that
rebuildVAOsIfNeeded() can safely recreate VAOs and bindVAO() cannot reference a
deleted object.
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts`:
- Line 127: Replace the magic numeric literal used for POINTS with the WebGL
constant from the existing GL context: change the declaration that sets POINTS =
0x0000 to use the in-scope WebGLRenderingContext constant (e.g., POINTS =
gl.POINTS or this.gl.POINTS depending on how the context is referenced in
ParticleTransformFeedbackSimulator) so the code uses the named constant instead
of the raw hex value.
- Around line 109-113: The code currently allocates a new VertexBufferBinding
each frame inside update(), causing GC churn; cache and reuse the binding
instead: add a class field (e.g., this._instanceBinding) and in update() only
create a new VertexBufferBinding when the instanceBuffer reference changes
(compare instanceBuffer to the cached binding's buffer or null), otherwise reuse
the cached binding when calling
this._primitive.rebuildVAOsIfNeeded(this._tfProgram,
ParticleTransformFeedbackSimulator._tfElements, [{ binding:
this._instanceBinding, elements:
ParticleTransformFeedbackSimulator._instanceElements }]); ensure the cache is
cleared/updated whenever the instanceBuffer is replaced or disposed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d59df521-c2b6-4d48-b48b-0b1f6cd258e6
⛔ Files ignored due to path filters (1)
e2e/fixtures/originImage/Particle_particleRenderer-limitVelocity.jpgis excluded by!**/*.jpg
📒 Files selected for processing (6)
e2e/case/particleRenderer-limitVelocity.tse2e/config.tspackages/core/src/graphic/TransformFeedbackPrimitive.tspackages/core/src/particle/ParticleRenderer.tspackages/core/src/particle/ParticleTransformFeedbackSimulator.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/config.ts
- Fix animatedVelocity composition: only VOL, not FOL/gravity - Fix gravity timing: add to base velocity before dampen/drag - Fix drag math: scale total then subtract animated (not scale base only) - Remove Unity references from comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GuoLei1990
force-pushed
the
feat/limitParticle
branch
from
3c88bf0 to
d023c18
Compare
…als in particle modules
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (1)
44-44: Inconsistent shader property naming.
_dragConstantPropertyuses"u_DragConstant"while all other properties in this module use the"renderer_LVL*"prefix (e.g.,renderer_LVLDampen,renderer_LVLDragMaxCurve). Consider renaming to"renderer_LVLDragConstant"for consistency.Suggested fix
- static readonly _dragConstantProperty = ShaderProperty.getByName("u_DragConstant"); + static readonly _dragConstantProperty = ShaderProperty.getByName("renderer_LVLDragConstant");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` at line 44, Update the shader property name for consistency: change the string passed to ShaderProperty.getByName in the static member _dragConstantProperty from "u_DragConstant" to "renderer_LVLDragConstant" and ensure any other usages that reference the shader property by name align with this new identifier (look for references to _dragConstantProperty and any direct "u_DragConstant" usages to update them).
🤖 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/LimitVelocityOverLifetimeModule.ts`:
- Around line 423-445: In _uploadDrag, constant-based drag modes never set a
"random" macro so the shader can't tell TwoConstants (random) from Constant;
update _uploadDrag to set and return a dedicated macro (e.g.,
LimitVelocityOverLifetimeModule._dragIsRandomMacro) when drag.mode ===
ParticleCurveMode.TwoConstants (in addition to writing _dragConstantProperty
using _dragConstantVec), keep existing behavior of returning _dragCurveModeMacro
for curve modes, and ensure the shader code uses that new _dragIsRandomMacro
(and related symbol _isRandomMode()) to choose interpolation; reference symbols:
_uploadDrag, _drag, _dragConstantVec, _dragConstantProperty,
_dragCurveModeMacro, _dragMin/Max curve properties, and add/return
LimitVelocityOverLifetimeModule._dragIsRandomMacro when appropriate.
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts`:
- Around line 64-83: The code marks the transform-feedback simulator initialized
even when resized to zero, allowing writeParticleData to write into
zero-capacity buffers; fix by only setting this->_initialized (in resize) when
particleCount > 0 (or set it false when particleCount === 0), and/or add a
defensive check at the start of writeParticleData to return if the primitive
buffers have insufficient capacity (use
ParticleTransformFeedbackSimulator._byteStride and the index to compute/validate
byteOffset before calling _primitive.readBuffer.setData and
_primitive.writeBuffer.setData) so writes never occur into zero-sized ping-pong
buffers.
---
Nitpick comments:
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Line 44: Update the shader property name for consistency: change the string
passed to ShaderProperty.getByName in the static member _dragConstantProperty
from "u_DragConstant" to "renderer_LVLDragConstant" and ensure any other usages
that reference the shader property by name align with this new identifier (look
for references to _dragConstantProperty and any direct "u_DragConstant" usages
to update them).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2703be57-bb7a-4843-8e3a-adad99afc684
⛔ Files ignored due to path filters (1)
packages/core/src/shaderlib/particle/particle_transform_feedback_update.glslis excluded by!**/*.glsl
📒 Files selected for processing (3)
packages/core/src/particle/ParticleRenderer.tspackages/core/src/particle/ParticleTransformFeedbackSimulator.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts
Comment on lines
+423
to
+445
| private _uploadDrag(shaderData: ShaderData): ShaderMacro { | ||
| const drag = this._drag; | ||
| let dragCurveMacro: ShaderMacro = null; | ||
|
|
||
| const isRandomCurveMode = drag.mode === ParticleCurveMode.TwoCurves; | ||
| if (isRandomCurveMode || drag.mode === ParticleCurveMode.Curve) { | ||
| shaderData.setFloatArray(LimitVelocityOverLifetimeModule._dragMaxCurveProperty, drag.curveMax._getTypeArray()); | ||
| dragCurveMacro = LimitVelocityOverLifetimeModule._dragCurveModeMacro; | ||
| if (isRandomCurveMode) { | ||
| shaderData.setFloatArray(LimitVelocityOverLifetimeModule._dragMinCurveProperty, drag.curveMin._getTypeArray()); | ||
| } | ||
| } else { | ||
| const dragVec = this._dragConstantVec; | ||
| if (drag.mode === ParticleCurveMode.TwoConstants) { | ||
| dragVec.set(drag.constantMin, drag.constantMax); | ||
| } else { | ||
| dragVec.set(drag.constantMax, drag.constantMax); | ||
| } | ||
| shaderData.setVector2(LimitVelocityOverLifetimeModule._dragConstantProperty, dragVec); | ||
| } | ||
|
|
||
| return dragCurveMacro; | ||
| } |
There was a problem hiding this comment.
Drag constant modes lack a random indicator macro.
For constant-based drag modes (TwoConstants vs Constant), no macro is returned to inform the shader whether to interpolate between min and max using a random value. The shader receives a Vector2 but cannot distinguish between "use both with random" and "use single value".
This is related to the _isRandomMode() issue—without proper random mode detection and a dedicated macro (similar to _limitIsRandomMacro), drag randomization may not work as expected.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around
lines 423 - 445, In _uploadDrag, constant-based drag modes never set a "random"
macro so the shader can't tell TwoConstants (random) from Constant; update
_uploadDrag to set and return a dedicated macro (e.g.,
LimitVelocityOverLifetimeModule._dragIsRandomMacro) when drag.mode ===
ParticleCurveMode.TwoConstants (in addition to writing _dragConstantProperty
using _dragConstantVec), keep existing behavior of returning _dragCurveModeMacro
for curve modes, and ensure the shader code uses that new _dragIsRandomMacro
(and related symbol _isRandomMode()) to choose interpolation; reference symbols:
_uploadDrag, _drag, _dragConstantVec, _dragConstantProperty,
_dragCurveModeMacro, _dragMin/Max curve properties, and add/return
LimitVelocityOverLifetimeModule._dragIsRandomMacro when appropriate.
packages/core/src/particle/ParticleTransformFeedbackSimulator.ts
Outdated
Show resolved
Hide resolved
- Unbind TF buffer from TRANSFORM_FEEDBACK_BUFFER target after TF draw to prevent conflict when render VAO rebuilds (e.g., VOL enabled at runtime) - Fix drag math: scale total velocity then subtract animated - Fix gravity: add to base velocity before dampen/drag - Fix animatedVelocity: only VOL, not FOL/gravity Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace raw GL constants with MeshTopology.Points - Reorder TransformFeedback methods by usage flow - Add @param annotations to TransformFeedback API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move VAO management to GLTransformFeedbackPrimitive (RHI layer) - Add IPlatformTransformFeedbackPrimitive interface - Remove GL calls from core layer TransformFeedbackPrimitive - Split draw into beginDraw/draw/endDraw (no array allocation) - Use VertexElement/VertexBufferBinding directly (no intermediate types) - Remove unused drawArrays/VAO methods from WebGLGraphicDevice - Fix TF buffer residual binding causing GL conflict on VAO rebuild Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GuoLei1990
force-pushed
the
feat/limitParticle
branch
from
68b2e13 to
2024ae1
Compare
…point Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Simplify to readBinding/writeBinding (remove separate buffer fields) - Rename update to updateVertexLayout - Split beginDraw/draw/endDraw for flexible ring buffer drawing - Move render binding cache into primitive (zero GC swap) - Clean up comments to use core-layer concepts - Move VAO logic to GLTransformFeedbackPrimitive via platform interface - Remove unused drawArrays/VAO methods from WebGLGraphicDevice Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove section separator comments from static fields - Simplify limit property comment - Clamp dampen to [0, 1] matching Unity behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…uffers - Core layer no longer accesses _platformBuffer - RHI layer extracts platform buffer internally (same pattern as GLPrimitive) - Fix cross-package TS error for Buffer._platformBuffer Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
TF object internally remembers buffer bindings. When bind() restores the TF object, old buffer binding resurfaces on TRANSFORM_FEEDBACK_BUFFER, conflicting with the same buffer on ARRAY_BUFFER via VAO. Clear with unbindBuffer(0) after bind() before setting new range. Also add invalidate() to force VAO rebuild after buffer resize. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pass _bindingA/_bindingB directly to updateVertexLayout instead of readBinding/writeBinding which vary with swap state. This prevents VAO A from accidentally binding buffer B (and vice versa) when program recompiles after an odd number of swaps. Also remove redundant unbindBuffer in beginDraw. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
VOL is animated velocity (reset each frame), should use the full instantaneous value, not delta between frames. Constant VOL had zero delta, making it invisible. Now matches Unity's behavior where animatedVelocity is reset to zero then VOL adds full value. Also fix VAO A/B correspondence to always match binding A/B. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LVL drag only works in TF mode. The non-TF analytical path had drag calculations using renderer_LVLDragConstant which was never uploaded (LVL enabled = TF mode). Removed getStartPosition, dragData, and the uniform declaration from particle.vs.glsl. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename _tfModeMacro to _transformFeedbackMacro - Rename _tfBufferBindingIndex to _feedbackBindingIndex - Rename _transformFeedback to _feedbackSimulator - Rename _setTFMode to _setTransformFeedback - Cache renderer_CurrentTime as static property - Replace all TF abbreviations in comments with full words Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…form upload - TF pass now runs in ParticleRenderer._update after shader data is ready - Remove redundant _updateShaderData and _currentTimeProperty from Generator - Extract _updateFeedback method for clean separation - Simplify condition checks (remove redundant _feedbackSimulator null checks) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Single curve drag mode was unconditionally mixing with uninitialized MinCurve (defaulting to zero), causing drag to be randomly weakened. Now only TwoCurves mode does min/max interpolation; single curve returns dragMax directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use ParticleFeedbackVertexAttribute enum for vertex element names - Simplify _setTransformFeedback (remove redundant guards and locals) - Remove unused Logger and ShaderProperty imports - Remove redundant _feedbackBindingIndex >= 0 checks - Clean up comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
limitVal → limitValue, minLimitVal → minLimitValue, ConstVec → ConstVector for uniform naming consistency. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use Vector3.transformByQuat instead of hand-written quaternion rotation - Accept Vector3 position in writeParticleData - Skip copy in local mode (pass shapePosition directly) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Deduplicate upload logic using compact flag for bufferByteOffset - Remove trailing periods from comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ack_simulation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
packages/rhi-webgl/src/GLTransformFeedbackPrimitive.ts (1)
27-35:⚠️ Potential issue | 🟠 MajorRebuild VAOs when buffer bindings change, not only when program changes.
Line 27 currently keys cache on
program.idonly. IfreadBinding,writeBinding, orinputBindingpoints to a new buffer with the same program, VAOs keep stale bindings.Suggested fix
export class GLTransformFeedbackPrimitive implements IPlatformTransformFeedbackPrimitive { private _gl: WebGL2RenderingContext; private _vaoA: WebGLVertexArrayObject; private _vaoB: WebGLVertexArrayObject; private _lastProgramId = -1; + private _lastReadBuffer: unknown = null; + private _lastWriteBuffer: unknown = null; + private _lastInputBuffer: unknown = null; @@ updateVertexLayout( program: any, readBinding: VertexBufferBinding, writeBinding: VertexBufferBinding, @@ ): void { - if (program.id === this._lastProgramId) return; + const readBuffer = readBinding?.buffer; + const writeBuffer = writeBinding?.buffer; + const inputBuffer = inputBinding?.buffer; + if ( + program.id === this._lastProgramId && + readBuffer === this._lastReadBuffer && + writeBuffer === this._lastWriteBuffer && + inputBuffer === this._lastInputBuffer + ) { + return; + } @@ this._vaoA = this._createVAO(attribs, readBinding, feedbackElements, inputBinding, inputElements); this._vaoB = this._createVAO(attribs, writeBinding, feedbackElements, inputBinding, inputElements); this._lastProgramId = program.id; + this._lastReadBuffer = readBuffer; + this._lastWriteBuffer = writeBuffer; + this._lastInputBuffer = inputBuffer; @@ private _deleteVAOs(): void { @@ this._lastProgramId = -1; + this._lastReadBuffer = null; + this._lastWriteBuffer = null; + this._lastInputBuffer = null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rhi-webgl/src/GLTransformFeedbackPrimitive.ts` around lines 27 - 35, The code only rebuilds VAOs when program.id changes (checked via this._lastProgramId), so VAOs stay stale if readBinding, writeBinding, or inputBinding change; update the logic to also detect binding changes by tracking unique identifiers for readBinding, writeBinding and inputBinding (e.g., buffer id or a composite key) and include them in the cache check before returning, and when any of those ids differ call this._deleteVAOs() and recreate this._vaoA/this._vaoB via this._createVAO(attribs, ...), then update the stored last-program and last-binding ids (e.g., this._lastReadBindingId, this._lastWriteBindingId, this._lastInputBindingId) alongside this._lastProgramId.packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts (1)
260-269:⚠️ Potential issue | 🟠 MajorInclude drag random modes in module random-mode detection.
Line 260-Line 269 currently ignores drag random modes. This can skip per-particle random generation when
dragisTwoConstants/TwoCurvesbut limits are non-random.Suggested fix
_isRandomMode(): boolean { + const dragRandom = + this._drag.mode === ParticleCurveMode.TwoConstants || this._drag.mode === ParticleCurveMode.TwoCurves; + + const limitRandom = this._separateAxes + ? (this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves) && + (this._limitY.mode === ParticleCurveMode.TwoConstants || this._limitY.mode === ParticleCurveMode.TwoCurves) && + (this._limitZ.mode === ParticleCurveMode.TwoConstants || this._limitZ.mode === ParticleCurveMode.TwoCurves) + : this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves; + - if (this._separateAxes) { - return ( - (this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves) && - (this._limitY.mode === ParticleCurveMode.TwoConstants || this._limitY.mode === ParticleCurveMode.TwoCurves) && - (this._limitZ.mode === ParticleCurveMode.TwoConstants || this._limitZ.mode === ParticleCurveMode.TwoCurves) - ); - } - return this._limitX.mode === ParticleCurveMode.TwoConstants || this._limitX.mode === ParticleCurveMode.TwoCurves; + return limitRandom || dragRandom; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts` around lines 260 - 269, The _isRandomMode method currently only checks limit curves and ignores drag random modes; update it to also consider drag being in random modes (ParticleCurveMode.TwoConstants or ParticleCurveMode.TwoCurves). Specifically, in the branch where this._separateAxes is true, include checks for this._dragX.mode, this._dragY.mode and this._dragZ.mode (alongside _limitX/_limitY/_limitZ) and in the non-separate branch include this._drag.mode in the OR logic with this._limitX.mode so that any drag TwoConstants/TwoCurves causes _isRandomMode to return true.packages/rhi-webgl/src/GLTransformFeedback.ts (1)
13-17:⚠️ Potential issue | 🔴 CriticalAdd explicit WebGL2 and handle-creation guards in constructor.
This constructor still assumes WebGL2 and does not check
createTransformFeedback()result, which can fail and produce invalid state.Suggested patch
constructor(rhi: WebGLGraphicDevice) { + if (!rhi.isWebGL2) { + throw new Error("Transform Feedback requires a WebGL2 context."); + } const gl = <WebGL2RenderingContext>rhi.gl; this._gl = gl; - this._glTransformFeedback = gl.createTransformFeedback(); + const tf = gl.createTransformFeedback(); + if (!tf) { + throw new Error("Failed to create WebGLTransformFeedback."); + } + this._glTransformFeedback = tf; }#!/bin/bash set -e # Confirm guard presence and constructor call paths sed -n '1,120p' packages/rhi-webgl/src/GLTransformFeedback.ts rg -nP --type=ts 'new GLTransformFeedback\s*\(' packages rg -nP --type=ts -C3 'createPlatformTransformFeedback|isWebGL2' packages/rhi-webgl/src🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rhi-webgl/src/GLTransformFeedback.ts` around lines 13 - 17, The GLTransformFeedback constructor assumes WebGL2 and doesn't validate createTransformFeedback() output; update the constructor in class GLTransformFeedback to assert/guard that rhi.gl is a WebGL2RenderingContext (use isWebGL2 or instanceof WebGL2RenderingContext) and check the result of gl.createTransformFeedback(); if it returns null handle it by throwing or returning a clear error (or set a safe fallback and log) and ensure _glTransformFeedback is only assigned when non-null, referencing the constructor, WebGL2RenderingContext, createTransformFeedback, and _glTransformFeedback symbols.e2e/case/particleRenderer-limitVelocity.ts (1)
27-28:⚠️ Potential issue | 🟡 MinorWire the screenshot harness in this case setup.
initScreenshot/updateForE2Eare imported but unused; the case never explicitly settles before capture. This can make snapshots timing-sensitive.Suggested patch
- .then((texture) => { - createScalarLimitParticle(engine, rootEntity, <Texture2D>texture); + .then((texture) => { + createScalarLimitParticle(engine, rootEntity, camera, <Texture2D>texture); }); }); -function createScalarLimitParticle(engine: Engine, rootEntity: Entity, texture: Texture2D): void { +function createScalarLimitParticle(engine: Engine, rootEntity: Entity, camera: Camera, texture: Texture2D): void { @@ limitVelocityOverLifetime.drag = new ParticleCompositeCurve(0.0); limitVelocityOverLifetime.multiplyDragByParticleSize = true; limitVelocityOverLifetime.multiplyDragByParticleVelocity = true; + + updateForE2E(engine, 500); + initScreenshot(engine, camera); }Also applies to: 64-68, 148-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/case/particleRenderer-limitVelocity.ts` around lines 27 - 28, The test imports initScreenshot and updateForE2E but never uses them, so wire the screenshot harness into the case: call initScreenshot() near the start of the test setup to initialize the harness and, after any actions that change state but before taking a snapshot, call await updateForE2E() to let the scene settle; update the particleRenderer-limitVelocity test case (references to initScreenshot and updateForE2E in the file) to include these calls in the setup and immediately before captures so snapshots are not timing-sensitive.
🧹 Nitpick comments (2)
packages/core/src/particle/ParticleTransformFeedbackSimulator.ts (1)
84-84: Avoid per-frameVertexBufferBindingallocation in hot update loop.Line 84 allocates every frame. Reusing a cached binding (or mutating an existing one) will reduce GC churn during particle-heavy scenes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts` at line 84, The per-frame allocation of a VertexBufferBinding (variable instanceBinding) inside ParticleTransformFeedbackSimulator causes GC churn; cache a reusable binding on the simulator instance instead of constructing it each update: add a class field (e.g., this.instanceBinding) created once using instanceBuffer and ParticleBufferUtils.instanceVertexStride and reuse it every frame, updating its buffer reference only if instanceBuffer changes (or mutate the existing binding) so you stop allocating a new VertexBufferBinding each frame.packages/core/src/shader/ShaderPass.ts (1)
153-163: Consider centralizing capability macro collection.
_getShaderLabProgram()rebuilds the same renderer-capability macro list that the non-ShaderLab path already needs. Pulling that into a shared helper would reduce drift if new capability macros get added later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/shader/ShaderPass.ts` around lines 153 - 163, The code in _getShaderLabProgram rebuilds renderer capability macros (e.g., GRAPHICS_API_WEBGL2/1, HAS_TEX_LOD, HAS_DERIVATIVES) into shaderMacroList; extract that logic into a shared helper (for example a new private function like collectRendererCapabilityMacros(engine: Engine, target: ShaderMacro[])) and call it from _getShaderLabProgram and the non-ShaderLab path so both use the same single source of truth; ensure the helper uses engine._hardwareRenderer and ShaderMacro.getByName to push the same macros and update callers to remove the duplicated push logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/case/particleRenderer-limitVelocity.ts`:
- Around line 123-129: Remove the leftover debug logging inside the setTimeout
callback: locate the anonymous callback that sets velocityOverLifetime.enabled
and its velocityX/Y/Z constants and delete the console.log("s") call so the E2E
path no longer emits debug output; ensure only the velocityOverLifetime changes
remain in that callback.
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 987-1002: The initial velocity is not rotated into world space
when this.main.simulationSpace !== ParticleSimulationSpace.Local, so rotate the
local direction by transform.worldRotationQuaternion before passing to
writeParticleData: after computing world position (using
ParticleGenerator._tempVector32 and transform.worldRotationQuaternion),
transform the local direction vector (the existing direction variable) by
transform.worldRotationQuaternion into a temp vector (e.g., a ParticleGenerator
temp Vector3), then use that rotated vector multiplied by startSpeed for the
x/y/z velocity args in this._feedbackSimulator.writeParticleData so
world-simulated particles respect emitter rotation.
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts`:
- Around line 85-105: beginUpdate()/endUpdate() pair in
ParticleTransformFeedbackSimulator is not exception-safe: if any
this._simulator.draw(...) throws, endUpdate() is skipped; wrap the draw logic in
a try/finally so endUpdate() always executes when beginUpdate(...) returned
true. Concretely, after the successful beginUpdate(...) call on this._simulator,
enclose the conditional draw(...) calls (including both branches using
MeshTopology.Points and the two draw calls) inside a try block and call
this._simulator.endUpdate() in the finally block; preserve the early return when
beginUpdate(...) returns false so endUpdate() is only invoked when
beginUpdate(...) succeeded.
In `@packages/rhi-webgl/src/WebGLGraphicDevice.ts`:
- Around line 279-281: createPlatformTransformFeedbackPrimitive() assumes WebGL2
and will crash on WebGL1; add a guard using this._isWebGL2 before instantiating
GLTransformFeedbackPrimitive. In createPlatformTransformFeedbackPrimitive(),
check this._isWebGL2 and if true return new
GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl); otherwise either
throw a clear Error (e.g. "Transform feedback requires WebGL2") or return a
no-op/fallback IPlatformTransformFeedbackPrimitive implementation to match how
other WebGL2-only methods are protected.
---
Duplicate comments:
In `@e2e/case/particleRenderer-limitVelocity.ts`:
- Around line 27-28: The test imports initScreenshot and updateForE2E but never
uses them, so wire the screenshot harness into the case: call initScreenshot()
near the start of the test setup to initialize the harness and, after any
actions that change state but before taking a snapshot, call await
updateForE2E() to let the scene settle; update the
particleRenderer-limitVelocity test case (references to initScreenshot and
updateForE2E in the file) to include these calls in the setup and immediately
before captures so snapshots are not timing-sensitive.
In `@packages/core/src/particle/modules/LimitVelocityOverLifetimeModule.ts`:
- Around line 260-269: The _isRandomMode method currently only checks limit
curves and ignores drag random modes; update it to also consider drag being in
random modes (ParticleCurveMode.TwoConstants or ParticleCurveMode.TwoCurves).
Specifically, in the branch where this._separateAxes is true, include checks for
this._dragX.mode, this._dragY.mode and this._dragZ.mode (alongside
_limitX/_limitY/_limitZ) and in the non-separate branch include this._drag.mode
in the OR logic with this._limitX.mode so that any drag TwoConstants/TwoCurves
causes _isRandomMode to return true.
In `@packages/rhi-webgl/src/GLTransformFeedback.ts`:
- Around line 13-17: The GLTransformFeedback constructor assumes WebGL2 and
doesn't validate createTransformFeedback() output; update the constructor in
class GLTransformFeedback to assert/guard that rhi.gl is a
WebGL2RenderingContext (use isWebGL2 or instanceof WebGL2RenderingContext) and
check the result of gl.createTransformFeedback(); if it returns null handle it
by throwing or returning a clear error (or set a safe fallback and log) and
ensure _glTransformFeedback is only assigned when non-null, referencing the
constructor, WebGL2RenderingContext, createTransformFeedback, and
_glTransformFeedback symbols.
In `@packages/rhi-webgl/src/GLTransformFeedbackPrimitive.ts`:
- Around line 27-35: The code only rebuilds VAOs when program.id changes
(checked via this._lastProgramId), so VAOs stay stale if readBinding,
writeBinding, or inputBinding change; update the logic to also detect binding
changes by tracking unique identifiers for readBinding, writeBinding and
inputBinding (e.g., buffer id or a composite key) and include them in the cache
check before returning, and when any of those ids differ call this._deleteVAOs()
and recreate this._vaoA/this._vaoB via this._createVAO(attribs, ...), then
update the stored last-program and last-binding ids (e.g.,
this._lastReadBindingId, this._lastWriteBindingId, this._lastInputBindingId)
alongside this._lastProgramId.
---
Nitpick comments:
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts`:
- Line 84: The per-frame allocation of a VertexBufferBinding (variable
instanceBinding) inside ParticleTransformFeedbackSimulator causes GC churn;
cache a reusable binding on the simulator instance instead of constructing it
each update: add a class field (e.g., this.instanceBinding) created once using
instanceBuffer and ParticleBufferUtils.instanceVertexStride and reuse it every
frame, updating its buffer reference only if instanceBuffer changes (or mutate
the existing binding) so you stop allocating a new VertexBufferBinding each
frame.
In `@packages/core/src/shader/ShaderPass.ts`:
- Around line 153-163: The code in _getShaderLabProgram rebuilds renderer
capability macros (e.g., GRAPHICS_API_WEBGL2/1, HAS_TEX_LOD, HAS_DERIVATIVES)
into shaderMacroList; extract that logic into a shared helper (for example a new
private function like collectRendererCapabilityMacros(engine: Engine, target:
ShaderMacro[])) and call it from _getShaderLabProgram and the non-ShaderLab path
so both use the same single source of truth; ensure the helper uses
engine._hardwareRenderer and ShaderMacro.getByName to push the same macros and
update callers to remove the duplicated push logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4fc14c71-df4e-45c3-86f7-9d341f40ef7b
⛔ Files ignored due to path filters (3)
packages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/limit_velocity_over_lifetime_module.glslis excluded by!**/*.glslpackages/core/src/shaderlib/particle/particle_feedback_simulation.glslis excluded by!**/*.glsl
📒 Files selected for processing (19)
e2e/case/particleRenderer-limitVelocity.tspackages/core/src/graphic/TransformFeedback.tspackages/core/src/graphic/TransformFeedbackPrimitive.tspackages/core/src/graphic/TransformFeedbackSimulator.tspackages/core/src/particle/ParticleBufferUtils.tspackages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/ParticleRenderer.tspackages/core/src/particle/ParticleTransformFeedbackSimulator.tspackages/core/src/particle/enums/attributes/ParticleFeedbackVertexAttribute.tspackages/core/src/particle/modules/LimitVelocityOverLifetimeModule.tspackages/core/src/renderingHardwareInterface/IPlatformTransformFeedback.tspackages/core/src/renderingHardwareInterface/IPlatformTransformFeedbackPrimitive.tspackages/core/src/renderingHardwareInterface/index.tspackages/core/src/shader/ShaderPass.tspackages/core/src/shaderlib/ShaderFactory.tspackages/core/src/shaderlib/particle/index.tspackages/rhi-webgl/src/GLTransformFeedback.tspackages/rhi-webgl/src/GLTransformFeedbackPrimitive.tspackages/rhi-webgl/src/WebGLGraphicDevice.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/renderingHardwareInterface/IPlatformTransformFeedback.ts
- packages/core/src/graphic/TransformFeedback.ts
Comment on lines
+987
to
+1002
| let position: Vector3; | ||
| if (this.main.simulationSpace === ParticleSimulationSpace.Local) { | ||
| position = shapePosition; | ||
| } else { | ||
| position = ParticleGenerator._tempVector32; | ||
| Vector3.transformByQuat(shapePosition, transform.worldRotationQuaternion, position); | ||
| position.add(transform.worldPosition); | ||
| } | ||
|
|
||
| this._feedbackSimulator.writeParticleData( | ||
| index, | ||
| position, | ||
| direction.x * startSpeed, | ||
| direction.y * startSpeed, | ||
| direction.z * startSpeed | ||
| ); |
There was a problem hiding this comment.
Rotate initial TF velocity in world simulation mode.
In world mode, position is rotated into world space (Lines 991–994), but velocity at Lines 999–1001 still uses local direction. This causes incorrect initial motion when emitter rotation is non-identity.
Suggested patch
private _addFeedbackParticle(
@@
): void {
let position: Vector3;
+ let velocity = direction;
if (this.main.simulationSpace === ParticleSimulationSpace.Local) {
position = shapePosition;
} else {
position = ParticleGenerator._tempVector32;
Vector3.transformByQuat(shapePosition, transform.worldRotationQuaternion, position);
position.add(transform.worldPosition);
+ velocity = ParticleGenerator._tempVector31;
+ Vector3.transformByQuat(direction, transform.worldRotationQuaternion, velocity);
}
this._feedbackSimulator.writeParticleData(
index,
position,
- direction.x * startSpeed,
- direction.y * startSpeed,
- direction.z * startSpeed
+ velocity.x * startSpeed,
+ velocity.y * startSpeed,
+ velocity.z * startSpeed
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/particle/ParticleGenerator.ts` around lines 987 - 1002, The
initial velocity is not rotated into world space when this.main.simulationSpace
!== ParticleSimulationSpace.Local, so rotate the local direction by
transform.worldRotationQuaternion before passing to writeParticleData: after
computing world position (using ParticleGenerator._tempVector32 and
transform.worldRotationQuaternion), transform the local direction vector (the
existing direction variable) by transform.worldRotationQuaternion into a temp
vector (e.g., a ParticleGenerator temp Vector3), then use that rotated vector
multiplied by startSpeed for the x/y/z velocity args in
this._feedbackSimulator.writeParticleData so world-simulated particles respect
emitter rotation.
Comment on lines
+85
to
+105
| if ( | ||
| !this._simulator.beginUpdate( | ||
| shaderData, | ||
| ParticleBufferUtils.feedbackVertexElements, | ||
| instanceBinding, | ||
| ParticleBufferUtils.feedbackInstanceElements | ||
| ) | ||
| ) | ||
| return; | ||
|
|
||
| if (firstActive < firstFree) { | ||
| this._simulator.draw(MeshTopology.Points, firstActive, firstFree - firstActive); | ||
| } else { | ||
| this._simulator.draw(MeshTopology.Points, firstActive, particleCount - firstActive); | ||
| if (firstFree > 0) { | ||
| this._simulator.draw(MeshTopology.Points, 0, firstFree); | ||
| } | ||
| } | ||
|
|
||
| this._simulator.endUpdate(); | ||
| } |
There was a problem hiding this comment.
Make TF update exception-safe with try/finally.
If any draw-path call throws after beginUpdate(), endUpdate() is skipped and TF state cleanup is missed.
Suggested patch
if (
!this._simulator.beginUpdate(
shaderData,
ParticleBufferUtils.feedbackVertexElements,
instanceBinding,
ParticleBufferUtils.feedbackInstanceElements
)
)
return;
- if (firstActive < firstFree) {
- this._simulator.draw(MeshTopology.Points, firstActive, firstFree - firstActive);
- } else {
- this._simulator.draw(MeshTopology.Points, firstActive, particleCount - firstActive);
- if (firstFree > 0) {
- this._simulator.draw(MeshTopology.Points, 0, firstFree);
+ try {
+ if (firstActive < firstFree) {
+ this._simulator.draw(MeshTopology.Points, firstActive, firstFree - firstActive);
+ } else {
+ this._simulator.draw(MeshTopology.Points, firstActive, particleCount - firstActive);
+ if (firstFree > 0) {
+ this._simulator.draw(MeshTopology.Points, 0, firstFree);
+ }
}
+ } finally {
+ this._simulator.endUpdate();
}
-
- this._simulator.endUpdate();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts` around
lines 85 - 105, beginUpdate()/endUpdate() pair in
ParticleTransformFeedbackSimulator is not exception-safe: if any
this._simulator.draw(...) throws, endUpdate() is skipped; wrap the draw logic in
a try/finally so endUpdate() always executes when beginUpdate(...) returned
true. Concretely, after the successful beginUpdate(...) call on this._simulator,
enclose the conditional draw(...) calls (including both branches using
MeshTopology.Points and the two draw calls) inside a try block and call
this._simulator.endUpdate() in the finally block; preserve the early return when
beginUpdate(...) returns false so endUpdate() is only invoked when
beginUpdate(...) succeeded.
Comment on lines
+279
to
+281
| createPlatformTransformFeedbackPrimitive(): IPlatformTransformFeedbackPrimitive { | ||
| return new GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
# Verify whether this factory can be reached without explicit WebGL2 gating
rg -nP --type=ts '\bcreatePlatformTransformFeedbackPrimitive\s*\(' packages
rg -nP --type=ts -C3 'new GLTransformFeedbackPrimitive|isWebGL2' packages/rhi-webgl/src
sed -n '1,140p' packages/rhi-webgl/src/GLTransformFeedbackPrimitive.tsRepository: galacean/engine
Length of output: 37600
Guard TF primitive creation behind WebGL2 capability.
createPlatformTransformFeedbackPrimitive() currently assumes a WebGL2 context without checking. In a WebGL1 runtime, this will fail at first API call. Other methods in the same class (e.g., enableRasterizerDiscard, blitFrameBuffer) properly guard WebGL2-only operations using this._isWebGL2.
Suggested patch
createPlatformTransformFeedbackPrimitive(): IPlatformTransformFeedbackPrimitive {
+ if (!this._isWebGL2) {
+ throw new Error("Transform Feedback primitive requires a WebGL2 context.");
+ }
return new GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| createPlatformTransformFeedbackPrimitive(): IPlatformTransformFeedbackPrimitive { | |
| return new GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl); | |
| } | |
| createPlatformTransformFeedbackPrimitive(): IPlatformTransformFeedbackPrimitive { | |
| if (!this._isWebGL2) { | |
| throw new Error("Transform Feedback primitive requires a WebGL2 context."); | |
| } | |
| return new GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/rhi-webgl/src/WebGLGraphicDevice.ts` around lines 279 - 281,
createPlatformTransformFeedbackPrimitive() assumes WebGL2 and will crash on
WebGL1; add a guard using this._isWebGL2 before instantiating
GLTransformFeedbackPrimitive. In createPlatformTransformFeedbackPrimitive(),
check this._isWebGL2 and if true return new
GLTransformFeedbackPrimitive(<WebGL2RenderingContext>this._gl); otherwise either
throw a clear Error (e.g. "Transform feedback requires WebGL2") or return a
no-op/fallback IPlatformTransformFeedbackPrimitive implementation to match how
other WebGL2-only methods are protected.
- Create entity before adding to scene to prevent early play() - Use updateForE2E + initScreenshot instead of engine.run() - Remove PostProcess/Bloom and cleanup unused code Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Update origin images for forceOverLifetime, limitVelocityOverLifetime, and particleEmitMeshCone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 `@e2e/case/particleRenderer-limitVelocity.ts`:
- Around line 122-123: Before enabling limitVelocityOverLifetime, add an
explicit WebGL2 capability guard that checks the renderer/context for WebGL2
(e.g. renderer.capabilities.isWebGL2 or gl instanceof WebGL2RenderingContext)
and fail or skip the test if WebGL2 is not available; specifically, insert the
guard just before the lines that set limitVelocityOverLifetime.enabled and
.separateAxes so the test does not silently no-op on WebGL1—use a clear early
exit such as throwing an Error("WebGL2 required for limit-velocity test") or
invoking the test runner's skip mechanism.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8fb17f70-130d-494e-add2-dc0b53b550af
📒 Files selected for processing (1)
e2e/case/particleRenderer-limitVelocity.ts
Comment on lines
+122
to
+123
| limitVelocityOverLifetime.enabled = true; | ||
| limitVelocityOverLifetime.separateAxes = true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify module behavior: enabling is gated by WebGL2.
module_file="$(fd 'LimitVelocityOverLifetimeModule.ts$' | head -n1)"
case_file="$(fd 'particleRenderer-limitVelocity.ts$' | head -n1)"
echo "Module file: ${module_file}"
echo "Case file: ${case_file}"
rg -n -C3 'override set enabled|isWebGL2|_setTransformFeedback' "$module_file"
rg -n -C3 'limitVelocityOverLifetime\.enabled|isWebGL2' "$case_file"
# Optional: inspect similar E2E patterns for capability gating.
rg -n --type=ts -C2 'isWebGL2|TransformFeedback|limitVelocityOverLifetime' e2e/caseRepository: galacean/engine
Length of output: 2685
Fail fast when WebGL2 is unavailable so this test actually exercises limit-velocity.
The module's enabled setter silently returns without enabling on WebGL1 (it checks isWebGL2 and returns early). Add an explicit WebGL2 capability guard before line 122 to ensure the test fails or skips if WebGL2 is not available, rather than silently passing without covering the intended transform-feedback behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@e2e/case/particleRenderer-limitVelocity.ts` around lines 122 - 123, Before
enabling limitVelocityOverLifetime, add an explicit WebGL2 capability guard that
checks the renderer/context for WebGL2 (e.g. renderer.capabilities.isWebGL2 or
gl instanceof WebGL2RenderingContext) and fail or skip the test if WebGL2 is not
available; specifically, insert the guard just before the lines that set
limitVelocityOverLifetime.enabled and .separateAxes so the test does not
silently no-op on WebGL1—use a clear early exit such as throwing an
Error("WebGL2 required for limit-velocity test") or invoking the test runner's
skip mechanism.
…ibute a_Random2 was conditionally declared using _FOL_MODULE_ENABLED, but this macro is defined inside force_over_lifetime_module.glsl which was included after the declaration. Move declaration after includes so the macro is available when the condition is evaluated. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GLSL requires declaration before use. a_Random2 must be declared before force_over_lifetime_module.glsl is included (which uses it). Use RENDERER_FOL_CONSTANT_MODE/RENDERER_FOL_CURVE_MODE (CPU macros) instead of _FOL_MODULE_ENABLED (defined inside the include). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GuoLei1990
approved these changes
Mar 17, 2026
Pass existing VertexBufferBinding from ParticleGenerator on resize, eliminating per-frame allocation in update(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/core/src/particle/ParticleGenerator.ts (2)
979-1001:⚠️ Potential issue | 🟠 MajorRotate initial TF velocity for world simulation.
In world mode, Line 991-992 rotates/emits position in world space, but Line 998-1000 still uses local
direction. This produces incorrect initial motion when emitter rotation is non-identity.Suggested patch
private _addFeedbackParticle( index: number, shapePosition: Vector3, direction: Vector3, startSpeed: number, transform: Transform ): void { let position: Vector3; + let velocity: Vector3 = direction; if (this.main.simulationSpace === ParticleSimulationSpace.Local) { position = shapePosition; } else { position = ParticleGenerator._tempVector32; Vector3.transformByQuat(shapePosition, transform.worldRotationQuaternion, position); position.add(transform.worldPosition); + velocity = ParticleGenerator._tempVector31; + Vector3.transformByQuat(direction, transform.worldRotationQuaternion, velocity); } this._feedbackSimulator.writeParticleData( index, position, - direction.x * startSpeed, - direction.y * startSpeed, - direction.z * startSpeed + velocity.x * startSpeed, + velocity.y * startSpeed, + velocity.z * startSpeed ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleGenerator.ts` around lines 979 - 1001, The feedback particle's initial velocity isn't rotated into world space in ParticleGenerator._addFeedbackParticle when this.main.simulationSpace !== ParticleSimulationSpace.Local; keep the existing world-space position transform but also rotate the local direction by transform.worldRotationQuaternion (e.g., into a temporary Vector3) and use that rotated direction's x/y/z multiplied by startSpeed in the call to this._feedbackSimulator.writeParticleData so emitted velocities respect emitter rotation.
614-628:⚠️ Potential issue | 🟠 MajorBackfill alive particles when enabling TF at runtime.
Line 621 only resizes TF buffers. Existing alive slots are not copied into TF state, so the first TF step can read default position/velocity for already-active particles.
Suggested patch sketch
_setTransformFeedback(enabled: boolean): void { this._useTransformFeedback = enabled; if (enabled) { if (!this._feedbackSimulator) { this._feedbackSimulator = new ParticleTransformFeedbackSimulator(this._renderer.engine); } this._feedbackSimulator.resize(this._currentParticleCount, this._instanceVertexBufferBinding); + this._syncAliveParticlesToFeedback(); this._renderer.shaderData.enableMacro(ParticleGenerator._transformFeedbackMacro); } else { this._renderer.shaderData.disableMacro(ParticleGenerator._transformFeedbackMacro); } this._reorganizeGeometryBuffers(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleGenerator.ts` around lines 614 - 628, When enabling transform-feedback in _setTransformFeedback, after creating/resizing this._feedbackSimulator call a method to initialize/copy the current alive particle state from the instance vertex data into the TF buffers so the first TF step doesn't read default values; use this._instanceVertexBufferBinding and this._currentParticleCount as the source and add/invoke a method on ParticleTransformFeedbackSimulator (e.g. initializeFromInstanceBinding or copyFromInstanceBuffers) to transfer per-particle attributes before enabling the macro and before calling _reorganizeGeometryBuffers().packages/core/src/particle/ParticleTransformFeedbackSimulator.ts (1)
84-104:⚠️ Potential issue | 🟠 MajorGuard TF state cleanup with
try/finally.Line 85 enters TF update, but if any draw on Line 95/Line 97/Line 99 throws, Line 103 is skipped and TF state is left unclosed.
Suggested patch
if ( !this._simulator.beginUpdate( shaderData, ParticleBufferUtils.feedbackVertexElements, this._instanceBinding, ParticleBufferUtils.feedbackInstanceElements ) ) return; - if (firstActive < firstFree) { - this._simulator.draw(MeshTopology.Points, firstActive, firstFree - firstActive); - } else { - this._simulator.draw(MeshTopology.Points, firstActive, particleCount - firstActive); - if (firstFree > 0) { - this._simulator.draw(MeshTopology.Points, 0, firstFree); + try { + if (firstActive < firstFree) { + this._simulator.draw(MeshTopology.Points, firstActive, firstFree - firstActive); + } else { + this._simulator.draw(MeshTopology.Points, firstActive, particleCount - firstActive); + if (firstFree > 0) { + this._simulator.draw(MeshTopology.Points, 0, firstFree); + } } + } finally { + this._simulator.endUpdate(); } - - this._simulator.endUpdate();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts` around lines 84 - 104, The TF (transform feedback) update calls into this._simulator beginUpdate/draw/endUpdate must be wrapped so endUpdate always runs even if a draw throws: capture the boolean result of this._simulator.beginUpdate(...) into a local (e.g., began), and if began then execute the draw logic inside a try block and call this._simulator.endUpdate() in a finally block; do not call endUpdate when beginUpdate returned false. Update the code around _simulator.beginUpdate / _simulator.draw / _simulator.endUpdate to use this try/finally pattern so TF state is always cleaned up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/particle/ParticleGenerator.ts`:
- Around line 979-1001: The feedback particle's initial velocity isn't rotated
into world space in ParticleGenerator._addFeedbackParticle when
this.main.simulationSpace !== ParticleSimulationSpace.Local; keep the existing
world-space position transform but also rotate the local direction by
transform.worldRotationQuaternion (e.g., into a temporary Vector3) and use that
rotated direction's x/y/z multiplied by startSpeed in the call to
this._feedbackSimulator.writeParticleData so emitted velocities respect emitter
rotation.
- Around line 614-628: When enabling transform-feedback in
_setTransformFeedback, after creating/resizing this._feedbackSimulator call a
method to initialize/copy the current alive particle state from the instance
vertex data into the TF buffers so the first TF step doesn't read default
values; use this._instanceVertexBufferBinding and this._currentParticleCount as
the source and add/invoke a method on ParticleTransformFeedbackSimulator (e.g.
initializeFromInstanceBinding or copyFromInstanceBuffers) to transfer
per-particle attributes before enabling the macro and before calling
_reorganizeGeometryBuffers().
In `@packages/core/src/particle/ParticleTransformFeedbackSimulator.ts`:
- Around line 84-104: The TF (transform feedback) update calls into
this._simulator beginUpdate/draw/endUpdate must be wrapped so endUpdate always
runs even if a draw throws: capture the boolean result of
this._simulator.beginUpdate(...) into a local (e.g., began), and if began then
execute the draw logic inside a try block and call this._simulator.endUpdate()
in a finally block; do not call endUpdate when beginUpdate returned false.
Update the code around _simulator.beginUpdate / _simulator.draw /
_simulator.endUpdate to use this try/finally pattern so TF state is always
cleaned up.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f9965229-ef8a-46c0-9911-ab36dbc8e709
⛔ Files ignored due to path filters (2)
e2e/fixtures/originImage/Particle_particleRenderer-force.jpgis excluded by!**/*.jpgpackages/core/src/shaderlib/extra/particle.vs.glslis excluded by!**/*.glsl
📒 Files selected for processing (2)
packages/core/src/particle/ParticleGenerator.tspackages/core/src/particle/ParticleTransformFeedbackSimulator.ts
packages/core/src/shaderlib/particle/particle_feedback_simulation.glsl
Outdated
Show resolved
Hide resolved
a_Random0.x is the gravity modifier random channel, not an independent drag factor. This caused drag TwoConstants/TwoCurves to be coupled with gravity config. Now shares a_Random2.w with LVL limit random factor. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| feedbackVaryings: string[] | ||
| ) { | ||
| this._engine = engine; | ||
| this._primitive = new TransformFeedbackPrimitive(engine, byteStride); |
Collaborator
There was a problem hiding this comment.
P1 这条 Transform Feedback 路径还没有真正接进设备恢复链路。TransformFeedbackSimulator 自己维护了一套独立的 ShaderProgramPool，不会随着 devicerestored 一起清空；恢复后 beginUpdate() 可能继续复用底层 GL program 已失效的 ShaderProgram 包装对象，而 GLTransformFeedbackPrimitive.updateVertexLayout() 又只在 program.id 变化时才重建 VAO。另外，feedback ping-pong buffer 恢复后是空的，但粒子路径只会重传 instance buffer，不会恢复已存活粒子的当前位置/速度。既然引擎显式支持 context lost/restore，这条 TF 模拟路径需要明确的 invalidate/恢复策略。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
为粒子系统新增 LimitVelocityOverLifetime 模块，通过 WebGL2 Transform Feedback 实现 GPU 端逐帧速度限制（dampen）和阻力（drag）模拟。
架构设计
三层 Transform Feedback 架构：
TransformFeedback— WebGL2 TF 对象封装（bind/unbind/begin/end）TransformFeedbackPrimitive— ping-pong 双缓冲 + VAO 管理TransformFeedbackSimulator— 通用 TF 模拟器（shader 编译、program 缓存、simulate 调度）ParticleTransformFeedbackSimulator— 粒子专用，继承通用模拟器平台抽象：
IPlatformTransformFeedback/IPlatformTransformFeedbackPrimitive— 核心接口GLTransformFeedback/GLTransformFeedbackPrimitive— WebGL2 实现TF Shader 算法（particle_feedback_simulation.glsl）
每帧对每个存活粒子执行四步：
drag * dt，支持 multiplyBySize / multiplyByVelocityBug 修复
curVOL - prevVOL计算，常量模式下结果为零RENDERER_LVL_DRAG_IS_RANDOM_TWO宏保护其他改动
ShaderPass.ts— 拆分 ShaderLab / 非 ShaderLab 编译路径，使手写 GLSL（TF shader）能正确编译ShaderFactory.ts— 新增compilePlatformSource统一非 ShaderLab shader 编译流程getStartPosition/u_DragConstant），drag 现在仅在 TF 模式下生效Summary by CodeRabbit