Skip to content

fix: Fix screen rotation issues#643

Merged
lzwind merged 2 commits intolinuxdeepin:masterfrom
re2zero:bugfix
Jun 22, 2025
Merged

fix: Fix screen rotation issues#643
lzwind merged 2 commits intolinuxdeepin:masterfrom
re2zero:bugfix

Conversation

@re2zero
Copy link
Contributor

@re2zero re2zero commented Jun 22, 2025

  • Commented out debug logging statements in VNCRecvThread and VNCSendWorker to reduce log clutter.
  • Enhanced VncViewer with QMutex for thread safety during image updates, size changes, and painting operations.
  • Added checks for image validity and surface size to prevent crashes during rendering.

Log: Fix screen rotation issues.

Summary by Sourcery

Improve VncViewer’s resilience to screen rotation and multithreading by adding mutex protection, image and surface validity checks, and more synchronized thread handling while silencing extraneous debug logs.

Bug Fixes:

  • Prevent crashes by skipping null or mismatched images and invalid surface sizes during rendering and rotation handling
  • Avoid clearing or drawing surfaces when not connected or when dimensions are invalid

Enhancements:

  • Introduce QMutex locks around size changes, image updates, painting, and viewer startup to ensure thread safety
  • Detect rotation in both onSizeChange and updateImage and adjust the display surface accordingly
  • Use BlockingQueuedConnection for sizeChangedSignal and reorder thread start-up to first launch the send worker then the receive thread

Chores:

  • Comment out redundant DLOG debug statements in VNCRecvThread and VNCSendWorker to reduce log clutter

- Commented out debug logging statements in VNCRecvThread and VNCSendWorker to reduce log clutter.
- Enhanced VncViewer with QMutex for thread safety during image updates, size changes, and painting operations.
- Added checks for image validity and surface size to prevent crashes during rendering.

Log: Fix screen rotation issues.
@sourcery-ai
Copy link

sourcery-ai bot commented Jun 22, 2025

Reviewer's Guide

This PR addresses screen rotation stability by enforcing thread safety in VncViewer via mutex locking and blocking signal-slot connections, adds validation checks to prevent invalid rendering and sizing operations, and reduces log clutter by commenting out debug statements in send/receive threads.

Sequence diagram for image update and screen rotation handling in VncViewer

sequenceDiagram
    participant VNCRecvThread
    participant VncViewer
    participant QMutex

    VNCRecvThread->>VncViewer: updateImage(QImage)
    VncViewer->>QMutex: lock (QMutexLocker)
    alt image is invalid
        VncViewer-->>QMutex: unlock
        VncViewer-->>VNCRecvThread: return
    else image is valid
        alt screen rotation detected
            VncViewer->>VncViewer: update m_phoneMode
            VncViewer->>VncViewer: setSurfaceSize(new size)
            VncViewer->>VncViewer: emit sizeChanged(new size)
        end
        VncViewer->>VncViewer: m_image = image
        VncViewer->>VncViewer: update()
    end
    VncViewer-->>QMutex: unlock
Loading

Class diagram for updated VncViewer thread safety and rendering logic

classDiagram
    class VncViewer {
        +QMutex m_mutex
        +void onSizeChange(int width, int height)
        +void clearSurface()
        +void updateImage(const QImage &image)
        +void paintEvent(QPaintEvent *event)
        +void setSurfaceSize(QSize surfaceSize)
        +void start()
    }
    VncViewer --|> QWidget
    VncViewer o-- VNCRecvThread : _vncRecvThread
    VncViewer o-- VNCSendWorker : _vncSendThread
    VncViewer o-- QImage : m_image
    VncViewer o-- QPixmap : m_surfacePixmap
    VncViewer o-- QPainter : m_painter
    VncViewer o-- QTimer : m_frameTimer
    VncViewer o-- QMutex : m_mutex

    class VNCRecvThread {
        +void startRun(rfbClient *cl)
        +signal updateImageSignal(QImage)
        +signal sizeChangedSignal(int, int)
    }
    class VNCSendWorker {
        +void sendMouseUpdateMsg(rfbClient *cl, int x, int y, int button=0)
        +void sendKeyUpdateMsg(rfbClient *cl, int key, bool down)
    }
Loading

File-Level Changes

Change Details Files
Applied thread safety mechanisms and updated signal-slot invocation
  • Added QMutex member and included QMutexLocker
  • Wrapped critical sections in onSizeChange, clearSurface, updateImage, paintEvent, and start() with QMutexLocker
  • Changed sizeChangedSignal connection to BlockingQueuedConnection
src/lib/cooperation/core/gui/phone/vncviewer.h
src/lib/cooperation/core/gui/phone/vncviewer.cpp
Introduced validation checks to prevent crashes during rendering and sizing
  • Early return if image.isNull() in updateImage and paintEvent
  • Skip clearSurface when not connected
  • Validate surfaceSize dimensions in setSurfaceSize
  • Skip drawing frames with mismatched orientation in paintEvent
src/lib/cooperation/core/gui/phone/vncviewer.cpp
Reduced log verbosity by commenting out debug statements
  • Disabled DLOG in VNCSendWorker sendMouseUpdateMsg and sendKeyUpdateMsg
  • Disabled DLOG in VNCRecvThread frameBufferUpdated
src/lib/cooperation/core/gui/phone/vncsendworker.cpp
src/lib/cooperation/core/gui/phone/vncrecvthread.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @re2zero - I've reviewed your changes - here's some feedback:

  • Switching the sizeChangedSignal connection to BlockingQueuedConnection risks deadlocking the UI thread; consider using a non-blocking approach or deferring the resize logic to avoid blocking.
  • The heavy use of QMutexLocker across paintEvent, updateImage, and onSizeChange may degrade rendering performance—review lock scope or consider a read/write lock to reduce contention.
  • You have duplicated rotation-detection and surface-resizing code in onSizeChange and updateImage—extract that into a shared helper to simplify maintenance.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Switching the sizeChangedSignal connection to BlockingQueuedConnection risks deadlocking the UI thread; consider using a non-blocking approach or deferring the resize logic to avoid blocking.
- The heavy use of QMutexLocker across paintEvent, updateImage, and onSizeChange may degrade rendering performance—review lock scope or consider a read/write lock to reduce contention.
- You have duplicated rotation-detection and surface-resizing code in onSizeChange and updateImage—extract that into a shared helper to simplify maintenance.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Update changelog for version 1.1.7.

 Log: New v1.1.7 tag.
@github-actions
Copy link

TAG Bot

TAG: 1.1.7
EXISTED: no
DISTRIBUTION: unstable

@lzwind lzwind merged commit 7023305 into linuxdeepin:master Jun 22, 2025
13 checks passed
@re2zero re2zero deleted the bugfix branch June 22, 2025 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants