Always update QPalette resolve mask, regardless of QBrush change

56bd1b76d2e81c58a80bf6b5d74219c6b1ab8499 changed the update
resolve mask behavior in QPalette to avoid detaching brush data
when modifying the resolve mask if the brush value is not changed.

But this behavior broke compatibility, it introduced unknown risks, and
we cannot ensure that other code in Qt does not depend on the old
behavior.

We both need to ensure that we don't detach when the value is not
changed, and ensure that the resolveMask is always updated regardless
of whether the value changes, so we need to split them up and
independently share the brush data.

QFont will update its corresponding resolveMask even if the value has
not changed, so it is better to correct this behavior so that QPalette
and QFont are consistent.

[ChangeLog][QtGui][QPalette] Always update resolve mask in
QPalette::setBrush, even if the value of brush has not changed.

Fixes: QTBUG-98762
Change-Id: Ib845361b30f21c3d78c16ced923c1678b12e05ac
Reviewed-by: JiDe Zhang <zhangjide@uniontech.com>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
(cherry picked from commit 1d961491d817490da156769ddce6fce48a0bce4a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
JiDe Zhang 2022-07-03 11:38:34 +08:00 committed by Qt Cherry-pick Bot
parent d8808604b7
commit b98b683bdc
2 changed files with 37 additions and 23 deletions

View File

@ -33,12 +33,25 @@ static_assert(bitPosition(QPalette::ColorGroup(QPalette::NColorGroups - 1),
class QPalettePrivate
{
public:
QPalettePrivate() : ref(1), ser_no(qt_palette_count++), detach_no(0) { }
class Data : public QSharedData {
public:
Data() : ser_no(qt_palette_count++) { }
QBrush br[QPalette::NColorGroups][QPalette::NColorRoles];
const int ser_no;
};
QPalettePrivate(const QExplicitlySharedDataPointer<Data> &data)
: ref(1), detach_no(0), data(data)
{ }
QPalettePrivate()
: QPalettePrivate(QExplicitlySharedDataPointer<Data>(new Data))
{ }
QAtomicInt ref;
QBrush br[QPalette::NColorGroups][QPalette::NColorRoles];
QPalette::ResolveMask resolveMask = {0};
int ser_no;
int detach_no;
QExplicitlySharedDataPointer<Data> data;
};
static QColor qt_mix_colors(QColor a, QColor b)
@ -710,7 +723,7 @@ const QBrush &QPalette::brush(ColorGroup gr, ColorRole cr) const
gr = Active;
}
}
return d->br[gr][cr];
return d->data->br[gr][cr];
}
/*!
@ -748,11 +761,18 @@ void QPalette::setBrush(ColorGroup cg, ColorRole cr, const QBrush &b)
cg = Active;
}
if (d->br[cg][cr] != b) {
const auto newResolveMask = d->resolveMask | ResolveMask(1) << bitPosition(cg, cr);
const auto valueChanged = d->data->br[cg][cr] != b;
if (valueChanged) {
detach();
d->data.detach();
d->data->br[cg][cr] = b;
} else if (d->resolveMask != newResolveMask) {
detach();
d->br[cg][cr] = b;
d->resolveMask |= ResolveMask(1) << bitPosition(cg, cr);
}
d->resolveMask = newResolveMask;
}
/*!
@ -793,11 +813,7 @@ bool QPalette::isBrushSet(ColorGroup cg, ColorRole cr) const
void QPalette::detach()
{
if (d->ref.loadRelaxed() != 1) {
QPalettePrivate *x = new QPalettePrivate;
for(int grp = 0; grp < (int)NColorGroups; grp++) {
for(int role = 0; role < (int)NColorRoles; role++)
x->br[grp][role] = d->br[grp][role];
}
QPalettePrivate *x = new QPalettePrivate(d->data);
x->resolveMask = d->resolveMask;
if (!d->ref.deref())
delete d;
@ -829,11 +845,11 @@ void QPalette::detach()
*/
bool QPalette::operator==(const QPalette &p) const
{
if (isCopyOf(p))
if (isCopyOf(p) || d->data == p.d->data)
return true;
for(int grp = 0; grp < (int)NColorGroups; grp++) {
for(int role = 0; role < (int)NColorRoles; role++) {
if (d->br[grp][role] != p.d->br[grp][role])
if (d->data->br[grp][role] != p.d->data->br[grp][role])
return false;
}
}
@ -867,7 +883,7 @@ bool QPalette::isEqual(QPalette::ColorGroup group1, QPalette::ColorGroup group2)
if (group1 == group2)
return true;
for(int role = 0; role < (int)NColorRoles; role++) {
if (d->br[group1][role] != d->br[group2][role])
if (d->data->br[group1][role] != d->data->br[group2][role])
return false;
}
return true;
@ -882,7 +898,7 @@ bool QPalette::isEqual(QPalette::ColorGroup group1, QPalette::ColorGroup group2)
*/
qint64 QPalette::cacheKey() const
{
return (((qint64) d->ser_no) << 32) | ((qint64) (d->detach_no));
return (((qint64) d->data->ser_no) << 32) | ((qint64) (d->detach_no));
}
static constexpr QPalette::ResolveMask allResolveMask()
@ -918,7 +934,8 @@ QPalette QPalette::resolve(const QPalette &other) const
for (int role = 0; role < int(NColorRoles); ++role) {
for (int grp = 0; grp < int(NColorGroups); ++grp) {
if (!(d->resolveMask & (ResolveMask(1) << bitPosition(ColorGroup(grp), ColorRole(role))))) {
palette.d->br[grp][role] = other.d->br[grp][role];
palette.d->data.detach();
palette.d->data->br[grp][role] = other.d->data->br[grp][role];
}
}
}
@ -981,7 +998,7 @@ QDataStream &operator<<(QDataStream &s, const QPalette &p)
if (s.version() == 1) {
// Qt 1.x
for (int i = 0; i < NumOldRoles; ++i)
s << p.d->br[grp][oldRoles[i]].color();
s << p.d->data->br[grp][oldRoles[i]].color();
} else {
int max = (int)QPalette::NColorRoles;
if (s.version() <= QDataStream::Qt_2_1)
@ -991,7 +1008,7 @@ QDataStream &operator<<(QDataStream &s, const QPalette &p)
else if (s.version() <= QDataStream::Qt_5_11)
max = QPalette::ToolTipText + 1;
for (int r = 0; r < max; r++)
s << p.d->br[grp][r];
s << p.d->data->br[grp][r];
}
}
return s;

View File

@ -190,9 +190,6 @@ void tst_QPalette::setBrush()
const QPalette pp = p;
QVERIFY(pp.isCopyOf(p));
// Setting the same brush won't detach
p.setBrush(QPalette::Disabled, QPalette::Button, Qt::green);
QVERIFY(pp.isCopyOf(p));
}
void tst_QPalette::isBrushSet()
@ -217,7 +214,7 @@ void tst_QPalette::isBrushSet()
QVERIFY(!p2.isBrushSet(QPalette::Active, QPalette::Dark));
p2.setBrush(QPalette::Active, QPalette::Dark, p2.brush(QPalette::Active, QPalette::Dark));
QVERIFY(!p3.isBrushSet(QPalette::Active, QPalette::Dark));
QVERIFY(!p2.isBrushSet(QPalette::Active, QPalette::Dark));
QVERIFY(p2.isBrushSet(QPalette::Active, QPalette::Dark));
}
void tst_QPalette::setAllPossibleBrushes()