Merged
Conversation
|
Hi @apr3vau. 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 adds TTY switching support by exposing the underlying wlr_session through WServer/WBackend, wiring up wlr_session_change_vt in the input helper, and hardening event filters to avoid null-dereference crashes after a VT switch. Sequence diagram for TTY switching via keyboard shortcutsequenceDiagram
actor User
participant Helper
participant WBackend
participant wlr_session
User->>Helper: Press Ctrl+Alt+F1-F6
Helper->>Helper: Detect modifiers and scanCode
Helper->>Helper: setCurrentMode(LockScreen)
Helper->>Helper: m_lockScreen->lock()
Helper->>WBackend: m_backend->session()
Helper->>wlr_session: wlr_session_change_vt(session, vt)
Class diagram for QWlrootsRenderWindow event filter hardeningclassDiagram
class QWlrootsRenderWindow {
+bool beforeDisposeEventFilter(QEvent *event)
+bool afterDisposeEventFilter(QEvent *event)
-lastActiveCursor
}
class WInputDevice {
+static from(device)
+seat()
}
class WSeat {
+cursor()
+filterEventBeforeDisposeStage(window, event)
+filterEventAfterDisposeStage(window, event)
}
QWlrootsRenderWindow --> WInputDevice : uses
WInputDevice --> WSeat : seat()
QWlrootsRenderWindow --> WSeat : calls filterEvent* methods
File-Level Changes
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 @apr3vau - I've reviewed your changes - here's some feedback:
- The Ctrl+Alt modifier check
kevent->modifiers() == Qt::ControlModifier | Qt::AltModifierhas a precedence bug—wrap the OR in parentheses or use a bitmask test (e.g.(mods & (Ctrl|Alt)) == (Ctrl|Alt)). - Replace the hard-coded scanCode range (67–72) with named key constants (e.g. KEY_F1–KEY_F6 or Qt::Key_F1–Qt::Key_F6) for clarity and maintainability.
- Guard the call to wlr_session_change_vt by verifying
m_backend->session()is non-null to avoid potential null-pointer dereferences.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Ctrl+Alt modifier check `kevent->modifiers() == Qt::ControlModifier | Qt::AltModifier` has a precedence bug—wrap the OR in parentheses or use a bitmask test (e.g. `(mods & (Ctrl|Alt)) == (Ctrl|Alt)`).
- Replace the hard-coded scanCode range (67–72) with named key constants (e.g. KEY_F1–KEY_F6 or Qt::Key_F1–Qt::Key_F6) for clarity and maintainability.
- Guard the call to wlr_session_change_vt by verifying `m_backend->session()` is non-null to avoid potential null-pointer dereferences.
## Individual Comments
### Comment 1
<location> `src/seat/helper.cpp:1200` </location>
<code_context>
}
+ // Switch TTY with Ctrl + Alt + F1-F6
+ if (kevent->modifiers() == Qt::ControlModifier | Qt::AltModifier) {
+ auto scanCode = kevent->nativeScanCode();
+ if (scanCode >= 67 && scanCode <= 72) {
</code_context>
<issue_to_address>
Bitwise OR operator precedence may cause incorrect modifier check.
Operator precedence causes the bitwise OR to be evaluated before equality, so this checks if modifiers equal the combined value. If you want to check that both modifiers are set, use bitwise AND or testFlag() instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
zccrs
requested changes
Jul 24, 2025
zccrs
requested changes
Jul 24, 2025
zccrs
requested changes
Jul 25, 2025
zccrs
requested changes
Jul 25, 2025
Collaborator
Author
|
经测试发现了问题：
|
Member
这才是符合我的预期的。因为是 vt_bound 模式，我准备移除掉对 vt_bound 模式的依赖，使用正常模式。在 ddm 里设置vt控制进程，使用ddm来阻塞tty的切换。 |
Member
这个提交里不用解决这个问题，先做到在lightdm启动时能切换tty即可。 |
zccrs
requested changes
Jul 29, 2025
Add qwlroots interface for wl_session object, which can serve for VT switching.
Support TTY switching by calling wlr_session_change_vt(). Currently not working with ddm because of the VT_BOUND mode.
When switching back from TTY the Greeter will be re-created, and somehow leads to an incorrect lockscreen animation on HiRes screen. This commit adds skip function to lockscreen animation, and skips the animation when the first time it adds to an output.
Qt will create virtual device for QInputEvent when no physical device available (e.g. switched to another VT), which will leads to a failed assertion and make treeland crash. This commit applies a special guarded getter for such case.
zccrs
requested changes
Jul 31, 2025
Use parameter to control lockscreen animation instead of QML property.
zccrs
approved these changes
Jul 31, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: apr3vau, zccrs 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 |
Dami-star
added a commit
to Dami-star/waylib
that referenced
this pull request
Aug 14, 2025
Qt will create virtual device for QInputEvent when no physical device available (e.g. switched to another VT), which will leads to a failed assertion and make treeland crash. This commit applies a special guarded getter for such case. check from linuxdeepin/treeland/pull/466
Dami-star
added a commit
to Dami-star/waylib
that referenced
this pull request
Aug 14, 2025
Qt will create virtual device for QInputEvent when no physical device available (e.g. switched to another VT), which will leads to a failed assertion and make treeland crash. This commit applies a special guarded getter for such case. check from linuxdeepin/treeland/pull/466
zccrs
pushed a commit
to vioken/waylib
that referenced
this pull request
Aug 14, 2025
Qt will create virtual device for QInputEvent when no physical device available (e.g. switched to another VT), which will leads to a failed assertion and make treeland crash. This commit applies a special guarded getter for such case. check from linuxdeepin/treeland/pull/466
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
3 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.
helper.cpp: 使用
wlr_session_change_vt实现TTY切换wbackend.cpp, wserver.h: 暴露
wlr_session对象，供wlr_session_change_vt使用qwlrootswindow.cpp: 解决切换TTY后崩溃退出的问题
Summary by Sourcery
Enable TTY switching support by exposing and using wlr_session_change_vt on Ctrl+Alt+F-keys, lock the screen during switch, and add safety checks to avoid crashes after switching.
New Features:
Bug Fixes:
Enhancements: