From fe59babf21364c9940ec404fab0fdc7086042f81 Mon Sep 17 00:00:00 2001
From: Magne Sjaastad <magne.sjaastad@ceetronsolutions.com>
Date: Tue, 12 May 2026 14:23:48 +0200
Subject: [PATCH] #13987 PDM: Fix IconProvider operator= clearing pixmap +
 regression test

IconProvider::operator= copied a new resource-string but never cleared
the existing m_pixmap. icon() prefers the pixmap when present, so a
pixmap-based default stamped earlier (e.g. by a base class) would
silently shadow a later resource-string-only assignment in a derived
class.

Reset m_pixmap in operator= so resource-string-only assignments fully
replace pixmap-based ones, and cover the replacement semantics with a
caf::IconProvider regression test: default-overwriting-pixmap,
resource-string-overwriting-pixmap and pixmap-overwriting-pixmap.

Needed for the upcoming nested-collection migration where a base class
stamps a folder pixmap and the topmost instance applies a branded
resource-string icon on top.
---
 .../cafPdmUiCore/cafIconProvider.cpp          |  1 +
 .../cafUserInterface_UnitTests/CMakeLists.txt | 10 ++-
 .../cafIconProviderTest.cpp                   | 63 +++++++++++++++++++
 3 files changed, 71 insertions(+), 3 deletions(-)
 create mode 100644 Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafIconProviderTest.cpp

diff --git a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafIconProvider.cpp b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafIconProvider.cpp
index af9f0a72f8..e84284f964 100644
--- a/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafIconProvider.cpp
+++ b/Fwk/AppFwk/cafProjectDataModel/cafPdmUiCore/cafIconProvider.cpp
@@ -98,6 +98,7 @@ IconProvider& IconProvider::operator=( const IconProvider& rhs )
     m_overlayResourceString  = rhs.m_overlayResourceString;
     m_backgroundColorStrings = rhs.m_backgroundColorStrings;
     m_preferredSize          = rhs.m_preferredSize;
+    m_pixmap.reset();
     copyPixmap( rhs );
 
     return *this;
diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt
index dc208b61b4..ebffdbe7e0 100644
--- a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt
+++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/CMakeLists.txt
@@ -14,9 +14,13 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR} # required for gtest-all.cpp
 )
 
 set(PROJECT_FILES
-    cafUserInterface_UnitTests.cpp cafPdmUiTreeViewModelTest.cpp
-    cafPdmUiTreeSelectionQModelTest.cpp cafPdmUiNumberFormatTest.cpp
-    cafPdmUiCheckBoxAndTextEditorTest.cpp gtest/gtest-all.cpp
+    cafUserInterface_UnitTests.cpp
+    cafPdmUiTreeViewModelTest.cpp
+    cafPdmUiTreeSelectionQModelTest.cpp
+    cafPdmUiNumberFormatTest.cpp
+    cafPdmUiCheckBoxAndTextEditorTest.cpp
+    cafIconProviderTest.cpp
+    gtest/gtest-all.cpp
 )
 
 qt_add_executable(${PROJECT_NAME} ${PROJECT_FILES})
diff --git a/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafIconProviderTest.cpp b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafIconProviderTest.cpp
new file mode 100644
index 0000000000..21db9691b4
--- /dev/null
+++ b/Fwk/AppFwk/cafUserInterface/cafUserInterface_UnitTests/cafIconProviderTest.cpp
@@ -0,0 +1,63 @@
+
+#include "gtest/gtest.h"
+
+#include "cafIconProvider.h"
+
+#include <QIcon>
+#include <QImage>
+#include <QPixmap>
+
+using namespace caf;
+
+//--------------------------------------------------------------------------------------------------
+/// Regression test for the topmost nested-collection icon: assigning a resource-string-only
+/// IconProvider on top of a pixmap-based one must fully replace the state. Previously
+/// operator= leaked the existing pixmap through copyPixmap(), so icon() kept returning the
+/// stale pixmap and the branded resource icon was never visible.
+//--------------------------------------------------------------------------------------------------
+TEST( IconProviderTest, AssignmentFromDefaultClearsPixmap )
+{
+    QPixmap pix( 16, 16 );
+    pix.fill( Qt::red );
+
+    IconProvider provider( pix );
+    ASSERT_TRUE( provider.valid() );
+
+    provider = IconProvider();
+
+    EXPECT_FALSE( provider.valid() );
+}
+
+TEST( IconProviderTest, AssignmentFromResourceStringReplacesPixmap )
+{
+    QPixmap pix( 16, 16 );
+    pix.fill( Qt::red );
+
+    IconProvider provider( pix );
+
+    // Assign a resource-string-only provider whose resource does not resolve to any real
+    // image. icon() should yield nullptr because nothing is renderable; with the bug the
+    // previous pixmap leaked through and icon() returned the red QIcon.
+    provider = IconProvider( ":/non-existent-resource.png" );
+
+    EXPECT_TRUE( provider.icon() == nullptr );
+}
+
+TEST( IconProviderTest, AssignmentBetweenPixmapsPicksNewPixmap )
+{
+    QPixmap red( 16, 16 );
+    red.fill( Qt::red );
+
+    QPixmap blue( 16, 16 );
+    blue.fill( Qt::blue );
+
+    IconProvider provider( red );
+    provider = IconProvider( blue );
+
+    auto icon = provider.icon();
+    ASSERT_TRUE( icon != nullptr );
+
+    QImage img = icon->pixmap( 16, 16 ).toImage();
+    ASSERT_FALSE( img.isNull() );
+    EXPECT_EQ( img.pixel( 0, 0 ), qRgb( 0, 0, 255 ) );
+}