[system/dolphin] src: views: Never let the item layout use a zero icon size

Méven Car <[email protected]>
Newsgroups gmane.comp.kde.cvs
Message-ID <[email protected]>
Git commit 3ce18dd17c41969796bdb62d0b7749b502b428a6 by Méven Car, on behalf of Iyán Méndez Veiga.
Committed on 07/08/2026 at 10:53.
Pushed by meven into branch 'master'.

views: Never let the item layout use a zero icon size

DolphinItemListView caches the icon size in m_iconSize/m_previewSize, but
those members are only ever written by setZoomLevel(), which returns early
when the requested zoom level equals the current one. DolphinView::
applyViewProperties() skips the call for the same reason.

A freshly constructed view starts with m_zoomLevel == 0 and both cached
sizes at 0. Details mode defaults to KIconLoader::SizeSmall, which is zoom
level 0, so the requested level matches the initial one, setZoomLevel() is
never called and updateGridSize() ends up putting a size of 0 into
KItemListStyleOption. KStandardItemListWidget then creates a null pixmap
for every item, and since it only refreshes the pixmap cache when the size
changes, nothing recovers until the view mode is switched.

Fall back to the configured size in updateGridSize() when the cache has not
been populated yet, and keep the cache in sync in setZoomLevel() even when
the zoom level itself did not change. The latter also covers toggling
previews, which switches to the other cache without going through
setZoomLevel().

Icons mode is unaffected because its default size maps to zoom level 2, and
so is any setup using global view properties, which reads the sizes from the
settings directly.

BUG: 523228

M  +5    -0    src/tests/CMakeLists.txt
A  +227  -0    src/tests/dolphinitemlistviewtest.cpp     [License: GPL(v2.0+)]
M  +24   -9    src/views/dolphinitemlistview.cpp
M  +3    -1    src/views/zoomlevelinfo.h

https://invent.kde.org/system/dolphin/-/commit/3ce18dd17c41969796bdb62d0b7749b502b428a6

diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 919769a67d..04725f990c 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -37,6 +37,11 @@ ecm_add_test(kfileitemlistviewtest.cpp testdir.cpp
 TEST_NAME kfileitemlistviewtest
 LINK_LIBRARIES dolphinprivate Qt6::Test)
 
+# DolphinItemListViewTest
+ecm_add_test(dolphinitemlistviewtest.cpp
+TEST_NAME dolphinitemlistviewtest
+LINK_LIBRARIES dolphinprivate Qt6::Test)
+
 # KFileItemModelTest
 ecm_add_test(kfileitemmodeltest.cpp testdir.cpp
 TEST_NAME kfileitemmodeltest
diff --git a/src/tests/dolphinitemlistviewtest.cpp b/src/tests/dolphinitemlistviewtest.cpp
new file mode 100644
index 0000000000..49fc85f0e6
--- /dev/null
+++ b/src/tests/dolphinitemlistviewtest.cpp
@@ -0,0 +1,227 @@
+/*
+ * SPDX-FileCopyrightText: 2026 Iyán Méndez Veiga <[email protected]>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "views/dolphinitemlistview.h"
+#include "dolphin_compactmodesettings.h"
+#include "dolphin_detailsmodesettings.h"
+#include "dolphin_generalsettings.h"
+#include "dolphin_iconsmodesettings.h"
+#include "kitemviews/kfileitemmodel.h"
+#include "kitemviews/kitemlistcontroller.h"
+#include "views/zoomlevelinfo.h"
+
+#include <QStandardPaths>
+#include <QTest>
+
+Q_DECLARE_METATYPE(KStandardItemListView::ItemLayout)
+
+class DolphinItemListViewTest : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void initTestCase();
+    void init();
+    void cleanup();
+
+    void testFreshViewFallsBackToTheConfiguredIconSize();
+
+    void testFreshViewFallsBackToTheConfiguredPreviewSize_data();
+    void testFreshViewFallsBackToTheConfiguredPreviewSize();
+
+    void testApplyingTheCurrentZoomLevelAppliesItsIconSize_data();
+    void testApplyingTheCurrentZoomLevelAppliesItsIconSize();
+
+    void testIconAndPreviewSizesAreCachedSeparately();
+
+    void testZoomLevelChangesAreApplied_data();
+    void testZoomLevelChangesAreApplied();
+
+    void testZoomLevelIsClamped();
+
+private:
+    /** The configured icon or preview size of @p layout, i.e. the size a view falls back to. */
+    static int configuredSize(KStandardItemListView::ItemLayout layout, bool previewsShown);
+
+    int iconSize() const
+    {
+        return m_view->styleOption().iconSize;
+    }
+
+    KFileItemModel *m_model = nullptr;
+    DolphinItemListView *m_view = nullptr;
+    KItemListController *m_controller = nullptr;
+    bool m_globalViewProps = true;
+};
+
+void DolphinItemListViewTest::initTestCase()
+{
+    QStandardPaths::setTestModeEnabled(true);
+}
+
+void DolphinItemListViewTest::init()
+{
+    m_globalViewProps = GeneralSettings::globalViewProps();
+    // With global view properties the sizes are always read back from the settings, so the cached
+    // m_iconSize/m_previewSize members are never used. Per-folder view properties are therefore the
+    // interesting case here, and also the one in which BUG 523228 showed up.
+    GeneralSettings::setGlobalViewProps(false);
+
+    m_model = new KFileItemModel();
+    m_view = new DolphinItemListView();
+    // The controller attaches the model to the view, which is what makes previews toggleable:
+    // without a model KFileItemListView::setPreviewsShown() is a no-op. It also takes ownership
+    // of both the view and the model.
+    m_controller = new KItemListController(m_model, m_view, nullptr);
+
+    // A view starts out in details layout without previews.
+    QCOMPARE(m_view->itemLayout(), KStandardItemListView::DetailsLayout);
+    QVERIFY(!m_view->previewsShown());
+}
+
+void DolphinItemListViewTest::cleanup()
+{
+    // The controller owns both the view and the model.
+    delete m_controller;
+    m_controller = nullptr;
+    m_view = nullptr;
+    m_model = nullptr;
+
+    GeneralSettings::setGlobalViewProps(m_globalViewProps);
+}
+
+int DolphinItemListViewTest::configuredSize(KStandardItemListView::ItemLayout layout, bool previewsShown)
+{
+    switch (layout) {
+    case KStandardItemListView::IconsLayout:
+        return previewsShown ? IconsModeSettings::previewSize() : IconsModeSettings::iconSize();
+    case KStandardItemListView::CompactLayout:
+        return previewsShown ? CompactModeSettings::previewSize() : CompactModeSettings::iconSize();
+    case KStandardItemListView::DetailsLayout:
+        return previewsShown ? DetailsModeSettings::previewSize() : DetailsModeSettings::iconSize();
+    }
+    Q_UNREACHABLE();
+}
+
+static void addLayoutRows()
+{
+    QTest::addColumn<KStandardItemListView::ItemLayout>("layout");
+    QTest::newRow("icons") << KStandardItemListView::IconsLayout;
+    QTest::newRow("compact") << KStandardItemListView::CompactLayout;
+    QTest::newRow("details") << KStandardItemListView::DetailsLayout;
+}
+
+/**
+ * A view that was just constructed must already be usable: an icon size of 0 means that no icons
+ * are rendered at all. Since the cached icon size starts out as 0, the configured size has to be
+ * used until a zoom level is applied.
+ */
+void DolphinItemListViewTest::testFreshViewFallsBackToTheConfiguredIconSize()
+{
+    QCOMPARE(iconSize(), configuredSize(KStandardItemListView::DetailsLayout, false));
+    QVERIFY(iconSize() > 0);
+}
+
+void DolphinItemListViewTest::testFreshViewFallsBackToTheConfiguredPreviewSize_data()
+{
+    addLayoutRows();
+}
+
+/** The same for the separately cached preview size, which is unset until previews are turned on. */
+void DolphinItemListViewTest::testFreshViewFallsBackToTheConfiguredPreviewSize()
+{
+    QFETCH(KStandardItemListView::ItemLayout, layout);
+
+    m_view->setItemLayout(layout);
+    m_view->setPreviewsShown(true);
+    QVERIFY(m_view->previewsShown());
+
+    QCOMPARE(iconSize(), configuredSize(layout, true));
+    QVERIFY(iconSize() > 0);
+}
+
+void DolphinItemListViewTest::testApplyingTheCurrentZoomLevelAppliesItsIconSize_data()
+{
+    addLayoutRows();
+}
+
+/**
+ * Regression test for BUG 523228: when a folder is opened, DolphinView applies the zoom level from
+ * its view properties. That zoom level often is the one the view already reports, in which case
+ * setZoomLevel() used to return early and left the cached size at 0, so the view rendered null
+ * pixmaps for every item until the view mode was changed.
+ */
+void DolphinItemListViewTest::testApplyingTheCurrentZoomLevelAppliesItsIconSize()
+{
+    QFETCH(KStandardItemListView::ItemLayout, layout);
+
+    m_view->setItemLayout(layout);
+
+    // Applying the zoom level the view already is at must not be a no-op.
+    const int level = m_view->zoomLevel();
+    m_view->setZoomLevel(level);
+
+    QCOMPARE(m_view->zoomLevel(), level);
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(level));
+    QVERIFY(iconSize() > 0);
+}
+
+/**
+ * Icon size and preview size are cached separately, so turning previews on after only the icon size
+ * has been populated must not leave the view with the still unset preview size of 0.
+ */
+void DolphinItemListViewTest::testIconAndPreviewSizesAreCachedSeparately()
+{
+    const int zoomLevel = 2;
+    m_view->setZoomLevel(zoomLevel);
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(zoomLevel));
+
+    // Only the icon size has been set so far, so the preview size falls back to the configured one.
+    m_view->setPreviewsShown(true);
+    QCOMPARE(iconSize(), configuredSize(KStandardItemListView::DetailsLayout, true));
+
+    // Once a zoom level is applied while previews are shown, that one wins again.
+    m_view->setZoomLevel(zoomLevel);
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(zoomLevel));
+
+    // Switching previews back off restores the cached icon size.
+    m_view->setPreviewsShown(false);
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(zoomLevel));
+}
+
+void DolphinItemListViewTest::testZoomLevelChangesAreApplied_data()
+{
+    addLayoutRows();
+}
+
+/** Makes sure that dropping the early return in setZoomLevel() did not break ordinary zooming. */
+void DolphinItemListViewTest::testZoomLevelChangesAreApplied()
+{
+    QFETCH(KStandardItemListView::ItemLayout, layout);
+
+    m_view->setItemLayout(layout);
+
+    for (int level = ZoomLevelInfo::minimumLevel(); level <= ZoomLevelInfo::maximumLevel(); ++level) {
+        m_view->setZoomLevel(level);
+        QCOMPARE(m_view->zoomLevel(), level);
+        QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(level));
+    }
+}
+
+void DolphinItemListViewTest::testZoomLevelIsClamped()
+{
+    m_view->setZoomLevel(ZoomLevelInfo::minimumLevel() - 1);
+    QCOMPARE(m_view->zoomLevel(), ZoomLevelInfo::minimumLevel());
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(ZoomLevelInfo::minimumLevel()));
+
+    m_view->setZoomLevel(ZoomLevelInfo::maximumLevel() + 1);
+    QCOMPARE(m_view->zoomLevel(), ZoomLevelInfo::maximumLevel());
+    QCOMPARE(iconSize(), ZoomLevelInfo::iconSizeForZoomLevel(ZoomLevelInfo::maximumLevel()));
+}
+
+QTEST_MAIN(DolphinItemListViewTest)
+
+#include "dolphinitemlistviewtest.moc"
diff --git a/src/views/dolphinitemlistview.cpp b/src/views/dolphinitemlistview.cpp
index 53d1c6c346..8dbe4a716d 100644
--- a/src/views/dolphinitemlistview.cpp
+++ b/src/views/dolphinitemlistview.cpp
@@ -39,17 +39,18 @@ void DolphinItemListView::setZoomLevel(int level)
         level = ZoomLevelInfo::maximumLevel();
     }
 
-    if (level == m_zoomLevel) {
-        return;
-    }
-
-    m_zoomLevel = level;
-
     const bool useGlobalViewProps = GeneralSettings::globalViewProps();
     ViewModeSettings settings(itemLayout());
 
+    // The size belonging to the requested zoom level must be applied even when the zoom level
+    // itself did not change: m_iconSize/m_previewSize may still be unset, e.g. because this is
+    // the first call for the current view mode. Otherwise the view would keep using a stale size.
+    const int size = ZoomLevelInfo::iconSizeForZoomLevel(level);
+    bool changed = (level != m_zoomLevel);
+
     if (previewsShown()) {
-        m_previewSize = ZoomLevelInfo::iconSizeForZoomLevel(level);
+        changed = changed || (m_previewSize != size);
+        m_previewSize = size;
         // Only update the icon size settings if we're using global view props
         // to prevent inconsistent state on zoom level changes
         if (useGlobalViewProps) {
@@ -57,12 +58,19 @@ void DolphinItemListView::setZoomLevel(int level)
         }
     } else {
         // Same as above
-        m_iconSize = ZoomLevelInfo::iconSizeForZoomLevel(level);
+        changed = changed || (m_iconSize != size);
+        m_iconSize = size;
         if (useGlobalViewProps) {
             settings.setIconSize(m_iconSize);
         }
     }
 
+    if (!changed) {
+        return;
+    }
+
+    m_zoomLevel = level;
+
     updateGridSize();
 }
 
@@ -180,7 +188,14 @@ void DolphinItemListView::updateGridSize()
 
     // Calculate the size of the icon
     // Only use zoom stored in settings if we're using global view props
-    const int iconSize = useGlobalViewProps ? (previewsShown() ? settings.previewSize() : settings.iconSize()) : (previewsShown() ? m_previewSize : m_iconSize);
+    int &cachedSize = previewsShown() ? m_previewSize : m_iconSize;
+    if (cachedSize <= 0) {
+        // setZoomLevel() has not been called yet for this view mode and preview state, so the
+        // per-folder zoom is not known yet. Falling back to the configured size keeps the view
+        // usable: a size of 0 would mean that no icons are rendered at all.
+        cachedSize = previewsShown() ? settings.previewSize() : settings.iconSize();
+    }
+    const int iconSize = useGlobalViewProps ? (previewsShown() ? settings.previewSize() : settings.iconSize()) : cachedSize;
     m_zoomLevel = ZoomLevelInfo::zoomLevelForIconSize(QSize(iconSize, iconSize));
     KItemListStyleOption option = styleOption();
 
diff --git a/src/views/zoomlevelinfo.h b/src/views/zoomlevelinfo.h
index 2d444eaf0a..26f249c8c0 100644
--- a/src/views/zoomlevelinfo.h
+++ b/src/views/zoomlevelinfo.h
@@ -7,13 +7,15 @@
 #ifndef ZOOMLEVELINFO_H
 #define ZOOMLEVELINFO_H
 
+#include "dolphin_export.h"
+
 class QSize;
 
 /**
  * @short Helper class for getting information about the zooming
  *        capabilities.
  */
-class ZoomLevelInfo
+class DOLPHIN_EXPORT ZoomLevelInfo
 {
 public:
     static int minimumLevel();
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.