tiff: consistently skip horizontal padding in tiled images The rightmost tiles of a tiled image can extend past the right edge of the image. When this happens, we should skip over the out-of-bounds data. For example, consider a 2x2 image with an 4x4 tile size: xx.. xx.. .... .... When decoding, we must skip over the padding (dots) at the end of each row. This skip was present for 16bpp greyscale images, but missing for a number of other cases. Add the missing skips where absent. Add a collection of test images for various image formats containing a tile larger than the image size. (Credit to Gemini for pointing out the bug and creating the test data.) Change-Id: I72cc342deb3f3a714ab7aa8f8111c7bd6a6a6964 Reviewed-on: https://go-review.googlesource.com/c/image/+/790222 Auto-Submit: Damien Neil <dneil@google.com> SLSA-Policy-Verified: SLSA Policy Verification Service <devtools-gerritcodereview-exitgate@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Neal Patel <neal@golang.org> Reviewed-by: Neal Patel <nealpatel@google.com>
diff --git a/tiff/reader.go b/tiff/reader.go index a0cd4e8..41260e4 100644 --- a/tiff/reader.go +++ b/tiff/reader.go
@@ -342,13 +342,25 @@ } } + // calcRowBytes returns the number of bytes in a row with numSamples samples per pixel + // and bytesPerSample bytes per sample. + calcRowBytes := func(xmin, xmax, numSamples int, bitsPerSample uint) int { + return int((int64(xmax-xmin)*int64(numSamples)*int64(bitsPerSample) + 7) / 8) + } + // calcRowOff returns the offset of the next row. + calcRowOff := func(y, ymin, rowBytes int) int { + return (y - ymin) * rowBytes + } + rMaxX := minInt(xmax, dst.Bounds().Max.X) rMaxY := minInt(ymax, dst.Bounds().Max.Y) switch d.mode { case mGray, mGrayInvert: + rowBytes := calcRowBytes(xmin, xmax, 1, d.bpp) if d.bpp == 16 { img := dst.(*image.Gray16) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) for x := xmin; x < rMaxX; x++ { if d.off+2 > len(d.buf) { return errNoPixels @@ -360,14 +372,13 @@ } img.SetGray16(x, y, color.Gray16{v}) } - if rMaxX == img.Bounds().Max.X { - d.off += 2 * (xmax - img.Bounds().Max.X) - } } } else { img := dst.(*image.Gray) max := uint32((1 << d.bpp) - 1) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) + d.flushBits() for x := xmin; x < rMaxX; x++ { v, ok := d.readBits(d.bpp) if !ok { @@ -379,13 +390,15 @@ } img.SetGray(x, y, color.Gray{uint8(v)}) } - d.flushBits() } } case mPaletted: img := dst.(*image.Paletted) pLen := len(d.palette) + rowBytes := calcRowBytes(xmin, xmax, 1, d.bpp) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) + d.flushBits() for x := xmin; x < rMaxX; x++ { v, ok := d.readBits(d.bpp) if !ok { @@ -397,12 +410,13 @@ } img.SetColorIndex(x, y, idx) } - d.flushBits() } case mRGB: + rowBytes := calcRowBytes(xmin, xmax, 3, d.bpp) if d.bpp == 16 { img := dst.(*image.RGBA64) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) for x := xmin; x < rMaxX; x++ { if d.off+6 > len(d.buf) { return errNoPixels @@ -417,6 +431,7 @@ } else { img := dst.(*image.RGBA) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) min := img.PixOffset(xmin, y) max := img.PixOffset(rMaxX, y) off := (y - ymin) * (xmax - xmin) * 3 @@ -433,9 +448,11 @@ } } case mNRGBA: + rowBytes := calcRowBytes(xmin, xmax, 4, d.bpp) if d.bpp == 16 { img := dst.(*image.NRGBA64) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) for x := xmin; x < rMaxX; x++ { if d.off+8 > len(d.buf) { return errNoPixels @@ -461,9 +478,11 @@ } } case mRGBA: + rowBytes := calcRowBytes(xmin, xmax, 4, d.bpp) if d.bpp == 16 { img := dst.(*image.RGBA64) for y := ymin; y < rMaxY; y++ { + d.off = calcRowOff(y, ymin, rowBytes) for x := xmin; x < rMaxX; x++ { if d.off+8 > len(d.buf) { return errNoPixels
diff --git a/tiff/reader_test.go b/tiff/reader_test.go index bf59fdd..8bbce2b 100644 --- a/tiff/reader_test.go +++ b/tiff/reader_test.go
@@ -6,6 +6,7 @@ import ( "bytes" + "compress/bzip2" "compress/zlib" "encoding/binary" "encoding/hex" @@ -564,7 +565,7 @@ ifd = enc.AppendUint32(ifd, 0) case 1: ifd = enc.AppendUint16(ifd, v[0]) - ifd = enc.AppendUint16(ifd, v[1]) + ifd = enc.AppendUint16(ifd, 0) default: ifd = enc.AppendUint32(ifd, uint32(len(b))) for _, e := range v { @@ -768,3 +769,58 @@ } }) } + +func TestTiledPadding(t *testing.T) { + // This test uses a set of files with various color modes and bpp. + // Each contains a 15x15 image of a single black diagonal line on a white background, + // encoded with a tile size of 16x16. + // + // Verify that we correctly handle padding. + tests := []string{ + "gray1", + "gray1-invert", + "gray8", + "gray8-invert", + "gray16", + "paletted1", + "paletted8", + "rgb8", + "rgb16", + "rgba8", + "rgba16", + "nrgba8", + "nrgba16", + } + for _, filename := range tests { + t.Run(filename, func(t *testing.T) { + f, err := os.Open("testdata/tiled-" + filename + ".tiff.bz2") + if err != nil { + t.Fatal(err) + } + defer f.Close() + img, _, err := image.Decode(bzip2.NewReader(f)) + if err != nil { + t.Fatal(err) + } + + const wantx, wanty = 15, 15 + bounds := img.Bounds() + if bounds.Dx() != wantx || bounds.Dy() != wantx { + t.Fatalf("Wrong image size: %dx%d, want %dx%d", bounds.Dx(), bounds.Dy(), wantx, wanty) + } + + for y := range bounds.Dy() { + for x := range bounds.Dx() { + gotr, gotg, gotb, gota := img.At(x, y).RGBA() + var wantr, wantg, wantb, wanta uint32 = 0xffff, 0xffff, 0xffff, 0xffff + if x == y { + wantr, wantg, wantb, wanta = 0, 0, 0, 0xffff + } + if gotr != wantr || gotg != wantg || gotb != wantb || gota != wanta { + t.Fatalf("(x=%d, y=%d): got RGBA (%v, %v, %v, %v) want (%v, %v, %v, %v)", x, y, gotr, gotg, gotb, gota, wantr, wantg, wantb, wanta) + } + } + } + }) + } +}
diff --git a/tiff/testdata/tiled-gray1-invert.tiff.bz2 b/tiff/testdata/tiled-gray1-invert.tiff.bz2 new file mode 100644 index 0000000..78a5145 --- /dev/null +++ b/tiff/testdata/tiled-gray1-invert.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-gray1.tiff.bz2 b/tiff/testdata/tiled-gray1.tiff.bz2 new file mode 100644 index 0000000..a662e7c --- /dev/null +++ b/tiff/testdata/tiled-gray1.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-gray16.tiff.bz2 b/tiff/testdata/tiled-gray16.tiff.bz2 new file mode 100644 index 0000000..b1ccbe1 --- /dev/null +++ b/tiff/testdata/tiled-gray16.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-gray8-invert.tiff.bz2 b/tiff/testdata/tiled-gray8-invert.tiff.bz2 new file mode 100644 index 0000000..9b86df7 --- /dev/null +++ b/tiff/testdata/tiled-gray8-invert.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-gray8.tiff.bz2 b/tiff/testdata/tiled-gray8.tiff.bz2 new file mode 100644 index 0000000..1f2507c --- /dev/null +++ b/tiff/testdata/tiled-gray8.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-nrgba16.tiff.bz2 b/tiff/testdata/tiled-nrgba16.tiff.bz2 new file mode 100644 index 0000000..1e32a35 --- /dev/null +++ b/tiff/testdata/tiled-nrgba16.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-nrgba8.tiff.bz2 b/tiff/testdata/tiled-nrgba8.tiff.bz2 new file mode 100644 index 0000000..af63ddf --- /dev/null +++ b/tiff/testdata/tiled-nrgba8.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-paletted1.tiff.bz2 b/tiff/testdata/tiled-paletted1.tiff.bz2 new file mode 100644 index 0000000..c3168fa --- /dev/null +++ b/tiff/testdata/tiled-paletted1.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-paletted8.tiff.bz2 b/tiff/testdata/tiled-paletted8.tiff.bz2 new file mode 100644 index 0000000..91060d3 --- /dev/null +++ b/tiff/testdata/tiled-paletted8.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-rgb16.tiff.bz2 b/tiff/testdata/tiled-rgb16.tiff.bz2 new file mode 100644 index 0000000..44772f6 --- /dev/null +++ b/tiff/testdata/tiled-rgb16.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-rgb8.tiff.bz2 b/tiff/testdata/tiled-rgb8.tiff.bz2 new file mode 100644 index 0000000..f71b690 --- /dev/null +++ b/tiff/testdata/tiled-rgb8.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-rgba16.tiff.bz2 b/tiff/testdata/tiled-rgba16.tiff.bz2 new file mode 100644 index 0000000..6c0d6cc --- /dev/null +++ b/tiff/testdata/tiled-rgba16.tiff.bz2 Binary files differ
diff --git a/tiff/testdata/tiled-rgba8.tiff.bz2 b/tiff/testdata/tiled-rgba8.tiff.bz2 new file mode 100644 index 0000000..9961025 --- /dev/null +++ b/tiff/testdata/tiled-rgba8.tiff.bz2 Binary files differ