From 12eb3b51c4c8c7ce90302d2c356fea09f2c42a5c Mon Sep 17 00:00:00 2001 From: Simon Hausmann Date: Tue, 29 Apr 2014 13:39:23 +0200 Subject: [PATCH] Fix incorrect repaints with plain text edit The plain text edit's smart repaint logic in QPlainTextDocumentLayout::documentChanged assumes that during a change inside just one block, the block is in the state before the edit and a call to layoutBlock() is going to bring it up-to-date. Then only a comparison of the bounding rect - before and after - is going to allow for smart repaints. The assumption of the layout being in the same state as before the edit got broken by commit cc57a2e90f18a39ce3c74b6ad0db9a64aa135ddf, which introduced code to use a QTextCursor within a slot connected to QTextDocument's contentsChange signal. The usage of the QTextCursor there ends up updating the layout of the block ahead of time, breaking the assumption and therefore the optimization, in the sense that during changes in the preedit that cause a change of height / line count, the old bounding rect in QPlainTextDocumentLayout::documentChanged and the new bounding rect will be the same. This causes a repaint of only the edited block, missing repaints of the following blocks, even though the line count effectively changed. So what's causing QTextCursor to mess with the layout is the attempt of updating the vertical movement x property. This patch inhibits the update, marking it as dirty for initialization later. This means that slots connected to this low-level signal cannot rely on the cursor's visual x position, but that doesn't seem useful anyway and isn't required for commit cc57a2e90f18a39ce3c74b6ad0db9a64aa135ddf. Task-number: QTBUG-38536 Change-Id: I5fae12d646a4b2d2cc22b9f2d021e5dc8cfdda94 Reviewed-by: Paul Olav Tvete Reviewed-by: Eskil Abrahamsen Blomfeldt Reviewed-by: Konstantin Ritt --- src/gui/text/qtextcursor.cpp | 2 +- .../text/qtextdocument/tst_qtextdocument.cpp | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/gui/text/qtextcursor.cpp b/src/gui/text/qtextcursor.cpp index ac9762b183..731b6adde8 100644 --- a/src/gui/text/qtextcursor.cpp +++ b/src/gui/text/qtextcursor.cpp @@ -132,7 +132,7 @@ QTextCursorPrivate::AdjustResult QTextCursorPrivate::adjustPosition(int position void QTextCursorPrivate::setX() { - if (priv->isInEditBlock()) { + if (priv->isInEditBlock() || priv->inContentsChange) { x = -1; // mark dirty return; } diff --git a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp index 53aef40df0..59c951332e 100644 --- a/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp +++ b/tests/auto/gui/text/qtextdocument/tst_qtextdocument.cpp @@ -192,6 +192,8 @@ private slots: void QTBUG28998_linkColor(); + void textCursorUsageWithinContentsChange(); + private: void backgroundImage_checkExpectedHtml(const QTextDocument &doc); @@ -3021,5 +3023,50 @@ void tst_QTextDocument::QTBUG28998_linkColor() QCOMPARE(format.foreground(), pal.link()); } +class ContentsChangeHandler : public QObject +{ + Q_OBJECT +public: + ContentsChangeHandler(QTextDocument *doc) + : verticalMovementX(-1) + , doc(doc) + { + connect(doc, SIGNAL(contentsChange(int,int,int)), + this, SLOT(saveModifiedText(int, int, int))); + } + +private slots: + void saveModifiedText(int from, int /*charsRemoved*/, int charsAdded) + { + QTextCursor tmp(doc); + tmp.setPosition(from); + tmp.setPosition(from + charsAdded, QTextCursor::KeepAnchor); + text = tmp.selectedText(); + verticalMovementX = tmp.verticalMovementX(); + } + +public: + QString text; + int verticalMovementX; +private: + QTextDocument *doc; +}; + +void tst_QTextDocument::textCursorUsageWithinContentsChange() +{ + // force creation of layout + doc->documentLayout(); + + QTextCursor cursor(doc); + cursor.insertText("initial text"); + + ContentsChangeHandler handler(doc); + + cursor.insertText("new text"); + + QCOMPARE(handler.text, QString("new text")); + QCOMPARE(handler.verticalMovementX, -1); +} + QTEST_MAIN(tst_QTextDocument) #include "tst_qtextdocument.moc"