Patch from Jeremy Michelson (with slight tweaks) - bug #48274 - fix erronious wrapping of byte colours in HSSFPalette.findSimilarColor

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

src/documentation/content/xdocs/status.xml

@@ -34,7 +34,8 @@
 
     <changes>
         <release version="3.6-beta1" date="2009-??-??">
-           <action dev="POI-DEVELOPERS" type="fix">24601 - fix fetching of error codes from XSSF formula cells</action>
+           <action dev="POI-DEVELOPERS" type="fix">48274 - fix erronious wrapping of byte colours in HSSFPalette.findSimilarColor</action>
+           <action dev="POI-DEVELOPERS" type="fix">48269 - fix fetching of error codes from XSSF formula cells</action>
            <action dev="POI-DEVELOPERS" type="fix">48229 - fixed javadoc for  HSSFSheet.setColumnWidth and XSSFSheet setColumnWidth </action>
            <action dev="POI-DEVELOPERS" type="fix">47757 - fixed XLSX2CSV to avoid exception when processing cells with multiple "t" elements</action>
            <action dev="POI-DEVELOPERS" type="add">48195 - short-circuit evaluation of IF() and CHOOSE()</action>

src/java/org/apache/poi/hssf/usermodel/HSSFPalette.java

@@ -98,16 +98,30 @@ public final class HSSFPalette {
      * @return  The closest color or null if there are no custom
      *          colors currently defined.
      */
-    public HSSFColor findSimilarColor(byte red, byte green, byte blue)
+    public HSSFColor findSimilarColor(byte red, byte green, byte blue) {
-    {
+    	return findSimilarColor(unsignedInt(red), unsignedInt(green), unsignedInt(blue));
+    }
+    /**
+     * Finds the closest matching color in the custom palette.  The
+     * method for finding the distance between the colors is fairly
+     * primative.
+     *
+     * @param red   The red component of the color to match.
+     * @param green The green component of the color to match.
+     * @param blue  The blue component of the color to match.
+     * @return  The closest color or null if there are no custom
+     *          colors currently defined.
+     */
+    public HSSFColor findSimilarColor(int red, int green, int blue) {
         HSSFColor result = null;
         int minColorDistance = Integer.MAX_VALUE;
         byte[] b = _palette.getColor(PaletteRecord.FIRST_COLOR_INDEX);
         for (short i = PaletteRecord.FIRST_COLOR_INDEX; b != null;
             b = _palette.getColor(++i))
         {
-            int colorDistance = Math.abs(red - b[0]) +
+            int colorDistance = Math.abs(red - unsignedInt(b[0])) +
-            	Math.abs(green - b[1]) + Math.abs(blue - b[2]);
+            	Math.abs(green - unsignedInt(b[1])) +
+            	Math.abs(blue - unsignedInt(b[2]));
             if (colorDistance < minColorDistance)
             {
                 minColorDistance = colorDistance;
@@ -117,6 +131,14 @@ public final class HSSFPalette {
         return result;
     }
     
+    /**
+     * Turn a byte of between -127 and 127 into something between
+     *  0 and 255, so distance calculations work as expected.
+     */
+    private int unsignedInt(byte b) {
+    	return 0xFF & ((int) b);
+    }
+
     /**
      * Sets the color at the given offset
      *

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFPalette.java

@@ -17,6 +17,7 @@
 
 package org.apache.poi.hssf.usermodel;
 
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.Map;
 
@@ -24,6 +25,7 @@ import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
 import org.apache.poi.hssf.record.PaletteRecord;
+import org.apache.poi.hssf.usermodel.HSSFPalette;
 import org.apache.poi.hssf.util.HSSFColor;
 
 /**
@@ -138,6 +140,49 @@ public final class TestHSSFPalette extends TestCase {
         HSSFWorkbook book = new HSSFWorkbook();
         HSSFPalette p = book.getCustomPalette();
         
+        /* first test the defaults */
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 255, (short) 255, (short) 0}, // not [204, 255, 255]
+        				p.findSimilarColor((byte) 204, (byte) 255, (byte) 0).getTriplet()
+        		)
+        );
+
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 153, (short) 204, (short) 0}, // not [128, 0, 0]
+        				p.findSimilarColor((byte) 128, (byte) 255, (byte) 0).getTriplet()
+        		)
+        );
+
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 0, (short) 255, (short) 0}, // not [0, 51, 102]
+        				p.findSimilarColor((byte) 0, (byte) 255, (byte) 102).getTriplet()
+        		)
+        );
+
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 0, (short) 102, (short) 204}, // not [255, 102, 0]
+        				p.findSimilarColor((byte) 0, (byte) 102, (byte) 255).getTriplet()
+        		)
+        );
+
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 255, (short) 0, (short) 255}, // not [128, 0, 0]
+        				p.findSimilarColor((byte) 128, (byte) 0, (byte) 255).getTriplet()
+        		)
+        );
+
+        assertTrue(
+        		Arrays.equals(
+        				new short[] {(short) 255, (short) 0, (short) 255}, // not [255, 255, 153]
+        				p.findSimilarColor((byte) 255, (byte) 0, (byte) 153).getTriplet()
+        		)
+        );
+
 
         // Add a few edge colours in
         p.setColorAtIndex((short)8, (byte)-1, (byte)0, (byte)0);
@@ -175,6 +220,16 @@ public final class TestHSSFPalette extends TestCase {
                 p.getColor((short)12).getHexString(),
                 p.findSimilarColor((byte)-1, (byte)2, (byte)10).getHexString()
         );
+        
+        // And with ints not bytes
+        assertEquals(
+                p.getColor((short)11).getHexString(),
+                p.findSimilarColor(255, 2, 1).getHexString()
+        );
+        assertEquals(
+                p.getColor((short)12).getHexString(),
+                p.findSimilarColor(255, 2, 10).getHexString()
+        );
     }
     
     /**