Fix QPalette::isBrushSet

The previous implementation did not take into account different color
groups in resolve mask. It led to some issues when resolving a
palette or checking whether a brush is set or not.

Task-number: QTBUG-78544
Change-Id: I9b67b2c444eb62c022643022a874dc400005e6ee
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
This commit is contained in:
Vitaly Fanaskov 2019-11-22 14:26:12 +01:00
parent deddafe0a6
commit 045250ed42
6 changed files with 150 additions and 44 deletions

View File

@ -48,6 +48,22 @@ QT_BEGIN_NAMESPACE
static int qt_palette_count = 1;
static constexpr QPalette::ResolveMask colorRoleOffset(QPalette::ColorGroup colorGroup)
{
return QPalette::NColorRoles * colorGroup;
}
static constexpr QPalette::ResolveMask bitPosition(QPalette::ColorGroup colorGroup,
QPalette::ColorRole colorRole)
{
return colorRole + colorRoleOffset(colorGroup);
}
Q_STATIC_ASSERT_X(bitPosition(QPalette::ColorGroup(QPalette::NColorGroups - 1),
QPalette::ColorRole(QPalette::NColorRoles - 1))
< sizeof(QPalette::ResolveMask) * CHAR_BIT,
"The resolve mask type is not wide enough to fit the entire bit mask.");
class QPalettePrivate {
public:
QPalettePrivate() : ref(1), ser_no(qt_palette_count++), detach_no(0) { }
@ -535,8 +551,6 @@ static void qt_palette_from_color(QPalette &pal, const QColor &button)
QPalette::QPalette()
: d(nullptr)
{
data.current_group = Active;
data.resolve_mask = 0;
// Initialize to application palette if present, else default to black.
// This makes it possible to instantiate QPalette outside QGuiApplication,
// for example in the platform plugins.
@ -546,7 +560,7 @@ QPalette::QPalette()
} else {
init();
qt_palette_from_color(*this, Qt::black);
data.resolve_mask = 0;
data.resolveMask = 0;
}
}
@ -678,8 +692,6 @@ QPalette::~QPalette()
/*!\internal*/
void QPalette::init() {
d = new QPalettePrivate;
data.resolve_mask = 0;
data.current_group = Active; //as a default..
}
/*!
@ -736,7 +748,7 @@ const QBrush &QPalette::brush(ColorGroup gr, ColorRole cr) const
Q_ASSERT(cr < NColorRoles);
if(gr >= (int)NColorGroups) {
if(gr == Current) {
gr = (ColorGroup)data.current_group;
gr = data.currentGroup;
} else {
qWarning("QPalette::brush: Unknown ColorGroup: %d", (int)gr);
gr = Active;
@ -774,7 +786,7 @@ void QPalette::setBrush(ColorGroup cg, ColorRole cr, const QBrush &b)
}
if (cg == Current) {
cg = ColorGroup(data.current_group);
cg = data.currentGroup;
} else if (cg >= NColorGroups) {
qWarning("QPalette::setBrush: Unknown ColorGroup: %d", cg);
cg = Active;
@ -784,7 +796,8 @@ void QPalette::setBrush(ColorGroup cg, ColorRole cr, const QBrush &b)
detach();
d->br[cg][cr] = b;
}
data.resolve_mask |= (1<<cr);
data.resolveMask |= ResolveMask(1) << bitPosition(cg, cr);
}
/*!
@ -793,12 +806,30 @@ void QPalette::setBrush(ColorGroup cg, ColorRole cr, const QBrush &b)
Returns \c true if the ColorGroup \a cg and ColorRole \a cr has been
set previously on this palette; otherwise returns \c false.
\sa setBrush()
The ColorGroup \a cg should be less than QPalette::NColorGroups,
but you can use QPalette::Current. In this case, the previously
set current color group will be used.
The ColorRole \a cr should be less than QPalette::NColorRoles.
\sa setBrush(), currentColorGroup()
*/
bool QPalette::isBrushSet(ColorGroup cg, ColorRole cr) const
{
Q_UNUSED(cg);
return (data.resolve_mask & (1<<cr));
if (cg == Current)
cg = data.currentGroup;
if (cg >= NColorGroups) {
qWarning() << "Wrong color group:" << cg;
return false;
}
if (cr >= NColorRoles) {
qWarning() << "Wrong color role:" << cr;
return false;
}
return data.resolveMask & (ResolveMask(1) << bitPosition(cg, cr));
}
/*!
@ -863,7 +894,7 @@ bool QPalette::isEqual(QPalette::ColorGroup group1, QPalette::ColorGroup group2)
{
if(group1 >= (int)NColorGroups) {
if(group1 == Current) {
group1 = (ColorGroup)data.current_group;
group1 = data.currentGroup;
} else {
qWarning("QPalette::brush: Unknown ColorGroup(1): %d", (int)group1);
group1 = Active;
@ -871,7 +902,7 @@ bool QPalette::isEqual(QPalette::ColorGroup group1, QPalette::ColorGroup group2)
}
if(group2 >= (int)NColorGroups) {
if(group2 == Current) {
group2 = (ColorGroup)data.current_group;
group2 = data.currentGroup;
} else {
qWarning("QPalette::brush: Unknown ColorGroup(2): %d", (int)group2);
group2 = Active;
@ -922,21 +953,25 @@ qint64 QPalette::cacheKey() const
*/
QPalette QPalette::resolve(const QPalette &other) const
{
if ((*this == other && data.resolve_mask == other.data.resolve_mask)
|| data.resolve_mask == 0) {
if ((*this == other && data.resolveMask == other.data.resolveMask)
|| data.resolveMask == 0) {
QPalette o = other;
o.data.resolve_mask = data.resolve_mask;
o.data.resolveMask = data.resolveMask;
return o;
}
QPalette palette(*this);
palette.detach();
for(int role = 0; role < (int)NColorRoles; role++)
if (!(data.resolve_mask & (1<<role)))
for(int grp = 0; grp < (int)NColorGroups; grp++)
for (int role = 0; role < int(NColorRoles); ++role) {
for (int grp = 0; grp < int(NColorGroups); ++grp) {
if (!(data.resolveMask & (ResolveMask(1) << bitPosition(ColorGroup(grp), ColorRole(role))))) {
palette.d->br[grp][role] = other.d->br[grp][role];
palette.data.resolve_mask |= other.data.resolve_mask;
}
}
}
palette.data.resolveMask |= other.data.resolveMask;
return palette;
}
@ -947,7 +982,12 @@ QPalette QPalette::resolve(const QPalette &other) const
*/
/*!
\fn void QPalette::resolve(uint mask)
\typedef ResolveMaskType
\internal
*/
/*!
\fn void QPalette::resolve(ResolveMaskType mask)
\internal
*/
@ -1081,10 +1121,15 @@ void QPalette::setColorGroup(ColorGroup cg, const QBrush &windowText, const QBru
QBrush(Qt::blue), QBrush(Qt::magenta), QBrush(toolTipBase),
QBrush(toolTipText));
data.resolve_mask &= ~(1 << Highlight);
data.resolve_mask &= ~(1 << HighlightedText);
data.resolve_mask &= ~(1 << LinkVisited);
data.resolve_mask &= ~(1 << Link);
for (int cr = Highlight; cr <= LinkVisited; ++cr) {
if (cg == All) {
for (int group = Active; group < NColorGroups; ++group) {
data.resolveMask &= ~(ResolveMask(1) << bitPosition(ColorGroup(group), ColorRole(cr)));
}
} else {
data.resolveMask &= ~(ResolveMask(1) << bitPosition(ColorGroup(cg), ColorRole(cr)));
}
}
}
@ -1189,7 +1234,7 @@ QDebug operator<<(QDebug dbg, const QPalette &p)
"ToolTipBase","ToolTipText", "PlaceholderText" };
QDebugStateSaver saver(dbg);
QDebug nospace = dbg.nospace();
const uint mask = p.resolve();
auto mask = p.resolve();
nospace << "QPalette(resolve=" << Qt::hex << Qt::showbase << mask << ',';
for (int role = 0; role < (int)QPalette::NColorRoles; ++role) {
if (mask & (1<<role)) {

View File

@ -72,14 +72,12 @@ public:
{ other.d = nullptr; }
inline QPalette &operator=(QPalette &&other) noexcept
{
for_faster_swapping_dont_use = other.for_faster_swapping_dont_use;
qSwap(d, other.d); return *this;
}
void swap(QPalette &other) noexcept
{
qSwap(d, other.d);
qSwap(for_faster_swapping_dont_use, other.for_faster_swapping_dont_use);
}
operator QVariant() const;
@ -103,8 +101,8 @@ public:
};
Q_ENUM(ColorRole)
inline ColorGroup currentColorGroup() const { return static_cast<ColorGroup>(data.current_group); }
inline void setCurrentColorGroup(ColorGroup cg) { data.current_group = cg; }
inline ColorGroup currentColorGroup() const { return data.currentGroup; }
inline void setCurrentColorGroup(ColorGroup cg) { data.currentGroup = cg; }
inline const QColor &color(ColorGroup cg, ColorRole cr) const
{ return brush(cg, cr).color(); }
@ -158,9 +156,11 @@ public:
#endif
qint64 cacheKey() const;
QPalette resolve(const QPalette &) const;
inline uint resolve() const { return data.resolve_mask; }
inline void resolve(uint mask) { data.resolve_mask = mask; }
QPalette resolve(const QPalette &other) const;
using ResolveMask = quint64;
inline ResolveMask resolve() const { return data.resolveMask; }
inline void resolve(ResolveMask mask) { data.resolveMask = mask; }
private:
void setColorGroup(ColorGroup cr, const QBrush &windowText, const QBrush &button,
@ -185,13 +185,11 @@ private:
QPalettePrivate *d;
struct Data {
uint current_group : 4;
uint resolve_mask : 28;
};
union {
Data data;
quint32 for_faster_swapping_dont_use;
ResolveMask resolveMask{0};
ColorGroup currentGroup{Active};
};
Data data;
friend Q_GUI_EXPORT QDataStream &operator<<(QDataStream &s, const QPalette &p);
};

View File

@ -1850,7 +1850,7 @@ void QWidgetPrivate::propagatePaletteChange()
if (q->isWindow() && !q->testAttribute(Qt::WA_WindowPropagation)) {
inheritedPaletteResolveMask = 0;
}
int mask = data.pal.resolve() | inheritedPaletteResolveMask;
QPalette::ResolveMask mask = data.pal.resolve() | inheritedPaletteResolveMask;
const bool useStyleSheetPropagationInWidgetStyles =
QCoreApplication::testAttribute(Qt::AA_UseStyleSheetPropagationInWidgetStyles);
@ -4374,7 +4374,7 @@ void QWidget::setPalette(const QPalette &palette)
widget's palette are implicitly imposed on this widget by the user). Note
that this font does not take into account the palette set on \a w itself.
*/
QPalette QWidgetPrivate::naturalWidgetPalette(uint inheritedMask) const
QPalette QWidgetPrivate::naturalWidgetPalette(QPalette::ResolveMask inheritedMask) const
{
Q_Q(const QWidget);

View File

@ -299,7 +299,7 @@ public:
void setPalette_helper(const QPalette &);
void resolvePalette();
QPalette naturalWidgetPalette(uint inheritedMask) const;
QPalette naturalWidgetPalette(QPalette::ResolveMask inheritedMask) const;
void setMask_sys(const QRegion &);
@ -672,7 +672,7 @@ public:
// Other variables.
uint directFontResolveMask;
uint inheritedFontResolveMask;
uint inheritedPaletteResolveMask;
QPalette::ResolveMask inheritedPaletteResolveMask;
short leftmargin;
short topmargin;
short rightmargin;

View File

@ -194,7 +194,7 @@ public:
template <typename T>
struct Tampered {
T oldWidgetValue;
uint resolveMask;
decltype(std::declval<T>().resolve()) resolveMask;
// only call this function on an rvalue *this (it mangles oldWidgetValue)
T reverted(T current)

View File

@ -41,6 +41,12 @@ private Q_SLOTS:
void copySemantics();
void moveSemantics();
void setBrush();
void isBrushSet();
void setAllPossibleBrushes();
void noBrushesSetForDefaultPalette();
void cannotCheckIfInvalidBrushSet();
void checkIfBrushForCurrentGroupSet();
};
void tst_QPalette::roleValues_data()
@ -194,5 +200,62 @@ void tst_QPalette::setBrush()
QVERIFY(pp.isCopyOf(p));
}
void tst_QPalette::isBrushSet()
{
QPalette p;
// Set only one color group
p.setBrush(QPalette::Active, QPalette::Mid, QBrush(Qt::red));
QVERIFY(p.isBrushSet(QPalette::Active, QPalette::Mid));
QVERIFY(!p.isBrushSet(QPalette::Inactive, QPalette::Mid));
QVERIFY(!p.isBrushSet(QPalette::Disabled, QPalette::Mid));
// Set all color groups
p.setBrush(QPalette::LinkVisited, QBrush(Qt::green));
QVERIFY(p.isBrushSet(QPalette::Active, QPalette::LinkVisited));
QVERIFY(p.isBrushSet(QPalette::Inactive, QPalette::LinkVisited));
QVERIFY(p.isBrushSet(QPalette::Disabled, QPalette::LinkVisited));
}
void tst_QPalette::setAllPossibleBrushes()
{
QPalette p;
QCOMPARE(p.resolve(), QPalette::ResolveMask(0));
for (int r = 0; r < QPalette::NColorRoles; ++r) {
p.setBrush(QPalette::All, QPalette::ColorRole(r), Qt::red);
}
for (int r = 0; r < QPalette::NColorRoles; ++r) {
for (int g = 0; g < QPalette::NColorGroups; ++g) {
QVERIFY(p.isBrushSet(QPalette::ColorGroup(g), QPalette::ColorRole(r)));
}
}
}
void tst_QPalette::noBrushesSetForDefaultPalette()
{
QCOMPARE(QPalette().resolve(), QPalette::ResolveMask(0));
}
void tst_QPalette::cannotCheckIfInvalidBrushSet()
{
QPalette p(Qt::red);
QVERIFY(!p.isBrushSet(QPalette::All, QPalette::LinkVisited));
QVERIFY(!p.isBrushSet(QPalette::Active, QPalette::NColorRoles));
}
void tst_QPalette::checkIfBrushForCurrentGroupSet()
{
QPalette p;
p.setCurrentColorGroup(QPalette::Disabled);
p.setBrush(QPalette::Current, QPalette::Link, QBrush(Qt::yellow));
QVERIFY(p.isBrushSet(QPalette::Current, QPalette::Link));
}
QTEST_MAIN(tst_QPalette)
#include "tst_qpalette.moc"