fix: fullscreen window size reduced by titlebar height#853
Open
glyvut wants to merge 1 commit into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: glyvut The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @glyvut. Thanks for your PR. 😃 |
|
Hi @glyvut. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
CLA Assistant Lite bot: |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts fullscreen state handling during animation readiness so the title bar padding is updated for the target state before resizing, preventing fullscreen windows from being reduced by the title bar height and restoring state/titlebar correctly on failure. Sequence diagram for SurfaceWrapper onAnimationReady fullscreen padding handlingsequenceDiagram
participant SurfaceWrapper
participant SurfaceState
participant TitleBar
participant GeometryAnimation
SurfaceWrapper->>SurfaceWrapper: onAnimationReady()
SurfaceWrapper->>SurfaceWrapper: Q_ASSERT(m_pendingState != m_surfaceState)
SurfaceWrapper->>SurfaceWrapper: Q_ASSERT(m_pendingGeometry.isValid())
SurfaceWrapper->>SurfaceState: originalState = m_surfaceState.value()
SurfaceWrapper->>SurfaceState: setValueBypassingBindings(m_pendingState)
SurfaceWrapper->>TitleBar: updateTitleBar()
SurfaceWrapper->>SurfaceState: setValueBypassingBindings(originalState)
SurfaceWrapper->>SurfaceWrapper: resize(m_pendingGeometry.size())
alt resize failed
SurfaceWrapper->>TitleBar: updateTitleBar()
SurfaceWrapper->>GeometryAnimation: disconnect(this)
SurfaceWrapper->>GeometryAnimation: deleteLater()
SurfaceWrapper->>SurfaceWrapper: m_geometryAnimation = nullptr
else resize succeeded
SurfaceWrapper->>SurfaceWrapper: proceed with state transition handling
end
Flow diagram for onAnimationReady padding and resize logicflowchart TD
A[onAnimationReady called] --> B[Assert m_pendingState != m_surfaceState]
B --> C[Assert m_pendingGeometry.isValid]
C --> D[Store originalState = m_surfaceState.value]
D --> E[Temporarily set m_surfaceState to m_pendingState via setValueBypassingBindings]
E --> F[updateTitleBar with target state padding]
F --> G[Restore m_surfaceState to originalState via setValueBypassingBindings]
G --> H{"resize(m_pendingGeometry.size) succeeds?"}
H -- No --> I[updateTitleBar to restore titlebar]
I --> J[disconnect m_geometryAnimation]
J --> K[deleteLater on m_geometryAnimation]
K --> L[set m_geometryAnimation to nullptr]
H -- Yes --> M[Continue normal state transition handling]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Temporarily mutating
m_surfaceStatearoundupdateTitleBar()is subtle; consider wrapping the state change in a small RAII/scope-guard helper to guarantee restoration even if early returns or additional error paths are added later. - Given that
updateTitleBar()may emit signals or trigger bindings, it would be helpful to explicitly document that observers must tolerate the brief discrepancy betweenm_surfaceStateand the actual compositor state when this function is called.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Temporarily mutating `m_surfaceState` around `updateTitleBar()` is subtle; consider wrapping the state change in a small RAII/scope-guard helper to guarantee restoration even if early returns or additional error paths are added later.
- Given that `updateTitleBar()` may emit signals or trigger bindings, it would be helpful to explicitly document that observers must tolerate the brief discrepancy between `m_surfaceState` and the actual compositor state when this function is called.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zccrs
requested changes
Apr 30, 2026
| Q_ASSERT(m_pendingState != m_surfaceState); | ||
| Q_ASSERT(m_pendingGeometry.isValid()); | ||
|
|
||
| // Temporarily set the target state so that updateTitleBar() can set the |
Member
There was a problem hiding this comment.
这样解决有些脏了，应该能有更好的解决方案，比如在实际configure size能识别到这种情况
glyvut
force-pushed
the
master
branch
3 times, most recently
from
ec9181c to
c1515e4
Compare
asterwyx
reviewed
May 7, 2026
Comment on lines
+1337
to
+1338
| } | ||
| else if (m_surfaceState == State::Fullscreen) { |
Comment on lines
+1327
to
+1330
| m_recoverPadding = QMarginsF(m_surfaceItem->leftPadding(), | ||
| m_surfaceItem->topPadding(), | ||
| m_surfaceItem->rightPadding(), | ||
| m_surfaceItem->bottomPadding()); |
asterwyx
reviewed
May 7, 2026
Comment on lines
+1333
to
+1336
| m_surfaceItem->setTopPadding(0); | ||
| m_surfaceItem->setLeftPadding(0); | ||
| m_surfaceItem->setRightPadding(0); | ||
| m_surfaceItem->setBottomPadding(0); |
Save and clear paddings before resizing to fullscreen state to ensure the configure size matches the fullscreen geometry. Recover paddings on resize failure or when exiting fullscreen. 在进入全屏前保存并清除padding，确保配置尺寸与全屏几何匹配。 resize失败或退出全屏时恢复padding。 Log: 修复全屏窗口配置尺寸被标题栏高度缩减的问题 PMS: BUG-359605 Issue: Fixes linuxdeepin#744 Influence: 全屏窗口获得正确的配置尺寸。
wineee
requested changes
May 9, 2026
|
|
||
| if (m_surfaceItem) { | ||
| if (m_pendingState == State::Fullscreen) { | ||
| m_recoverPadding = QMarginsF(m_surfaceItem->leftPadding(), |
|
TAG Bot New tag: 0.8.6 |
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
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Temporarily set the target state before resize() so that
updateTitleBar() updates topPadding correctly (zero for fullscreen).
Restore the original state afterwards so doSetSurfaceState() can
properly track the state transition (e.g. issuing setFullScreen(false)
when exiting fullscreen).
在全屏状态切换时，resize()先于updateTitleBar()执行，导致
resizeSurface()使用了带有标题栏高度的topPadding计算configure
尺寸，客户端收到的全屏尺寸比实际小了标题栏高度。修复方案是在
resize前临时设置目标状态以更新padding，随后恢复原始状态。
Log: 修复全屏窗口尺寸被标题栏高度缩减的bug
PMS: BUG-359605
Issue: Fixes #744
Influence: 全屏窗口现在能正确覆盖整个屏幕，退出全屏时客户端也能正确收到状态变更。
Summary by Sourcery
Bug Fixes: