Fix usage of IntCachedColorMap in Indexed PNG encoding

raster-test/src/test/scala/geotrellis/raster/render/ColorMapSpec.scala

@@ -113,6 +113,44 @@ class ColorMapSpec extends FunSpec with Matchers
       }
     }
 
+    it("should correctly map values to colors using IntCacheColorMap") {
+      val limits = Array(25,50,80,100)
+      val colors = Array(100,110,120,130)
+
+      val colorRamp: ColorRamp = colors
+      val breaksToColors: Map[Int, Int] = (limits zip colorRamp.stops(limits.size).colors).toMap
+      val colorMap1 = new IntColorMap(breaksToColors, ColorMap.Options(noDataColor =  0, classBoundaryType = LessThanOrEqualTo))
+      val arr = (0 until 90 by 5).toArray
+      val r = createTile(arr)
+      val h = r.histogram
+      val colorMap = colorMap1.cache(h)
+
+      val color: IndexedPngEncoding =
+        PngColorEncoding(colorMap.colors, colorMap.options.noDataColor, colorMap.options.fallbackColor) match {
+          case i @ IndexedPngEncoding(_, _) => i
+          case _ =>
+            withClue(s"Color should be Indexed") { sys.error("") }
+        }
+
+      // check that PNG will correctly convert raster to their color index values
+      val pngMap = color.convertColorMap(colorMap)
+      h.foreachValue { z =>
+        val color = colorMap.map(z)
+        val colorIndex = colorMap.colors.indexOf(color)
+        pngMap.map(z) should be(colorIndex)
+      }
+
+      // check that we can convert all raster values we have seen in our histogram
+      withClue(s"Cached colors: ${colorMap.colors.toList}") {
+        for (x <- arr) {
+          if (x <= 25) colorMap.map(x) should be(100)
+          else if (x <= 50) colorMap.map(x) should be(110)
+            // tile does not have any values past 75, since we didn't see them, we don't expect to color them
+          else if (x > 75) colorMap.options.noDataColor
+        }
+      }
+    }
+
     it("should correctly map redundant values to colors") {
       val limits = Array(25,42,60)
       val colors = Array(10,20,30)

raster/src/main/scala/geotrellis/raster/render/ColorMap.scala

@@ -19,7 +19,7 @@ package geotrellis.raster.render
 import geotrellis.raster._
 import geotrellis.raster.histogram.Histogram
 import geotrellis.util._
-
+import spire.syntax.cfor._
 import spire.std.any._
 
 import scala.util.Try
@@ -179,6 +179,8 @@ trait ColorMap extends Serializable {
   def mapDouble(v: Double): Int
 
   def mapColors(f: Int => Int): ColorMap
+  /** Maps each color value to its index in the [[colors]] sequence.
+    * This is useful for table lookup encodings (ex: [[geotrellis.raster.render.png.IndexedPngEncoding]]) */
   def mapColorsToIndex(): ColorMap
 
   def withNoDataColor(color: Int): ColorMap
@@ -228,8 +230,15 @@ class IntColorMap(breaksToColors: Map[Int, Int], val options: Options = Options.
 
   def cache(h: Histogram[Int]): ColorMap = {
     val ch = h.mutable
-    h.foreachValue(z => ch.setItem(z, map(z)))
-    new IntCachedColorMap(colors, ch, options)
+    val cachedColors = h.values()
+    cfor(0)( _ < cachedColors.length, _ + 1) { i =>
+      val z = cachedColors(i)
+      val itemColor = map(z)
+      cachedColors(i) = itemColor
+      ch.setItem(z, itemColor)
+    }
+    // multiple values could have mapped to same color, use only distinct color values
+    new IntCachedColorMap(cachedColors.distinct.toVector, ch, options)
   }
 
   def breaksString: String = {
@@ -243,6 +252,14 @@ class IntColorMap(breaksToColors: Map[Int, Int], val options: Options = Options.
 /** Caches a color ramp based on a histogram of values. This is an optimization, since
   * often times we create a histogram while classifying, and can reuse that computed
   * information in the color mapping.
+  *
+  * In order for this class to work correctly the histogram acts as a map from values to their colors
+  * and must contain a value for each pixel value that we expect to encounter.
+  *
+  * The performance benefit is `eC` lookup cost instead of `Log` provided by [[IntColorMap]]
+  *
+  * @param colors All color values that can be encountered
+  * @param h      Histogram where counts of values have been replaced cached RGBA color
   */
 class IntCachedColorMap(val colors: Vector[Int], h: Histogram[Int], val options: Options)
     extends ColorMap {
@@ -259,8 +276,9 @@ class IntCachedColorMap(val colors: Vector[Int], h: Histogram[Int], val options:
   }
 
   def mapColorsToIndex(): ColorMap = {
-    val colorIndexMap = colors.zipWithIndex.map { case (c, i) => (i, c) }.toMap
+    val colorIndexMap = colors.zipWithIndex.toMap
     val ch = h.mutable
+
     h.foreachValue(z => ch.setItem(z, colorIndexMap(h.itemCount(z).toInt)))
     new IntCachedColorMap((0 to colors.length).toVector, ch, options)
   }

raster/src/main/scala/geotrellis/raster/render/png/PngColorEncoding.scala

@@ -18,6 +18,7 @@ package geotrellis.raster.render.png
 
 import geotrellis.raster.render._
 
+/** Captures the conversion strategies from RGBA color space to PNG pixels. */
 sealed abstract class PngColorEncoding(val n: Byte, val depth: Int) {
  def convertColorMap(colorMap: ColorMap): ColorMap
 }