Internal
Public Access
RDP: fix distorted text on HiDPI monitors, reduce resize glitches
The RDP session pipeline never accounted for display scale: it
requested a desktop canvas sized in Qt logical pixels (never
multiplied by devicePixelRatio()), and FreeRDP_DesktopScaleFactor/
DeviceScaleFactor were read but never actually set anywhere. On a
HiDPI monitor this meant the remote session rendered assuming a
96 DPI / 100% display, and the resulting canvas got stretched
locally — ClearType's subpixel hinting doesn't survive that kind of
resampling, producing distorted glyph shapes and color fringing
rather than plain blur.
RdpDisplayWidget now reports physical pixel dimensions and the real
devicePixelRatio (recomputed on resize and on screen changes, e.g.
dragging the window to a different-DPI monitor). RdpSessionBackend
maps that to the nearest FreeRDP-legal scale value ({100, 140, 180},
per MS-RDPEDISP and FreeRDP's own reference client) and sets it at
both connect time and on every dynamic resize, including the
FreeRDP_MonitorOverrideFlags required for the values to actually be
honored rather than silently ignored.
While testing this against real infrastructure, found and fixed two
related (pre-existing, not caused by this change) resize issues:
- A stale-frame race where the old frame could be drawn at the wrong
scale for a moment after a resize, before a correctly-sized one
arrives — now the frame is cleared during that transition instead.
- No debounce on outgoing resize requests — every single resize event
fired an immediate request to the server, which can visibly
contribute to host-side redraw glitches during rapid layout churn
(e.g. right after connecting). Coalesced into one request per burst,
plus an explicit refresh-rect request after each resize completes
as a best-effort nudge for hosts that don't fully repaint on their
own.
A separate, deeper issue was also found during testing (the remote
guest's actual resolution sometimes not changing despite the resize
channel reporting success) and is tracked separately, not fixed here.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
#include "rdp_display_widget.h"
|
||||
|
||||
#include <QCursor>
|
||||
#include <QEvent>
|
||||
#include <QKeyEvent>
|
||||
#include <QMouseEvent>
|
||||
#include <QPainter>
|
||||
@@ -15,20 +16,38 @@ QSize sanitizeSize(const QSize& size)
|
||||
{
|
||||
return QSize(qMax(1, size.width()), qMax(1, size.height()));
|
||||
}
|
||||
|
||||
qreal sanitizeDevicePixelRatio(qreal ratio)
|
||||
{
|
||||
if (!(ratio > 0.0)) {
|
||||
return 1.0;
|
||||
}
|
||||
return qBound(1.0, ratio, 4.0);
|
||||
}
|
||||
|
||||
// Windows' virtual-display driver can visibly glitch (stale composited
|
||||
// content left on screen) when asked to change resolution repeatedly in
|
||||
// quick succession, which naturally happens as the window's layout settles
|
||||
// right after creation/connect. Coalescing bursts of resize events into one
|
||||
// request avoids triggering that.
|
||||
constexpr int kResizeDebounceMs = 150;
|
||||
}
|
||||
|
||||
RdpDisplayWidget::RdpDisplayWidget(QWidget* parent)
|
||||
: QWidget(parent), m_remoteSize(1280, 720), m_cursorMode(CursorMode::Default)
|
||||
: QWidget(parent),
|
||||
m_remoteSize(1280, 720),
|
||||
m_cursorMode(CursorMode::Default),
|
||||
m_resizeDebounceTimer(new QTimer(this))
|
||||
{
|
||||
setFocusPolicy(Qt::StrongFocus);
|
||||
setMouseTracking(true);
|
||||
setAutoFillBackground(false);
|
||||
setMinimumSize(320, 200);
|
||||
|
||||
QTimer::singleShot(0, this, [this]() {
|
||||
const QSize size = sanitizeSize(this->size());
|
||||
emit viewportSizeChanged(size.width(), size.height());
|
||||
});
|
||||
m_resizeDebounceTimer->setSingleShot(true);
|
||||
connect(m_resizeDebounceTimer, &QTimer::timeout, this, &RdpDisplayWidget::emitViewportGeometry);
|
||||
|
||||
scheduleViewportGeometryEmit();
|
||||
}
|
||||
|
||||
void RdpDisplayWidget::setFrame(const QImage& frame)
|
||||
@@ -54,6 +73,13 @@ void RdpDisplayWidget::setRemoteDesktopSize(int width, int height)
|
||||
}
|
||||
|
||||
m_remoteSize = nextSize;
|
||||
// The next actual frame (via setFrame) arrives asynchronously and isn't
|
||||
// guaranteed to be sized to match yet. Drawing the old frame stretched
|
||||
// to a renderRect() computed from the new m_remoteSize would scale it
|
||||
// by the wrong factor for the transition window, producing visibly
|
||||
// distorted/duplicated-looking content. Clear it and show the existing
|
||||
// "waiting for frame" placeholder until a correctly-sized frame lands.
|
||||
m_frame = QImage();
|
||||
update();
|
||||
}
|
||||
|
||||
@@ -140,11 +166,39 @@ void RdpDisplayWidget::paintEvent(QPaintEvent* event)
|
||||
void RdpDisplayWidget::resizeEvent(QResizeEvent* event)
|
||||
{
|
||||
QWidget::resizeEvent(event);
|
||||
const QSize size = sanitizeSize(event->size());
|
||||
emit viewportSizeChanged(size.width(), size.height());
|
||||
scheduleViewportGeometryEmit();
|
||||
applyCursor();
|
||||
}
|
||||
|
||||
bool RdpDisplayWidget::event(QEvent* event)
|
||||
{
|
||||
// Fires when this widget's effective screen changes (e.g. dragged to a
|
||||
// different monitor), which is what changes devicePixelRatio(). Newer
|
||||
// Qt versions add a more specific QEvent::DevicePixelRatioChange, but
|
||||
// this project's Qt 6.2 floor doesn't have it.
|
||||
if (event->type() == QEvent::ScreenChangeInternal) {
|
||||
scheduleViewportGeometryEmit();
|
||||
}
|
||||
return QWidget::event(event);
|
||||
}
|
||||
|
||||
void RdpDisplayWidget::scheduleViewportGeometryEmit()
|
||||
{
|
||||
// Restarting an already-running single-shot timer resets its countdown,
|
||||
// so a burst of resize events collapses into one emission after things
|
||||
// settle, rather than one request per event.
|
||||
m_resizeDebounceTimer->start(kResizeDebounceMs);
|
||||
}
|
||||
|
||||
void RdpDisplayWidget::emitViewportGeometry()
|
||||
{
|
||||
const QSize logicalSize = sanitizeSize(this->size());
|
||||
const qreal ratio = sanitizeDevicePixelRatio(this->devicePixelRatioF());
|
||||
const QSize physicalSize(qRound(logicalSize.width() * ratio), qRound(logicalSize.height() * ratio));
|
||||
emit viewportSizeChanged(physicalSize.width(), physicalSize.height());
|
||||
emit displayScaleChanged(ratio);
|
||||
}
|
||||
|
||||
void RdpDisplayWidget::keyPressEvent(QKeyEvent* event)
|
||||
{
|
||||
if (event == nullptr) {
|
||||
|
||||
@@ -8,6 +8,7 @@ class QKeyEvent;
|
||||
class QMouseEvent;
|
||||
class QPaintEvent;
|
||||
class QResizeEvent;
|
||||
class QTimer;
|
||||
class QWheelEvent;
|
||||
|
||||
class RdpDisplayWidget : public QWidget
|
||||
@@ -30,10 +31,12 @@ signals:
|
||||
void mouseButtonInput(int x, int y, int button, bool pressed);
|
||||
void mouseWheelInput(int x, int y, int deltaX, int deltaY);
|
||||
void viewportSizeChanged(int width, int height);
|
||||
void displayScaleChanged(qreal devicePixelRatio);
|
||||
|
||||
protected:
|
||||
void paintEvent(QPaintEvent* event) override;
|
||||
void resizeEvent(QResizeEvent* event) override;
|
||||
bool event(QEvent* event) override;
|
||||
void keyPressEvent(QKeyEvent* event) override;
|
||||
void keyReleaseEvent(QKeyEvent* event) override;
|
||||
void mousePressEvent(QMouseEvent* event) override;
|
||||
@@ -54,11 +57,14 @@ private:
|
||||
QImage m_cursorImage;
|
||||
QPoint m_cursorHotspot;
|
||||
CursorMode m_cursorMode;
|
||||
QTimer* m_resizeDebounceTimer;
|
||||
|
||||
QRectF renderRect() const;
|
||||
QPoint mapToRemote(const QPointF& pos) const;
|
||||
QSize effectiveRemoteSize() const;
|
||||
void applyCursor();
|
||||
void emitViewportGeometry();
|
||||
void scheduleViewportGeometryEmit();
|
||||
};
|
||||
|
||||
#endif
|
||||
|
||||
@@ -104,6 +104,22 @@ QString normalizedRdpSecurityMode(const QString& value)
|
||||
return QStringLiteral("Negotiate");
|
||||
}
|
||||
|
||||
// MS-RDPEDISP restricts DesktopScaleFactor/DeviceScaleFactor to exactly
|
||||
// these three values; FreeRDP's own reference client enforces the same
|
||||
// set (client/common/cmdline.c, parse_scale_options). Anything else is
|
||||
// silently ignored by the server, so map the real, continuous
|
||||
// devicePixelRatio down to the nearest one.
|
||||
UINT32 nearestFreeRdpScaleValue(qreal ratio)
|
||||
{
|
||||
if (ratio <= 1.2) {
|
||||
return 100;
|
||||
}
|
||||
if (ratio <= 1.6) {
|
||||
return 140;
|
||||
}
|
||||
return 180;
|
||||
}
|
||||
|
||||
QString normalizedRdpPerformanceProfile(const QString& value)
|
||||
{
|
||||
const QString profile = value.trimmed();
|
||||
@@ -1304,6 +1320,7 @@ RdpSessionBackend::RdpSessionBackend(const Profile& profile, QObject* parent)
|
||||
m_userInitiatedDisconnect(false),
|
||||
m_requestedDesktopWidth(kDefaultDesktopWidth),
|
||||
m_requestedDesktopHeight(kDefaultDesktopHeight),
|
||||
m_devicePixelRatio(1.0),
|
||||
m_workerRunning(false),
|
||||
m_stopRequested(false),
|
||||
m_instance(nullptr),
|
||||
@@ -1312,6 +1329,7 @@ RdpSessionBackend::RdpSessionBackend(const Profile& profile, QObject* parent)
|
||||
m_resizeFailureLogged(false),
|
||||
m_lastResizeWidth(0),
|
||||
m_lastResizeHeight(0),
|
||||
m_lastResizeScale(0),
|
||||
m_cliprdrContext(nullptr)
|
||||
{
|
||||
}
|
||||
@@ -1403,6 +1421,28 @@ void RdpSessionBackend::updateTerminalSize(int columns, int rows)
|
||||
enqueueInputEvent(event);
|
||||
}
|
||||
|
||||
void RdpSessionBackend::updateDisplayScale(qreal devicePixelRatio)
|
||||
{
|
||||
const qreal clamped = qBound(1.0, devicePixelRatio, 4.0);
|
||||
m_devicePixelRatio.store(clamped);
|
||||
|
||||
if (!m_workerRunning.load()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Reuses the resize input-event path so the worker thread (which owns
|
||||
// m_instance) picks this up safely; processInputEvents' dedup check
|
||||
// also compares the quantized scale value, so this correctly triggers
|
||||
// a fresh SendMonitorLayout even when width/height haven't changed
|
||||
// (e.g. the window moved to a different-DPI monitor at the same
|
||||
// logical size).
|
||||
InputEvent event;
|
||||
event.type = InputEventType::Resize;
|
||||
event.width = sanitizeDesktopWidth(m_requestedDesktopWidth.load());
|
||||
event.height = sanitizeDesktopHeight(m_requestedDesktopHeight.load());
|
||||
enqueueInputEvent(event);
|
||||
}
|
||||
|
||||
void RdpSessionBackend::sendKeyEvent(int key,
|
||||
quint32 nativeScanCode,
|
||||
const QString& text,
|
||||
@@ -1637,9 +1677,29 @@ void RdpSessionBackend::workerMain()
|
||||
const QString performanceProfile = normalizedRdpPerformanceProfile(p.rdpPerformanceProfile);
|
||||
freerdp_settings_set_bool(settings, FreeRDP_SupportDisplayControl, TRUE);
|
||||
freerdp_settings_set_bool(settings, FreeRDP_DynamicResolutionUpdate, TRUE);
|
||||
// Lets us actively request a full-screen repaint after a resize (see
|
||||
// sendDisplayResize) — some RDP hosts (particularly VMs using a
|
||||
// synthetic/virtual display driver) occasionally fail to fully redraw
|
||||
// their own desktop after a resolution change; requesting a refresh
|
||||
// forces them to resend everything rather than leaving stale content.
|
||||
freerdp_settings_set_bool(settings, FreeRDP_RefreshRect, TRUE);
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopWidth, static_cast<UINT32>(desktopWidth));
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopHeight, static_cast<UINT32>(desktopHeight));
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_ColorDepth, 32);
|
||||
// Tell the remote session the real display scale so it renders its own
|
||||
// UI/ClearType text natively at that size, instead of assuming 96 DPI
|
||||
// and having the result stretched (and ClearType's subpixel hinting
|
||||
// distorted) by client-side scaling. The override flags are required —
|
||||
// without them FreeRDP ignores the scale factor values entirely.
|
||||
{
|
||||
const UINT32 scaleValue = nearestFreeRdpScaleValue(m_devicePixelRatio.load());
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopScaleFactor, scaleValue);
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DeviceScaleFactor, scaleValue);
|
||||
freerdp_settings_set_uint64(settings,
|
||||
FreeRDP_MonitorOverrideFlags,
|
||||
FREERDP_MONITOR_OVERRIDE_DESKTOP_SCALE
|
||||
| FREERDP_MONITOR_OVERRIDE_DEVICE_SCALE);
|
||||
}
|
||||
freerdp_settings_set_bool(settings, FreeRDP_AuthenticationOnly, FALSE);
|
||||
freerdp_settings_set_bool(settings, FreeRDP_AutoLogonEnabled, TRUE);
|
||||
if (!applyRdpSecurityMode(settings, securityMode)) {
|
||||
@@ -1831,9 +1891,14 @@ bool RdpSessionBackend::sendDisplayResize(rdp_freerdp* instance, int width, int
|
||||
return false;
|
||||
}
|
||||
|
||||
const qreal ratio = m_devicePixelRatio.load();
|
||||
const UINT32 scaleValue = nearestFreeRdpScaleValue(ratio);
|
||||
|
||||
rdpSettings* settings = instance->context->settings;
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopWidth, static_cast<UINT32>(width));
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopHeight, static_cast<UINT32>(height));
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DesktopScaleFactor, scaleValue);
|
||||
freerdp_settings_set_uint32(settings, FreeRDP_DeviceScaleFactor, scaleValue);
|
||||
|
||||
DispClientContext* dispContext = nullptr;
|
||||
bool displayControlReady = false;
|
||||
@@ -1851,17 +1916,34 @@ bool RdpSessionBackend::sendDisplayResize(rdp_freerdp* instance, int width, int
|
||||
layout.Width = static_cast<UINT32>(width);
|
||||
layout.Height = static_cast<UINT32>(height);
|
||||
layout.Orientation = ORIENTATION_LANDSCAPE;
|
||||
layout.DesktopScaleFactor = freerdp_settings_get_uint32(settings, FreeRDP_DesktopScaleFactor);
|
||||
layout.DeviceScaleFactor = freerdp_settings_get_uint32(settings, FreeRDP_DeviceScaleFactor);
|
||||
layout.DesktopScaleFactor = scaleValue;
|
||||
layout.DeviceScaleFactor = scaleValue;
|
||||
// Physical size in mm must reflect the real DPI (96 * ratio), not
|
||||
// the plain baseline, so it stays consistent with the scale factor
|
||||
// above rather than implying a standard-DPI display of this size.
|
||||
const double effectiveDpi = kDefaultDpi * ratio;
|
||||
layout.PhysicalWidth = static_cast<UINT32>(
|
||||
std::lround((static_cast<double>(width) / kDefaultDpi) * kMillimetersPerInch));
|
||||
std::lround((static_cast<double>(width) / effectiveDpi) * kMillimetersPerInch));
|
||||
layout.PhysicalHeight = static_cast<UINT32>(
|
||||
std::lround((static_cast<double>(height) / kDefaultDpi) * kMillimetersPerInch));
|
||||
std::lround((static_cast<double>(height) / effectiveDpi) * kMillimetersPerInch));
|
||||
|
||||
const UINT rc = dispContext->SendMonitorLayout(dispContext, 1, &layout);
|
||||
if (rc == CHANNEL_RC_OK) {
|
||||
m_lastResizeWidth = width;
|
||||
m_lastResizeHeight = height;
|
||||
m_lastResizeScale = static_cast<int>(scaleValue);
|
||||
|
||||
// Best-effort nudge: some hosts don't fully repaint their own
|
||||
// desktop after a resolution change (observed: taskbar missing
|
||||
// until something else forces a redraw). Explicitly asking for
|
||||
// the whole new area to be resent costs little and helps
|
||||
// recover from that when it happens.
|
||||
if (instance->context->update != nullptr
|
||||
&& instance->context->update->RefreshRect != nullptr) {
|
||||
RECTANGLE_16 fullArea = {0, 0, static_cast<UINT16>(qMin(width, 65535)),
|
||||
static_cast<UINT16>(qMin(height, 65535))};
|
||||
instance->context->update->RefreshRect(instance->context, 1, &fullArea);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -2035,7 +2117,9 @@ void RdpSessionBackend::processInputEvents(rdp_freerdp* instance)
|
||||
if (hasResize) {
|
||||
const int width = sanitizeDesktopWidth(resizeWidth);
|
||||
const int height = sanitizeDesktopHeight(resizeHeight);
|
||||
if (width != m_lastResizeWidth || height != m_lastResizeHeight) {
|
||||
const int scaleValue = static_cast<int>(nearestFreeRdpScaleValue(m_devicePixelRatio.load()));
|
||||
if (width != m_lastResizeWidth || height != m_lastResizeHeight
|
||||
|| scaleValue != m_lastResizeScale) {
|
||||
if (sendDisplayResize(instance, width, height)) {
|
||||
if (m_resizeFailureLogged) {
|
||||
emit eventLogged(QStringLiteral("Dynamic RDP resize recovered."));
|
||||
|
||||
@@ -26,6 +26,7 @@ public slots:
|
||||
void sendInput(const QString& input) override;
|
||||
void confirmHostKey(bool trustHost) override;
|
||||
void updateTerminalSize(int columns, int rows) override;
|
||||
void updateDisplayScale(qreal devicePixelRatio) override;
|
||||
void sendKeyEvent(int key,
|
||||
quint32 nativeScanCode,
|
||||
const QString& text,
|
||||
@@ -68,6 +69,7 @@ private:
|
||||
|
||||
std::atomic_int m_requestedDesktopWidth;
|
||||
std::atomic_int m_requestedDesktopHeight;
|
||||
std::atomic<qreal> m_devicePixelRatio;
|
||||
|
||||
std::thread m_worker;
|
||||
std::atomic_bool m_workerRunning;
|
||||
@@ -85,6 +87,7 @@ private:
|
||||
bool m_resizeFailureLogged;
|
||||
int m_lastResizeWidth;
|
||||
int m_lastResizeHeight;
|
||||
int m_lastResizeScale;
|
||||
|
||||
std::mutex m_cliprdrMutex;
|
||||
void* m_cliprdrContext;
|
||||
|
||||
@@ -47,6 +47,10 @@ public slots:
|
||||
virtual void sendInput(const QString& input) = 0;
|
||||
virtual void confirmHostKey(bool trustHost) = 0;
|
||||
virtual void updateTerminalSize(int columns, int rows) = 0;
|
||||
virtual void updateDisplayScale(qreal devicePixelRatio)
|
||||
{
|
||||
Q_UNUSED(devicePixelRatio);
|
||||
}
|
||||
virtual void setClipboardText(const QString& text)
|
||||
{
|
||||
Q_UNUSED(text);
|
||||
|
||||
@@ -176,6 +176,11 @@ SessionTab::SessionTab(const Profile& profile,
|
||||
m_backend,
|
||||
&SessionBackend::updateTerminalSize,
|
||||
Qt::QueuedConnection);
|
||||
connect(this,
|
||||
&SessionTab::requestDisplayScale,
|
||||
m_backend,
|
||||
&SessionBackend::updateDisplayScale,
|
||||
Qt::QueuedConnection);
|
||||
connect(this,
|
||||
&SessionTab::requestKeyEvent,
|
||||
m_backend,
|
||||
@@ -860,6 +865,10 @@ void SessionTab::setupUi()
|
||||
&RdpDisplayWidget::viewportSizeChanged,
|
||||
this,
|
||||
[this](int width, int height) { emit requestTerminalSize(width, height); });
|
||||
connect(m_rdpDisplay,
|
||||
&RdpDisplayWidget::displayScaleChanged,
|
||||
this,
|
||||
[this](qreal ratio) { emit requestDisplayScale(ratio); });
|
||||
connect(m_rdpDisplay,
|
||||
&RdpDisplayWidget::keyInput,
|
||||
this,
|
||||
|
||||
@@ -74,6 +74,7 @@ signals:
|
||||
void requestInput(const QString& input);
|
||||
void requestHostKeyConfirmation(bool trustHost);
|
||||
void requestTerminalSize(int columns, int rows);
|
||||
void requestDisplayScale(qreal devicePixelRatio);
|
||||
void requestKeyEvent(int key,
|
||||
quint32 nativeScanCode,
|
||||
const QString& text,
|
||||
|
||||
Reference in New Issue
Block a user