From 826fb1cb298fbaa21d7dea754ca225754c6851a7 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 23 Jan 2023 14:58:09 +0200 Subject: [PATCH] Fix a crash that happens when resizing quick tiled window QuickTile::setRelativeGeometry() and QuickRootTile::setVerticalSplit() or QuickRootTile::setHorizontalSplit() can hit recursion when size constraints start taking effect. This change reworks how other quick tiles are resized. With the proposed design, when relative geometry changes, QuickRootTile will notice that and start resizing other tiles. When QuickRootTile resizes horizontal or vertical split, it is going to ignore QuickRootTile::relativeGeometryChanged() signals (m_resizedTile). It prevents hitting the recursion and makes moving h/v splits more predictable. I do think that in order to make the tile design more robust to this kind of bugs, it's worth splitting geometry in two kinds though - the one that indicates the preferred geometry (implicitWidth/implicitHeight in qtquick lingua) and the current geometry, the parent node then monitors the preferred geometries and updates the current geometries. BUG: 464621 --- src/tiles/quicktile.cpp | 161 ++++++++++++++-------------------------- src/tiles/quicktile.h | 37 +++------ src/tiles/tile.cpp | 12 +-- src/tiles/tile.h | 6 +- 4 files changed, 77 insertions(+), 139 deletions(-) diff --git a/src/tiles/quicktile.cpp b/src/tiles/quicktile.cpp index 1ce12fe0a0..241e4bf627 100644 --- a/src/tiles/quicktile.cpp +++ b/src/tiles/quicktile.cpp @@ -19,22 +19,69 @@ QuickRootTile::QuickRootTile(TileManager *tiling, Tile *parentItem) setRelativeGeometry(QRectF(0, 0, 1, 1)); setQuickTileMode(QuickTileFlag::Maximize); - m_leftVerticalTile = std::make_unique(tiling, ElectricBorder::ElectricLeft, this); - m_rightVerticalTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricRight, this)); + auto createTile = [this, &tiling](const QRectF &geometry, QuickTileMode tileMode) { + Tile *tile = new Tile(tiling, this); + tile->setPadding(0.0); + tile->setQuickTileMode(tileMode); + tile->setRelativeGeometry(geometry); - m_topHorizontalTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricTop, this)); - m_bottomHorizontalTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricBottom, this)); + connect(tile, &Tile::relativeGeometryChanged, this, [this, tile]() { + relayoutToFit(tile); + }); - m_topLeftTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricTopLeft, this)); - m_topRightTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricTopRight, this)); - m_bottomLeftTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricBottomLeft, this)); - m_bottomRightTile = std::unique_ptr(new QuickTile(tiling, ElectricBorder::ElectricBottomRight, this)); + return std::unique_ptr(tile); + }; + + m_leftVerticalTile = createTile(QRectF(0, 0, 0.5, 1), QuickTileFlag::Left); + m_rightVerticalTile = createTile(QRectF(0.5, 0, 0.5, 1), QuickTileFlag::Right); + m_topHorizontalTile = createTile(QRectF(0, 0, 1, 0.5), QuickTileFlag::Top); + m_bottomHorizontalTile = createTile(QRectF(0, 0.5, 1, 0.5), QuickTileFlag::Bottom); + + m_topLeftTile = createTile(QRectF(0, 0, 0.5, 0.5), QuickTileFlag::Top | QuickTileFlag::Left); + m_topRightTile = createTile(QRectF(0.5, 0, 0.5, 0.5), QuickTileFlag::Top | QuickTileFlag::Right); + m_bottomLeftTile = createTile(QRectF(0, 0.5, 0.5, 0.5), QuickTileFlag::Bottom | QuickTileFlag::Left); + m_bottomRightTile = createTile(QRectF(0.5, 0.5, 0.5, 0.5), QuickTileFlag::Bottom | QuickTileFlag::Right); } QuickRootTile::~QuickRootTile() { } +void QuickRootTile::relayoutToFit(Tile *tile) +{ + if (m_resizedTile) { + return; + } + + m_resizedTile = tile; + + const QRectF geometry = tile->relativeGeometry(); + + if (m_topHorizontalTile.get() == tile) { + setVerticalSplit(geometry.bottom()); + } else if (m_bottomHorizontalTile.get() == tile) { + setVerticalSplit(geometry.top()); + } else if (m_leftVerticalTile.get() == tile) { + setHorizontalSplit(geometry.right()); + } else if (m_rightVerticalTile.get() == tile) { + setHorizontalSplit(geometry.left()); + } else if (m_topLeftTile.get() == tile) { + setHorizontalSplit(geometry.right()); + setVerticalSplit(geometry.bottom()); + } else if (m_topRightTile.get() == tile) { + setHorizontalSplit(geometry.left()); + setVerticalSplit(geometry.bottom()); + } else if (m_bottomRightTile.get() == tile) { + setHorizontalSplit(geometry.left()); + setVerticalSplit(geometry.top()); + } else if (m_bottomLeftTile.get() == tile) { + setHorizontalSplit(geometry.right()); + setVerticalSplit(geometry.top()); + } + + m_resizedTile = nullptr; +} + Tile *QuickRootTile::tileForMode(QuickTileMode mode) { switch (mode) { @@ -152,102 +199,4 @@ void QuickRootTile::setVerticalSplit(qreal split) m_bottomRightTile->setRelativeGeometry(geom); } -QuickTile::QuickTile(TileManager *tiling, ElectricBorder border, QuickRootTile *parentItem) - : Tile(tiling, parentItem) - , m_root(parentItem) - , m_border(border) -{ - m_geometryLock = true; - setPadding(0); - switch (border) { - case ElectricTop: - setQuickTileMode(QuickTileFlag::Top); - setRelativeGeometry(QRectF(0, 0, 1, 0.5)); - break; - case ElectricTopRight: - setQuickTileMode(QuickTileFlag::Top | QuickTileFlag::Right); - setRelativeGeometry(QRectF(0.5, 0, 0.5, 0.5)); - break; - case ElectricRight: - setQuickTileMode(QuickTileFlag::Right); - setRelativeGeometry(QRectF(0.5, 0, 0.5, 1)); - break; - case ElectricBottomRight: - setQuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Right); - setRelativeGeometry(QRectF(0.5, 0.5, 0.5, 0.5)); - break; - case ElectricBottom: - setQuickTileMode(QuickTileFlag::Bottom); - setRelativeGeometry(QRectF(0, 0.5, 1, 0.5)); - break; - case ElectricBottomLeft: - setQuickTileMode(QuickTileFlag::Bottom | QuickTileFlag::Left); - setRelativeGeometry(QRectF(0, 0.5, 0.5, 0.5)); - break; - case ElectricLeft: - setQuickTileMode(QuickTileFlag::Left); - setRelativeGeometry(QRectF(0, 0, 0.5, 1)); - break; - case ElectricTopLeft: - setQuickTileMode(QuickTileFlag::Top | QuickTileFlag::Left); - setRelativeGeometry(QRectF(0, 0, 0.5, 0.5)); - break; - case ElectricNone: - setQuickTileMode(QuickTileFlag::None); - default: - break; - } -} - -QuickTile::~QuickTile() -{ -} - -void QuickTile::setRelativeGeometry(const QRectF &geom) -{ - if (relativeGeometry() == geom) { - return; - } - - if (!m_geometryLock) { - m_geometryLock = true; - switch (m_border) { - case ElectricTop: - m_root->setVerticalSplit(geom.bottom()); - break; - case ElectricTopRight: - m_root->setHorizontalSplit(geom.left()); - m_root->setVerticalSplit(geom.bottom()); - break; - case ElectricRight: - m_root->setHorizontalSplit(geom.left()); - break; - case ElectricBottomRight: - m_root->setHorizontalSplit(geom.left()); - m_root->setVerticalSplit(geom.top()); - break; - case ElectricBottom: - m_root->setVerticalSplit(geom.top()); - break; - case ElectricBottomLeft: - m_root->setHorizontalSplit(geom.right()); - m_root->setVerticalSplit(geom.top()); - break; - case ElectricLeft: - m_root->setHorizontalSplit(geom.right()); - break; - case ElectricTopLeft: - m_root->setHorizontalSplit(geom.right()); - m_root->setVerticalSplit(geom.bottom()); - break; - case ElectricNone: - default: - break; - } - } - - Tile::setRelativeGeometry(geom); - m_geometryLock = false; -} - } // namespace KWin diff --git a/src/tiles/quicktile.h b/src/tiles/quicktile.h index 82bbd17133..f12ee1a553 100644 --- a/src/tiles/quicktile.h +++ b/src/tiles/quicktile.h @@ -16,23 +16,6 @@ namespace KWin { -class QuickRootTile; - -class KWIN_EXPORT QuickTile : public Tile -{ - Q_OBJECT -public: - QuickTile(TileManager *tiling, ElectricBorder border, QuickRootTile *parentItem = nullptr); - ~QuickTile(); - - void setRelativeGeometry(const QRectF &geom) override; - -private: - QuickRootTile *m_root = nullptr; - ElectricBorder m_border; - bool m_geometryLock = false; -}; - class KWIN_EXPORT QuickRootTile : public Tile { Q_OBJECT @@ -50,16 +33,20 @@ public: void setVerticalSplit(qreal split); private: - std::unique_ptr m_leftVerticalTile; - std::unique_ptr m_rightVerticalTile; + void relayoutToFit(Tile *tile); - std::unique_ptr m_topHorizontalTile; - std::unique_ptr m_bottomHorizontalTile; + Tile *m_resizedTile = nullptr; - std::unique_ptr m_topLeftTile; - std::unique_ptr m_topRightTile; - std::unique_ptr m_bottomLeftTile; - std::unique_ptr m_bottomRightTile; + std::unique_ptr m_leftVerticalTile; + std::unique_ptr m_rightVerticalTile; + + std::unique_ptr m_topHorizontalTile; + std::unique_ptr m_bottomHorizontalTile; + + std::unique_ptr m_topLeftTile; + std::unique_ptr m_topRightTile; + std::unique_ptr m_bottomLeftTile; + std::unique_ptr m_bottomRightTile; }; } // namespace KWin diff --git a/src/tiles/tile.cpp b/src/tiles/tile.cpp index 9dd58cf4a7..328c7043d4 100644 --- a/src/tiles/tile.cpp +++ b/src/tiles/tile.cpp @@ -91,15 +91,17 @@ void Tile::setGeometryFromAbsolute(const QRectF &geom) void Tile::setRelativeGeometry(const QRectF &geom) { - if (m_relativeGeometry == geom) { + QRectF constrainedGeom = geom; + constrainedGeom.setWidth(std::max(constrainedGeom.width(), s_minimumSize.width())); + constrainedGeom.setHeight(std::max(constrainedGeom.height(), s_minimumSize.height())); + + if (m_relativeGeometry == constrainedGeom) { return; } - m_relativeGeometry = geom; - m_relativeGeometry.setWidth(std::max(m_relativeGeometry.width(), s_minimumSize.width())); - m_relativeGeometry.setHeight(std::max(m_relativeGeometry.height(), s_minimumSize.height())); + m_relativeGeometry = constrainedGeom; - Q_EMIT relativeGeometryChanged(geom); + Q_EMIT relativeGeometryChanged(); Q_EMIT absoluteGeometryChanged(); Q_EMIT windowGeometryChanged(); diff --git a/src/tiles/tile.h b/src/tiles/tile.h index 84ee47fbe1..8eefff400b 100644 --- a/src/tiles/tile.h +++ b/src/tiles/tile.h @@ -44,6 +44,7 @@ public: }; Q_ENUM(LayoutDirection) + explicit Tile(TileManager *tiling, Tile *parentItem = nullptr); ~Tile(); void setGeometryFromWindow(const QRectF &geom); @@ -85,6 +86,7 @@ public: void setPadding(qreal padding); QuickTileMode quickTileMode() const; + void setQuickTileMode(QuickTileMode mode); /** * All tiles directly children of this tile @@ -129,7 +131,7 @@ public: } Q_SIGNALS: - void relativeGeometryChanged(const QRectF &relativeGeometry); + void relativeGeometryChanged(); void absoluteGeometryChanged(); void windowGeometryChanged(); void paddingChanged(qreal padding); @@ -141,8 +143,6 @@ Q_SIGNALS: void windowsChanged(); protected: - explicit Tile(TileManager *tiling, Tile *parentItem = nullptr); - void setQuickTileMode(QuickTileMode mode); void insertChild(int position, Tile *item); void removeChild(Tile *child);