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
This commit is contained in:
Vlad Zahorodnii 2023-01-23 14:58:09 +02:00
parent 3650250c31
commit 826fb1cb29
4 changed files with 77 additions and 139 deletions

View file

@ -19,22 +19,69 @@ QuickRootTile::QuickRootTile(TileManager *tiling, Tile *parentItem)
setRelativeGeometry(QRectF(0, 0, 1, 1));
setQuickTileMode(QuickTileFlag::Maximize);
m_leftVerticalTile = std::make_unique<QuickTile>(tiling, ElectricBorder::ElectricLeft, this);
m_rightVerticalTile = std::unique_ptr<QuickTile>(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<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricTop, this));
m_bottomHorizontalTile = std::unique_ptr<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricBottom, this));
connect(tile, &Tile::relativeGeometryChanged, this, [this, tile]() {
relayoutToFit(tile);
});
m_topLeftTile = std::unique_ptr<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricTopLeft, this));
m_topRightTile = std::unique_ptr<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricTopRight, this));
m_bottomLeftTile = std::unique_ptr<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricBottomLeft, this));
m_bottomRightTile = std::unique_ptr<QuickTile>(new QuickTile(tiling, ElectricBorder::ElectricBottomRight, this));
return std::unique_ptr<Tile>(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

View file

@ -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<QuickTile> m_leftVerticalTile;
std::unique_ptr<QuickTile> m_rightVerticalTile;
void relayoutToFit(Tile *tile);
std::unique_ptr<QuickTile> m_topHorizontalTile;
std::unique_ptr<QuickTile> m_bottomHorizontalTile;
Tile *m_resizedTile = nullptr;
std::unique_ptr<QuickTile> m_topLeftTile;
std::unique_ptr<QuickTile> m_topRightTile;
std::unique_ptr<QuickTile> m_bottomLeftTile;
std::unique_ptr<QuickTile> m_bottomRightTile;
std::unique_ptr<Tile> m_leftVerticalTile;
std::unique_ptr<Tile> m_rightVerticalTile;
std::unique_ptr<Tile> m_topHorizontalTile;
std::unique_ptr<Tile> m_bottomHorizontalTile;
std::unique_ptr<Tile> m_topLeftTile;
std::unique_ptr<Tile> m_topRightTile;
std::unique_ptr<Tile> m_bottomLeftTile;
std::unique_ptr<Tile> m_bottomRightTile;
};
} // namespace KWin

View file

@ -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();

View file

@ -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);