From ec40e505068c9e75d69f13666aa3f182cf2fc2c4 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@kdab.com>
Date: Wed, 4 Aug 2021 08:13:56 +0200
Subject: [PATCH] QXpmHandler: avoid double-lookup

The code used the if (!contains()) { insert() } anti-pattern,
necessitated by Qt's deviation from the STL of allowing insert() to
overwrite an existing entry, causing two lookups of the same key.

Since QMap these days is a wrapper around std::map, fix by using the
real thing (a std::map) instead, which sports the non-broken insert()
semantics already, avoiding the need to play tricks like detecting a
size increase in order to find whether an insertion took place. It
also simplifies the loop later on, and we can transparently use pmr,
when available.

Change-Id: Iedd8d5691514a7705a55c27376446304b20af071
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/gui/image/qxpmhandler.cpp | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/gui/image/qxpmhandler.cpp b/src/gui/image/qxpmhandler.cpp
index d00c887b71..f9247f9ff0 100644
--- a/src/gui/image/qxpmhandler.cpp
+++ b/src/gui/image/qxpmhandler.cpp
@@ -50,6 +50,7 @@
 #include <qvariant.h>
 
 #include <private/qcolor_p.h>
+#include <private/qduplicatetracker_p.h> // for easier std::pmr detection
 
 #include <algorithm>
 #include <array>
@@ -1116,7 +1117,13 @@ static bool write_xpm_image(const QImage &sourceImage, QIODevice *device, const
     else
         image = sourceImage;
 
-    QMap<QRgb, int> colorMap;
+#ifdef __cpp_lib_memory_resource
+    char buffer[1024];
+    std::pmr::monotonic_buffer_resource res{&buffer, sizeof buffer};
+    std::pmr::map<QRgb, int> colorMap(&res);
+#else
+    std::map<QRgb, int> colorMap;
+#endif
 
     const int w = image.width();
     const int h = image.height();
@@ -1128,8 +1135,9 @@ static bool write_xpm_image(const QImage &sourceImage, QIODevice *device, const
         const QRgb *yp = reinterpret_cast<const QRgb *>(image.constScanLine(y));
         for(x=0; x<w; x++) {
             QRgb color = *(yp + x);
-            if (!colorMap.contains(color))
-                colorMap.insert(color, ncolors++);
+            const auto [it, inserted] = colorMap.try_emplace(color, ncolors);
+            if (inserted)
+                ++ncolors;
         }
     }
 
@@ -1150,14 +1158,11 @@ static bool write_xpm_image(const QImage &sourceImage, QIODevice *device, const
       << '\"' << w << ' ' << h << ' ' << ncolors << ' ' << cpp << '\"';
 
     // write palette
-    QMap<QRgb, int>::Iterator c = colorMap.begin();
-    while (c != colorMap.end()) {
-        QRgb color = c.key();
+    for (const auto [color, index] : colorMap) {
         const QString line = image.format() != QImage::Format_RGB32 && !qAlpha(color)
-            ? QString::asprintf("\"%s c None\"", xpm_color_name(cpp, *c))
-            : QString::asprintf("\"%s c #%02x%02x%02x\"", xpm_color_name(cpp, *c),
+            ? QString::asprintf("\"%s c None\"", xpm_color_name(cpp, index))
+            : QString::asprintf("\"%s c #%02x%02x%02x\"", xpm_color_name(cpp, index),
                                 qRed(color), qGreen(color), qBlue(color));
-        ++c;
         s << ',' << Qt::endl << line;
     }