qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix bad rendering after text selection 

In 9db23e0e04 we added a new call to
createTextNode(), but it turns out that it could be called again for the
same block, a few lines below (in code that was originally added in
dfdc4ce825), if we have not reached the
"last node that needed replacing or last block of the frame". This
resulted in a memory leak: TransformNodes were created without parents,
which b616028dae tried to fix by calling
addCurrentTextNodeToRoot(). That change made the code in "Update the
position of the subsequent text blocks" re-add offsets that had already
been added, which had the effect of moving down the blocks below the
selected ones by the height of one line; but the new call to
addCurrentTextNodeToRoot() was anyway just adding empty TransformNode
instances to the scene graph. Afterwards we can see that any
TransformNode that does not have a GeometryNode as a child is not
actually rendering any text, it's just wasting memory. Having a debug
operator for QQuickTextEditPrivate::Node, and using a QQuickTextEdit
subclass in the autotest that checks the nodes corresponding to blocks
rendered at the end of updatePaintNode(), makes this easier to see
and verify.

So now, if createTextNode() has already been called, we avoid calling
it again a few lines below; but we keep the checks in place to make
sure that every node which does not have a parent will get added to
the scene graph (regardless of which line of code it was created on),
so that there's no leak.

It remains to be seen if this could be cleaned up further.

Amends 9db23e0e04 and
b616028dae

Pick-to: 6.3.2 6.3 6.4
Task-number: QTBUG-103819
Fixes: QTBUG-105208
Fixes: QTBUG-105555
Fixes: QTBUG-105728
Change-Id: I321980c705887bfdb37825f7e6ed8da3d200654e
Reviewed-by: Eskil Abrahamsen Blomfeldt <eskil.abrahamsen-blomfeldt@qt.io>
This commit is contained in:
Shawn Rutledge 2022-08-17 21:57:52 +02:00
parent 47490648b1
commit b271755e6c
4 changed files with 116 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
src/quick/items/qquicktextedit.cpp
View File

@ -2271,11 +2271,13 @@ QSGNode *QQuickTextEdit::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeData *
}
}
bool createdNodeInView = false;
if (inView) {
if (!engine.hasContents()) {
if (node && !node->parent())
d->addCurrentTextNodeToRoot(&engine, rootNode, node, nodeIterator, nodeStart);
node = d->createTextNode();
createdNodeInView = true;
updateNodeTransform(node, nodeOffset);
nodeStart = block.position();
}
@ -2285,13 +2287,13 @@ QSGNode *QQuickTextEdit::updatePaintNode(QSGNode *oldNode, UpdatePaintNodeData *
if ((it.atEnd()) || block.next().position() >= firstCleanNode.startPos())
break; // last node that needed replacing or last block of the frame
QList<int>::const_iterator lowerBound = std::lower_bound(frameBoundaries.constBegin(), frameBoundaries.constEnd(), block.next().position());
if (node && (currentNodeSize > nodeBreakingSize || lowerBound == frameBoundaries.constEnd() || *lowerBound > nodeStart)) {
currentNodeSize = 0;
if (!node->parent())
d->addCurrentTextNodeToRoot(&engine, rootNode, node, nodeIterator, nodeStart);
node = d->createTextNode();
if (!createdNodeInView)
node = d->createTextNode();
resetEngine(&engine, d->color, d->selectedTextColor, d->selectionColor);
nodeStart = block.next().position();
}
@ -3357,6 +3359,16 @@ void QQuickTextEdit::clear()
d->control->clear();
}
#ifndef QT_NO_DEBUG_STREAM
QDebug operator<<(QDebug debug, const QQuickTextEditPrivate::Node &n)
{
QDebugStateSaver saver(debug);
debug.space();
debug << "Node(startPos:" << n.m_startPos << "dirty:" << n.m_dirty << n.m_node << ')';
return debug;
}
#endif
#if QT_VERSION < QT_VERSION_CHECK(7, 0, 0)
void QQuickTextEdit::setOldSelectionDefault()
{

8
src/quick/items/qquicktextedit_p_p.h
View File

@ -53,6 +53,10 @@ public:
int m_startPos;
QQuickTextNode* m_node;
bool m_dirty;
#ifndef QT_NO_DEBUG_STREAM
friend QDebug Q_QUICK_PRIVATE_EXPORT operator<<(QDebug, const Node &);
#endif
};
typedef QList<Node>::iterator TextNodeIterator;
@ -202,6 +206,10 @@ public:
static const int largeTextSizeThreshold;
};
#ifndef QT_NO_DEBUG_STREAM
QDebug Q_QUICK_PRIVATE_EXPORT operator<<(QDebug debug, const QQuickTextEditPrivate::Node &);
#endif
QT_END_NAMESPACE
#endif // QQUICKTEXTEDIT_P_P_H

7
tests/auto/quick/qquicktextedit/data/threeLines.qml Normal file
View File

@ -0,0 +1,7 @@
import QtQuick
import Qt.test 1.0
NodeCheckerTextEdit {
width: 200; height: 100
text: "Line 1\nLine 2\nLine 3\n"
}

87
tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp
View File

@ -57,6 +57,8 @@ public:
tst_qquicktextedit();
private slots:
void initTestCase() override;
void cleanup();
void text();
void width();
@ -138,6 +140,7 @@ private slots:
void implicitSizeBinding();
void largeTextObservesViewport_data();
void largeTextObservesViewport();
void renderingAroundSelection();
void signal_editingfinished();
@ -336,6 +339,56 @@ tst_qquicktextedit::tst_qquicktextedit()
//
}
class NodeCheckerTextEdit : public QQuickTextEdit
{
public:
NodeCheckerTextEdit(QQuickItem *parent = nullptr) : QQuickTextEdit(parent) {}
void populateLinePositions(QSGNode *node)
{
linePositions.clear();
lastLinePosition = 0;
QSGNode *ch = node->firstChild();
while (ch != node->lastChild()) {
QCOMPARE(ch->type(), QSGNode::TransformNodeType);
QSGTransformNode *tn = static_cast<QSGTransformNode *>(ch);
int y = 0;
if (!tn->matrix().isIdentity())
y = tn->matrix().column(3).y();
if (tn->childCount() == 0) {
// A TransformNode with no children is a waste of memory.
// So far, QQuickTextEdit still creates a couple of extras.
qCDebug(lcTests) << "ignoring leaf TransformNode" << tn << "@ y" << y;
} else {
qCDebug(lcTests) << "child" << tn << "@ y" << y << "has children" << tn->childCount();
if (!linePositions.contains(y)) {
linePositions.append(y);
lastLinePosition = qMax(lastLinePosition, y);
}
}
ch = ch->nextSibling();
}
std::sort(linePositions.begin(), linePositions.end());
}
QSGNode *updatePaintNode(QSGNode *node, UpdatePaintNodeData *data) override
{
QSGNode *ret = QQuickTextEdit::updatePaintNode(node, data);
qCDebug(lcTests) << "updated root node" << ret;
populateLinePositions(ret);
return ret;
}
QList<int> linePositions;
int lastLinePosition;
};
void tst_qquicktextedit::initTestCase()
{
QQmlDataTest::initTestCase();
qmlRegisterType<NodeCheckerTextEdit>("Qt.test", 1, 0, "NodeCheckerTextEdit");
}
void tst_qquicktextedit::cleanup()
{
// ensure not even skipped tests with custom input context leave it dangling
@ -3823,6 +3876,40 @@ void tst_qquicktextedit::largeTextObservesViewport()
QCOMPARE(textPriv->cursorItem->isVisible(), textPriv->renderedRegion.intersects(textItem->cursorRectangle()));
}
void tst_qquicktextedit::renderingAroundSelection()
{
QQuickView window;
QVERIFY(QQuickTest::showView(window, testFileUrl("threeLines.qml")));
NodeCheckerTextEdit *textItem = qmlobject_cast<NodeCheckerTextEdit*>(window.rootObject());
QVERIFY(textItem);
QTRY_VERIFY(textItem->linePositions.count() > 0);
const auto linePositions = textItem->linePositions;
const int lastLinePosition = textItem->lastLinePosition;
QQuickTextEditPrivate *textPriv = QQuickTextEditPrivate::get(textItem);
QSignalSpy renderSpy(&window, &QQuickWindow::afterRendering);
if (lcTests().isDebugEnabled())
QTest::qWait(500); // for visual check; not needed in CI
const int renderCount = renderSpy.count();
QPoint p1 = textItem->mapToScene(textItem->positionToRectangle(8).center()).toPoint();
QPoint p2 = textItem->mapToScene(textItem->positionToRectangle(10).center()).toPoint();
qCDebug(lcTests) << "drag from" << p1 << "to" << p2;
QTest::mousePress(&window, Qt::LeftButton, Qt::NoModifier, p1);
QTest::mouseMove(&window, p2);
QTest::mouseRelease(&window, Qt::LeftButton, Qt::NoModifier, p2);
// ensure that QQuickTextEdit::updatePaintNode() has a chance to run
QTRY_COMPARE_GT(renderSpy.count(), renderCount);
if (lcTests().isDebugEnabled())
QTest::qWait(500); // for visual check; not needed in CI
qCDebug(lcTests) << "TextEdit's nodes" << textPriv->textNodeMap;
qCDebug(lcTests) << "font" << textItem->font() << "line positions" << textItem->linePositions << "should be" << linePositions;
QCOMPARE(textItem->lastLinePosition, lastLinePosition);
QTRY_COMPARE(textItem->linePositions, linePositions);
}
void tst_qquicktextedit::signal_editingfinished()
{
QQuickView *window = new QQuickView(nullptr);