#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.
This commit is contained in:
Magne Sjaastad committed 2026-05-13 14:53:00 +02:00
1 parent 92da6633bb
commit fe59babf21
3 files changed
+71 -3

No files matched your search

@@ -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;
@@ -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})
@@ -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 ) );
}