Merge origin/main into origin/feat/2.5 #1045
Conversation
fix: composition freeze issue
* fix: fix rich-text character overlapping bug * test: 补充富文本字体遮挡的帧对比 case --------- Co-authored-by: yiiqii <[email protected]>
Merge origin/dev into origin/main
docs: changelog 2.4.8
WalkthroughThis update increments the version to 2.4.8 across multiple packages and plugins, documents the release in changelogs, and introduces fixes for rich-text character overlapping and composition freeze issues. It refactors time advancement logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Scene
participant Player
TestRunner->>Scene: Extract url, name, variables
TestRunner->>Player: Initialize with url and variables
Player->>Scene: Render with variables
TestRunner->>Player: Compare rendered output (stricter thresholds)
sequenceDiagram
participant Composition
participant RootItem
participant Video
participant Plugins
participant Scene
Composition->>Composition: forwardTime(deltaTime)
Composition->>Composition: updateCompositionTime(scaledDeltaTime)
Composition->>RootItem: Update time, handle end behavior
Composition->>Video: Update with actual deltaTime
Composition->>Plugins: Update loaders
Composition->>Scene: Tick with deltaTime
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)
packages/effects-core/src/composition.ts (1)
587-587: Address the FIXME comment for optimization.The FIXME comment indicates a known issue where the update process might modify composition time, requiring optimization of the update logic.
Since this relates to the composition freeze fix, would you like me to help analyze potential race conditions or timing issues that could arise from composition time modifications during updates?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
CHANGELOG-zh_CN.md(1 hunks)CHANGELOG.md(1 hunks)packages/effects-core/package.json(1 hunks)packages/effects-core/src/composition.ts(4 hunks)packages/effects-core/src/plugins/timeline/track.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/package.json(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/orientation-transformer/package.json(1 hunks)plugin-packages/rich-text/package.json(1 hunks)plugin-packages/rich-text/src/rich-text-component.ts(8 hunks)plugin-packages/spine/package.json(1 hunks)plugin-packages/stats/package.json(1 hunks)web-packages/test/case/src/2d/assets/dynamic.ts(1 hunks)web-packages/test/case/src/2d/dynamic.spec.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
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.
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.
packages/effects-core/src/plugins/timeline/track.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/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.
web-packages/test/case/src/2d/assets/dynamic.ts (1)
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.
web-packages/test/case/src/2d/dynamic.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.
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.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (34)
packages/effects/package.json (1)
3-3: Version bump looks good and follows SemVer patch semantics
No further issues spotted for this package.json.packages/effects-helper/package.json (1)
3-3: Patch release number correctly updated
Change is minimal and consistent with the overall 2.4.8 release train.packages/effects-core/package.json (2)
3-3: Core package version properly incremented
No metadata, script, or dependency anomalies detected.
1-60: Verify every workspace package now reports2.4.8
A quick sanity-check helps catch any stragglers that may have been missed in the version sweep.#!/bin/bash # List every package.json version in the repo to confirm they are at 2.4.8 fd -t f package.json | while read -r f; do name=$(jq -r '.name' "$f") ver=$(jq -r '.version' "$f") printf '%-60s %s\n' "$name" "$ver" done | sortplugin-packages/spine/package.json (1)
3-3: Spine plugin aligned with 2.4.8 tag
All good here.plugin-packages/stats/package.json (1)
3-3: Stats plugin version sync confirmed
Nothing else to address.packages/effects-webgl/package.json (1)
3-3: Version bump to 2.4.8 looks good and consistent with the release train.
No further changes required.plugin-packages/rich-text/package.json (1)
3-3: Rich-text plugin correctly aligned to 2.4.8.
Nothing else stands out—good to go.plugin-packages/orientation-transformer/package.json (1)
3-3: Orientation-transformer plugin version updated correctly.
No additional adjustments needed.plugin-packages/alipay-downgrade/package.json (1)
3-3: Alipay-downgrade plugin now at 2.4.8—change looks straightforward.
All good from my side.plugin-packages/multimedia/package.json (1)
3-3: Multimedia plugin version increment verified.
Change is minimal and consistent.plugin-packages/model/package.json (2)
3-3: Version bump looks good – matches the global 2.4.8 release.
No other metadata impacted.
3-3: (Optional) sanity-check all workspace packages share the same new version.
A quick script can prevent an accidental mismatch.#!/usr/bin/env bash # List any package.json whose version is NOT 2.4.8 fd package.json | xargs jq -r '.name + " " + .version' | grep -v ' 2.4.8$' || echo "✅ all packages on 2.4.8"plugin-packages/downgrade/package.json (1)
3-3: Consistent patch upgrade – no issues.packages/effects-threejs/package.json (1)
3-3: Three-js bridge package correctly bumped to 2.4.8.plugin-packages/editor-gizmo/package.json (1)
3-3: Gizmo plugin version updated; everything else unchanged.CHANGELOG.md (1)
10-16: Changelog entry added and formatted correctly.
Links, dates and authors render fine.CHANGELOG-zh_CN.md (1)
11-17: LGTM! Properly documents the version 2.4.8 fixes.The changelog entries correctly document the rich-text character overlapping fix and composition freeze issue resolution, with proper PR references and contributor attribution.
packages/effects-core/src/plugins/timeline/track.ts (1)
232-232: Good precision improvement by removing unnecessary rounding.Removing the
toFixed(3)rounding preserves full floating-point precision in time calculations, which should improve timing accuracy and help resolve composition freeze issues.web-packages/test/case/src/2d/dynamic.spec.ts (3)
1-1: Good addition of spec type for template variables support.Adding the
specimport enables proper typing for the template variables functionality being tested.
8-9: Appropriately stricter test thresholds for improved rendering precision.The significantly tightened thresholds (accumRatioThreshold: 0.02 → 0.0002, pixelDiffThreshold: 10 → 1) reflect confidence in the rich-text character overlapping fixes and ensure high-precision validation of the rendering improvements.
31-33: Excellent integration of template variables into the testing framework.The addition of
variablesextraction and passing to player initialization enables comprehensive testing of dynamic text scenarios, which is crucial for validating the rich-text improvements.Also applies to: 38-38, 44-45
web-packages/test/case/src/2d/assets/dynamic.ts (1)
5-6: Comprehensive template variables for rich-text testing.The added variables provide excellent test coverage for various rich-text scenarios including multiline text, size formatting, and color formatting, which are essential for validating the character overlapping fixes.
Also applies to: 11-14, 19-21, 23-29
plugin-packages/rich-text/src/rich-text-component.ts (4)
45-46: Clear documentation of line spacing calculation.The comment effectively explains the relationship between font height and line spacing, improving code maintainability.
150-161: Excellent improvement in text width calculation precision.The enhanced width calculation using
actualBoundingBoxLeftandactualBoundingBoxRightwhen available, combined withmeasureText().width, provides more accurate text measurements and should help resolve character overlapping issues.
169-170: Good detailed explanation of width calculation formula.The comment clearly explains the complex width calculation involving font size, scale factors, and letter spacing, which aids in understanding and maintaining this critical rendering logic.
239-239: Simplified and more efficient rendering approach.The change from character-by-character rendering to segment-based
fillTextcalls should improve both performance and rendering accuracy, addressing the character overlapping bug effectively.Also applies to: 250-250, 252-252, 255-255
packages/effects-core/src/composition.ts (7)
581-581: Time calculation precision looks good.The millisecond conversion maintains precision for the delta time calculation in the forwardTime method.
633-636: Excellent refactor for accurate time delta calculation.This change improves time management by:
- Capturing the composition time before updates
- Using the actual time difference after
updateCompositionTime- Providing more accurate delta time for downstream systems
This approach is much more robust than using the input deltaTime directly, especially when dealing with end behaviors that might clamp or modify the composition time.
640-643: Consistent usage of recalculated delta time.Good implementation using
deltaTimeInMsfor both plugin loaders and scene ticking updates, ensuring all downstream systems receive the actual time progression rather than the requested time change.
706-706: Method rename improves clarity.The rename from
updateRootCompositiontoupdateCompositionTimebetter reflects the method's primary responsibility of updating composition time rather than the broader composition updates.
711-716: Robust handling of negative time scenarios.The improved logic handles edge cases better:
- Calculates local time relative to start time more clearly
- Prevents negative local time when deltaTime is negative
- This should help resolve composition freeze issues when time calculations go negative
721-721: Clean variable initialization.Moving the
isEndeddeclaration to the top improves code readability and follows better scoping practices.
750-750: Correct composition time assignment.The time assignment
this.rootComposition.time = localTime + this.startTimeproperly combines the calculated local time with the start time, maintaining the absolute time reference for the composition.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores