[kcm/kwinrules] Fix size properties not being stored

Summary:
Use `QSize`/`QPoint` to handle and store coordinate values (size and position)

Previously, the rules model stored the "coordinate" type properties as a
`QString` with format `x, y`.

This fails when setting the properties to the config schema, as it requires
a proper `QPoint` or `QSize` value, specially the latter which can't be
convert from such a string.

BUG: 421055
FIXED-IN: 5.19.0

Test Plan:
- Add a new rule and set its position and size properties
- Hitting apply stores the right values in `~\.config\kwinrulesrc`
- Close the kcm and reopen, the values are loaded
- Property detection still works for size and position

Please note that there is a pre-existing bug of some position/sizes not being
applied to the windows in some cases, when using `Apply Initially`.
Better try using the `Force` policy.

Reviewers: ngraham, #kwin, #plasma, zzag

Reviewed By: #kwin, #plasma, zzag

Subscribers: zzag, ltoscano, yurchor, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D29764

kcmkwin/kwinrules/package/contents/ui/RulesEditor.qml

@@ -218,9 +218,10 @@ ScrollViewKCM {
                 return value ? i18n("Yes") : i18n("No");
             case RuleItem.Percentage:
                 return i18n("%1 %", value);
-            case RuleItem.Coordinate:
-                var point = value.split(',');
-                return i18nc("Coordinates (x, y)", "(%1, %2)", point[0], point[1]);
+            case RuleItem.Point:
+                return i18nc("Coordinates (x, y)", "(%1, %2)", value.x, value.y);
+            case RuleItem.Size:
+                return i18nc("Size (width, height)", "(%1, %2)", value.width, value.height);
             case RuleItem.Option:
                 return options.textOfValue(value);
             case RuleItem.FlagsOption:

kcmkwin/kwinrules/package/contents/ui/ValueEditor.qml

@@ -45,7 +45,8 @@ Loader {
             case RuleItem.Option: return optionEditor
             case RuleItem.FlagsOption: return flagsEditor
             case RuleItem.Percentage: return percentageEditor
-            case RuleItem.Coordinate: return coordinateEditor
+            case RuleItem.Point: return coordinateEditor
+            case RuleItem.Size: return coordinateEditor
             case RuleItem.Shortcut: return shortcutEditor
             default: return emptyEditor
         }
@@ -154,7 +155,9 @@ Loader {
             id: coordItem
             spacing: Kirigami.Units.smallSpacing
 
-            property var coords: ruleValue ? ruleValue.split(',') : [0, 0]
+            readonly property var coord: (controlType == RuleItem.Size) ? Qt.size(coordX.value, coordY.value)
+                                                                        : Qt.point(coordX.value, coordY.value)
+            onCoordChanged: valueEditor.valueEdited(coord)
 
             QQC2.SpinBox {
                 id: coordX
@@ -163,8 +166,7 @@ Loader {
                 Layout.fillWidth: true
                 from: 0
                 to: 4098
-                value: coords[0]
-                onValueModified: valueEditor.valueEdited(coordX.value + "," + coordY.value)
+                value: (controlType == RuleItem.Size) ? ruleValue.width : ruleValue.x
             }
             QQC2.Label {
                 id: coordSeparator
@@ -179,8 +181,7 @@ Loader {
                 to: 4098
                 Layout.preferredWidth: 50   // 50%
                 Layout.fillWidth: true
-                value: coords[1]
-                onValueModified: valueEditor.valueEdited(coordX.value + "," + coordY.value)
+                value: (controlType == RuleItem.Size) ? ruleValue.height : ruleValue.y
             }
         }
     }

kcmkwin/kwinrules/ruleitem.cpp

@@ -208,11 +208,10 @@ QVariant RuleItem::typedValue(const QVariant &value, const RuleItem::Type type)
                 return 0x3FF - 0x040;  //All possible flags minus NET::Override (deprecated)
             }
             return value.toInt();
-        case Coordinate:
-            if (value.toString().isEmpty()) {
-                return QStringLiteral("0,0");
-            }
-            return value.toString();
+        case Point:
+            return value.toPoint();
+        case Size:
+            return value.toSize();
         case String:
             return value.toString().trimmed();
         case Shortcut:

kcmkwin/kwinrules/ruleitem.h

@@ -43,7 +43,8 @@ public:
         Option,
         FlagsOption,
         Percentage,
-        Coordinate,
+        Point,
+        Size,
         Shortcut
     };
     Q_ENUM(Type)

kcmkwin/kwinrules/rulesmodel.cpp

@@ -396,12 +396,12 @@ void RulesModel::populateRuleList()
 
     // Size & Position
     addRule(new RuleItem(QLatin1String("position"),
-                         RulePolicy::SetRule, RuleItem::Coordinate,
+                         RulePolicy::SetRule, RuleItem::Point,
                          i18n("Position"), i18n("Size & Position"),
                          QIcon::fromTheme("transform-move")));
 
     addRule(new RuleItem(QLatin1String("size"),
-                         RulePolicy::SetRule, RuleItem::Coordinate,
+                         RulePolicy::SetRule, RuleItem::Size,
                          i18n("Size"), i18n("Size & Position"),
                          QIcon::fromTheme("image-resize-symbolic")));
 
@@ -474,12 +474,12 @@ void RulesModel::populateRuleList()
                               "to unconditionally popup in the middle of your screen.")));
 
     addRule(new RuleItem(QLatin1String("minsize"),
-                         RulePolicy::ForceRule, RuleItem::Coordinate,
+                         RulePolicy::ForceRule, RuleItem::Size,
                          i18n("Minimum Size"), i18n("Size & Position"),
                          QIcon::fromTheme("image-resize-symbolic")));
 
     addRule(new RuleItem(QLatin1String("maxsize"),
-                         RulePolicy::ForceRule, RuleItem::Coordinate,
+                         RulePolicy::ForceRule, RuleItem::Size,
                          i18n("Maximum Size"), i18n("Size & Position"),
                          QIcon::fromTheme("image-resize-symbolic")));
 
@@ -647,10 +647,8 @@ const QHash<QString, QString> RulesModel::x11PropertyHash()
 void RulesModel::setWindowProperties(const QVariantMap &info, bool forceValue)
 {
     // Properties that cannot be directly applied via x11PropertyHash
-    const QString position = QStringLiteral("%1,%2").arg(info.value("x").toInt())
-    .arg(info.value("y").toInt());
-    const QString size = QStringLiteral("%1,%2").arg(info.value("width").toInt())
-    .arg(info.value("height").toInt());
+    const QPoint position = QPoint(info.value("x").toInt(), info.value("y").toInt());
+    const QSize size = QSize(info.value("width").toInt(), info.value("height").toInt());
 
     m_rules["position"]->setSuggestedValue(position, forceValue);
     m_rules["size"]->setSuggestedValue(size, forceValue);