Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Overviews Read Incorrectly when Per Dataset Masks Present bug #3271

Merged
merged 3 commits into from
Jul 16, 2020
Merged

Fix Overviews Read Incorrectly when Per Dataset Masks Present bug #3271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8df8ff9
GeoTiff Reader supports only ReducedImage IFDs
pomadchin Jul 15, 2020
d1ea372
Rename NewSubfileTypes
pomadchin Jul 15, 2020
a3d8ebd
Fix a typo in the NewSubfileType.fromCode function
pomadchin Jul 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- GeoTiffRasterSource is not threadsafe enough [#3265](https://github.com/locationtech/geotrellis/pull/3265)
- Fix ShortArrayTileResult result ArrayTile fulfillment [#3268](https://github.com/locationtech/geotrellis/pull/3268)
- GeoTiffRasterSource TiledToLayout reads by SpatialKey can produce non-256x256 tiles [3267](https://github.com/locationtech/geotrellis/issues/3267)
- Fix Overviews Read Incorrectly when Per Dataset Masks Present [#3269](https://github.com/locationtech/geotrellis/issues/3269)

## [3.4.0] - 2020-06-19

Expand Down
Binary file modified raster/data/geotiff-test-files.zip
View file
Open in desktop
Binary file not shown.
22 changes: 22 additions & 0 deletions raster/src/main/scala/geotrellis/raster/io/geotiff/NewSubfileType.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,44 @@ package geotrellis.raster.io.geotiff
/**
* A general indication of the kind of data contained in this subfile.
* URL: https://www.awaresystems.be/imaging/tiff/tifftags/newsubfiletype.html
* URL #2: https://exiftool.org/TagNames/EXIF.html
*/
abstract sealed class NewSubfileType extends Serializable { val code: Int }

/** Full resolution image */
case object FullResolutionImage extends NewSubfileType { val code = 0 }
/** Reduced-resolution version of another image in this TIFF file */
case object ReducedImage extends NewSubfileType { val code = 1 }
/** Single page of a multi-page image (see the PageNumber field description) */
case object Page extends NewSubfileType { val code = 2 }
/** Transparency mask for another image in this TIFF file */
case object Mask extends NewSubfileType { val code = 4 }
/** Reduced-resolution version of a transparency mask */
case object ReducedImageMask extends NewSubfileType { val code = 5 }
/** Transparency mask of multi-page image */
case object MultiPageMask extends NewSubfileType { val code = 6 }
/** Transparency mask of reduced-resolution multi-page image */
case object MultiPageReducedImageMask extends NewSubfileType { val code = 7 }
/** Depth map */
case object Depth extends NewSubfileType { val code = 8 }
/** Depth map of reduced-resolution image */
case object ReducedImageDepth extends NewSubfileType { val code = 9 }
/** Enhanced image data */
case object Enhanced extends NewSubfileType { val code = 10 }

object NewSubfileType {
def fromCode(code: Long): Option[NewSubfileType] = fromCode(code.toInt)
def fromCode(code: Int): Option[NewSubfileType] = code match {
case FullResolutionImage.code => Some(FullResolutionImage)
case ReducedImage.code => Some(ReducedImage)
case Page.code => Some(Page)
case Mask.code => Some(Mask)
case ReducedImageMask.code => Some(ReducedImageMask)
case MultiPageMask.code => Some(MultiPageMask)
case MultiPageReducedImageMask.code => Some(MultiPageReducedImageMask)
case Depth.code => Some(Depth)
case ReducedImageDepth.code => Some(ReducedImageDepth)
case Enhanced.code => Some(Enhanced)
case _ => None
}
}
12 changes: 10 additions & 2 deletions raster/src/main/scala/geotrellis/raster/io/geotiff/reader/GeoTiffInfo.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,21 @@ object GeoTiffInfo {
case Tiff =>
var ifdOffset = byteReader.getInt
while (ifdOffset > 0) {
tiffTagsBuffer += TiffTags.read(byteReader, ifdOffset)(IntTiffTagOffsetSize)
val ifdTiffTags = TiffTags.read(byteReader, ifdOffset)(IntTiffTagOffsetSize)
// TIFF Reader supports only overviews at this point
// Overview is a reduced-resolution IFD
val subfileType = ifdTiffTags.nonBasicTags.newSubfileType.flatMap(NewSubfileType.fromCode)
if(subfileType.contains(ReducedImage)) tiffTagsBuffer += ifdTiffTags
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that we're skipping everything that is not a ReducedImage (anyway we can't work with anything else)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 but also noting that this kind of comment is super helpful for a user to see in the method docstring. It would be helpful for the user to know at a glance something like "Only SubfileType.ReducedImage tiffs are supported".

ifdOffset = byteReader.getInt
}
case _ =>
var ifdOffset = byteReader.getLong
while (ifdOffset > 0) {
tiffTagsBuffer += TiffTags.read(byteReader, ifdOffset)(LongTiffTagOffsetSize)
val ifdTiffTags = TiffTags.read(byteReader, ifdOffset)(LongTiffTagOffsetSize)
// TIFF Reader supports only overviews at this point
// Overview is a reduced-resolution IFD
val subfileType = ifdTiffTags.nonBasicTags.newSubfileType.flatMap(NewSubfileType.fromCode)
if(subfileType.contains(ReducedImage)) tiffTagsBuffer += ifdTiffTags
ifdOffset = byteReader.getLong
}
}
Expand Down
2 changes: 0 additions & 2 deletions raster/src/main/scala/geotrellis/raster/io/geotiff/tags/GeoKeyDirectory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ package geotrellis.raster.io.geotiff.tags

import ProjectionTypesMap._
import geotrellis.raster.io.geotiff.reader.MalformedGeoTiffException
import geotrellis.raster.io.geotiff.util._
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 big fan of removal of unused imports whenever possible!


import monocle.syntax._
import monocle.macros.Lenses
import io.circe._
import io.circe.generic.semiauto._
Expand Down
27 changes: 23 additions & 4 deletions raster/src/test/scala/geotrellis/raster/io/geotiff/reader/SinglebandGeoTiffReaderSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ import geotrellis.util.Filesystem
import spire.syntax.cfor._
import org.scalatest._

class SinglebandGeoTiffReaderSpec extends FunSpec
with RasterMatchers
with GeoTiffTestUtils {

class SinglebandGeoTiffReaderSpec extends FunSpec with RasterMatchers with GeoTiffTestUtils {
def geoTiff(storage: String, cellType: String): SinglebandGeoTiff =
SinglebandGeoTiff(geoTiffPath(s"uncompressed/$storage/${cellType}.tif"))

Expand Down Expand Up @@ -137,6 +134,28 @@ class SinglebandGeoTiffReaderSpec extends FunSpec
}
}

it("should read tiff with masks and mask overviews correct (skip everything that is not a reduced image)") {
// sizes of overviews, starting with the base ifd
val sizes = List(1024 -> 1024, 512 -> 512)

val tiff = SinglebandGeoTiff(geoTiffPath("overviews/per-dataset-mask.tif"))
val tile = tiff.tile

tiff.getOverviewsCount should be (1)
tile.isNoDataTile should be (false)

tile.cols -> tile.rows should be (sizes(0))

tiff.overviews.zip(sizes.tail).foreach { case (ovrTiff, ovrSize) =>
val ovrTile = ovrTiff.tile

ovrTiff.getOverviewsCount should be (0)
ovrTile.isNoDataTile should be (false)

ovrTile.cols -> ovrTile.rows should be (ovrSize)
}
}

it("should pick up the tiff overview correct (AutoHigherResolution test)") {
def cellSizesSequence(cellSize: CellSize): List[CellSize] = {
val CellSize(w, h) = cellSize
Expand Down