Skip to content

qt: Handle fonts of deleted widgets properly, streamline the flow in GUIUtil::updateFonts #3772

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UdjinM6 merged 8 commits into from
Oct 28, 2020
Merged

qt: Handle fonts of deleted widgets properly, streamline the flow in GUIUtil::updateFonts #3772

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7491a56
Refactor and fix `GUIUtil::updateFonts`
UdjinM6 Oct 13, 2020
f0eba2f
Add some debug output
UdjinM6 Oct 13, 2020
491fe7d
qt: Rename mapNormalFontUpdates -> mapFontUpdates
xdustinface Oct 9, 2020
5c5c2d2
qt: Count removed items, adjust debug logs
xdustinface Oct 21, 2020
f09d911
qt: Use the emplace result for the default font size
xdustinface Oct 21, 2020
634ccc8
qt: Perform all widget font updates later in a seperate step
xdustinface Oct 22, 2020
c6bf0d3
qt: Drop pointSize checks
xdustinface Oct 22, 2020
d06ea03
qt: Refactor app class font scaling
xdustinface Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 69 additions & 69 deletions src/qt/guiutil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include <QFontDatabase>
#include <QKeyEvent>
#include <QLineEdit>
#include <QPointer>
#include <QSettings>
#include <QTextDocument> // for Qt::mightBeRichText
#include <QThread>
Expand Down Expand Up @@ -133,7 +134,7 @@ static QFont::Weight fontWeightNormal = defaultFontWeightNormal;
static QFont::Weight fontWeightBold = defaultFontWeightBold;

// Contains all widgets and its font attributes (weight, italic, size) with font changes due to GUIUtil::setFont
static std::map<QWidget*, std::tuple<FontWeight, bool, int>> mapNormalFontUpdates;
static std::map<QPointer<QWidget>, std::tuple<FontWeight, bool, int>> mapFontUpdates;
// Contains a list of supported font weights for all members of GUIUtil::FontFamily
static std::map<FontFamily, std::vector<QFont::Weight>> mapSupportedWeights;

Expand Down Expand Up @@ -1529,7 +1530,7 @@ void setFont(const std::vector<QWidget*>& vecWidgets, FontWeight weight, int nPo
{
for (auto it : vecWidgets) {
auto fontAttributes = std::make_tuple(weight, fItalic, nPointSize);
auto itFontUpdate = mapNormalFontUpdates.emplace(std::make_pair(it, fontAttributes));
auto itFontUpdate = mapFontUpdates.emplace(std::make_pair(it, fontAttributes));
if (!itFontUpdate.second) {
itFontUpdate.first->second = fontAttributes;
}
Expand All @@ -1543,10 +1544,36 @@ void updateFonts()
return;
}

static std::map<QWidget*, int> mapWidgetDefaultFontSizes;
static std::map<QString, int> mapClassDefaultFontSizes;
std::map<QWidget*, std::pair<QFont, bool>> mapWidgetFonts;
static std::map<QPointer<QWidget>, int> mapWidgetDefaultFontSizes;

// QPointer becomes nullptr for objects that were deleted.
// Remove them from mapDefaultFontSize and mapFontUpdates
// before proceeding any further.
size_t nRemovedDefaultFonts{0};
auto itd = mapWidgetDefaultFontSizes.begin();
while (itd != mapWidgetDefaultFontSizes.end()) {
if (itd->first.isNull()) {
itd = mapWidgetDefaultFontSizes.erase(itd);
++nRemovedDefaultFonts;
} else {
++itd;
}
}

size_t nRemovedFontUpdates{0};
auto itn = mapFontUpdates.begin();
while (itn != mapFontUpdates.end()) {
if (itn->first.isNull()) {
itn = mapFontUpdates.erase(itn);
++nRemovedFontUpdates;
} else {
++itn;
}
}

size_t nUpdatable{0}, nUpdated{0};
std::map<QWidget*, QFont> mapWidgetFonts;
// Loop through all widgets
for (QWidget* w : qApp->allWidgets()) {
std::vector<QString> vecIgnore{
"QWidget", "QDialog", "QFrame", "QStackedWidget", "QDesktopWidget", "QDesktopScreenWidget",
Expand All @@ -1556,86 +1583,59 @@ void updateFonts()
if (std::find(vecIgnore.begin(), vecIgnore.end(), w->metaObject()->className()) != vecIgnore.end()) {
continue;
}
++nUpdatable;

QFont font = w->font();
assert(font.pointSize() > 0);
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't love using asserts... If they do catch they provide basically no info to the user for us to debug with...

Copy link
Copy Markdown
Author

@UdjinM6 UdjinM6 Oct 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>0 means that it's either a default font size (set via setApplicationFont) or some another valid value and font size was not set via setPixelSize. IMO it's safe to use it here, should only crash if we break smth in code (e.g. use setPixelSize).

font.setFamily(qApp->font().family());
font.setWeight(getFontWeightNormal());
font.setStyleName(qApp->font().styleName());
font.setStyle(qApp->font().style());
// Set the font size based on the widgets default font size + the font scale
bool fAdded = false;
if (!mapWidgetDefaultFontSizes.count(w)) {
mapWidgetDefaultFontSizes.emplace(std::make_pair(w, font.pointSize() > 0 ? font.pointSize() : defaultFontSize));
fAdded = true;
}
font.setPointSizeF(getScaledFontSize(mapWidgetDefaultFontSizes[w]));
bool fUpdateRequired = fAdded || (mapNormalFontUpdates.find(w) == mapNormalFontUpdates.end() && font != w->font());
mapWidgetFonts.emplace(w, std::make_pair(font, fUpdateRequired));
}

auto itn = mapNormalFontUpdates.begin();
while (itn != mapNormalFontUpdates.end()) {
auto itw = mapWidgetFonts.find(itn->first);
if (itw != mapWidgetFonts.end()) {
int nSize = std::get<2>(itn->second);
// Insert/Get the default font size of the widget
auto itDefault = mapWidgetDefaultFontSizes.emplace(w, font.pointSize());

auto it = mapFontUpdates.find(w);
if (it != mapFontUpdates.end()) {
int nSize = std::get<2>(it->second);
if (nSize == -1) {
nSize = mapWidgetDefaultFontSizes[itn->first];
}
QFont&& font = getFont(std::get<0>(itn->second), std::get<1>(itn->second), nSize);
if (itn->first->font() != font) {
itw->second.first = font;
itw->second.second = true;
nSize = itDefault.first->second;
}
++itn;
font = getFont(std::get<0>(it->second), std::get<1>(it->second), nSize);
} else {
itn = mapNormalFontUpdates.erase(itn);
font.setPointSizeF(getScaledFontSize(itDefault.first->second));
}
}

for (auto it : mapWidgetFonts) {
if (it.second.second) {
it.first->setFont(it.second.first);
if (w->font() != font) {
auto itWidgetFont = mapWidgetFonts.emplace(w, font);
assert(itWidgetFont.second);
++nUpdated;
}
}
qDebug().nospace() << __func__ << " - widget counts: updated/updatable/total(" << nUpdated << "/" << nUpdatable << "/" << qApp->allWidgets().size() << ")"
<< ", removed items: mapWidgetDefaultFontSizes/mapFontUpdates(" << nRemovedDefaultFonts << "/" << nRemovedFontUpdates << ")";

// Cleanup mapDefaultFontSize to remove deleted widgets
auto itd = mapWidgetDefaultFontSizes.begin();
while (itd != mapWidgetDefaultFontSizes.end()) {
if (qApp->allWidgets().contains(itd->first)) {
++itd;
} else {
itd = mapWidgetDefaultFontSizes.erase(itd);
}
// Perform the required font updates
// NOTE: This is done as seperate step to avoid scaling issues due to font inheritance
// hence all fonts are calculated and stored in mapWidgetFonts above.
for (auto it : mapWidgetFonts) {
it.first->setFont(it.second);
}

// Scale the global font for QToolTip labels, QMenu and QMessageBox instances
QFont fontToolTip = qApp->font("QTipLabel");
QFont fontMenu = qApp->font("QMenu");
QFont fontMessageBox = qApp->font("QMessageBox");
// Store their default font sizes before ever applying any scale to it
if (!mapClassDefaultFontSizes.count("QTipLabel")) {
mapClassDefaultFontSizes.emplace("QTipLabel", fontToolTip.pointSize());
}
if (!mapClassDefaultFontSizes.count("QMenu")) {
mapClassDefaultFontSizes.emplace("QMenu", fontMenu.pointSize());
}
if (!mapClassDefaultFontSizes.count("QMessageBox")) {
mapClassDefaultFontSizes.emplace("QMessageBox", fontMessageBox.pointSize());
}
// And give them the proper scaled size based on their default sizes if required
double dSize = getScaledFontSize(mapClassDefaultFontSizes["QTipLabel"]);
if (fontToolTip.pointSizeF() != dSize) {
fontToolTip.setPointSizeF(dSize);
qApp->setFont(fontToolTip, "QTipLabel");
}
dSize = getScaledFontSize(mapClassDefaultFontSizes["QMenu"]);
if (fontMenu.pointSizeF() != dSize) {
fontMenu.setPointSizeF(dSize);
qApp->setFont(fontMenu, "QMenu");
}
dSize = getScaledFontSize(getScaledFontSize(mapClassDefaultFontSizes["QMessageBox"]));
if (fontMessageBox.pointSizeF() != dSize) {
fontMessageBox.setPointSizeF(dSize);
qApp->setFont(fontMessageBox, "QMessageBox");
// Scale the global font size for the classes in the map below
static std::map<std::string, int> mapClassFontUpdates{
{"QTipLabel", -1}, {"QMenu", -1}, {"QMessageBox", -1}
};
for (auto& it : mapClassFontUpdates) {
QFont fontClass = qApp->font(it.first.c_str());
if (it.second == -1) {
it.second = fontClass.pointSize();
}
double dSize = getScaledFontSize(it.second);
if (fontClass.pointSizeF() != dSize) {
fontClass.setPointSizeF(dSize);
qApp->setFont(fontClass, it.first.c_str());
}
}
}

Expand Down