From 813bbc515bdd8af843ef56115711691e926d0bcb Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 30 Jun 2023 21:36:22 +0200 Subject: [PATCH] QStyle: don't use the QPixmapCache when a style sheet is set If a style sheet is active, then the palette in the QStyleOption might be a generated, short-lived, temporary QPalette instance. As QPalette's cacheKey is based on instance-counters of the private data structures, it will always be unique for such a QPalette, even if the brushes are the same as in a previous instance. (QPalette::cacheKey is just a 64bit integer, we cannot possibly encode the entire QPalette data in it in a collision-free way, and since a brush in the palette might contain a pixmap or a gradient we couldn't even generate an efficient string representation for it. We could at most cache and reuse QPalette instances based on the attributes in the style sheet rule and in the base palette of the widget. However, this seems fragile; it might be an opportunity for future optimization.) Some styles use the QPixmapCache, with a key that includes the palette's cache key. The key will always be unique if the palette is based on style sheet rules, and then we fill pixmap cache with pixmaps that can never be reused, making the cache both useless and wasteful. To solve this, generate an empty key if we detect that it is for a style object that is the target of a style sheet. Return an empty cache key string from QStyleHelper::uniqueName, which will make QPixmapCache return immediatey when trying to insert or find an object. This is not pretty, but it makes the change minimal and low-risk. Refactoring the respective code paths to e.g. consistently use the BEGIN_STYLE_PIXMAPCACHE helper macro requires larger changes that can only be verified visually, and so are out of scope for a bug fix. This requires changes to code that uses QStyleHelper::uniqueName, as we need to avoid that other key elements are appended to the generated (and maybe empty) key. As a side effect, this ends up with code that makes better use of QStringBuilder. Pick-to: 6.6 Fixes: QTBUG-114473 Change-Id: I011aed0885f105cbf1e8c0bc6b94c46df47761a3 Reviewed-by: Marc Mutz --- src/widgets/styles/qcommonstyle.cpp | 9 ++++---- src/widgets/styles/qfusionstyle.cpp | 33 +++++++++++++---------------- src/widgets/styles/qstylehelper.cpp | 10 +++++++++ 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/widgets/styles/qcommonstyle.cpp b/src/widgets/styles/qcommonstyle.cpp index 08c22240925..b247cf6f67f 100644 --- a/src/widgets/styles/qcommonstyle.cpp +++ b/src/widgets/styles/qcommonstyle.cpp @@ -736,11 +736,10 @@ void QCommonStyle::drawPrimitive(PrimitiveElement pe, const QStyleOption *opt, Q QRect r = opt->rect; int size = qMin(r.height(), r.width()); QPixmap pixmap; - QString pixmapName = - QStyleHelper::uniqueName("$qt_ia-"_L1 - % QLatin1StringView(metaObject()->className()), - opt, QSize(size, size)) - % HexString(pe); + const QString pixmapName = QStyleHelper::uniqueName("$qt_ia-"_L1 + % QLatin1StringView(metaObject()->className()) + % HexString(pe), + opt, QSize(size, size)); if (!QPixmapCache::find(pixmapName, &pixmap)) { qreal pixelRatio = p->device()->devicePixelRatio(); int border = qRound(pixelRatio*(size/5)); diff --git a/src/widgets/styles/qfusionstyle.cpp b/src/widgets/styles/qfusionstyle.cpp index 918f9dbe520..723549f3d10 100644 --- a/src/widgets/styles/qfusionstyle.cpp +++ b/src/widgets/styles/qfusionstyle.cpp @@ -1236,12 +1236,12 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio // Draws the header in tables. if (const QStyleOptionHeader *header = qstyleoption_cast(option)) { const QStyleOptionHeaderV2 *headerV2 = qstyleoption_cast(option); - QString pixmapName = QStyleHelper::uniqueName("headersection"_L1, option, option->rect.size()); - pixmapName += QString::number(- int(header->position)); - pixmapName += QString::number(- int(header->orientation)); - if (headerV2) - pixmapName += QString::number(- int(headerV2->isSectionDragTarget)); - + const bool isSectionDragTarget = headerV2 ? headerV2->isSectionDragTarget : false; + const QString pixmapName = QStyleHelper::uniqueName("headersection-"_L1 + % HexString(header->position) + % HexString(header->orientation) + % (isSectionDragTarget ? '1' : '0'), + option, option->rect.size()); QPixmap cache; if (!QPixmapCache::find(pixmapName, &cache)) { cache = styleCachePixmap(rect.size()); @@ -1251,7 +1251,7 @@ void QFusionStyle::drawControl(ControlElement element, const QStyleOption *optio QColor buttonColor = d->buttonColor(option->palette); QColor gradientStartColor = buttonColor.lighter(104); QColor gradientStopColor = buttonColor.darker(102); - if (headerV2 && headerV2->isSectionDragTarget) { + if (isSectionDragTarget) { gradientStopColor = gradientStartColor.darker(130); gradientStartColor = gradientStartColor.darker(130); } @@ -2665,16 +2665,12 @@ void QFusionStyle::drawComplexControl(ComplexControl control, const QStyleOption bool sunken = comboBox->state & State_On; // play dead, if combobox has no items bool isEnabled = (comboBox->state & State_Enabled); QPixmap cache; - QString pixmapName = QStyleHelper::uniqueName("combobox"_L1, option, comboBox->rect.size()); - if (sunken) - pixmapName += "-sunken"_L1; - if (comboBox->editable) - pixmapName += "-editable"_L1; - if (isEnabled) - pixmapName += "-enabled"_L1; - if (!comboBox->frame) - pixmapName += "-frameless"_L1; - + const QString pixmapName = QStyleHelper::uniqueName("combobox"_L1 + % (sunken ? "-sunken" : "") + % (comboBox->editable ? "-editable" : "") + % (isEnabled ? "-enabled" : "") + % (!comboBox->frame ? "-frameless" : ""), + option, comboBox->rect.size()); if (!QPixmapCache::find(pixmapName, &cache)) { cache = styleCachePixmap(comboBox->rect.size()); cache.fill(Qt::transparent); @@ -2817,7 +2813,8 @@ void QFusionStyle::drawComplexControl(ComplexControl control, const QStyleOption // draw blue groove highlight QRect clipRect; - groovePixmapName += "_blue"_L1; + if (!groovePixmapName.isEmpty()) + groovePixmapName += "_blue"_L1; if (!QPixmapCache::find(groovePixmapName, &cache)) { cache = styleCachePixmap(pixmapRect.size()); cache.fill(Qt::transparent); diff --git a/src/widgets/styles/qstylehelper.cpp b/src/widgets/styles/qstylehelper.cpp index f79e83be617..1084761d30d 100644 --- a/src/widgets/styles/qstylehelper.cpp +++ b/src/widgets/styles/qstylehelper.cpp @@ -25,8 +25,18 @@ Q_GUI_EXPORT int qt_defaultDpiX(); namespace QStyleHelper { +static inline bool usePixmapCache(const QStyleOption *opt) +{ + if (QWidget *widget = qobject_cast(opt->styleObject)) + return !widget->testAttribute(Qt::WA_StyleSheetTarget); + return true; +} + QString uniqueName(const QString &key, const QStyleOption *option, const QSize &size) { + if (!usePixmapCache(option)) + return {}; + const QStyleOptionComplex *complexOption = qstyleoption_cast(option); QString tmp = key % HexString(option->state) % HexString(option->direction)