Bug 51222 - XSSFColor.getARGBHex() returns wrong color for Excel 2007 xlsx file

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1621393 13f79535-47bb-0310-9956-ffa450edef68

src/ooxml/java/org/apache/poi/xssf/model/ThemesTable.java

@@ -23,20 +23,22 @@ import org.apache.poi.openxml4j.opc.PackagePart;
 import org.apache.poi.openxml4j.opc.PackageRelationship;
 import org.apache.poi.xssf.usermodel.XSSFColor;
 import org.apache.xmlbeans.XmlException;
-import org.apache.xmlbeans.XmlObject;
+import org.openxmlformats.schemas.drawingml.x2006.main.CTColor;
 import org.openxmlformats.schemas.drawingml.x2006.main.CTColorScheme;
 import org.openxmlformats.schemas.drawingml.x2006.main.ThemeDocument;
-import org.openxmlformats.schemas.drawingml.x2006.main.CTColor;
 
 /**
  * Class that represents theme of XLSX document. The theme includes specific
  * colors and fonts.
- * 
- * @author Petr Udalau(Petr.Udalau at exigenservices.com) - theme colors
  */
 public class ThemesTable extends POIXMLDocumentPart {
     private ThemeDocument theme;
 
+    /**
+     * Construct a ThemesTable.
+     * @param part A PackagePart.
+     * @param rel A PackageRelationship.
+     */
     public ThemesTable(PackagePart part, PackageRelationship rel) throws IOException {
         super(part, rel);
         
@@ -47,35 +49,53 @@ public class ThemesTable extends POIXMLDocumentPart {
         }
     }
 
+    /**
+     * Construct a ThemesTable from an existing ThemeDocument.
+     * @param theme A ThemeDocument.
+     */
     public ThemesTable(ThemeDocument theme) {
         this.theme = theme;
     }
 
+    /**
+     * Convert a theme "index" into a color.
+     * @param idx A theme "index"
+     * @return The mapped XSSFColor, or null if not mapped.
+     */
     public XSSFColor getThemeColor(int idx) {
+        // Theme color references are NOT positional indices into the color scheme,
+        // i.e. these keys are NOT the same as the order in which theme colors appear
+        // in theme1.xml. They are keys to a mapped color.
         CTColorScheme colorScheme = theme.getTheme().getThemeElements().getClrScheme();
-        CTColor ctColor = null;
+        CTColor ctColor;
-        int cnt = 0;
+        switch (idx) {
-        for (XmlObject obj : colorScheme.selectPath("./*")) {
+            case  0: ctColor = colorScheme.getLt1(); break;
-            if (obj instanceof org.openxmlformats.schemas.drawingml.x2006.main.CTColor) {
+            case  1: ctColor = colorScheme.getDk1(); break;
-                if (cnt == idx) {
+            case  2: ctColor = colorScheme.getLt2(); break;
-                    ctColor = (org.openxmlformats.schemas.drawingml.x2006.main.CTColor) obj;
+            case  3: ctColor = colorScheme.getDk2(); break;
+            case  4: ctColor = colorScheme.getAccent1(); break;
+            case  5: ctColor = colorScheme.getAccent2(); break;
+            case  6: ctColor = colorScheme.getAccent3(); break;
+            case  7: ctColor = colorScheme.getAccent4(); break;
+            case  8: ctColor = colorScheme.getAccent5(); break;
+            case  9: ctColor = colorScheme.getAccent6(); break;
+            case 10: ctColor = colorScheme.getHlink(); break;
+            case 11: ctColor = colorScheme.getFolHlink(); break;
+            default: return null;
+        }
 
         byte[] rgb = null;
-                    if (ctColor.getSrgbClr() != null) {
+        if (ctColor.isSetSrgbClr()) {
-                       // Colour is a regular one 
+            // Color is a regular one
             rgb = ctColor.getSrgbClr().getVal();
-                    } else if (ctColor.getSysClr() != null) {
+        } else if (ctColor.isSetSysClr()) {
-                       // Colour is a tint of white or black
+            // Color is a tint of white or black
             rgb = ctColor.getSysClr().getLastClr();
-                    }
+        } else {
-
-                    return new XSSFColor(rgb);
-                }
-                cnt++;
-            }
-        }
             return null;
         }
+        return new XSSFColor(rgb);        
+    }
     
     /**
      * If the colour is based on a theme, then inherit

src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFColor.java

@@ -80,27 +80,6 @@ public class XSSFColor implements Color {
         ctColor.setIndexed(indexed);
     }
 
-	/**
-    * For RGB colours, but not ARGB (we think...)
-    * Excel gets black and white the wrong way around, so switch them 
-	 */
-	private byte[] correctRGB(byte[] rgb) {
-	   if(rgb.length == 4) {
-	      // Excel doesn't appear to get these wrong
-	      // Nothing to change
-	      return rgb;
-	   } else {
-         // Excel gets black and white the wrong way around, so switch them 
-         if (rgb[0] == 0 && rgb[1] == 0 && rgb[2] == 0) {
-            rgb = new byte[] {-1, -1, -1};
-         }
-         else if (rgb[0] == -1 && rgb[1] == -1 && rgb[2] == -1) {
-            rgb = new byte[] {0, 0, 0};
-         }
-         return rgb;
-	   }
-	}
-
    /**
     * Standard Red Green Blue ctColor value (RGB).
     * If there was an A (Alpha) value, it will be stripped.
@@ -158,9 +137,7 @@ public class XSSFColor implements Color {
 
          // Grab the colour
          rgb = ctColor.getRgb();
-
+         return rgb;
-         // Correct it as needed, and return
-         return correctRGB(rgb);
     }
 
     /**
@@ -220,8 +197,7 @@ public class XSSFColor implements Color {
      * Standard Alpha Red Green Blue ctColor value (ARGB).
      */
     public void setRgb(byte[] rgb) {
-	   // Correct it and save
+       ctColor.setRgb(rgb);
-		ctColor.setRgb(correctRGB(rgb));
     }
 
     /**

src/ooxml/testcases/org/apache/poi/xssf/model/TestThemesTable.java

@@ -0,0 +1,78 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.xssf.model;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.FileOutputStream;
+
+import org.apache.commons.codec.binary.Hex;
+import org.apache.poi.ss.usermodel.CellStyle;
+import org.apache.poi.ss.usermodel.Row;
+import org.apache.poi.xssf.XSSFTestDataSamples;
+import org.apache.poi.xssf.usermodel.XSSFCellStyle;
+import org.apache.poi.xssf.usermodel.XSSFColor;
+import org.apache.poi.xssf.usermodel.XSSFFont;
+import org.apache.poi.xssf.usermodel.XSSFRow;
+import org.apache.poi.xssf.usermodel.XSSFWorkbook;
+import org.junit.Test;
+
+public class TestThemesTable {
+    private String testFile = "Themes.xlsx";
+
+    @Test
+    public void testThemesTableColors() throws Exception {
+        XSSFWorkbook workbook = XSSFTestDataSamples.openSampleWorkbook(testFile);
+        String rgbExpected[] = {
+            "ffffff", // Lt1
+            "000000", // Dk1
+            "eeece1", // Lt2
+            "1f497d", // DK2
+            "4f81bd", // Accent1
+            "c0504d", // Accent2
+            "9bbb59", // Accent3
+            "8064a2", // Accent4
+            "4bacc6", // Accent5
+            "f79646", // Accent6
+            "0000ff", // Hlink
+            "800080"  // FolHlink
+        };
+        boolean createFile = false;
+        int i=0;
+        for (Row row : workbook.getSheetAt(0)) {
+            XSSFFont font = ((XSSFRow)row).getCell(0).getCellStyle().getFont();
+            XSSFColor color = font.getXSSFColor();
+            assertEquals("Failed color theme "+i, rgbExpected[i], Hex.encodeHexString(color.getRgb()));
+            long themeIdx = font.getCTFont().getColorArray(0).getTheme();
+            assertEquals("Failed color theme "+i, i, themeIdx);
+            if (createFile) {
+                XSSFCellStyle cs = (XSSFCellStyle)row.getSheet().getWorkbook().createCellStyle();
+                cs.setFillForegroundColor(color);
+                cs.setFillPattern(CellStyle.SOLID_FOREGROUND);
+                row.createCell(1).setCellStyle(cs);
+            }
+            i++;
+        }
+        
+        if (createFile) {
+            FileOutputStream fos = new FileOutputStream("foobaa.xlsx");
+            workbook.write(fos);
+            fos.close();
+        }
+    }
+}

src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFColor.java

@@ -93,15 +93,17 @@ public final class TestXSSFColor extends TestCase {
       assertEquals(-1,  rgb3.getARgb()[3]);
 
       // Tint doesn't have the alpha
+      // tint = -0.34999
+      // 255 * (1 + tint) = 165 truncated
+      // or (byte) -91 (which is 165 - 256)
       assertEquals(3, rgb3.getRgbWithTint().length);
-      assertEquals(0, rgb3.getRgbWithTint()[0]);
+      assertEquals(-91, rgb3.getRgbWithTint()[0]);
-      assertEquals(0,  rgb3.getRgbWithTint()[1]);
+      assertEquals(-91,  rgb3.getRgbWithTint()[1]);
-      assertEquals(0,  rgb3.getRgbWithTint()[2]);
+      assertEquals(-91,  rgb3.getRgbWithTint()[2]);
 
-      // Set the colour to black, will get translated internally
+      // Set the color to black (no theme).
-      // (Excel stores 3 colour white and black wrong!)
+      rgb3.setRgb(new byte[] {0, 0, 0});
-      rgb3.setRgb(new byte[] {-1,-1,-1});
+      assertEquals("FF000000", rgb3.getARGBHex());
-      assertEquals("FFFFFFFF", rgb3.getARGBHex());
       assertEquals(0, rgb3.getCTColor().getRgb()[0]);
       assertEquals(0, rgb3.getCTColor().getRgb()[1]);
       assertEquals(0, rgb3.getCTColor().getRgb()[2]);

src/ooxml/testcases/org/apache/poi/xssf/usermodel/extensions/TestXSSFCellFill.java

@@ -75,11 +75,16 @@ public class TestXSSFCellFill extends TestCase {
         XSSFColor foregroundColor = cellWithThemeColor.getCellStyle().getFillForegroundXSSFColor();
         byte[] rgb = foregroundColor.getRgb();
         byte[] rgbWithTint = foregroundColor.getRgbWithTint();
-        assertEquals(rgb[0],-18);
+        // Dk2
-        assertEquals(rgb[1],-20);
+        assertEquals(rgb[0],31);
-        assertEquals(rgb[2],-31);
+        assertEquals(rgb[1],73);
-        assertEquals(rgbWithTint[0],-12);
+        assertEquals(rgb[2],125);
-        assertEquals(rgbWithTint[1],-13);
+        // Dk2, lighter 40% (tint is about 0.39998)
-        assertEquals(rgbWithTint[2],-20);
+        // 31 * (1.0 - 0.39998) + (255 - 255 * (1.0 - 0.39998)) = 120.59552 => 120 (byte)
+        // 73 * (1.0 - 0.39998) + (255 - 255 * (1.0 - 0.39998)) = 145.79636 => -111 (byte)
+        // 125 * (1.0 - 0.39998) + (255 - 255 * (1.0 - 0.39998)) = 176.99740 => -80 (byte)
+        assertEquals(rgbWithTint[0],120);
+        assertEquals(rgbWithTint[1],-111);
+        assertEquals(rgbWithTint[2],-80);
     }
 }