Merge origin/main into origin/dev #1073
Conversation
* fix: opt canvas and gl dispose logic * chore: update * chore: update
fix: composition autoplay issue
fix: bezier path build const bezier easing
* fix: composition play issue * chore: add multi load type demo * chore: update case test version --------- Co-authored-by: yiiqii <[email protected]>
docs: changelog 2.5.3
WalkthroughThis update introduces patch versions 2.5.2 and 2.5.3 across multiple packages, reflecting several bug fixes and internal improvements. Key changes include adjustments to composition playback control, canvas ownership tracking, bezier easing instantiation, and demo asset management. Documentation and demo files were updated to align with these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Player
participant DOM
participant Composition
User->>Player: new Player(container, canvas?)
Player->>DOM: (optionally create canvas)
Player->>Player: set useExternalCanvas flag
Note over Player: On error during construction
alt useExternalCanvas is false
Player->>DOM: Remove internally created canvas
end
User->>Player: loadScene(sceneAssets)
loop For each asset
Player->>Composition: createComposition(asset)
alt autoplay
Player->>Composition: play()
else
Player->>Composition: pause()
end
Player->>Player: compositions.push(composition)
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web-packages/demo/src/multi-load-type.ts (1)
31-43: Consider improving code maintainability and consistency.The delayed scene loading demonstration is effective, but there are a few suggestions:
- The comment on line 31 is in Chinese - consider using English for consistency
- Consider extracting the hardcoded URLs to constants for better maintainability
+const SCENE_URLS = { + MAIN: 'https://gw.alipayobjects.com/os/gltf-asset/mars-cli/ILDKKFUFMVJA/1705406034-80896.json', + CARD: 'https://mdn.alipayobjects.com/mars/afts/file/A*u-NFTK_DS0IAAAAAAAAAAAAAelB4AQ', + DELAYED: 'https://mdn.alipayobjects.com/mars/afts/file/A*k6vyTpyAkykAAAAARYAAAAgAelB4AQ' +}; const [_, card] = await player.loadScene([ - json, + SCENE_URLS.MAIN, { - url: 'https://mdn.alipayobjects.com/mars/afts/file/A*u-NFTK_DS0IAAAAAAAAAAAAAelB4AQ', + url: SCENE_URLS.CARD, options: { autoplay: false, }, }, ]); -// 改为点击触发即可 +// Could be triggered by click event instead setTimeout(async () => { const composition = await player.loadScene({ - url: 'https://mdn.alipayobjects.com/mars/afts/file/A*k6vyTpyAkykAAAAARYAAAAgAelB4AQ', + url: SCENE_URLS.DELAYED, options: { autoplay: false, }, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
CHANGELOG-zh_CN.md(1 hunks)CHANGELOG.md(1 hunks)packages/effects-core/package.json(1 hunks)packages/effects-core/src/composition.ts(1 hunks)packages/effects-core/src/math/bezier.ts(1 hunks)packages/effects-helper/package.json(1 hunks)packages/effects-threejs/package.json(1 hunks)packages/effects-webgl/package.json(1 hunks)packages/effects-webgl/src/gl-engine.ts(1 hunks)packages/effects/package.json(1 hunks)packages/effects/src/player.ts(4 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/orientation-transformer/package.json(1 hunks)plugin-packages/rich-text/package.json(1 hunks)plugin-packages/spine/package.json(1 hunks)plugin-packages/stats/package.json(1 hunks)web-packages/demo/html/multi-load-type.html(2 hunks)web-packages/demo/index.html(1 hunks)web-packages/demo/src/multi-load-type.ts(1 hunks)web-packages/demo/src/shape.ts(0 hunks)web-packages/demo/vite.config.js(1 hunks)web-packages/test/case/src/common/test-controller.ts(1 hunks)
💤 Files with no reviewable changes (1)
- web-packages/demo/src/shape.ts
🧰 Additional context used
🧠 Learnings (9)
plugin-packages/rich-text/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.
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-webgl/src/gl-engine.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/downgrade/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/composition.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-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.
web-packages/demo/src/multi-load-type.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/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.
🧬 Code Graph Analysis (2)
packages/effects-core/src/math/bezier.ts (1)
packages/effects-core/src/math/utils.ts (1)
decimalEqual(86-88)
web-packages/demo/src/multi-load-type.ts (1)
packages/effects/src/player.ts (1)
Player(24-958)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (30)
packages/effects-webgl/package.json (1)
3-3: Version bump to 2.5.3 looks good.No further issues spotted.
Please remember to regenerate / commit the workspace lockfile (pnpm-lock.yaml) so the registry receives the exact dependency graph corresponding to this version.packages/effects-core/package.json (1)
3-3: Patch-version alignment confirmed.
@galacean/effects-coreis now at 2.5.3, matching the ecosystem.
Same note about updating the lockfile before publishing.packages/effects/package.json (1)
3-3: Player package version increment acknowledged.Everything else in the manifest remains unchanged and consistent.
Ensure you runpnpm install(orpnpm -r exec npm packin CI) to verify there are no peer-dependency warnings after the bump.packages/effects-threejs/package.json (1)
3-3: ThreeJS plugin version updated correctly.No discrepancies found.
Lockfile refresh still required before publish.plugin-packages/stats/package.json (1)
3-3: Stats plugin patch release accepted.Manifest remains otherwise untouched and valid.
plugin-packages/model/package.json (1)
3-3: Patch bump is correct and consistentVersion updated to
2.5.3, matching the coordinated patch release across the repo.
No further action required.plugin-packages/rich-text/package.json (1)
3-3: Version synchronized successfully
2.5.3aligns with the core and sibling plugin packages.
Looks good.packages/effects-helper/package.json (1)
3-3: Helper package version bump verifiedPatch version set to
2.5.3; this keeps the helper library in lock-step with the runtime packages.plugin-packages/orientation-transformer/package.json (1)
3-3: Orientation-transformer version updatedChange to
2.5.3is correct and consistent with the mono-repo release strategy.plugin-packages/spine/package.json (1)
3-3: Spine plugin patch release acknowledged
2.5.3version bump matches the rest of the ecosystem; nothing else to flag.plugin-packages/editor-gizmo/package.json (1)
3-3: Version bump looks goodPatch-level increment to 2.5.3 aligns with the rest of the workspace.
No other metadata affected—nothing else to flag.plugin-packages/alipay-downgrade/package.json (1)
3-3: Consistent patch upgradeThe package now targets 2.5.3, keeping it in step with the core and sibling plugins.
Ship it.plugin-packages/downgrade/package.json (1)
3-3: Patch version synced
2.5.3is correctly applied. All other manifest sections remain intact.plugin-packages/multimedia/package.json (1)
3-3: Patch version bump acknowledgedNo additional changes observed. Good to merge.
web-packages/test/case/src/common/test-controller.ts (1)
7-7: Confirm availability of old artefacts on CDNThe fallback version for the legacy player/plugins moved to
2.5.2.
Before merging, make sure every required package (player +model,rich-text,spine,orientation-transformer) is published to npm at exactly 2.5.2; otherwise the tests will break whenunpkg404s.If 2.5.3 is already published for all those artefacts, consider switching the constant to
2.5.3for consistency.packages/effects-core/src/math/bezier.ts (1)
407-411: LGTM! Proper handling of constant bezier easing.The conditional instantiation of
BezierEasingcorrectly handles the case wherevalueIntervalis zero by creating a default (constant) easing instance. This optimization avoids unnecessary computations for constant bezier curves and aligns with the constructor's design where no parameters result inisConstant = true.web-packages/demo/html/multi-load-type.html (2)
6-6: LGTM! Title update reflects demo content change.The title change from "图形元素 - demo" to "多加载类型 - demo" accurately reflects the shift from a shape-focused demo to a multi-load-type demo.
19-19: LGTM! Script source updated consistently.The script source change from "../src/shape.ts" to "../src/multi-load-type.ts" is consistent with the demo content update and aligns with the coordinated changes across the demo files.
web-packages/demo/index.html (1)
24-24: LGTM! Navigation updated to reflect demo changes.The navigation link update from "图形元素" (shape.html) to "多加载类型" (multi-load-type.html) is consistent with the demo content restructuring and maintains proper navigation flow.
packages/effects-webgl/src/gl-engine.ts (1)
13-13: LGTM! Property name standardization.The change from
this.isDestroyedtothis.destroyedappears to be a property name standardization that maintains the same disposal logic while improving consistency across the codebase.CHANGELOG.md (1)
10-23: LGTM! Well-formatted changelog entries.The changelog entries for versions 2.5.3 and 2.5.2 are properly formatted, include appropriate PR references, and accurately describe the fixes implemented in the codebase. The entries align with the code changes reviewed in the other files.
web-packages/demo/vite.config.js (1)
31-31: LGTM! Demo configuration update is correct.The replacement of the 'shape' demo with 'multi-load-type' demo is properly configured in the build input entries.
CHANGELOG-zh_CN.md (1)
11-24: LGTM! Changelog entries are properly formatted and documented.The version entries for 2.5.3 and 2.5.2 correctly document the composition autoplay fixes, bezier easing improvements, and canvas dispose logic optimizations with proper dates and PR references.
packages/effects-core/src/composition.ts (1)
619-621: Removal ofassignedcheck in Composition.update is safeAll references to the old
assignedflag have been removed, andComposition.updateis only ever called by:
- Player (in its ticker loop via
this.compositions.forEach(comp => comp.update(dt)))- The Three.js display‐object plugin (
composition.update(delta))Player’s own methods (
createComposition/loadScene→this.compositionsanddestroyCurrentCompositions) fully manage when compositions are added and removed, ensuring they’re initialized before any update runs. No further action required.web-packages/demo/src/multi-load-type.ts (2)
1-15: LGTM! Well-structured demo with proper async patterns and error handling.The Player initialization and error handling setup look good, properly demonstrating the interactive and transparent background features.
17-29: Good demonstration of multi-scene loading with different autoplay settings.The code properly demonstrates loading multiple scenes with different configurations and composition manipulation. The destructuring assignment and subsequent manipulation of the
cardcomposition shows the key features well.packages/effects/src/player.ts (4)
71-71: Clean property addition for canvas ownership tracking.The
useExternalCanvasproperty provides a clear way to track canvas ownership, which is essential for proper cleanup logic.
120-120: Proper canvas ownership flag setting.The flag is correctly set when an external canvas is provided, enabling proper cleanup decisions later.
156-158: Excellent canvas cleanup improvement.This conditional removal logic properly respects canvas ownership - only cleaning up internally created canvases while preserving externally provided ones. This prevents potential DOM manipulation issues and maintains proper resource management.
386-388: Critical timing fix for composition management.This placement prevents premature rendering of non-autoplay compositions during multi-composition loading scenarios. The explicit comment warns against moving this code, indicating this addresses a specific and important rendering issue.
Summary by CodeRabbit
New Features
Bug Fixes
Chores