fix: Fix secondary screen highlight display anomaly in 90° and 270° rotation#3175
Open
leibn123 wants to merge 4 commits intolinuxdeepin:masterfrom
Open
fix: Fix secondary screen highlight display anomaly in 90° and 270° rotation#3175leibn123 wants to merge 4 commits intolinuxdeepin:masterfrom
leibn123 wants to merge 4 commits intolinuxdeepin:masterfrom
Conversation
1.修改添加新用户界面的背景透明度问题 PMS: BUG-316577
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: leibn123 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 @leibn123. 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. |
Reviewer's GuideThis PR fixes secondary screen highlight and selection issues when displays are rotated 90°/270° by correcting how Qt screens are matched to virtual screens, ensuring highlight selection reacts to rotation changes, and making screen selection persist across virtual screen list updates. Sequence diagram for rotated screen selection and highlight updatesequenceDiagram
actor User
participant DisplayMainQml as DisplayMainQml
participant ScreenTabQml as ScreenTabQml
participant ScreenItemQml as ScreenItemQml
participant dccData as dccData
participant QtApplication as QtApplication
participant Indicator as Indicator
rect rgb(235,235,255)
User ->> DisplayMainQml: open display settings
activate DisplayMainQml
DisplayMainQml ->> dccData: read virtualScreens[0]
dccData -->> DisplayMainQml: initial screen
DisplayMainQml ->> DisplayMainQml: Component.onCompleted
DisplayMainQml ->> DisplayMainQml: lastScreenName = screen.name
end
rect rgb(235,255,235)
User ->> ScreenItemQml: click secondary screen preview
ScreenItemQml ->> DisplayMainQml: pressedItem(item)
DisplayMainQml ->> DisplayMainQml: screen = item.screen
DisplayMainQml ->> DisplayMainQml: lastScreenName = item.screen.name
alt dccData.isX11
DisplayMainQml ->> QtApplication: getQtScreen(screen)
loop for each s in Qt.application.screens
DisplayMainQml ->> DisplayMainQml: isRotated = (screen.rotate == 2 or 8)
DisplayMainQml ->> DisplayMainQml: targetW,targetH based on isRotated
DisplayMainQml ->> DisplayMainQml: compare s.virtualX,Y and s.width,height
end
QtApplication -->> DisplayMainQml: matched screen s or null
DisplayMainQml ->> Indicator: createObject(screen = matched s)
end
end
rect rgb(255,235,235)
User ->> dccData: change display rotation 90 or 270
dccData ->> DisplayMainQml: emit virtualScreensChanged
DisplayMainQml ->> DisplayMainQml: onVirtualScreensChanged
alt lastScreenName matches a virtual screen
DisplayMainQml ->> DisplayMainQml: root.screen = matchedScreen
else
DisplayMainQml ->> DisplayMainQml: root.screen = virtualScreens[0]
end
DisplayMainQml ->> ScreenItemQml: update delegates
ScreenItemQml ->> ScreenItemQml: selected = (root.screen === screen and screen.rotate defined)
ScreenItemQml ->> ScreenItemQml: onScreenRotateChanged -> selected = (root.screen === screen)
ScreenItemQml ->> ScreenItemQml: Loader.active = selected
ScreenItemQml ->> ScreenItemQml: Loader.visible = selected
end
deactivate DisplayMainQml
Updated class diagram for display QML components and selection stateclassDiagram
class DisplayMainQml {
+var screen
+string lastScreenName
+var activeDialogs
+var scaleModelConst
+getScaleModelIndex(model, scale)
+getQtScreen(screen)
+getControlCenterScreen()
+closeInvalidDialogs()
+Component_onCompleted()
+onVirtualScreensChanged()
}
class ScreenItemQml {
+var screen
+bool selected
+int width
+int height
+int radius
+int offset
+updatePosition()
+onWidthChanged()
+onHeightChanged()
+onRotateChanged()
+Loader_active
+Loader_visible
+onScreenChanged()
+onScreenRotationChanged()
}
class ScreenTabQml {
+var screen
+Repeater repeater
+delegate_isSelect
+delegate_hovered
}
class dccData {
+list virtualScreens
+bool isX11
+signal virtualScreensChanged
}
class QtApplication {
+list screens
}
DisplayMainQml --> dccData : reads virtualScreens
DisplayMainQml --> QtApplication : getQtScreen
DisplayMainQml "1" o-- "many" ScreenItemQml : screen delegates
DisplayMainQml "1" o-- "many" ScreenTabQml : tab delegates
dccData --> DisplayMainQml : emits virtualScreensChanged
ScreenItemQml --> DisplayMainQml : pressedItem sets screen
ScreenTabQml --> DisplayMainQml : tab click sets screen
class ScreenTabDelegate {
+var modelData
+bool isSelect
+bool hovered
}
ScreenTabQml "1" o-- "many" ScreenTabDelegate : delegate
ScreenTabDelegate --> dccData : uses modelData.name
ScreenTabDelegate --> DisplayMainQml : compares with screen.name
class VirtualScreen {
+string name
+int x
+int y
+int rotate
+Resolution currentResolution
}
class Resolution {
+int width
+int height
}
dccData "1" o-- "many" VirtualScreen
VirtualScreen *-- Resolution
DisplayMainQml --> VirtualScreen : screen
ScreenItemQml --> VirtualScreen : screen
ScreenTabDelegate --> VirtualScreen : modelData
class QtScreen {
+int virtualX
+int virtualY
+int width
+int height
+double devicePixelRatio
}
QtApplication "1" o-- "many" QtScreen
DisplayMainQml --> QtScreen : getQtScreen match
Flow diagram for rotation aware getQtScreen matchingflowchart TD
A[Start getQtScreen screen] --> B[Initialize matchedScreen as null]
B --> C{Any Qt.application.screens remaining}
C -->|No| Z[Return null]
C -->|Yes| D[Take next screen s]
D --> E{screen.rotate is 2 or 8}
E -->|Yes| F[Set isRotated true
set targetW = screen.currentResolution.height
set targetH = screen.currentResolution.width]
E -->|No| G[Set isRotated false
set targetW = screen.currentResolution.width
set targetH = screen.currentResolution.height]
F --> H
G --> H
H{Match position and size}
H -->|Yes| I[Return s as matched Qt screen]
H -->|No| C
Z --> J[End]
I --> J
Flow diagram for ScreenItem highlight update on rotationflowchart TD
A[Start ScreenItem] --> B["Compute selected = (root.screen == screen and screen.rotate defined)"]
B --> C[Set Loader.active = selected]
C --> D[Set Loader.visible = selected]
D --> E{screen property changes}
E -->|Yes| F[onScreenChanged -> updatePosition]
E -->|No| G
G{screen rotation changes} -->|Yes| H[onScreenRotationChanged -> updatePosition]
G -->|No| I
H --> J[Recalculate highlight border position]
F --> J
J --> K[Highlight border aligned with rotated screen]
K --> L[End]
File-Level Changes
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 found 1 issue, and left some high level feedback:
- In
ScreenItem.qml, the newonScreenChanged/onScreenRotationChangedhandlers on theLoader(and the innerRectangle) reference signals that don't exist on those types, so they will never fire; consider moving these updates to an object that actually owns ascreen/rotationproperty (e.g., the rootItemor an existingConnectionstoscreen). - The updated
isSelectproperty inScreenTab.qmldeclaresmodelNameandscreenNamebut doesn't use them, and the ternary is redundant; you can simplify this toproperty bool isSelect: model.modelData && screen && model.modelData.name === screen.nameto keep the logic clearer. - Several modified QML blocks (e.g.,
getQtScreeninDisplayMain.qmland theLoaderinScreenItem.qml) lost their previous indentation, which makes the structure harder to read; restoring consistent indentation would significantly improve maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ScreenItem.qml`, the new `onScreenChanged`/`onScreenRotationChanged` handlers on the `Loader` (and the inner `Rectangle`) reference signals that don't exist on those types, so they will never fire; consider moving these updates to an object that actually owns a `screen`/`rotation` property (e.g., the root `Item` or an existing `Connections` to `screen`).
- The updated `isSelect` property in `ScreenTab.qml` declares `modelName` and `screenName` but doesn't use them, and the ternary is redundant; you can simplify this to `property bool isSelect: model.modelData && screen && model.modelData.name === screen.name` to keep the logic clearer.
- Several modified QML blocks (e.g., `getQtScreen` in `DisplayMain.qml` and the `Loader` in `ScreenItem.qml`) lost their previous indentation, which makes the structure harder to read; restoring consistent indentation would significantly improve maintainability.
## Individual Comments
### Comment 1
<location path="src/plugin-display/qml/ScreenItem.qml" line_range="127-129" />
<code_context>
+ smooth: true
}
+
+ // ✅ 关键：旋转后强制重新渲染高亮框
+ onScreenChanged: updatePosition()
+ onScreenRotationChanged: updatePosition()
+}
MouseArea {
</code_context>
<issue_to_address>
**issue (bug_risk):** `onScreenChanged` / `onScreenRotationChanged` on `Loader` are likely invalid signal handlers.
`Loader` does not provide `screenChanged` or `screenRotationChanged` signals, so these handlers will never run and may emit QML warnings. To react to changes in the parent `ScreenItem`’s `screen`, either bind the geometry directly to `screen`-based values, or use a `Connections { target: root.screen; function onRotateChanged() { updatePosition() } }` block (as you do later in the file).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+127
to
+129
| // ✅ 关键：旋转后强制重新渲染高亮框 | ||
| onScreenChanged: updatePosition() | ||
| onScreenRotationChanged: updatePosition() |
There was a problem hiding this comment.
issue (bug_risk): onScreenChanged / onScreenRotationChanged on Loader are likely invalid signal handlers.
Loader does not provide screenChanged or screenRotationChanged signals, so these handlers will never run and may emit QML warnings. To react to changes in the parent ScreenItem’s screen, either bind the geometry directly to screen-based values, or use a Connections { target: root.screen; function onRotateChanged() { updatePosition() } } block (as you do later in the file).
leibn123
force-pushed
the
master
branch
3 times, most recently
from
f4e1ce1 to
20168db
Compare
…otation 1.修改副屏旋转90度和270度的高亮显示异常问题 PMS: BUG-355449
|
TAG Bot New tag: 6.1.82 |
|
TAG Bot New tag: 6.1.83 |
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
2 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.
Summary by Sourcery
Fix display configuration UI to correctly track and highlight the selected screen, especially when screens are rotated 90° or 270°, and when the virtual screen list changes.
Bug Fixes:
Enhancements: