broulik added a comment.
Thanks a lot for your patch! This makes the effect of those settings a lot easier to understand. I have a couple of nitpicks below. INLINE COMMENTS > main.qml:22 > import QtQuick.Layouts 1.1 > -import QtQuick.Controls 2.0 as QtControls > +import QtQuick.Controls 2.3 as QtControls > import QtQuick.Dialogs 1.2 as QtDialogs Is `2.2` also sufficient (Qt 5.9)? > main.qml:143 > + id: subPixelDelegate > + width: subPixelComboImage.implicitWidth > + contentItem: ColumnLayout { The popup width is too small, the text cut off. Either make it larger or at least elide the text on the right > main.qml:147 > + width: subPixelComboImage.implicitWidth > + Text { > + id: subPixelComboText Use QtQuick Controls `Label` instead of plain `Text` for proper rendering and text color > main.qml:153 > + id: subPixelComboImage > + fillMode: Image.Pad > + sourceSize: undefined This is the default, no need to explicitly specify > main.qml:154 > + fillMode: Image.Pad > + sourceSize: undefined > + source: > "image://preview/"+model.index+"_"+kcm.fontAASettings.hintingCurrentIndex+".png" Also not needed > main.qml:155 > + sourceSize: undefined > + source: > "image://preview/"+model.index+"_"+kcm.fontAASettings.hintingCurrentIndex+".png" > + } Coding style, add spaces between: source: "image://preview/" + model.index + "_" + kcm.fontAASettings.hintCurrentIndex + ".png" > previewimageprovider.cpp:35 > +{ > + int subPixelIndex = id.split('_')[0].toInt(); > + int hintingIndex = id.split('_')[1].toInt(); avoid splitting twice, I suggest storing const auto sections = id.splitRef(QLatin1Char('_')); Also do a bounds check > previewimageprovider.cpp:66 > + PreviewRenderEngine eng(true); > + QImage img = eng.drawAutoSize(m_font, text, bgnd, > eng.getDefaultPreviewString()); > + Your image does not take into account `devicePixelRatio` which makes it blurred on my high dpi screen. I'm not sure how to exactly fix that, perhaps `@2x` works for the image provider, or you can manually implement this, to test run QT_SCREEN_SCALE_FACTORS=2 kcmshell5 kcm_fonts > previewimageprovider.h:1 > +#ifndef __PREVIEW_IMAGE_PROVIDER_H__ > +#define __PREVIEW_IMAGE_PROVIDER_H__ Include guard typically goes below copyright, you can also use `#pragma once` in plasma-desktop REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D11064 To: progwolff, #plasma, harmathy, mart Cc: broulik, plasma-devel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart