Bug 138524

Summary: Krita does not properly parse GIMP palettes with no tab whitespace
Product: [Applications] krita Reporter: Juan Carlos Torres <carlosdgtorres>
Component: GeneralAssignee: Krita Bugs <krita-bugs-null>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 1.6.1   
Target Milestone: ---   
Platform: Ubuntu   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: Oxygen color palette

Description Juan Carlos Torres 2006-12-08 09:24:25 UTC
Version:           1.6.1 (using KDE KDE 3.5.5)
Installed from:    Ubuntu Packages
OS:                Linux

Krita's parser does not process lines in GIMP palette files that use spaces instead of a tab to separate the RGB color codes from the color name. 

The code is in kis_palette.cc, line 241:
 if (lines[i].contains("\t") > 0)

The parser recognizes only a tab as a valid separator to split the RGB code and the color name when checking whether the line is a valid palette entry. The GIMP, however, allows both tabs and spaces to be used in separating each component and parses the line correctly.

An example of GIMP palette that doesn't always use tabs is the Oxygen color palette. Only some of the lines use a tab, while others use spaces. As a result, only those lines with a tab are parsed and imported into Krita. The GIMP parses and imports all colors properly.

http://www.kde-artists.org/Oxygen-palette
Comment 1 Juan Carlos Torres 2006-12-08 09:27:08 UTC
Created attachment 18842 [details]
Oxygen color palette

This is the Oxygen color palette that is provided
http://www.kde-artists.org/Oxygen-palette. Not all colors are parsed and
imported properly.
Comment 2 Halla Rempt 2006-12-08 09:55:19 UTC
SVN commit 611459 by rempt:

BUG: 138524 
* Load gimp palettes without tabs (I think those are broken, but lets load
them anyway)
* Also: make closing Krita while drying not crash


 M  +1 -1      kis_paint_device.cc  
 M  +11 -2     kis_palette.cc  


--- branches/koffice/1.6/koffice/krita/core/kis_paint_device.cc #611458:611459
@@ -1271,7 +1271,7 @@
             (*it)->process(this, this, 0, rc);
         }
     }
-    undoAdapter()->addCommand(cmd);
+    if (cmd && undoAdapter()) undoAdapter()->addCommand(cmd);
 
     if (m_parentLayer) m_parentLayer->setDirty(rc);
 }
--- branches/koffice/1.6/koffice/krita/core/kis_palette.cc #611458:611459
@@ -238,8 +238,14 @@
                 m_comment += lines[i].mid(1).stripWhiteSpace() + " ";
             }
             else {
-                if (lines[i].contains("\t") > 0) {
-                    QStringList a = QStringList::split("\t", lines[i]);
+                if (lines[i].contains("\t") > 0 || lines[i].contains("     ") > 0) {
+
+                    QStringList a;
+                   
+                    if (lines[i].contains("\t") > 0)
+                        a = QStringList::split("\t", lines[i]);
+                    else if (lines[i].contains("     ") > 0)
+                        a = QStringList::split("     ", lines[i]);
                     e.name = a[1];
 
                     QStringList c = QStringList::split(" ", a[0]);
@@ -254,6 +260,9 @@
 
                     add(e);
                 }
+                else {
+                    kdWarning() << filename() << ": could not parse palette line " << lines[i] << endl;
+                }
             }
         }
 
Comment 3 Juan Carlos Torres 2006-12-08 10:02:35 UTC
I have also sent a working version of the Oxygen palette to the kde-artists mailing list, in the meantime.
Comment 4 Halla Rempt 2006-12-08 10:04:42 UTC
SVN commit 611461 by rempt:

Better way of loading palettes, thanks to Eike Hein.
CCBUG:138524 


 M  +26 -24    kis_palette.cc  


--- branches/koffice/1.6/koffice/krita/core/kis_palette.cc #611460:611461
@@ -232,40 +232,42 @@
             index = 3;
         }
 
-        // Loop over the rest of the lines
         for (Q_UINT32 i = index; i < lines.size(); i++) {
             if (lines[i].startsWith("#")) {
                 m_comment += lines[i].mid(1).stripWhiteSpace() + " ";
             }
-            else {
-                if (lines[i].contains("\t") > 0 || lines[i].contains("     ") > 0) {
+            else if (!lines[i].isEmpty())
+            {
+                QStringList a = QStringList::split(" ", lines[i].replace(QChar('\t'), " "));
 
-                    QStringList a;
-                   
-                    if (lines[i].contains("\t") > 0)
-                        a = QStringList::split("\t", lines[i]);
-                    else if (lines[i].contains("     ") > 0)
-                        a = QStringList::split("     ", lines[i]);
-                    e.name = a[1];
+                if (a.count() < 3)
+                {
+                    break;
+                }
 
-                    QStringList c = QStringList::split(" ", a[0]);
-                    channel = c[0].stripWhiteSpace();
-                    r = channel.toInt();
-                    channel = c[1].stripWhiteSpace();
-                    g = channel.toInt();
-                    channel = c[2].stripWhiteSpace();
-                    b = channel.toInt();
-                    color = QColor(r, g, b);
-                    e.color = color;
+                r = a[0].toInt();
+                a.pop_front();
+                g = a[0].toInt();
+                a.pop_front();
+                b = a[0].toInt();
+                a.pop_front();
 
-                    add(e);
+                if (r < 0 || r > 255 ||
+                    g < 0 || g > 255 ||
+                    b < 0 || b > 255)
+                {
+                    break;
                 }
-                else {
-                    kdWarning() << filename() << ": could not parse palette line " << lines[i] << endl;
-                }
+
+                color = QColor(r, g, b);
+                e.color = color;
+
+                QString name = a.join(" ");
+                e.name = name.isEmpty() ? i18n("Untitled") : name;
+
+                add(e);
             }
         }
-
         setValid(true);
         return true;
     }
Comment 5 Halla Rempt 2006-12-08 10:12:52 UTC
On Friday 08 December 2006 10:02, Juan Carlos Torres wrote:
> I have also sent a working version of the Oxygen palette to the
> kde-artists mailing list, in the meantime.


Great!