Fix NoData handling in the ColorMethods.color function for the RGB and RGBA Multiband Rasters

CHANGELOG.md

@@ -6,6 +6,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+- Fix NoData handling in the ColorMethods.color function for the RGB and RGBA Multiband Rasters [#3278](https://github.com/locationtech/geotrellis/pull/3278)
+- Fix renderJpg() color is bluer than expected [#3203](https://github.com/locationtech/geotrellis/issues/3203)
+
 ## [3.4.1] - 2020-07-16
 
 ### Changed

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

@@ -48,14 +48,13 @@ trait MultibandColorMethods extends MethodExtensions[MultibandTile] {
 
     if(self.bandCount == 3) {
       self.convert(IntConstantNoDataCellType).combine(0, 1, 2) { (rBand, gBand, bBand) =>
-        val r = if (isData(rBand)) { rBand } else 0
-        val g = if (isData(gBand)) { gBand } else 0
-        val b = if (isData(bBand)) { bBand } else 0
+        var transparent = true
+        val r = if (isData(rBand)) { transparent = false; rBand } else 0
+        val g = if (isData(gBand)) { transparent = false; gBand } else 0
+        val b = if (isData(bBand)) { transparent = false; bBand } else 0
 
-        if(r + g + b == 0) 0
-        else {
-          ((r & 0xFF) << 24) | ((g & 0xFF) << 16) | ((b & 0xFF) << 8) | 0xFF
-        }
+        if(transparent) 0
+        else ((r & 0xFF) << 24) | ((g & 0xFF) << 16) | ((b & 0xFF) << 8) | 0xFF
       }
     } else {
       self.convert(IntConstantNoDataCellType).combine(0, 1, 2, 3) { (rBand, gBand, bBand, aBand) =>
@@ -64,10 +63,8 @@ trait MultibandColorMethods extends MethodExtensions[MultibandTile] {
         val b = if (isData(bBand)) { bBand } else 0
         val a = if (isData(aBand)) { aBand } else 0
 
-        if(r + g + b == 0) 0
-        else {
-          ((r & 0xFF) << 24) | ((g & 0xFF) << 16) | ((b & 0xFF) << 8) | (a & 0xFF)
-        }
+        if(a == 0) 0
+        else ((r & 0xFF) << 24) | ((g & 0xFF) << 16) | ((b & 0xFF) << 8) | (a & 0xFF)
       }
     }
   }

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

@@ -40,7 +40,7 @@ trait JpgRenderMethods extends MethodExtensions[Tile] {
     *
     */
   def renderJpg(settings: Settings): Jpg =
-    new JpgEncoder(settings).writeByteArray(self)
+    JpgEncoder(settings).writeByteArray(self.map(_.toARGB))
 
   def renderJpg(colorRamp: ColorRamp): Jpg =
     renderJpg(colorRamp, Settings.DEFAULT)
@@ -73,8 +73,6 @@ trait JpgRenderMethods extends MethodExtensions[Tile] {
     * geotrellis.raster.stats.op.stat.GetClassBreaks operation to
     * generate quantile class breaks.
     */
-  def renderJpg(colorMap: ColorMap, settings: Settings): Jpg = {
-    val encoder = new JpgEncoder(new Settings(1.0, false))
-    encoder.writeByteArray(colorMap.render(self).map(_.toARGB))
-  }
+  def renderJpg(colorMap: ColorMap, settings: Settings): Jpg =
+    JpgEncoder(settings).writeByteArray(colorMap.render(self).map(_.toARGB))
 }

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

@@ -18,16 +18,8 @@ package geotrellis.raster.render.png
 
 import java.io.ByteArrayOutputStream
 import java.io.DataOutputStream
-import java.io.File
-import java.io.FileOutputStream
-import java.io.OutputStream
-import java.nio.ByteBuffer
 import java.util.zip.CRC32
 import java.util.zip.CheckedOutputStream
-import java.util.zip.Deflater
-import java.util.zip.DeflaterOutputStream
-
-import scala.math.abs
 
 import Util._
 

raster/src/test/scala/geotrellis/raster/render/jpg/RenderJpgTests.scala

@@ -17,13 +17,15 @@
 package geotrellis.raster.render.jpg
 
 import geotrellis.raster._
+import geotrellis.raster.geotiff.GeoTiffRasterSource
 import geotrellis.raster.render.Jpg
 import geotrellis.raster.testkit._
 
 import spire.syntax.cfor._
 import org.scalatest._
 
 import java.io._
+import java.nio.file.{Files, Paths}
 import javax.imageio._
 
 class RenderJpgTests extends FunSuite with Matchers with TileBuilders with RasterMatchers {
@@ -90,9 +92,9 @@ class RenderJpgTests extends FunSuite with Matchers with TileBuilders with Raste
         )
       )
 
-    val jpg = tile.renderJpg(colorMap)
+    val jpg = tile.renderJpg(colorMap, Settings.HIGHEST_QUALITY)
 
-    testJpg(jpg, tile, colorMap, threshold=250.0)
+    testJpg(jpg, tile, colorMap, threshold = 250.0)
   }
 
   test("render a JPG from an Int tile and ensure it (roughly) matches what is read in by ImageIO when written") {
@@ -118,9 +120,9 @@ class RenderJpgTests extends FunSuite with Matchers with TileBuilders with Raste
         )
       )
 
-    val jpg = tile.renderJpg(colorMap)
+    val jpg = tile.renderJpg(colorMap, Settings.HIGHEST_QUALITY)
 
-    testJpg(jpg, tile, colorMap, threshold=250.0)
+    testJpg(jpg, tile, colorMap, threshold = 250.0)
   }
 
   test("render int and double tiles similarly") {
@@ -188,17 +190,27 @@ class RenderJpgTests extends FunSuite with Matchers with TileBuilders with Raste
         )
       )
 
-    val jpg = tile.renderJpg(colorMap)
+    val jpg = tile.renderJpg(colorMap, Settings.HIGHEST_QUALITY)
     val img = ImageIO.read(new ByteArrayInputStream(jpg.bytes))
 
     img.getWidth should be (tile.cols)
     img.getHeight should be (tile.rows)
 
-    var distances = 0.0
     cfor(0)(_ < img.getWidth, _ + 1) { col =>
       cfor(0)(_ < img.getHeight, _ + 1) { row =>
         img.getRGB(col, row).isTransparent should be (true)
       }
     }
   }
+
+  test("should render rgb raster") {
+    val baseDataPath = "raster/data/jpg"
+    val path = s"$baseDataPath/tiled_compressed.tif"
+    val pathExpected = s"$baseDataPath/tiled_compressed_expected.jpg"
+
+    val expected = Files.readAllBytes(Paths.get(pathExpected))
+    val actual = GeoTiffRasterSource(path).tiff.tile.renderJpg().bytes
+
+    actual should contain theSameElementsAs expected
+  }
 }

raster/src/test/scala/geotrellis/raster/render/png/RenderPngTests.scala

@@ -17,15 +17,16 @@
 package geotrellis.raster.render.png
 
 import geotrellis.raster._
-import geotrellis.raster.render.{RGB, RGBA, Png}
+import geotrellis.raster.geotiff.GeoTiffRasterSource
+import geotrellis.raster.render.{ColorMap, ColorRamp, Png, RGB, RGBA}
 import geotrellis.raster.testkit._
-
 import spire.syntax.cfor._
-import org.scalatest._
 
 import java.io._
 import javax.imageio._
 
+import org.scalatest._
+
 class RenderPngTests extends FunSuite with Matchers with TileBuilders with RasterMatchers {
   def testPng(png: Png, tile: Tile, colorMap: ColorMap): Unit = {
     val img = ImageIO.read(new ByteArrayInputStream(png.bytes))
@@ -376,4 +377,148 @@ class RenderPngTests extends FunSuite with Matchers with TileBuilders with Raste
       }
     }
   }
+
+  test("RGBA raster should respect NoData colors set") {
+    val baseDataPath = "raster/data"
+    val path = s"$baseDataPath/rgba-test.tif"
+
+    val tiff = GeoTiffRasterSource(path).tiff
+    val colors = Vector(0x000000FF, 0xFFFFFFFF)
+    val colorRamp = ColorRamp(colors)
+    val (min, max) = 54D -> 255D
+    val steps = 255
+
+    val step = (max - min) / steps
+    val breaks = for (j <- 0 until steps) yield min + j * step
+
+    val singleBandTile = tiff.raster.tile.band(0)
+
+    val cmap =
+      ColorMap(breaks.toArray, ColorRamp(colorRamp.stops(steps)), ColorMapOptions.DEFAULT)
+        .withFallbackColor(colors.last)
+
+    val renderedTile = cmap.render(singleBandTile)
+
+    val r = renderedTile.map(_.red).interpretAs(UByteCellType)
+    val g = renderedTile.map(_.green).interpretAs(UByteCellType)
+    val b = renderedTile.map(_.blue).interpretAs(UByteCellType)
+    val a = renderedTile.map(_.alpha).interpretAs(UByteCellType)
+
+    val rmm = r.findMinMax
+    val gmm = g.findMinMax
+    val bmm = b.findMinMax
+    val amm = a.findMinMax
+
+    rmm shouldBe (0, 36)
+    gmm shouldBe (0, 36)
+    bmm shouldBe (0, 36)
+    amm shouldBe (0, 255)
+
+    val color = MultibandTile(r, g, b, a).color
+
+    color.findMinMax shouldBe (0, rmm._2 << 24 | gmm._2 << 16 | bmm._2 << 8 | amm._2)
+
+    cfor(0)(_ < color.cols, _ + 1) { col =>
+      cfor(0)(_ < color.rows, _ + 1) { row =>
+        val az = a.get(col, row)
+        val rz = r.get(col, row)
+        val gz = g.get(col, row)
+        val bz = b.get(col, row)
+        val z = color.get(col, row)
+
+        if(az == 0) z shouldBe 0
+        else z shouldBe rz << 24 | gz << 16 | bz << 8 | az
+      }
+    }
+  }
+
+  test("RGB raster should respect NoData colors set") {
+    val baseDataPath = "raster/data"
+    val path = s"$baseDataPath/rgba-test.tif"
+
+    val tiff = GeoTiffRasterSource(path).tiff
+    val colors = Vector(0x000000FF, 0xFFFFFFFF)
+    val colorRamp = ColorRamp(colors)
+    val (min, max) = 54D -> 255D
+    val steps = 255
+
+    val step = (max - min) / steps
+    val breaks = for (j <- 0 until steps) yield min + j * step
+
+    val singleBandTile = tiff.raster.tile.band(0)
+
+    val cmap =
+      ColorMap(breaks.toArray, ColorRamp(colorRamp.stops(steps)), ColorMapOptions.DEFAULT)
+        .withFallbackColor(colors.last)
+
+    val renderedTile = cmap.render(singleBandTile)
+
+    val r = renderedTile.map(_.red).interpretAs(IntUserDefinedNoDataCellType(36))
+    val g = renderedTile.map(_.green).interpretAs(IntUserDefinedNoDataCellType(36))
+    val b = renderedTile.map(_.blue).interpretAs(IntUserDefinedNoDataCellType(36))
+
+    val rmm = r.findMinMax
+    val gmm = g.findMinMax
+    val bmm = b.findMinMax
+
+    rmm shouldBe (0, 35)
+    gmm shouldBe (0, 35)
+    bmm shouldBe (0, 35)
+
+    val color = MultibandTile(r, g, b).color
+
+    color.findMinMax shouldBe (0, rmm._2 << 24 | gmm._2 << 16 | bmm._2 << 8 | 0xFF)
+
+    // original tiles not with the reinterpreted cellType
+    val rr = renderedTile.map(_.red)
+    val gg = renderedTile.map(_.green)
+    val bb = renderedTile.map(_.blue)
+
+    var expectedTransparentCounter = 0
+    var expectedNonTransparentZerosCounter = 0
+
+    var transparentCounter = 0
+    var nonTransparentZerosCounter = 0
+
+    cfor(0)(_ < color.cols, _ + 1) { col =>
+      cfor(0)(_ < color.rows, _ + 1) { row =>
+        val rz = r.get(col, row)
+        val gz = g.get(col, row)
+        val bz = b.get(col, row)
+
+        // count the amount of NoDatas and Zeros
+        // to prepare "Expected" accumulators
+        val rzz = rr.get(col, row)
+        val gzz = gg.get(col, row)
+        val bzz = bb.get(col, row)
+
+        if(rzz == 36 && gzz == 36 && bzz == 36)
+          expectedTransparentCounter += 1
+
+        if(rzz + gzz + bzz == 0)
+          expectedNonTransparentZerosCounter += 1
+
+        val transparent = isNoData(rz) && isNoData(gz) && isNoData(bz)
+
+        val z = color.get(col, row)
+
+        if(transparent) {
+          transparentCounter += 1
+          z shouldBe 0
+          rz shouldBe NODATA
+          gz shouldBe NODATA
+          bz shouldBe NODATA
+        }
+        else if(z == 0xFF) {
+          nonTransparentZerosCounter += 1
+          rz shouldNot be(NODATA)
+          gz shouldNot be(NODATA)
+          bz shouldNot be(NODATA)
+        } else z shouldBe rz << 24 | gz << 16 | bz << 8 | 0xFF
+      }
+    }
+
+    transparentCounter shouldBe expectedTransparentCounter
+    nonTransparentZerosCounter shouldBe expectedNonTransparentZerosCounter
+  }
 }