QColorTrcLut: hold in shared_ptr

... instead of raw pointers or QSharedPointer.

Raw pointers are, of course, a no-no in modern code. In particular,
when the result is then held in shared_ptr or QSharedPointer,
make_shared or QSharedPointer::create() should be used to reduce
number of memory allocations.

Since this is private API, we're free to use std::shared_ptr, which
does only half the atomic operations on copies, compared to
QSharedPointer, so is more efficient.

For either make_shared or QSharedPointer::create(), we need to work
around the private ctor, which we do by inheriting a member-function
local class from QColorTrcLut and make_shared'ing that. As a
member-function-local class, it has access to the otherwise private
parts of QColorTrcLut, including its default constructor. As a public
subclass, shared_ptr has no problem performing the derived-to-base
pointer adjustment in the return statement. This way, we can use
make_shared even though our target's class' ctor is private.

Change-Id: Icb11249b54cd5e544e692f6a0bf1f9dda1710454
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>

src/gui/kernel/qguiapplication.cpp

@@ -4156,10 +4156,8 @@ void QGuiApplicationPrivate::notifyDragStarted(const QDrag *drag)
 const QColorTrcLut *QGuiApplicationPrivate::colorProfileForA8Text()
 {
 #ifdef Q_OS_WIN
-    if (!m_a8ColorProfile){
-        QColorTrcLut *cs = QColorTrcLut::fromGamma(2.31); // This is a hard-coded thing for Windows text rendering
-        m_a8ColorProfile.reset(cs);
-    }
+    if (!m_a8ColorProfile)
+        m_a8ColorProfile = QColorTrcLut::fromGamma(2.31); // This is a hard-coded thing for Windows text rendering
     return m_a8ColorProfile.get();
 #else
     return colorProfileForA32Text();
@@ -4168,10 +4166,8 @@ const QColorTrcLut *QGuiApplicationPrivate::colorProfileForA8Text()
 
 const QColorTrcLut *QGuiApplicationPrivate::colorProfileForA32Text()
 {
-    if (!m_a32ColorProfile) {
-        QColorTrcLut *cs = QColorTrcLut::fromGamma(fontSmoothingGamma);
-        m_a32ColorProfile.reset(cs);
-    }
+    if (!m_a32ColorProfile)
+        m_a32ColorProfile = QColorTrcLut::fromGamma(fontSmoothingGamma);
     return m_a32ColorProfile.get();
 }
 

src/gui/kernel/qguiapplication_p.h

@@ -56,7 +56,6 @@
 #include <QtGui/qicon.h>
 
 #include <QtCore/QPointF>
-#include <QtCore/QSharedPointer>
 #include <QtCore/private/qcoreapplication_p.h>
 
 #include <QtCore/qnativeinterface.h>
@@ -68,6 +67,8 @@
 #  include "private/qshortcutmap_p.h"
 #endif
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 
 class QColorTrcLut;
@@ -332,9 +333,9 @@ private:
     static QGuiApplicationPrivate *self;
     static int m_fakeMouseSourcePointId;
 #ifdef Q_OS_WIN
-    QSharedPointer<QColorTrcLut> m_a8ColorProfile;
+    std::shared_ptr<QColorTrcLut> m_a8ColorProfile;
 #endif
-    QSharedPointer<QColorTrcLut> m_a32ColorProfile;
+    std::shared_ptr<QColorTrcLut> m_a32ColorProfile;
 
     bool ownGlobalShareContext;
 

src/gui/painting/qcolorspace_p.h

@@ -60,6 +60,8 @@
 #include <QtCore/qpoint.h>
 #include <QtCore/qshareddata.h>
 
+#include <memory>
+
 QT_BEGIN_NAMESPACE
 
 class Q_AUTOTEST_EXPORT QColorSpacePrimaries
@@ -150,9 +152,9 @@ public:
                 generated.storeRelaxed(1);
             }
         }
-        QSharedPointer<QColorTrcLut> &operator[](int i) { return table[i]; }
-        const QSharedPointer<QColorTrcLut> &operator[](int i) const  { return table[i]; }
-        QSharedPointer<QColorTrcLut> table[3];
+        std::shared_ptr<QColorTrcLut> &operator[](int i) { return table[i]; }
+        const std::shared_ptr<QColorTrcLut> &operator[](int i) const  { return table[i]; }
+        std::shared_ptr<QColorTrcLut> table[3];
         QAtomicInt generated;
     } mutable lut;
 };

src/gui/painting/qcolortransform.cpp

@@ -56,7 +56,7 @@
 
 QT_BEGIN_NAMESPACE
 
-QColorTrcLut *lutFromTrc(const QColorTrc &trc)
+std::shared_ptr<QColorTrcLut> lutFromTrc(const QColorTrc &trc)
 {
     if (trc.m_type == QColorTrc::Type::Table)
         return QColorTrcLut::fromTransferTable(trc.m_table);
@@ -80,12 +80,12 @@ void QColorTransformPrivate::updateLutsIn() const
     }
 
     if (colorSpaceIn->trc[0] == colorSpaceIn->trc[1] && colorSpaceIn->trc[0] == colorSpaceIn->trc[2]) {
-        colorSpaceIn->lut[0].reset(lutFromTrc(colorSpaceIn->trc[0]));
+        colorSpaceIn->lut[0] = lutFromTrc(colorSpaceIn->trc[0]);
         colorSpaceIn->lut[1] = colorSpaceIn->lut[0];
         colorSpaceIn->lut[2] = colorSpaceIn->lut[0];
     } else {
         for (int i = 0; i < 3; ++i)
-            colorSpaceIn->lut[i].reset(lutFromTrc(colorSpaceIn->trc[i]));
+            colorSpaceIn->lut[i] = lutFromTrc(colorSpaceIn->trc[i]);
     }
 
     colorSpaceIn->lut.generated.storeRelease(1);
@@ -104,12 +104,12 @@ void QColorTransformPrivate::updateLutsOut() const
     }
 
     if (colorSpaceOut->trc[0] == colorSpaceOut->trc[1] && colorSpaceOut->trc[0] == colorSpaceOut->trc[2]) {
-        colorSpaceOut->lut[0].reset(lutFromTrc(colorSpaceOut->trc[0]));
+        colorSpaceOut->lut[0] = lutFromTrc(colorSpaceOut->trc[0]);
         colorSpaceOut->lut[1] = colorSpaceOut->lut[0];
         colorSpaceOut->lut[2] = colorSpaceOut->lut[0];
     } else {
         for (int i = 0; i < 3; ++i)
-            colorSpaceOut->lut[i].reset(lutFromTrc(colorSpaceOut->trc[i]));
+            colorSpaceOut->lut[i] = lutFromTrc(colorSpaceOut->trc[i]);
     }
 
     colorSpaceOut->lut.generated.storeRelease(1);

src/gui/painting/qcolortrclut.cpp

@@ -43,10 +43,15 @@
 #include <qmath.h>
 
 QT_BEGIN_NAMESPACE
-
-QColorTrcLut *QColorTrcLut::fromGamma(qreal gamma)
+std::shared_ptr<QColorTrcLut> QColorTrcLut::create()
 {
-    QColorTrcLut *cp = new QColorTrcLut;
+    struct Access : QColorTrcLut {};
+    return std::make_shared<Access>();
+}
+
+std::shared_ptr<QColorTrcLut> QColorTrcLut::fromGamma(qreal gamma)
+{
+    auto cp = create();
 
     for (int i = 0; i <= (255 * 16); ++i) {
         cp->m_toLinear[i] = ushort(qRound(qPow(i / qreal(255 * 16), gamma) * (255 * 256)));
@@ -56,9 +61,9 @@ QColorTrcLut *QColorTrcLut::fromGamma(qreal gamma)
     return cp;
 }
 
-QColorTrcLut *QColorTrcLut::fromTransferFunction(const QColorTransferFunction &fun)
+std::shared_ptr<QColorTrcLut> QColorTrcLut::fromTransferFunction(const QColorTransferFunction &fun)
 {
-    QColorTrcLut *cp = new QColorTrcLut;
+    auto cp = create();
     QColorTransferFunction inv = fun.inverted();
 
     for (int i = 0; i <= (255 * 16); ++i) {
@@ -69,9 +74,9 @@ QColorTrcLut *QColorTrcLut::fromTransferFunction(const QColorTransferFunction &f
     return cp;
 }
 
-QColorTrcLut *QColorTrcLut::fromTransferTable(const QColorTransferTable &table)
+std::shared_ptr<QColorTrcLut> QColorTrcLut::fromTransferTable(const QColorTransferTable &table)
 {
-    QColorTrcLut *cp = new QColorTrcLut;
+    auto cp = create();
 
     float minInverse = 0.0f;
     for (int i = 0; i <= (255 * 16); ++i) {

src/gui/painting/qcolortrclut_p.h

@@ -52,11 +52,11 @@
 //
 
 #include <QtGui/private/qtguiglobal_p.h>
-#include <QtCore/qsharedpointer.h>
 #include <QtGui/qrgb.h>
 #include <QtGui/qrgba64.h>
 
 #include <cmath>
+#include <memory>
 
 #if defined(__SSE2__)
 #include <emmintrin.h>
@@ -72,9 +72,9 @@ class QColorTransferTable;
 class Q_GUI_EXPORT QColorTrcLut
 {
 public:
-    static QColorTrcLut *fromGamma(qreal gamma);
-    static QColorTrcLut *fromTransferFunction(const QColorTransferFunction &transfn);
-    static QColorTrcLut *fromTransferTable(const QColorTransferTable &transTable);
+    static std::shared_ptr<QColorTrcLut> fromGamma(qreal gamma);
+    static std::shared_ptr<QColorTrcLut> fromTransferFunction(const QColorTransferFunction &transfn);
+    static std::shared_ptr<QColorTrcLut> fromTransferTable(const QColorTransferTable &transTable);
 
     // The following methods all convert opaque or unpremultiplied colors:
 
@@ -227,7 +227,9 @@ public:
     ushort m_fromLinear[(255 * 16) + 1]; // [0-4080] -> [0-65280]
 
 private:
-    QColorTrcLut() { }
+    QColorTrcLut() { } // force uninitialized members
+
+    static std::shared_ptr<QColorTrcLut> create();
 
     Q_ALWAYS_INLINE static QRgb convertWithTable(QRgb rgb32, const ushort *table)
     {

tests/auto/gui/painting/qcolor/tst_qcolor.cpp

@@ -1861,7 +1861,7 @@ void tst_QColor::qcolorprofile()
 {
     QFETCH(float, gammaC);
     QFETCH(int, tolerance);
-    QColorTrcLut *cp = QColorTrcLut::fromGamma(gammaC);
+    std::shared_ptr cp = QColorTrcLut::fromGamma(gammaC);
 
     // Test we are accurate for most values after converting through gamma-correction.
     int error = 0;
@@ -1872,7 +1872,6 @@ void tst_QColor::qcolorprofile()
         error += qAbs(qRed(cin) - qRed(cout));
     }
     QVERIFY(error <= tolerance);
-    delete cp;
 }
 
 QTEST_MAIN(tst_QColor)