Skip to content

Commit 35d872e

Browse files
fuddlesworthruvnet
andcommitted
fix(effect): decouple zone selector from drag activation trigger gating (#175)
Disabling the zone selector popup broke drag-based zone activation because the effect's D-Bus event gating used m_cachedZoneSelectorEnabled as a bypass for trigger checking. When zone selector was enabled (default), drag events always reached the daemon even if trigger deserialization failed. With it disabled, the gate relied solely on detectActivationAndGrab() which could fail if QDBusArgument types weren't handled. Three-part fix: - Handle QDBusArgument explicitly when deserializing trigger lists from D-Bus (Qt may deliver QVariantList<QVariantMap> as QDBusArgument instead of native types, causing silent toList()/toMap() failures) - Add m_triggersLoaded flag with permissive default: before triggers load (or on failure), all three drag event gates stay open, matching pre- optimization behavior - Add diagnostic logging: per-trigger debug dump, warning for all-zero triggers (deserialization failure indicator), warning on load failure Co-Authored-By: claude-flow <ruv@ruv.net>
1 parent d98efcc commit 35d872e

File tree

2 files changed

+69
-7
lines changed

2 files changed

+69
-7
lines changed

kwin-effect/plasmazoneseffect.cpp

Lines changed: 68 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "plasmazoneseffect.h"
55

66
#include <QBuffer>
7+
#include <QDBusArgument>
78
#include <QDBusConnection>
89
#include <QDBusConnectionInterface>
910
#include <QDBusMessage>
@@ -217,7 +218,10 @@ PlasmaZonesEffect::PlasmaZonesEffect()
217218
// Check if zones are needed right now. If so, send dragStarted
218219
// immediately and grab keyboard. Otherwise defer until activation
219220
// is detected mid-drag (or skip entirely if user never activates).
220-
if (detectActivationAndGrab() || m_cachedZoneSelectorEnabled) {
221+
//
222+
// When triggers haven't loaded yet (!m_triggersLoaded), stay
223+
// permissive — send immediately to avoid masking trigger issues (#175).
224+
if (detectActivationAndGrab() || m_cachedZoneSelectorEnabled || !m_triggersLoaded) {
221225
sendDeferredDragStarted();
222226
}
223227
});
@@ -226,7 +230,9 @@ PlasmaZonesEffect::PlasmaZonesEffect()
226230
// Gate D-Bus calls: if no activation trigger is held, toggle mode is off,
227231
// and zone selector is disabled, skip the D-Bus call entirely. This
228232
// eliminates 60Hz D-Bus traffic during non-zone drags.
229-
if (!detectActivationAndGrab() && !m_cachedZoneSelectorEnabled) {
233+
//
234+
// When triggers haven't loaded yet, stay permissive (#175).
235+
if (!detectActivationAndGrab() && !m_cachedZoneSelectorEnabled && m_triggersLoaded) {
230236
return;
231237
}
232238
// Ensure dragStarted was sent before any dragMoved
@@ -572,7 +578,8 @@ void PlasmaZonesEffect::slotMouseChanged(const QPointF& pos, const QPointF& oldp
572578
//
573579
// Gating: same logic as poll-based dragMoved — skip if no activation
574580
// detected and no reason to send (avoids D-Bus traffic for non-zone drags).
575-
if (detectActivationAndGrab() || m_cachedZoneSelectorEnabled) {
581+
// When triggers haven't loaded yet, stay permissive (#175).
582+
if (detectActivationAndGrab() || m_cachedZoneSelectorEnabled || !m_triggersLoaded) {
576583
sendDeferredDragStarted();
577584
callDragMoved(m_dragTracker->draggedWindowId(), pos, m_currentModifiers, static_cast<int>(m_currentMouseButtons));
578585
}
@@ -1057,6 +1064,7 @@ void PlasmaZonesEffect::loadCachedSettings()
10571064
m_minimumWindowWidth = 200;
10581065
m_minimumWindowHeight = 150;
10591066
m_snapAssistEnabled = false;
1067+
m_triggersLoaded = false; // Permissive until new triggers arrive (#175)
10601068

10611069
// Use QDBusMessage + QDBusConnection::asyncCall instead of QDBusInterface to avoid
10621070
// synchronous D-Bus introspection that blocks the compositor thread.
@@ -1141,19 +1149,72 @@ void PlasmaZonesEffect::loadCachedSettings()
11411149
w->deleteLater();
11421150
QDBusPendingReply<QVariant> reply = *w;
11431151
if (reply.isValid()) {
1144-
m_cachedDragActivationTriggers = extractVariant(reply).toList();
1152+
QVariant triggerVariant = extractVariant(reply);
1153+
1154+
// D-Bus may deliver QVariantList-of-QVariantMap as a QDBusArgument
1155+
// instead of a native QVariantList. Extract manually if needed (#175).
1156+
// D-Bus wire format is typically `av` (array of variants), so we
1157+
// extract QVariants first, then handle inner maps per-element below.
1158+
QVariantList triggerList;
1159+
if (triggerVariant.canConvert<QDBusArgument>()) {
1160+
const QDBusArgument arg = triggerVariant.value<QDBusArgument>();
1161+
arg.beginArray();
1162+
while (!arg.atEnd()) {
1163+
QVariant element;
1164+
arg >> element;
1165+
triggerList.append(element);
1166+
}
1167+
arg.endArray();
1168+
} else {
1169+
triggerList = triggerVariant.toList();
1170+
}
1171+
m_cachedDragActivationTriggers = triggerList;
1172+
11451173
// Pre-parse to POD structs so anyLocalTriggerHeld() avoids QVariant
11461174
// unboxing on every call (~30x/sec during drag).
11471175
m_parsedTriggers.clear();
1148-
m_parsedTriggers.reserve(m_cachedDragActivationTriggers.size());
1149-
for (const auto& t : std::as_const(m_cachedDragActivationTriggers)) {
1150-
const auto map = t.toMap();
1176+
m_parsedTriggers.reserve(triggerList.size());
1177+
for (const auto& t : std::as_const(triggerList)) {
1178+
// Each trigger element may also arrive as QDBusArgument
1179+
QVariantMap map;
1180+
if (t.canConvert<QDBusArgument>()) {
1181+
const QDBusArgument elemArg = t.value<QDBusArgument>();
1182+
elemArg >> map;
1183+
} else {
1184+
map = t.toMap();
1185+
}
11511186
ParsedTrigger pt;
11521187
pt.modifier = map.value(QStringLiteral("modifier"), 0).toInt();
11531188
pt.mouseButton = map.value(QStringLiteral("mouseButton"), 0).toInt();
11541189
m_parsedTriggers.append(pt);
11551190
}
1191+
11561192
qCDebug(lcEffect) << "Loaded dragActivationTriggers:" << m_parsedTriggers.size() << "triggers";
1193+
for (int i = 0; i < m_parsedTriggers.size(); ++i) {
1194+
qCDebug(lcEffect) << " trigger" << i << "modifier:" << m_parsedTriggers[i].modifier
1195+
<< "mouseButton:" << m_parsedTriggers[i].mouseButton;
1196+
}
1197+
1198+
// Safety: if triggers loaded but are all empty (modifier=0, mouseButton=0),
1199+
// the gating would block ALL drag events when zoneSelectorEnabled=false.
1200+
// Log a warning so this misconfiguration is visible in debug output.
1201+
bool anyValidTrigger = false;
1202+
for (const auto& pt : std::as_const(m_parsedTriggers)) {
1203+
if (pt.modifier != 0 || pt.mouseButton != 0) {
1204+
anyValidTrigger = true;
1205+
break;
1206+
}
1207+
}
1208+
if (!m_parsedTriggers.isEmpty() && !anyValidTrigger) {
1209+
qCWarning(lcEffect) << "All loaded triggers have modifier=0 mouseButton=0"
1210+
<< "— possible D-Bus deserialization issue;"
1211+
<< "zone activation may not work when zone selector is disabled";
1212+
}
1213+
m_triggersLoaded = true;
1214+
} else {
1215+
qCWarning(lcEffect) << "Failed to load dragActivationTriggers from daemon"
1216+
<< "— trigger gating will remain permissive";
1217+
// Leave m_triggersLoaded = false so gating stays permissive
11571218
}
11581219
});
11591220

kwin-effect/plasmazoneseffect.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,7 @@ public Q_SLOTS:
381381
// Once real settings arrive, they override these conservative defaults.
382382
QVariantList m_cachedDragActivationTriggers; // raw D-Bus data, kept for reload
383383
QVector<ParsedTrigger> m_parsedTriggers; // pre-parsed from QVariantList at load time (avoids QVariant unboxing in hot path)
384+
bool m_triggersLoaded = false; // false until D-Bus reply arrives — permissive default bypasses trigger gating (#175)
384385
bool m_cachedToggleActivation = false;
385386
bool m_cachedZoneSelectorEnabled = true; // true until proven false — ensures dragMoved passes through at startup
386387

0 commit comments

Comments
 (0)