fix(stream): adapt floating window viewport sizing#47
Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough变更说明重构了窗口模式下 XComponent 尺寸的计算流程： 变更详情窗口模式视口尺寸重构
代码审查估计🎯 3 (Moderate) | ⏱️ ~20 minutes 可能相关的 PR
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
entry/src/main/ets/pages/StreamPage.ets (1)
913-916: ⚡ Quick win窗口模式切换触发了两次尺寸重算，建议保留单一触发源
toggleWindowMode()内部已经通过notifyWindowSizeChanged()触发重算（并且已防抖）；Line 915 再await this.recalculateXComponentSize()会重复执行同一套异步计算，容易带来短时抖动和额外开销。建议修复
onToggleWindowMode: async () => { await this.windowManager?.toggleWindowMode(); - await this.recalculateXComponentSize(); },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entry/src/main/ets/pages/StreamPage.ets` around lines 913 - 916, The onToggleWindowMode method is triggering size recalculation twice: once internally within toggleWindowMode via notifyWindowSizeChanged (which is already debounced), and again explicitly with the await this.recalculateXComponentSize() call on line 915. Remove the explicit recalculateXComponentSize call since toggleWindowMode already handles the size recalculation internally, eliminating the duplicate computation and potential visual jitter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@entry/src/main/ets/service/streaming/StreamLifecycleManager.ets`:
- Around line 201-218: The StreamLifecycleManager currently updates
viewModel.displayInfo and calls the updateXComponentSize callback after rotation
or resolution changes, but it does not sync the parent container size for
PanZoomHandler, causing touch coordinate offsets when zoom is active. After
updating the XComponent size in both code paths (the fullscreen path around line
201-218 and the viewport path around line 243-260), you must also add a callback
to update the PanZoomHandler's parent container dimensions to match the same
viewport width and height being used for the XComponent size calculation. This
ensures that subsequent inverseTransform operations in PanZoomHandler use the
correct parent size rather than stale dimensions.
---
Nitpick comments:
In `@entry/src/main/ets/pages/StreamPage.ets`:
- Around line 913-916: The onToggleWindowMode method is triggering size
recalculation twice: once internally within toggleWindowMode via
notifyWindowSizeChanged (which is already debounced), and again explicitly with
the await this.recalculateXComponentSize() call on line 915. Remove the explicit
recalculateXComponentSize call since toggleWindowMode already handles the size
recalculation internally, eliminating the duplicate computation and potential
visual jitter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72737bb6-18f3-432b-80cc-89beebcec90b
📒 Files selected for processing (3)
entry/src/main/ets/pages/StreamPage.etsentry/src/main/ets/service/streaming/StreamLifecycleManager.etsentry/src/main/ets/service/streaming/StreamWindowManager.ets
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
改了啥呀
drawableRect/windowRect计算 XComponent 尺寸，别再拿整块屏幕尺寸硬塞进小窗口里了，裁剪感这个小坏蛋先按住。windowSizeChange监听和防抖通知收敛到StreamWindowManager，窗口recover/maximize、拖拽缩放、系统布局 settle 后统一触发重算。为啥要改
验证
git diff --checkJAVA_HOME=/Applications/DevEco-Studio.app/Contents/jbr/Contents/Home PATH=/Applications/DevEco-Studio.app/Contents/jbr/Contents/Home/bin:$PATH DEVECO_SDK_HOME=/Applications/DevEco-Studio.app/Contents/sdk NODE_PATH=/Users/mac/Program/moonlight-harmony/node_modules node hvigorw.js --no-daemon assembleHap --mode module -p module=entry --stacktracehdc install -r entry/build/default/outputs/default/entry-default-unsigned.hapaa start -a EntryAbility -b com.alkaidlab.sdream -m entry需要测试
需要回归
Summary by CodeRabbit