D27735: [KConfigGui] Clear styleName font property for Regular font sytles
This revision was automatically updated to reflect the committed changes. Closed by commit R237:a2774ff5b419: [KConfigGui] Clear styleName font property for Regular font sytles (authored by ahmadsamir). REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27735?vs=76721=76759 REVISION DETAIL https://phabricator.kde.org/D27735 AFFECTED FILES src/gui/kconfiggroupgui.cpp To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir added a comment.
To get those numbers I tested with this code:
QFontDatabase fdb;
const QStringList fontFamilies = fdb.families();
QStringList list;
int regularCnt = 0;
int normalCnt = 0;
int bookCnt = 0;
int romanCnt = 0;
for (const QString : fontFamilies) {
const QStringList styles = fdb.styles(family);
for (const QString : styles) {
if (s == QLatin1String("Regular")) {
++regularCnt;
} else if (s == QLatin1String("Normal")) {
++normalCnt;
} else if (s == QLatin1String("Book")) {
++bookCnt;
} else if (s == QLatin1String("Roman")) {
++romanCnt;
}
}
}
qDebug() << "Regular: " << regularCnt;
qDebug() << "Normal: " << normalCnt;
qDebug() << "Book: " << bookCnt;
qDebug() << "Roman: " << romanCnt;
REPOSITORY
R237 KConfig
BRANCH
l-font-sytleName (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D27735
To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir updated this revision to Diff 76721. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Add some relevant statistics REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27735?vs=76682=76721 BRANCH l-font-sytleName (branched from master) REVISION DETAIL https://phabricator.kde.org/D27735 AFFECTED FILES src/gui/kconfiggroupgui.cpp To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir updated this revision to Diff 76682. ahmadsamir edited the summary of this revision. ahmadsamir added a comment. Tweak commit message REPOSITORY R237 KConfig CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27735?vs=76671=76682 BRANCH l-font-sytleName (branched from master) REVISION DETAIL https://phabricator.kde.org/D27735 AFFECTED FILES src/gui/kconfiggroupgui.cpp To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. Wow that Qt bug report has really extensive investigation, very impressive. The fix looks ok to me (without being an expert on the topic, it does seem to match what that bug report is saying). REPOSITORY R237 KConfig BRANCH l-font-sytleName (branched from master) REVISION DETAIL https://phabricator.kde.org/D27735 To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir added a comment. Caveat: setting the general font to e.g. "Noto Sans" and then restarting the fonts KCM; opening the general font selection dialog, "Semi condensed" is selected instead of Regular. That looks like a bug in the static method nearestExistingFont() from plasma-desktop/kcms/fonts/fonts.cpp , I am working on porting the fonts KCM to QFontDialog, and will try to fix that there. REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27735 To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir added a comment. Konsole's workaroun: https://phabricator.kde.org/R319:41693fe9ee263f8f2281852a740ee52d55f003ef KTextEditor workaround: https://phabricator.kde.org/R39:4d91fa7e918d983e6569798dfe20c7c9faf4bb9e REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27735 To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir added a comment. Some relevant info: https://bugreports.qt.io/browse/QTBUG-63792?focusedCommentId=377225=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-377225 https://codereview.qt-project.org/c/qt/qtbase/+/181645/ REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D27735 To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
D27735: [KConfigGui] Clear styleName font property for Regular font sytles
ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure, davidedmundson, cfeck, ervin. Herald added a project: Frameworks. ahmadsamir requested review of this revision. REVISION SUMMARY If the styleName property is set for a QFont, using setBold(true) would lead to Qt using an "emboldended"/synthetic font style instead of using the bold style provided by the font itself (usually as a standalone font file), the former looks ugly (IIUC, Freetype emboldens fonts as a last resort for fonts that don't provide a bold style at all). Accoring to upstream[1] the styleName property is useful for fonts with fancy style names, and also it shouldn't be set if it's not needed; and indeed using styleName with e.g. "Regular" doesn't make sense, as there is no "Regular Bold" style AFAICS. Checking for "Regular|Normal|Book|Roman" is based on examining the font styles provided by the font packages available on OpenSuse Tumbleweed ATM, and not checking for some of the weird/non-common ones e.g. I've seen "Roma" and "Rounded". For more details see: [1] https://bugreports.qt.io/browse/QTBUG-63792 https://bugs.kde.org/show_bug.cgi?id=378523 BUG: 378523 FIXED-IN: 5.68 TEST PLAN All unit tests still pass. Changing the fonts via e.g. the fonts KCM doesn't append the font sytleName, to the relevant font config entry, if the "Regular" style or co. is used. A simple test, look at the current dir name in the Dolphin url bar with and without ",Regular" appended to the font= entry (assuming you're using Noto Sans or DejaVu Sans as the styleName varies from font to font). REPOSITORY R237 KConfig BRANCH l-font-sytleName (branched from master) REVISION DETAIL https://phabricator.kde.org/D27735 AFFECTED FILES src/gui/kconfiggroupgui.cpp To: ahmadsamir, #frameworks, dfaure, davidedmundson, cfeck, ervin Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns
