From ff6156204d2cc33771540ef71500605817dc4911 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Fri, 9 Jul 2021 15:51:40 +0200 Subject: [PATCH] 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 Reviewed-by: Friedemann Kleint --- src/gui/kernel/qguiapplication.cpp | 12 ++++-------- src/gui/kernel/qguiapplication_p.h | 7 ++++--- src/gui/painting/qcolorspace_p.h | 8 +++++--- src/gui/painting/qcolortransform.cpp | 10 +++++----- src/gui/painting/qcolortrclut.cpp | 19 ++++++++++++------- src/gui/painting/qcolortrclut_p.h | 12 +++++++----- tests/auto/gui/painting/qcolor/tst_qcolor.cpp | 3 +-- 7 files changed, 38 insertions(+), 33 deletions(-) diff --git a/src/gui/kernel/qguiapplication.cpp b/src/gui/kernel/qguiapplication.cpp index 101b33f94a..e6bc471ec7 100644 --- a/src/gui/kernel/qguiapplication.cpp +++ b/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(); } diff --git a/src/gui/kernel/qguiapplication_p.h b/src/gui/kernel/qguiapplication_p.h index ca4ed3f12e..b98a1b9ecf 100644 --- a/src/gui/kernel/qguiapplication_p.h +++ b/src/gui/kernel/qguiapplication_p.h @@ -56,7 +56,6 @@ #include #include -#include #include #include @@ -68,6 +67,8 @@ # include "private/qshortcutmap_p.h" #endif +#include + QT_BEGIN_NAMESPACE class QColorTrcLut; @@ -332,9 +333,9 @@ private: static QGuiApplicationPrivate *self; static int m_fakeMouseSourcePointId; #ifdef Q_OS_WIN - QSharedPointer m_a8ColorProfile; + std::shared_ptr m_a8ColorProfile; #endif - QSharedPointer m_a32ColorProfile; + std::shared_ptr m_a32ColorProfile; bool ownGlobalShareContext; diff --git a/src/gui/painting/qcolorspace_p.h b/src/gui/painting/qcolorspace_p.h index 79b1774d0b..f555ad5169 100644 --- a/src/gui/painting/qcolorspace_p.h +++ b/src/gui/painting/qcolorspace_p.h @@ -60,6 +60,8 @@ #include #include +#include + QT_BEGIN_NAMESPACE class Q_AUTOTEST_EXPORT QColorSpacePrimaries @@ -150,9 +152,9 @@ public: generated.storeRelaxed(1); } } - QSharedPointer &operator[](int i) { return table[i]; } - const QSharedPointer &operator[](int i) const { return table[i]; } - QSharedPointer table[3]; + std::shared_ptr &operator[](int i) { return table[i]; } + const std::shared_ptr &operator[](int i) const { return table[i]; } + std::shared_ptr table[3]; QAtomicInt generated; } mutable lut; }; diff --git a/src/gui/painting/qcolortransform.cpp b/src/gui/painting/qcolortransform.cpp index f1f8019a15..fa7cb0e6b4 100644 --- a/src/gui/painting/qcolortransform.cpp +++ b/src/gui/painting/qcolortransform.cpp @@ -56,7 +56,7 @@ QT_BEGIN_NAMESPACE -QColorTrcLut *lutFromTrc(const QColorTrc &trc) +std::shared_ptr 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); diff --git a/src/gui/painting/qcolortrclut.cpp b/src/gui/painting/qcolortrclut.cpp index 268d7252b4..8e2235bfa0 100644 --- a/src/gui/painting/qcolortrclut.cpp +++ b/src/gui/painting/qcolortrclut.cpp @@ -43,10 +43,15 @@ #include QT_BEGIN_NAMESPACE - -QColorTrcLut *QColorTrcLut::fromGamma(qreal gamma) +std::shared_ptr QColorTrcLut::create() { - QColorTrcLut *cp = new QColorTrcLut; + struct Access : QColorTrcLut {}; + return std::make_shared(); +} + +std::shared_ptr 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::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::fromTransferTable(const QColorTransferTable &table) { - QColorTrcLut *cp = new QColorTrcLut; + auto cp = create(); float minInverse = 0.0f; for (int i = 0; i <= (255 * 16); ++i) { diff --git a/src/gui/painting/qcolortrclut_p.h b/src/gui/painting/qcolortrclut_p.h index 1f75a2b4f0..50168652ff 100644 --- a/src/gui/painting/qcolortrclut_p.h +++ b/src/gui/painting/qcolortrclut_p.h @@ -52,11 +52,11 @@ // #include -#include #include #include #include +#include #if defined(__SSE2__) #include @@ -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 fromGamma(qreal gamma); + static std::shared_ptr fromTransferFunction(const QColorTransferFunction &transfn); + static std::shared_ptr 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 create(); Q_ALWAYS_INLINE static QRgb convertWithTable(QRgb rgb32, const ushort *table) { diff --git a/tests/auto/gui/painting/qcolor/tst_qcolor.cpp b/tests/auto/gui/painting/qcolor/tst_qcolor.cpp index 03cece5135..5c93a41b71 100644 --- a/tests/auto/gui/painting/qcolor/tst_qcolor.cpp +++ b/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)