|
|
Ok, I have an update.
Short version:
============
I rewrote some parts in the icon manager to support icon rendering in different sizes. It is tested on a Mac with retina display (2x) and on a full HD screen (1x).
Long version:
===========
Scribus has some icon size definitions in some UI widgets (scrAction for toolbar icons, prefDialog, etc.).
An icon cache is created once when the program starts. This contains all icons in their original size (e.g. 16 pixels). However, the toolbar expects 20 pixels and the settings dialog expects 32 pixels. As a result, some icons were simply displayed too small. With the current implementation, the rendered pixel graphics could only be scaled up afterwards, which led to blurring.
I implemented a new lookup table with icon meta information such as ID, color and file path. This is created instead of the cache when the program starts. When a widget requests an icon from the icon manager, it will be rendered at a specific size upon request. This is then stored in the cache. This allows the same SVG file with the same ID and color specifications to be rendered in different sizes. Now it works almost like old icon manager implementation before I implemented SVG support. Back to the roots!
fixes_2023-11-16_02.patch.txt (18,484 bytes)
Index: scribus/iconmanager.cpp
===================================================================
--- scribus/iconmanager.cpp (Revision 25832)
+++ scribus/iconmanager.cpp (Arbeitskopie)
@@ -28,7 +28,6 @@
#include "api/api_application.h"
#include "iconmanager.h"
-#include "prefsmanager.h"
#include "scpaths.h"
IconManager* IconManager::m_instance = nullptr;
@@ -38,6 +37,7 @@
{
m_splashScreenRect = QRect();
m_splashScreen = QPixmap();
+
}
IconManager& IconManager::instance()
@@ -46,10 +46,11 @@
return m_instance;
}
-bool IconManager::setup(qreal devicePixelRatio)
+bool IconManager::setup()
{
- m_devicePixelRatio = devicePixelRatio;
+ m_devicePixelRatio = qApp->devicePixelRatio();
+
if (!initIconSets())
{
qWarning()<<"No icons found :(";
@@ -56,7 +57,7 @@
return false;
}
- if (!createCache())
+ if (!createLookupTable())
{
qWarning()<<"Can't load icons from iconset.";
return false;
@@ -77,26 +78,34 @@
QPixmap IconManager::loadPixmap(const QString &name, int width)
{
- if (m_pxCache.contains(name))
+ QString cName = buildName(name, "icon_", QString::number(width));
+
+ // Use icon from icon cache
+ if (m_pxCache.contains(cName))
+ return *m_pxCache[cName];
+
+ // Check in item cache
+ if (m_lookupTable.contains(name))
{
- QPixmap pix = *m_pxCache[name];
- if (width > -1)
- pix = pix.scaledToWidth(width * m_devicePixelRatio, Qt::SmoothTransformation);
- return pix;
+ Item item = m_lookupTable[name];
+ m_pxCache.insert(cName, pixmapFromFile(item.filePath, item.color, width));
+ return *m_pxCache[cName];
}
- else
- {
- qWarning() << "IconManager: No icon with name '" + name + "' in icon cache.";
- return QPixmap();
- }
+
+ // Show error only if icon file doesn't exist!
+ qWarning() << QString("IconManager: No icon with name '%1' found in %2").arg(name, m_activeSetBasename) ;
+
+ return QPixmap();
}
-void IconManager::addIcon(const QString &name, QPainterPath path)
+void IconManager::addIconFromPainterPath(const QString &name, QPainterPath path)
{
- if (!m_pxCache.contains(name))
+ QString bn = buildName(name, "path_");
+
+ if (!m_pxCache.contains(bn))
{
- m_iconPaths.insert(name, &path);
- m_pxCache.insert(name, renderPath(path));
+ m_iconPaths.insert(bn, &path);
+ m_pxCache.insert(bn, pixmapFromPainterPath(path));
}
else
{
@@ -114,72 +123,77 @@
return (baseColor().lightness() >= 128) ? true : false;
}
-void IconManager::clearCache()
+void IconManager::rebuildCache()
{
m_pxCache.clear();
-}
-void IconManager::rebuildCache()
-{
- clearCache();
-
// add icons from icon set
- if (!createCache())
+ if (!createLookupTable())
{
qWarning()<<"IconManager: Can't load icons from iconset.";
}
// add icons from path
- renderIcons();
+ insertPathIconsToCache();
}
-bool IconManager::createCache()
+bool IconManager::createLookupTable()
{
- QString iconSubdir(m_iconSets[m_activeSetBasename].path);
- QString iconSetPath(QString("%1%2%3").arg(ScPaths::instance().iconDir(), iconSubdir, ".xml"));
- QDomDocument document;
+ QString iconSubdir(m_iconSets[m_activeSetBasename].path);
+ QString iconSetPath(QString("%1%2%3").arg(ScPaths::instance().iconDir(), iconSubdir, ".xml"));
+ QDomDocument document;
- if (!readXMLFile(iconSetPath, document, "xml"))
- return false;
+ if (!readXMLFile(iconSetPath, document, "xml"))
+ return false;
- QDomElement documentElement = document.documentElement();
- QDomNodeList elements = documentElement.elementsByTagName( tagIcon );
+ m_lookupTable.clear();
- for (int i = 0; i < elements.length(); i++)
- {
- QDomElement icon = elements.at(i).toElement();
+ QDomElement documentElement = document.documentElement();
+ QDomNodeList elements = documentElement.elementsByTagName( tagIcon );
- QString iconPath = QString("%1%2%3").arg(ScPaths::instance().iconDir(), iconSubdir, "/" + icon.attribute("file"));
- QString iconName = icon.attribute("id");
- QColor iconColor = baseColor();
+ for (int i = 0; i < elements.length(); i++)
+ {
+ QDomElement icon = elements.at(i).toElement();
- if (m_pxCache.contains(iconName))
- continue;
+ QString iconPath = QString("%1%2%3").arg(ScPaths::instance().iconDir(), iconSubdir, "/" + icon.attribute("file"));
+ QString iconName = icon.attribute("id");
+ QColor iconColor = baseColor();
- // if defined, override icon base color with color from iconset
- if (iconsForDarkMode() && icon.hasAttribute(colorOnDark))
- {
- iconColor = parseColor(icon.attribute(colorOnDark));
+ if (m_lookupTable.contains(iconName))
+ continue;
+
+ // if defined, override icon base color with color from iconset
+ if (iconsForDarkMode() && icon.hasAttribute(colorOnDark))
+ {
+ iconColor = parseColor(icon.attribute(colorOnDark));
+ }
+ else if (!iconsForDarkMode() && icon.hasAttribute(colorOnLight))
+ {
+ iconColor = parseColor(icon.attribute(colorOnLight));
+ }
+
+ Item item;
+ item.name = iconName;
+ item.filePath = iconPath;
+ item.color = iconColor;
+
+ m_lookupTable.insert(iconName, item);
}
- else if (!iconsForDarkMode() && icon.hasAttribute(colorOnLight))
- {
- iconColor = parseColor(icon.attribute(colorOnLight));
- }
- m_pxCache.insert(iconName, initPixmap(iconPath, iconColor));
- }
-
- return true;
+ return true;
}
-void IconManager::renderIcons()
+void IconManager::insertPathIconsToCache()
{
QMapIterator<QString, QPainterPath*> i(m_iconPaths);
while (i.hasNext())
{
i.next();
- if (!m_pxCache.contains(i.key()))
- m_pxCache.insert(i.key(), renderPath(*i.value()));
+
+ QString bn = buildName(i.key(), "path_");
+
+ if (!m_pxCache.contains(bn))
+ m_pxCache.insert(bn, pixmapFromPainterPath(*i.value()));
}
}
@@ -206,7 +220,7 @@
{
QString styleImport = nodeStyle.childNodes().at(0).toText().data();
- QRegularExpression rx_css("(.*?@import.*?')(.*?)('.*)");
+ static QRegularExpression rx_css("(.*?@import.*?')(.*?)('.*)");
QRegularExpressionMatch mImport = rx_css.match(styleImport);
// Check if stylesheet import is available
@@ -268,12 +282,12 @@
}
// Remove C-style comments /* */ or // from stylesheet
- QRegularExpression rx_comments("(\\/\\*[\\s\\S]*?(.*?)\\*\\/)|(\\/\\/.*)");
+ static QRegularExpression rx_comments("(\\/\\*[\\s\\S]*?(.*?)\\*\\/)|(\\/\\/.*)");
styleString.remove(rx_comments);
// Group 1: tags, like ".onDark .success"
// Group 2: strings within {}
- QRegularExpression rx_Settings("(.*?)\\s*{([^\\}]+)}");
+ static QRegularExpression rx_Settings("(.*?)\\s*{([^\\}]+)}");
QRegularExpressionMatchIterator i = rx_Settings.globalMatch(styleString);
while (i.hasNext())
@@ -287,7 +301,7 @@
{
if (str.startsWith("rgba"))
{
- QRegularExpression rx_rgba("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_rgba("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mRGBA = rx_rgba.match(str);
if (mRGBA.hasMatch())
@@ -302,7 +316,7 @@
}
else if (str.startsWith("rgb"))
{
- QRegularExpression rx_rgb("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_rgb("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mRGB = rx_rgb.match(str);
if (mRGB.hasMatch())
@@ -316,7 +330,7 @@
}
else if (str.startsWith("hsva"))
{
- QRegularExpression rx_hsva("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_hsva("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mHSVA = rx_hsva.match(str);
if (mHSVA.hasMatch())
@@ -331,7 +345,7 @@
}
else if (str.startsWith("hsv"))
{
- QRegularExpression rx_hsv("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_hsv("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mHSV = rx_hsv.match(str);
if (mHSV.hasMatch())
@@ -345,7 +359,7 @@
}
else if (str.startsWith("hsla"))
{
- QRegularExpression rx_hsla("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_hsla("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mHSLA = rx_hsla.match(str);
if (mHSLA.hasMatch())
@@ -360,7 +374,7 @@
}
else if (str.startsWith("hsl"))
{
- QRegularExpression rx_hsl("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
+ static QRegularExpression rx_hsl("\\(\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*,\\s*(\\d{1,3})\\s*\\)");
QRegularExpressionMatch mHSL = rx_hsl.match(str);
if (mHSL.hasMatch())
@@ -373,7 +387,7 @@
}
else
{
- QRegularExpression rx("#?[0-9A-Fa-f]+");
+ static QRegularExpression rx("#?[0-9A-Fa-f]+");
QRegularExpressionMatch mHex = rx.match(str);
if (mHex.hasMatch())
@@ -382,10 +396,9 @@
return QColor();
}
-QPixmap *IconManager::initPixmap(const QString filePath, QColor color)
+QPixmap *IconManager::pixmapFromFile(const QString filePath, QColor color, int width)
{
QDomDocument document;
-
// Load SVG file
if (readXMLFile(filePath, document, "svg"))
{
@@ -394,24 +407,29 @@
QSvgRenderer svgRenderer(document.toByteArray());
- QRect pxRect(0, 0, svgRenderer.defaultSize().width() * m_devicePixelRatio, svgRenderer.defaultSize().height() * m_devicePixelRatio);
- QPixmap *iconPixmap = new QPixmap(pxRect.size());
+ QSize size = svgRenderer.defaultSize();
+ QPixmap *iconPixmap = new QPixmap(size * m_devicePixelRatio);
+ if (width > -1)
+ {
+ iconPixmap = new QPixmap(iconPixmap->scaledToWidth(width * m_devicePixelRatio, Qt::SmoothTransformation));
+ size = iconPixmap->size() / m_devicePixelRatio;
+ }
+ iconPixmap->setDevicePixelRatio(m_devicePixelRatio);
iconPixmap->fill(Qt::transparent);
QPainter painter(iconPixmap);
- svgRenderer.render(&painter, pxRect);
+ svgRenderer.render(&painter, QRect(QPoint(0, 0), size));
painter.end();
return iconPixmap;
}
+
// Load any other file format
- else
- {
- return new QPixmap(filePath);
- }
+ return new QPixmap(filePath);
+
}
-QPixmap *IconManager::renderPath(QPainterPath path)
+QPixmap *IconManager::pixmapFromPainterPath(QPainterPath path)
{
QSize size(path.boundingRect().width(), path.boundingRect().height());
QPixmap pixmap(size);
@@ -426,40 +444,20 @@
return new QPixmap(pixmap);
}
-bool IconManager::readXMLFile(QString filePath, QDomDocument &document, QString fileExtension)
+QString IconManager::buildName(const QString &name, const QString &prefix, const QString &suffix) const
{
+ return prefix + name + suffix;
+}
-// QFileInfo file(filePath);
-// QFile dataFile(file.absoluteFilePath());
-// if (!dataFile.exists())
-// return false;
-// if (!dataFile.open(QIODevice::ReadOnly))
-// return false;
-// QTextStream ts(&dataFile);
-// ts.setEncoding(QStringConverter::Utf8); // --> Qt6
-// //ts.setCodec(QTextCodec::codecForName("UTF-8")); // --> Qt5
-// QString errorMsg;
-// int eline;
-// int ecol;
-// document = QDomDocument( QString(file.baseName()));
-// QString data(ts.readAll());
-// dataFile.close();
-// if (!document.setContent( data, &errorMsg, &eline, &ecol))
-// {
-// qDebug()<<data<<errorMsg<<eline<<ecol;
-// if (data.contains("404 not found", Qt::CaseInsensitive))
-// qDebug()<<"File not found on server";
-// else
-// qDebug()<<"Could not open file"<<dataFile.fileName();
-// return false;
-// }
-// return true;
-
+bool IconManager::readXMLFile(QString filePath, QDomDocument &document, QString fileExtension)
+{
QFileInfo fileInfo(filePath);
if (fileInfo.completeSuffix() != fileExtension)
+ {
return false;
+ }
QFile inFile( filePath );
if (!inFile.open(QIODevice::ReadOnly | QIODevice::Text))
@@ -479,14 +477,6 @@
return true;
}
-//void IconManager::tintPixmap(QPixmap &pixmap, QColor color)
-//{
-// //QPixmap pm = pixmap;
-// QPainter painter(&pixmap);
-// painter.setCompositionMode(QPainter::CompositionMode_SourceAtop);
-// painter.fillRect(pixmap.rect(), color);
-// painter.end();
-//}
bool IconManager::initIconSets()
{
@@ -644,13 +634,16 @@
if (QFile::exists(iconFilePath))
return iconFilePath;
- QString iconFilePath2(QString("%1%2%3").arg(ScPaths::instance().iconDir(), primaryIconSubdir, name));
- if (QFile::exists(iconFilePath2))
- return iconFilePath2;
+ qWarning("pathForIcon: Unable to load icon %s: File not found", iconFilePath.toLatin1().constData());
+ iconFilePath = QString("%1%2%3").arg(ScPaths::instance().iconDir(), primaryIconSubdir, name);
+ if (QFile::exists(iconFilePath))
+ {
+ return iconFilePath;
+ }
+#ifdef WANT_DEBUG
qWarning("pathForIcon: Unable to load icon %s: File not found", iconFilePath.toLatin1().constData());
- qWarning("pathForIcon: Unable to load icon %s: File not found", iconFilePath2.toLatin1().constData());
-
+#endif
return QString();
}
Index: scribus/iconmanager.h
===================================================================
--- scribus/iconmanager.h (Revision 25832)
+++ scribus/iconmanager.h (Arbeitskopie)
@@ -46,18 +46,15 @@
void setIconsForDarkMode(bool forDarkMode);
QColor baseColor() const;
bool iconsForDarkMode() const;
+ bool setup();
- void clearCache();
void rebuildCache();
- bool createCache();
- bool setup(qreal devicePixelRatio);
-
QCursor loadCursor(const QString& name, int hotX = -1, int hotY = -1, int width = -1);
QIcon loadIcon(const QString& name, int width = -1);
QPixmap loadPixmap(const QString& name, int width = -1);
- void addIcon(const QString & name, QPainterPath path);
+ void addIconFromPainterPath(const QString & name, QPainterPath path);
bool setActiveFromPrefs(const QString& prefsSet);
QString activeSetBasename() { return m_activeSetBasename; }
@@ -76,6 +73,12 @@
private:
+ struct Item {
+ QString name;
+ QColor color;
+ QString filePath;
+ };
+
IconManager(QObject *parent = nullptr);
~IconManager() = default;
@@ -83,6 +86,7 @@
QMap<QString, ScIconSetData> m_iconSets;
QMap<QString, QPainterPath*> m_iconPaths;
+ QMap<QString, Item> m_lookupTable;
ScPixmapCache<QString> m_pxCache;
QString m_activeSetBasename;
QString m_activeSetVersion;
@@ -93,15 +97,16 @@
qreal m_devicePixelRatio { 1.0 };
bool initIconSets();
- void readIconConfigFiles();
+ bool createLookupTable();
+ bool readXMLFile(QString filePath, QDomDocument &document, QString fileExtension);
- QPixmap *initPixmap(const QString filePath, QColor color);
- QPixmap *renderPath(QPainterPath path);
- void renderIcons();
+ QPixmap *pixmapFromFile(const QString filePath, QColor color, int width = -1);
+ QPixmap *pixmapFromPainterPath(QPainterPath path);
- bool readXMLFile(QString filePath, QDomDocument &document, QString fileExtension);
-// void tintPixmap(QPixmap &pixmap, QColor color);
+ QString buildName(const QString &name, const QString &prefix = "", const QString &suffix = "") const;
+ void insertPathIconsToCache();
+ void readIconConfigFiles();
void applyColors(QDomDocument &doc, QString fileName, QColor color);
void applyInlineStyleToElement(const QDomElement &elem, QMap<QString, QString> *styles);
void parseStyleSheet(QString styleString, QMap<QString, QString> *styles);
Index: scribus/scraction.cpp
===================================================================
--- scribus/scraction.cpp (Revision 25832)
+++ scribus/scraction.cpp (Arbeitskopie)
@@ -283,7 +283,7 @@
if (!m_iconPath16.isEmpty())
{
- QPixmap pix = iconManager.loadPixmap(m_iconPath16);
+ QPixmap pix = iconManager.loadPixmap(m_iconPath16, 16);
if (!pix.isNull())
newIcon.addPixmap(pix, QIcon::Normal, QIcon::On);
}
@@ -290,7 +290,7 @@
if (!m_iconPath22.isEmpty())
{
- QPixmap pix = iconManager.loadPixmap(m_iconPath22);
+ QPixmap pix = iconManager.loadPixmap(m_iconPath22, 20);
if (!pix.isNull())
newIcon.addPixmap(pix, QIcon::Normal, QIcon::On);
}
Index: scribus/scribusapp.cpp
===================================================================
--- scribus/scribusapp.cpp (Revision 25832)
+++ scribus/scribusapp.cpp (Arbeitskopie)
@@ -561,7 +561,8 @@
void ScribusQApp::changeIconSet(const QString& newIconSet)
{
IconManager& iconManager = IconManager::instance();
- iconManager.clearCache();
+ //iconManager.clearCache();
+ iconManager.rebuildCache();
iconManager.setActiveFromPrefs(newIconSet);
emit iconSetChanged();
}
Index: scribus/scribuscore.cpp
===================================================================
--- scribus/scribuscore.cpp (Revision 25832)
+++ scribus/scribuscore.cpp (Arbeitskopie)
@@ -178,8 +178,7 @@
CommonStrings::languageChange();
LanguageManager::instance()->languageChange();
- qreal devicePixelRatio = QApplication::primaryScreen()->devicePixelRatio();
- if (!m_iconManager.setup(devicePixelRatio))
+ if (!m_iconManager.setup())
return EXIT_FAILURE;
// FIXME: Splash needs the prefs loaded by initDefaults() to know if it must force the image to grayscale
Index: scribus/ui/preferencesdialog.cpp
===================================================================
--- scribus/ui/preferencesdialog.cpp (Revision 25832)
+++ scribus/ui/preferencesdialog.cpp (Arbeitskopie)
@@ -239,7 +239,7 @@
preferencesTypeList->setDragEnabled(false);
preferencesTypeList->setViewMode(QListView::ListMode);
preferencesTypeList->setFlow(QListView::TopToBottom);
- preferencesTypeList->setIconSize(QSize(32,32));
+ preferencesTypeList->setIconSize(QSize(16,16));
preferencesTypeList->setSortingEnabled(false);
preferencesTypeList->setWrapping(false);
preferencesTypeList->setWordWrap(true);
@@ -255,7 +255,7 @@
void PreferencesDialog::addWidget(Prefs_Pane* tab)
{
//TODO: Can we avoid using this name and duplicating strings by getting it from the tab UIs
- QListWidgetItem* newItem = new QListWidgetItem(IconManager::instance().loadIcon(tab->icon()), tab->caption(), preferencesTypeList);
+ QListWidgetItem* newItem = new QListWidgetItem(IconManager::instance().loadIcon(tab->icon(), 16), tab->caption(), preferencesTypeList);
newItem->setTextAlignment(Qt::AlignLeft | Qt::AlignVCenter);
int i = prefsStackWidget->addWidget(tab);
stackWidgetMap.insert(newItem, i);
|
