QFont: Don't invalidate engine unless request has been changed

This makes QFont do a "light" detach when the font attributes data has been changed.
The new test clearly shows that the engine is now shared between
two font instances after changing the kerning attribute.

Change-Id: I59db822f459f02d111686dba7101b98e361fada9
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

src/gui/text/qfont.cpp

@@ -648,6 +648,24 @@ void QFont::detach()
     d.detach();
 }
 
+/*!
+    \internal
+    Detaches the font object from common font attributes data.
+    Call this instead of QFont::detach() if the only font attributes data
+    has been changed (underline, letterSpacing, kerning, etc.).
+*/
+void QFontPrivate::detachButKeepEngineData(QFont *font)
+{
+    if (font->d->ref.load() == 1)
+        return;
+
+    QFontEngineData *engineData = font->d->engineData;
+    if (engineData)
+        engineData->ref.ref();
+    font->d.detach();
+    font->d->engineData = engineData;
+}
+
 /*!
     Constructs a font object that uses the application's default font.
 
@@ -1149,7 +1167,7 @@ void QFont::setUnderline(bool enable)
     if ((resolve_mask & QFont::UnderlineResolved) && d->underline == enable)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->underline = enable;
     resolve_mask |= QFont::UnderlineResolved;
@@ -1175,7 +1193,7 @@ void QFont::setOverline(bool enable)
     if ((resolve_mask & QFont::OverlineResolved) && d->overline == enable)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->overline = enable;
     resolve_mask |= QFont::OverlineResolved;
@@ -1202,7 +1220,7 @@ void QFont::setStrikeOut(bool enable)
     if ((resolve_mask & QFont::StrikeOutResolved) && d->strikeOut == enable)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->strikeOut = enable;
     resolve_mask |= QFont::StrikeOutResolved;
@@ -1262,7 +1280,7 @@ void QFont::setKerning(bool enable)
     if ((resolve_mask & QFont::KerningResolved) && d->kerning == enable)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->kerning = enable;
     resolve_mask |= QFont::KerningResolved;
@@ -1512,7 +1530,7 @@ void QFont::setLetterSpacing(SpacingType type, qreal spacing)
         d->letterSpacing == newSpacing)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->letterSpacing = newSpacing;
     d->letterSpacingIsAbsolute = absoluteSpacing;
@@ -1562,7 +1580,7 @@ void QFont::setWordSpacing(qreal spacing)
         d->wordSpacing == newSpacing)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->wordSpacing = newSpacing;
     resolve_mask |= QFont::WordSpacingResolved;
@@ -1596,7 +1614,7 @@ void QFont::setCapitalization(Capitalization caps)
         capitalization() == caps)
         return;
 
-    detach();
+    QFontPrivate::detachButKeepEngineData(this);
 
     d->capital = caps;
     resolve_mask |= QFont::CapitalizationResolved;
@@ -1635,6 +1653,7 @@ void QFont::setRawMode(bool enable)
 {
     if ((bool) d->rawMode == enable) return;
 
+    // might change behavior, thus destroy engine data
     detach();
 
     d->rawMode = enable;

src/gui/text/qfont_p.h

@@ -185,6 +185,9 @@ public:
     }
 
     void resolve(uint mask, const QFontPrivate *other);
+
+    static void detachButKeepEngineData(QFont *font);
+
 private:
     QFontPrivate &operator=(const QFontPrivate &) { return *this; }
 };

tests/auto/gui/text/qfont/qfont.pro

@@ -2,6 +2,7 @@ CONFIG += testcase
 CONFIG += parallel_test
 TARGET = tst_qfont
 QT += testlib
+QT += core-private gui-private
 !contains(QT_CONFIG, no-widgets): QT += widgets
 SOURCES  += tst_qfont.cpp
 

tests/auto/gui/text/qfont/tst_qfont.cpp

@@ -44,6 +44,7 @@
 
 
 #include <qfont.h>
+#include <private/qfont_p.h>
 #include <qfontdatabase.h>
 #include <qfontinfo.h>
 #include <qstringlist.h>
@@ -81,6 +82,8 @@ private slots:
     void styleName();
     void defaultFamily_data();
     void defaultFamily();
+
+    void sharing();
 };
 
 // Testing get/set functions
@@ -685,5 +688,45 @@ void tst_QFont::defaultFamily()
     QVERIFY2(isAcceptable, msgNotAcceptableFont(familyForHint, acceptableFamilies));
 }
 
+void tst_QFont::sharing()
+{
+    QFont f;
+    f.setStyleHint(QFont::Serif);
+    f.exactMatch(); // loads engine
+    QCOMPARE(QFontPrivate::get(f)->ref.load(), 1);
+    QVERIFY(QFontPrivate::get(f)->engineData);
+    QCOMPARE(QFontPrivate::get(f)->engineData->ref.load(), 1);
+
+    QFont f2(f);
+    QVERIFY(QFontPrivate::get(f2) == QFontPrivate::get(f));
+    QCOMPARE(QFontPrivate::get(f2)->ref.load(), 2);
+    QVERIFY(QFontPrivate::get(f2)->engineData);
+    QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1);
+
+    f2.setKerning(!f.kerning());
+    QVERIFY(QFontPrivate::get(f2) != QFontPrivate::get(f));
+    QCOMPARE(QFontPrivate::get(f2)->ref.load(), 1);
+    QVERIFY(QFontPrivate::get(f2)->engineData);
+    QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 2);
+
+    f2 = f;
+    QVERIFY(QFontPrivate::get(f2) == QFontPrivate::get(f));
+    QCOMPARE(QFontPrivate::get(f2)->ref.load(), 2);
+    QVERIFY(QFontPrivate::get(f2)->engineData);
+    QVERIFY(QFontPrivate::get(f2)->engineData == QFontPrivate::get(f)->engineData);
+    QCOMPARE(QFontPrivate::get(f2)->engineData->ref.load(), 1);
+
+    if (f.pointSize() > 0)
+        f2.setPointSize(f.pointSize() * 2 / 3);
+    else
+        f2.setPixelSize(f.pixelSize() * 2 / 3);
+    QVERIFY(QFontPrivate::get(f2) != QFontPrivate::get(f));
+    QCOMPARE(QFontPrivate::get(f2)->ref.load(), 1);
+    QVERIFY(!QFontPrivate::get(f2)->engineData);
+    QVERIFY(QFontPrivate::get(f2)->engineData != QFontPrivate::get(f)->engineData);
+}
+
 QTEST_MAIN(tst_QFont)
 #include "tst_qfont.moc"