-
Notifications
You must be signed in to change notification settings - Fork 18
Merge origin/main into origin/dev #1061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Merge origin/main into origin/feat/2.5
Merge origin/main into origin/feat/2.5
fix: type issues
refactor: sprite shape geometry generation logic
Merge origin/main into origin/feat/2.5
* feat: sprite component support custom geometry * chore: remove unused type --------- Co-authored-by: yiiqii <[email protected]>
Merge origin/main into origin/feat/2.5
Merge origin/dev into origin/main
docs: changelog 2.4.7
Merge origin/main into origin/feat/2.5
* fix: optimize Player dispose processing during loadScene * chore: update
* feat: support custom stats metric callbacks * feat: 使用事件回调方式
feat: add alpha mask option
Merge origin/dev into origin/main
docs: changelog 2.4.8
Merge origin/main into origin/feat/2.5
* fix: baseRenderer splits processing * chore: rename * chore: update * chore: update
…alization refactor: sprite geometry initialization
* fix: video update * chore: add sprite video update logic * test: 补充动态数据帧对比案例 --------- Co-authored-by: yiiqii <[email protected]>
fix: sprite geometry split rotate
fix: ignore shader textures compile error
fix: sprite split rotate issue
fix: audio asset loading
* perf: opt bezier evaluation performance * chore: update * style: import --------- Co-authored-by: yiiqii <[email protected]>
Co-authored-by: linxi <[email protected]>
Merge origin/feat/2.5 into origin/main
docs: changelog 2.5.0
WalkthroughThis update implements a broad set of changes across the codebase. It introduces new features such as sprite support for custom geometry, runtime version checking, and improved mask handling. The Bezier curve logic is refactored into a dedicated module, and changelogs and package versions are updated. Numerous bug fixes, refactorings, and documentation improvements are also included. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Player
participant SceneLoader
participant Composition
participant Stats
participant Monitor
User->>Player: loadScene()
Player->>SceneLoader: start async scene loading
alt Player disposed during load
SceneLoader->>Player: check disposed
Player-->>SceneLoader: clear compositions, return
else
SceneLoader->>Composition: createComposition()
Composition-->>Player: add to compositions
end
User->>Stats: new Stats(player)
Stats->>Monitor: create Monitor instance
Monitor-->>Stats: ready
User->>Stats: await getComponent()
Stats-->>User: StatsComponent
User->>Monitor: on('update', ...)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (15)
⏰ Context from checks skipped due to timeout of 90000ms (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
packages/effects-core/src/shape/geometry.ts (2)
148-156
: Loop condition usesdatas[i]
instead ofdatas.length
, causing runtime failure
datas[i]
is an object, so the comparisoni < datas[i]
producesfalse
wheni
is0
(object → NaN), and the body never executes.
The aggregatedaPointData
/indexData
buffers therefore stay zero-filled, breaking every consumer that relies on the merged geometry.-// @ts-expect-error -for (let i = 0, pointIndex = 0, idx = 0; i < datas[i]; i++) { +for (let i = 0, pointIndex = 0, idx = 0; i < datas.length; i++) {Remove the
@ts-expect-error
as the code now type-checks.
56-60
: Default rotation is applied even when nouvTransform
is supplied
r
is leftundefined
whenuvTransform
is not provided.
const angle = r === 0 ? 0 : -Math.PI / 2;
therefore evaluates to-π/2
, rotating all UVs by default.-const angle = r === 0 ? 0 : -Math.PI / 2; +const angle = r == null || r === 0 ? 0 : -Math.PI / 2;Safer yet: move the angle computation inside the
if (uvTransform)
block.
🧹 Nitpick comments (6)
packages/effects-core/src/shape/geometry.ts (1)
164-170
: Parametervec2
shadows imported typevec2
Shadowing a type import with a value parameter is legal but confusing.
Rename the parameter to avoid ambiguity.packages/effects-core/src/fallback/index.ts (1)
20-20
: Consider importing version instead of duplicating.The
version
constant is already exported frompackages/effects-core/src/index.ts
(line 59). Consider importing it instead of duplicating the declaration to maintain consistency and avoid potential sync issues.+import { version } from '../index'; -const version = __VERSION__;packages/effects-core/src/utils/index.ts (1)
10-10
: New util re-export added – double-check name collisions.Re-exporting
./promise-util
is fine, but confirm that none of its named exports clash with existing ones in this index (e.g.Deferred
,noop
, etc.) to avoid accidental overwrites in the rolled-up.d.ts
.packages/effects-core/src/math/value-getters/value-getter-map.ts (1)
56-57
: Address the TODO comment for migration logic.The migration logic using
oldBezierKeyFramesToNew
is currently inline. Consider moving this to a dedicated migration module or documenting the timeline for removing this temporary code.Would you like me to help create a migration strategy or track this TODO in a separate issue?
packages/effects-core/src/math/value-getters/bezier-curve.ts (1)
84-113
: Consider using binary search for keyframe lookupThe current implementation uses linear search to find the correct keyframe interval. For curves with many keyframes, consider using binary search to improve performance from O(n) to O(log n).
Example implementation:
- for (let i = 0; i < this.keyFrames.length - 1; i++) { - const xMin = this.keyFrames[i].time; - const xMax = this.keyFrames[i + 1].time; - - if (time >= xMin && time < xMax) { + // Binary search for the correct interval + let left = 0; + let right = this.keyFrames.length - 2; + while (left <= right) { + const mid = Math.floor((left + right) / 2); + const xMin = this.keyFrames[mid].time; + const xMax = this.keyFrames[mid + 1].time; + + if (time >= xMin && time < xMax) { + const i = mid;packages/effects-core/src/components/base-render-component.ts (1)
317-359
: Track removal of legacy texture split logicThe TODO comment indicates this is legacy code for packed texture splitting. Consider creating an issue to track its removal and ensure it's properly deprecated before removal.
Would you like me to create an issue to track the removal of this legacy texture splitting logic?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/effects-core/src/shader/item.vert.glsl
is excluded by!**/*.glsl
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (55)
CHANGELOG-zh_CN.md
(1 hunks)CHANGELOG.md
(1 hunks)package.json
(0 hunks)packages/effects-core/package.json
(2 hunks)packages/effects-core/src/asset-loader.ts
(1 hunks)packages/effects-core/src/components/base-render-component.ts
(8 hunks)packages/effects-core/src/components/shape-component.ts
(3 hunks)packages/effects-core/src/composition.ts
(0 hunks)packages/effects-core/src/fallback/index.ts
(3 hunks)packages/effects-core/src/fallback/migration.ts
(1 hunks)packages/effects-core/src/fallback/utils.ts
(2 hunks)packages/effects-core/src/material/mask-ref-manager.ts
(2 hunks)packages/effects-core/src/material/types.ts
(0 hunks)packages/effects-core/src/math/bezier.ts
(2 hunks)packages/effects-core/src/math/value-getters/bezier-curve.ts
(1 hunks)packages/effects-core/src/math/value-getters/color-curve.ts
(1 hunks)packages/effects-core/src/math/value-getters/index.ts
(1 hunks)packages/effects-core/src/math/value-getters/value-getter-map.ts
(2 hunks)packages/effects-core/src/math/value-getters/value-getter.ts
(1 hunks)packages/effects-core/src/math/value-getters/vector-curves.ts
(1 hunks)packages/effects-core/src/plugins/particle/particle-system.ts
(4 hunks)packages/effects-core/src/plugins/sprite/sprite-item.ts
(4 hunks)packages/effects-core/src/plugins/text/text-item.ts
(4 hunks)packages/effects-core/src/shape/geometry.ts
(1 hunks)packages/effects-core/src/utils/color.ts
(1 hunks)packages/effects-core/src/utils/index.ts
(1 hunks)packages/effects-core/src/utils/promise-util.ts
(1 hunks)packages/effects-helper/package.json
(1 hunks)packages/effects-threejs/package.json
(1 hunks)packages/effects-threejs/src/three-sprite-component.ts
(2 hunks)packages/effects-threejs/src/three-text-component.ts
(3 hunks)packages/effects-webgl/package.json
(1 hunks)packages/effects-webgl/src/gl-shader-library.ts
(3 hunks)packages/effects/package.json
(1 hunks)packages/effects/src/player.ts
(1 hunks)plugin-packages/alipay-downgrade/package.json
(1 hunks)plugin-packages/downgrade/package.json
(1 hunks)plugin-packages/editor-gizmo/package.json
(1 hunks)plugin-packages/model/package.json
(1 hunks)plugin-packages/multimedia/package.json
(1 hunks)plugin-packages/multimedia/src/audio/audio-component.ts
(1 hunks)plugin-packages/multimedia/src/video/video-component.ts
(4 hunks)plugin-packages/orientation-transformer/package.json
(1 hunks)plugin-packages/rich-text/package.json
(1 hunks)plugin-packages/spine/package.json
(1 hunks)plugin-packages/spine/src/spine-component.ts
(2 hunks)plugin-packages/stats/package.json
(1 hunks)plugin-packages/stats/src/hooks/texture-hook.ts
(3 hunks)plugin-packages/stats/src/monitor.ts
(6 hunks)plugin-packages/stats/src/stats-component.ts
(2 hunks)plugin-packages/stats/src/stats.ts
(3 hunks)web-packages/demo/src/render-level.ts
(1 hunks)web-packages/test/case/src/2d/assets/dynamic.ts
(1 hunks)web-packages/test/case/src/common/test-controller.ts
(1 hunks)web-packages/test/unit/src/effects-core/composition/graph.spec.ts
(2 hunks)
💤 Files with no reviewable changes (3)
- package.json
- packages/effects-core/src/material/types.ts
- packages/effects-core/src/composition.ts
🧰 Additional context used
🧠 Learnings (32)
plugin-packages/model/package.json (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-core/src/utils/index.ts (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-core/src/math/value-getters/vector-curves.ts (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-helper/package.json (2)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-core/src/math/value-getters/color-curve.ts (3)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-webgl/src/gl-shader-library.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/asset-loader.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/math/value-getters/index.ts (3)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/utils/color.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
plugin-packages/multimedia/src/audio/audio-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
plugin-packages/spine/src/spine-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
plugin-packages/stats/src/stats-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
web-packages/demo/src/render-level.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects/src/player.ts (2)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
Learnt from: yiiqii
PR: galacean/effects-runtime#214
File: packages/effects/src/player.ts:416-429
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Data compatibility or processing logic related to replacing text variables in scene compositions should be handled in the AssetManager class through the updateOptions method, not in the Player class.
packages/effects-core/src/fallback/utils.ts (3)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-core/src/components/shape-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/fallback/index.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-threejs/src/three-sprite-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/math/value-getters/value-getter.ts (3)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
packages/effects-core/src/math/value-getters/value-getter-map.ts (3)
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
web-packages/test/unit/src/effects-core/composition/graph.spec.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
plugin-packages/stats/src/monitor.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-threejs/src/three-text-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/plugins/particle/particle-system.ts (3)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-07-26T21:12:19.313Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
Learnt from: liuxi150
PR: galacean/effects-runtime#0
File: :0-0
Timestamp: 2024-10-17T07:15:16.523Z
Learning: Imports in `plugin-packages/model/src/utility/plugin-helper.ts` do not need to be merged as per project preference or coding standards.
plugin-packages/stats/src/stats.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
plugin-packages/multimedia/src/video/video-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/plugins/sprite/sprite-item.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/plugins/text/text-item.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/material/mask-ref-manager.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/math/value-getters/bezier-curve.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/math/bezier.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
packages/effects-core/src/components/base-render-component.ts (1)
Learnt from: wumaolinmaoan
PR: galacean/effects-runtime#691
File: packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts:39-42
Timestamp: 2024-10-18T09:15:08.038Z
Learning: In `packages/effects-core/src/plugins/timeline/playables/color-property-mixer-playable.ts`, when accumulating color components, avoid using `clone()` methods because it can cause additional garbage collection overhead.
🧬 Code Graph Analysis (5)
packages/effects-core/src/fallback/index.ts (1)
packages/effects-core/src/index.ts (1)
version
(59-59)
plugin-packages/stats/src/stats.ts (3)
packages/effects-core/src/utils/index.ts (1)
Disposable
(36-38)plugin-packages/stats/src/stats-component.ts (1)
StatsComponent
(9-37)packages/effects-core/src/utils/promise-util.ts (1)
Deferred
(5-13)
plugin-packages/multimedia/src/video/video-component.ts (1)
packages/effects-core/src/asset.ts (1)
Asset
(3-5)
packages/effects-core/src/material/mask-ref-manager.ts (1)
packages/effects-core/src/material/types.ts (1)
Maskable
(90-92)
packages/effects-core/src/math/value-getters/bezier-curve.ts (4)
packages/effects-core/src/math/bezier.ts (3)
BezierEasing
(258-358)BezierMap
(16-16)getControlPoints
(422-487)packages/effects-core/src/math/value-getters/value-getter.ts (2)
ValueGetter
(28-81)KeyFrameMeta
(14-20)packages/effects-core/src/utils/asserts.ts (1)
assertExist
(1-5)packages/effects-core/src/math/utils.ts (2)
numberToFix
(90-94)decimalEqual
(86-88)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (82)
packages/effects/package.json (1)
3-3
: Version bump looks good – remember to tag & publishNo further issues found for the root package version update.
packages/effects-webgl/package.json (1)
3-3
: Consistent version bump confirmedPackage is in sync with the root
@galacean/effects
version.packages/effects-helper/package.json (1)
3-3
: Helper package version alignedNo additional comments.
plugin-packages/multimedia/package.json (1)
3-3
: Multimedia plugin version alignedLooks good.
plugin-packages/rich-text/package.json (1)
3-3
: Version bump looks good.The change aligns with the coordinated 2.5.0 release; no additional metadata affected.
packages/effects-threejs/package.json (1)
3-3
: All publishable packages are at version 2.5.0 – no action neededVerified with a workspace-wide scan: the only manifests without a 2.5.0 version field are the monorepo root and the demo/test packages, all marked private. Every publishable package is correctly bumped to 2.5.0.
plugin-packages/downgrade/package.json (1)
3-3
: Downgrade plugin version bumped correctly.No other manifest fields impacted; change is safe to merge.
plugin-packages/model/package.json (1)
3-3
: Model plugin version updated as expected.Matches the rest of the 2.5.0 release train.
plugin-packages/editor-gizmo/package.json (1)
3-3
: Editor-gizmo plugin follows the 2.5.0 release.Nothing else changed—good to go.
plugin-packages/alipay-downgrade/package.json (1)
3-3
: Version bump looks good.The version update to 2.5.0 aligns with the coordinated release across the codebase.
packages/effects-core/package.json (2)
3-3
: Version bump is consistent.The version update to 2.5.0 aligns with the coordinated release.
54-54
: Verify compatibility with @galacean/effects-specification v2.4.0No public changelog is available for v2.4.0, and our imports of
@galacean/effects-specification
span the entirepackages/effects-core/src/
folder. Please ensure that none of the Types, DataTypes, or interfaces you rely on have changed or been removed:
- Review all imports from
@galacean/effects-specification
inpackages/effects-core/src/**/*.ts
.- Run existing unit and integration tests against the bumped version to catch any type or runtime errors.
- If tests pass but uncertainty remains, consult the spec’s GitHub issues or reach out to the maintainers for detailed release notes.
plugin-packages/orientation-transformer/package.json (1)
3-3
: Version bump is consistent.The version update to 2.5.0 aligns with the coordinated release across all packages.
plugin-packages/spine/package.json (1)
3-3
: Version bump is consistent.The version update to 2.5.0 aligns with the coordinated release strategy.
packages/effects-core/src/utils/color.ts (1)
23-23
: Good simplification of hex color parsing.The direct assignment of the parsed RGBA array makes the code more concise while maintaining the same functionality.
packages/effects-webgl/src/gl-shader-library.ts (3)
123-123
: LGTM: Unnecessary type assertion removed.The type assertion is redundant since TypeScript can infer the type from the destructuring operation.
175-177
: Good addition: Enhanced error filtering for shader validation.The new error constant
err1
and updated conditional logic properly handle additional shader validation errors that should be ignored. This aligns with the changelog fixes for shader texture compile errors.
208-208
: LGTM: Unnecessary type assertion removed.Consistent cleanup with the earlier change, removing redundant type assertion for
cacheId
destructuring.web-packages/test/unit/src/effects-core/composition/graph.spec.ts (1)
26-33
: Excellent type safety improvement.The explicit
Node
type definition and its usage enhance type safety and code clarity without altering the existing logic. This follows TypeScript best practices for improving type inference and catching potential type-related issues.Also applies to: 35-35, 55-55
packages/effects/src/player.ts (1)
348-353
: Good optimization: Prevents unnecessary processing during disposal.The disposal check during scene loading is a smart optimization that clears compositions and prevents further processing if the player is disposed while loading. This addresses the performance issue mentioned in the changelog regarding player disposal during
loadScene
.CHANGELOG-zh_CN.md (1)
11-45
: Well-documented changelog entries.The changelog entries properly document the new features, fixes, and improvements for versions 2.5.0, 2.4.8, and 2.4.7, providing good transparency for users about what changed in each release.
packages/effects-core/src/fallback/index.ts (2)
30-34
: Good addition: Runtime version checking with advisory warnings.The early version check provides valuable feedback to users when their JSON requires a higher runtime version, helping them understand potential compatibility issues.
342-361
: Solid implementation: Version comparison logic.The
checkRuntimeVersion
function correctly parses and compares version strings using semantic versioning principles. The logic properly handles major and minor version comparisons to determine if the JSON requires a higher runtime version.packages/effects-core/src/math/value-getters/vector-curves.ts (1)
6-6
: Import correctly switched to new module – looks good.Using
import type
keeps the identifier erased at runtime and avoids increasing bundle size, while the cast stays purely for typing.
No further action needed.packages/effects-core/src/math/value-getters/color-curve.ts (1)
5-5
: Consistent type-only import – OK.The move to
./bezier-curve
matches the recent extraction and keeps tree-shaking intact.
Nothing to change.plugin-packages/stats/package.json (1)
3-3
: Synchronise workspace versions.Package version bumped to 2.5.0 – ensure every sibling workspace package depending on
@galacean/effects-plugin-stats
(or specifying the monorepo wildcard) is also aligned to 2.5.0 to prevent mismatched peer resolutions.packages/effects-core/src/math/value-getters/index.ts (1)
4-4
: LGTM! Clean modularization of bezier curve functionality.The new export properly exposes the refactored bezier curve module, supporting the broader modularization effort mentioned in the PR summary.
packages/effects-core/src/fallback/utils.ts (2)
3-3
: LGTM! Improved import organization.Consolidating vec2, vec3, and vec4 imports on the same line improves readability and follows consistent import patterns.
125-125
: LGTM! Simplified color assignment logic.The removal of the unnecessary fallback
[0, 0, 0, 255]
is appropriate since the function already handles the case correctly through the existing logic flow.packages/effects-core/src/asset-loader.ts (1)
24-24
: LGTM! Improved null safety.Moving the
guid
assignment after the null check prevents potential runtime errors from accessingdataPath.id
whendataPath
is falsy. This is a good defensive programming practice.plugin-packages/multimedia/src/audio/audio-component.ts (1)
41-41
: Confirm audio presence check updateThe switch from
if (options.audio.id)
toif (options.audio)
matches the change made in the video component and aligns with how we pass an asset ID (or identifier) directly intofindObject
. It’s a valid simplification—but please verify that:
options.audio
is always the ID (or a truthy identifier) and never an object with its own.id
field.this.engine.findObject(options.audio)
continues to resolve correctly when given that identifier type.Locations to review:
- plugin-packages/multimedia/src/audio/audio-component.ts (around line 41)
- Any callers or tests that construct the
options.audio
payload (e.g. builder code or fixtures)web-packages/demo/src/render-level.ts (1)
15-25
: LGTM! Excellent async refactoring with monitoring capabilities.The refactoring improves the code by:
- Using
Promise.all
to run scene loading and stats initialization concurrently- Properly awaiting the stats component before accessing its properties
- Adding event listener for real-time performance monitoring
This follows modern async patterns and adds valuable monitoring functionality to the demo.
packages/effects-core/src/math/value-getters/value-getter-map.ts (1)
3-3
: LGTM: Clean import reorganization.The import restructuring aligns with the BezierCurve extraction into a dedicated module. The imports are logically organized.
Also applies to: 10-10
packages/effects-threejs/src/three-sprite-component.ts (1)
1-1
: LGTM: Type standardization aligns with codebase refactoring.The change from
SpriteItemProps
tospec.SpriteComponentData
is consistent with the broader refactoring mentioned in the AI summary. The import cleanup and type update are correctly implemented.Also applies to: 70-70
plugin-packages/stats/src/stats-component.ts (1)
6-8
: LGTM: Good documentation enhancements.The JSDoc comments provide clear descriptions of the component's purpose and method functionality. The documentation improves code maintainability without affecting logic.
Also applies to: 22-25, 30-32
packages/effects-core/src/utils/promise-util.ts (1)
1-13
: LGTM: Clean implementation of the deferred pattern.The
Deferred
class properly implements the deferred pattern by exposingresolve
andreject
methods. The implementation is straightforward and follows TypeScript best practices.plugin-packages/spine/src/spine-component.ts (2)
4-4
: LGTM: Import cleanup aligns with mask handling refactoring.The import changes are consistent with the broader mask handling refactoring across the codebase.
131-131
: LGTM: Mask handling simplification improves code clarity.The simplified mask handling logic directly uses
maskManager.getMaskMode(data.mask)
instead of manual reference resolution. This is cleaner and consistent with the refactoring across other components.web-packages/test/case/src/2d/assets/dynamic.ts (1)
30-44
: LGTM! Test assets properly configured.The new dynamic assets follow the established pattern and provide appropriate test data for multimedia functionality testing.
packages/effects-core/src/fallback/migration.ts (1)
225-250
: Mask property refactoring looks correct.The changes align with the broader mask handling updates across the codebase:
mask
→isMask
provides clearer boolean semanticsmode
→inverted
simplifies the obscured mode logicref
→reference
improves property naming claritypackages/effects-core/src/math/value-getters/value-getter.ts (2)
12-12
: Good modularization approach.Extracting
BezierCurve
to a dedicated module improves code organization and maintainability.
14-20
: Interface export enhances reusability.Making
KeyFrameMeta
exported allows it to be used across modules, supporting the refactored architecture.packages/effects-core/src/components/shape-component.ts (2)
183-194
: Simplified geometry initialization is cleaner.Removing explicit attribute creation and draw modes in favor of just adding empty subMeshes aligns with the improved geometry handling approach.
527-534
: Mask handling refactoring is consistent.The simplified mask processing using
maskManager.getMaskMode(data.mask)
aligns with the standardized mask handling pattern across components.packages/effects-threejs/src/three-text-component.ts (3)
2-2
: Type import cleanup improves consistency.Removing
SpriteItemProps
import aligns with the standardization onspec.SpriteComponentData
across the codebase.
18-18
: Type casting maintains compatibility.The cast to
spec.SpriteComponentData
ensures compatibility while moving to standardized types.
39-39
: Method signature aligns with specification types.Updating
fromData
to acceptspec.SpriteComponentData
maintains consistency with the core specification interfaces.CHANGELOG.md (1)
10-44
: LGTM! Comprehensive changelog documentation.The changelog entries for versions 2.5.0, 2.4.8, and 2.4.7 are well-structured and provide comprehensive documentation of the changes. The format is consistent with existing entries, and the content aligns with the broader codebase updates mentioned in the AI summary.
plugin-packages/stats/src/stats.ts (5)
2-2
: Good addition of Deferred utility import.The import of
Deferred
utility enables the implementation of proper asynchronous initialization tracking, which improves the reliability of the Stats plugin API.
104-132
: Excellent documentation with practical examples.The enhanced JSDoc documentation provides clear usage patterns and practical examples that will significantly help developers understand how to use the Stats plugin correctly. The examples show both simple usage and advanced patterns with custom event handling.
136-136
: Well-implemented asynchronous initialization tracking.The
initialized
Deferred promise provides a clean way to track when the Stats component is ready for use. This prevents race conditions and ensures safe access to the component.
161-163
: Proper promise resolution in finally block.Resolving the
initialized
promise in the finally block ensures that consumers can proceed regardless of whether initialization succeeded or failed, which is the correct pattern for completion tracking.
166-174
: Excellent async API design.The
getComponent()
method provides a clean, async API that automatically waits for initialization to complete before returning the component. This eliminates the need for consumers to manually coordinate timing and makes the API much more user-friendly.packages/effects-core/src/plugins/particle/particle-system.ts (3)
11-11
: Good addition of Maskable type import.Adding the explicit import for
Maskable
improves type safety and makes the interface implementation more explicit.
129-129
: Excellent type standardization for mask options.Replacing the inline mask property types with
spec.MaskOptions
standardizes mask handling across the codebase and reduces type duplication. This aligns with the broader mask refactoring mentioned in the AI summary.Also applies to: 143-143
1168-1181
: Simplified and improved mask handling.The refactored
getMaskOptions
method is cleaner and more maintainable by directly usingmaskManager.getMaskMode(data.mask)
instead of manual mask reference resolution. This leverages the centralized mask processing logic and reduces code duplication.plugin-packages/stats/src/monitor.ts (5)
62-67
: Well-designed event type definition.The
MonitorEvent
type provides clear type safety for the event emitter interface, ensuring consumers can properly handle the'update'
event with typed performance data.
72-76
: Good architectural improvement with EventEmitter.The Monitor class extending EventEmitter with the core property being public enables better integration patterns. This allows external consumers to access performance data both through events and direct property access.
105-108
: Proper initialization pattern.The constructor correctly calls
super()
and initializes thevisible
state from options, maintaining consistency with the existing options pattern.
147-151
: Efficient event emission with early return.The event emission before DOM updates is well-placed, and the early return when not visible prevents unnecessary DOM operations while still allowing event subscribers to receive updates.
169-177
: Consistent visibility state management.The
hide()
andshow()
methods properly synchronize the internalvisible
flag with the DOM display style, ensuring state consistency.packages/effects-core/src/plugins/text/text-item.ts (2)
20-26
: Good type standardization with spec.MaskOptions.The update from extending
MaskProps
to using an optionalmask
property of typespec.MaskOptions
aligns with the broader refactoring effort to standardize mask handling across components.
101-101
: Consistent fromData method signature update.The
fromData
method signature update to usespec.TextComponentData
maintains consistency with the constructor changes and spec type standardization.packages/effects-core/src/material/mask-ref-manager.ts (2)
10-11
: Good addition of alpha mask support.The new
alphaMaskEnabled
andmaskMode
properties provide better control over mask processing, with sensible default values (false
andMaskMode.NONE
respectively).
24-39
: Improved mask processing with simplified logic.The updated
getMaskMode
method effectively:
- Uses destructuring with sensible defaults for
spec.MaskOptions
- Sets the
alphaMaskEnabled
property from the input- Simplifies the mask mode determination logic
- Handles reference resolution internally via
engine.findObject
This is a cleaner implementation that centralizes mask reference resolution.
packages/effects-core/src/plugins/sprite/sprite-item.ts (3)
45-51
: Consistent type standardization.The constructor signature update to use
spec.SpriteComponentData
aligns with the broader refactoring effort to standardize component data types across the codebase.
72-72
: Important video frame update addition.The
this.renderer.texture.uploadCurrentVideoFrame()
call ensures video frames are properly updated during playback, which is crucial for video texture rendering.
135-145
: Simplified fromData method.The
fromData
method is now cleaner, focusing only on interaction and color setup, with geometry creation likely delegated to theBaseRenderComponent
as part of the architectural refactor.plugin-packages/stats/src/hooks/texture-hook.ts (4)
1-2
: Excellent type safety improvement.The
CreateTexture
andDeleteTexture
type aliases provide better type safety and code clarity by capturing the exact signatures of the WebGL methods being hooked.
10-11
: Consistent usage of type aliases.The private fields now use the defined type aliases, providing better type inference and consistency throughout the class.
29-39
: Improved method signature flexibility.The refactored
hookedCreateTexture
using arrow function syntax with rest parameters (...args
) provides better flexibility while maintaining the same functionality. The method correctly forwards all arguments to the original WebGL method.
41-48
: Type-safe parameter handling.The
hookedDeleteTexture
method usesParameters<DeleteTexture>
for proper type safety while maintaining the rest parameter pattern. This ensures all arguments are correctly typed and forwarded.plugin-packages/multimedia/src/video/video-component.ts (1)
108-125
: Good addition of safety checks for video asset access.The null checks prevent potential runtime errors when the video asset is not found. This defensive programming approach improves robustness.
packages/effects-core/src/math/bezier.ts (3)
262-287
: Good refactoring to use Vector2 for control pointsThe refactoring to use
Vector2
instances for control points improves code organization and type safety. The multiple constructor overloads provide good backward compatibility.
289-308
: Well-structured getValue method with appropriate optimizationsThe method correctly handles different cases:
- Constant curves return 0
- Linear curves (where control x equals y) return the input directly
- Simple non-weighted curves use direct interpolation
- Complex weighted curves use the optimized Newton-Raphson/binary subdivision approach
310-320
: Correct implementation of cubic Bezier interpolationThe method correctly implements the standard cubic Bezier curve formula with proper coefficients.
packages/effects-core/src/math/value-getters/bezier-curve.ts (3)
51-82
: Well-structured curve initializationThe
onCreate
method properly initializes the bezier curve segments from keyframes, building the necessary data structures for efficient curve evaluation.
204-289
: Robust bezier easing construction with proper validationThe function correctly handles different curve types (constant, weighted, simple) and includes important validation for control point x-coordinates to ensure they remain within [0,1] range. The caching mechanism is a good optimization.
298-376
: Proper legacy format conversion with division-by-zero protectionThe conversion function correctly transforms legacy bezier keyframes to the new format. Good use of
NumberEpsilon
in the slope calculation to prevent division by zero.packages/effects-core/src/components/base-render-component.ts (3)
65-66
: Good encapsulation of color property with automatic material synchronizationThe refactoring to use a private
_color
field with getter/setter ensures that material uniforms are always synchronized when the color changes. The backward compatibility is well maintained.Also applies to: 147-179
477-528
: Well-implemented custom geometry support with UV transformationThe implementation correctly handles custom geometry with proper UV transformation including rotation support. Good defensive programming with
slice()
to avoid modifying shared data.
85-110
: Note on defaultGeometry instance scope
ThedefaultGeometry
field is initialized in the constructor as an instance property (notstatic
), so each component gets its own Geometry object. Mutations inconfigureDefaultGeometry
only affect that component’s geometry—you won’t inadvertently alter other component instances.
private defaultGeometry: Geometry;
is assigned in the constructor.this.geometry = this.defaultGeometry;
and all subsequentsetIndexData
/setAttributeData
calls operate on that one per-instance object.- There’s no global or shared singleton Geometry.
No changes are required to guard against cross-instance sharing.
Likely an incorrect or invalid review comment.
const params = new URLSearchParams(location.search); | ||
const oldVersion = params.get('version') || '2.4.5'; // 旧版 Player 版本 | ||
const oldVersion = params.get('version') || '2.4.8'; // 旧版 Player 版本 | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify availability of version 2.4.8 on all CDN artefacts.
Bumping the fallback version is sensible, but any missing artefact on UNPKG (@galacean/effects
, plus each plugin) will break the test harness at runtime.
Please make sure 2.4.8 is published for the core package and every plugin referenced below.
🏁 Script executed:
#!/bin/bash
# Quick smoke-check that the core and four plugins have a 2.4.8 tag on UNPKG
for pkg in "@galacean/effects" \
"@galacean/effects-plugin-model" \
"@galacean/effects-plugin-rich-text" \
"@galacean/effects-plugin-spine" \
"@galacean/effects-plugin-orientation-transformer"
do
printf "%s: " "$pkg"
curl -s -I "https://unpkg.com/${pkg}@2.4.8/dist/index.min.js" | grep -q '200 OK' \
&& echo "found" || echo "MISSING!"
done
Length of output: 2016
Missing UNPKG artifacts for version 2.4.8
The fallback version 2.4.8
isn’t published on UNPKG, so the test harness will break at runtime. All of these artefacts returned MISSING!
:
- @galacean/effects
- @galacean/effects-plugin-model
- @galacean/effects-plugin-rich-text
- @galacean/effects-plugin-spine
- @galacean/effects-plugin-orientation-transformer
Please either publish v2.4.8 for the core package and each plugin, or update the fallback to the latest available version.
🤖 Prompt for AI Agents
In web-packages/test/case/src/common/test-controller.ts around lines 6 to 8, the
fallback version '2.4.8' is not published on UNPKG, causing runtime failures.
Update the fallback version string to the latest available published version for
@galacean/effects and its plugins to ensure the test harness can load the
necessary artifacts without errors.
@@ -69,11 +70,10 @@ export class TextComponent extends BaseRenderComponent { | |||
protected readonly SCALE_FACTOR = 0.1; | |||
protected readonly ALPHA_FIX_VALUE = 1 / 255; | |||
|
|||
constructor (engine: Engine, props?: TextItemProps) { | |||
constructor (engine: Engine, props?: spec.TextComponentData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify constructor parameter type consistency.
The constructor signature change from TextItemProps
to spec.TextComponentData
is consistent with the interface updates and spec type standardization.
🏁 Script executed:
#!/bin/bash
# Verify that all TextComponent usage has been updated to use spec.TextComponentData
ast-grep --pattern 'new TextComponent($_, $_)' -A 2
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Search for TextItem instantiations to verify constructor usage
rg -n "new TextItem" -A 2
# Search for any remaining references to the old TextItemProps type
rg -n "TextItemProps" -A 2
# Ensure spec.TextComponentData is used consistently
rg -n "TextComponentData" -A 2
Length of output: 1308
Normalize TextItemProps
and constructor signature
The local TextItemProps
interface is still declared but never used—yet the constructor now accepts spec.TextComponentData
, leading to a mismatch. Please either remove TextItemProps
(if it’s obsolete) or update the constructor to accept TextItemProps
and convert it to spec.TextComponentData
internally.
Locations to update:
- packages/effects-core/src/plugins/text/text-item.ts:20–22 — remove or refactor the
export interface TextItemProps { … }
block - packages/effects-core/src/plugins/text/text-item.ts:73 — change the constructor parameter from
props?: spec.TextComponentData
toprops?: TextItemProps
(or remove the unused interface if opting to keepspec.TextComponentData
)
Suggested diff if removing TextItemProps
:
--- a/packages/effects-core/src/plugins/text/text-item.ts
@@ -20,7 +20,6 @@ export interface TextItemProps extends Omit<spec.TextContent, 'renderer' | 'mask'> {
listIndex?: number,
mask?: spec.MaskOptions,
-}
+}
--- a/packages/effects-core/src/plugins/text/text-item.ts
@@ -73,7 +72,7 @@ export class TextItem extends SomeBase {
- constructor(engine: Engine, props?: spec.TextComponentData) {
+ constructor(engine: Engine, props?: spec.TextComponentData) {
super(engine);
// …
—or—
Suggested diff if keeping TextItemProps
:
--- a/packages/effects-core/src/plugins/text/text-item.ts
@@ -73,7 +73,7 @@ export class TextItem extends SomeBase {
- constructor(engine: Engine, props?: spec.TextComponentData) {
+ constructor(engine: Engine, props?: TextItemProps) {
super(engine);
this.applyProps(props);
// transform `TextItemProps` → `spec.TextComponentData` before calling super.fromData()
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/effects-core/src/plugins/text/text-item.ts around lines 20 to 22 and
line 73, the TextItemProps interface is declared but unused, while the
constructor accepts spec.TextComponentData causing a mismatch. To fix this,
either remove the TextItemProps interface entirely if it is obsolete, and keep
the constructor parameter as spec.TextComponentData, or update the constructor
to accept props?: TextItemProps and internally convert it to
spec.TextComponentData. Also, remove or refactor the TextItemProps interface
declaration accordingly to maintain consistency.
this.renderer.texture.uploadCurrentVideoFrame(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify performance impact of per-component video frame uploads.
Moving video frame upload responsibility to individual components provides better control but could impact performance with multiple video components. Consider whether frame upload coordination or throttling might be needed at a higher level.
🏁 Script executed:
#!/bin/bash
# Check for other video components and their update patterns
rg -A 5 "uploadCurrentVideoFrame|updateVideoFrame" --type ts
Length of output: 1974
Centralize or throttle per-component video frame uploads
The call to this.renderer.texture.uploadCurrentVideoFrame()
now lives in multiple spots:
- plugin-packages/multimedia/src/video/video-component.ts (line 152)
- packages/effects-core/src/plugins/sprite/sprite-item.ts
- Underlying implementations in packages/effects-webgl/src/gl-texture.ts and packages/effects-core/src/texture/texture.ts
Individually uploading frames per component can trigger redundant GPU work and hurt performance when multiple videos or sprites render simultaneously. To mitigate this, consider:
- Introducing a shared scheduler or manager that batches/throttles calls to
uploadCurrentVideoFrame()
. - Driving all uploads from a single
requestAnimationFrame
loop or composition-level update. - Profiling upload times under load to identify and address any bottlenecks.
🤖 Prompt for AI Agents
In plugin-packages/multimedia/src/video/video-component.ts around lines 152-153,
the direct call to this.renderer.texture.uploadCurrentVideoFrame() should be
refactored to avoid redundant GPU uploads when multiple components invoke it
independently. Implement a centralized scheduler or manager that batches or
throttles these upload calls, ideally driven by a single requestAnimationFrame
loop or a composition-level update mechanism, to coordinate and minimize GPU
workload. Replace the direct call with a registration or enqueueing method to
this shared manager, ensuring uploads happen efficiently and only once per
frame.
fix: text render issue
docs: changelog 2.5.1
Summary by CodeRabbit
New Features
Bug Fixes
Performance Improvements
Documentation
Other