diff --git a/pkg/images/bench_test.go b/pkg/images/bench_test.go index 30e3a7e0f..13f6a8dc3 100644 --- a/pkg/images/bench_test.go +++ b/pkg/images/bench_test.go @@ -27,7 +27,7 @@ func benchRescale(b *testing.B, w, h, thumbW, thumbH int) { o := &DecodeOpts{MaxWidth: thumbW, MaxHeight: thumbH} b.ResetTimer() for i := 0; i < b.N; i++ { - _ = rescale(im, o) + _ = rescale(im, o, false) } } diff --git a/pkg/images/images.go b/pkg/images/images.go index 47871a2d0..3af88cfe4 100644 --- a/pkg/images/images.go +++ b/pkg/images/images.go @@ -205,14 +205,19 @@ func ScaledDimensions(w, h, mw, mh int) (newWidth int, newHeight int) { return } -func rescale(im image.Image, opts *DecodeOpts) image.Image { +func rescale(im image.Image, opts *DecodeOpts, swapDimensions bool) image.Image { mw, mh := opts.MaxWidth, opts.MaxHeight mwf, mhf := opts.ScaleWidth, opts.ScaleHeight b := im.Bounds() // only do downscaling, otherwise just serve the original image - if !opts.wantRescale(b) { + if !opts.wantRescale(b, swapDimensions) { return im } + + if swapDimensions { + mw, mh = mh, mw + } + // ScaleWidth and ScaleHeight overrule MaxWidth and MaxHeight if mwf > 0.0 && mwf <= 1 { mw = int(mwf * float32(b.Dx())) @@ -220,7 +225,6 @@ func rescale(im image.Image, opts *DecodeOpts) image.Image { if mhf > 0.0 && mhf <= 1 { mh = int(mhf * float32(b.Dy())) } - // If it's gigantic, it's more efficient to downsample first // and then resize; resizing will smooth out the roughness. // (trusting the moustachio guys on that one). @@ -233,12 +237,36 @@ func rescale(im image.Image, opts *DecodeOpts) image.Image { return resize.Resize(im, b, mw, mh) } -func (opts *DecodeOpts) wantRescale(b image.Rectangle) bool { - return opts != nil && - (opts.MaxWidth > 0 && opts.MaxWidth < b.Dx() || - opts.MaxHeight > 0 && opts.MaxHeight < b.Dy() || - opts.ScaleWidth > 0.0 && opts.ScaleWidth < float32(b.Dx()) || - opts.ScaleHeight > 0.0 && opts.ScaleHeight < float32(b.Dy())) +func (opts *DecodeOpts) wantRescale(b image.Rectangle, swapDimensions bool) bool { + if opts == nil { + return false + } + + // In rescale Scale* trumps Max* so we assume the same relationship here. + + // Floating point compares probably only allow this to work if the values + // were specified as the literal 1 or 1.0, computed values will likely be + // off. If Scale{Width,Height} end up being 1.0-epsilon we'll rescale + // when it probably wouldn't even be noticible but that's okay. + if opts.ScaleWidth == 1.0 && opts.ScaleHeight == 1.0 { + return false + } + if opts.ScaleWidth > 0 && opts.ScaleWidth < 1.0 || + opts.ScaleHeight > 0 && opts.ScaleHeight < 1.0 { + return true + } + + w, h := b.Dx(), b.Dy() + if swapDimensions { + w, h = h, w + } + + // Same size, don't rescale. + if opts.MaxWidth == w && opts.MaxHeight == h { + return false + } + return opts.MaxWidth > 0 && opts.MaxWidth < w || + opts.MaxHeight > 0 && opts.MaxHeight < h } func (opts *DecodeOpts) forcedRotate() bool { @@ -318,8 +346,8 @@ func Decode(r io.Reader, opts *DecodeOpts) (image.Image, Config, error) { ex, err := exif.Decode(tr) maybeRescale := func() (image.Image, Config, error) { im, format, err := image.Decode(io.MultiReader(&buf, r)) - if err == nil && opts.wantRescale(im.Bounds()) { - im = rescale(im, opts) + if err == nil && opts.wantRescale(im.Bounds(), false) { + im = rescale(im, opts, false) c.Modified = true } c.Format = format @@ -379,8 +407,15 @@ func Decode(r io.Reader, opts *DecodeOpts) (image.Image, Config, error) { return nil, c, err } rescaled := false - if opts.wantRescale(im.Bounds()) { - im = rescale(im, opts) + // Orientation changing rotations should have their dimensions swapped + // when scaling. + var swapDimensions bool + switch angle { + case 90, -90: + swapDimensions = true + } + if opts.wantRescale(im.Bounds(), swapDimensions) { + im = rescale(im, opts, swapDimensions) rescaled = true } im = flip(rotate(im, angle), flipMode) diff --git a/pkg/images/images_test.go b/pkg/images/images_test.go index f4c830451..1b81c3060 100644 --- a/pkg/images/images_test.go +++ b/pkg/images/images_test.go @@ -21,6 +21,7 @@ import ( "image/jpeg" "os" "path/filepath" + "sort" "strings" "testing" "time" @@ -31,6 +32,9 @@ import ( const datadir = "testdata" func equals(im1, im2 image.Image) bool { + if !im1.Bounds().Eq(im2.Bounds()) { + return false + } for y := 0; y < im1.Bounds().Dy(); y++ { for x := 0; x < im1.Bounds().Dx(); x++ { r1, g1, b1, a1 := im1.At(x, y).RGBA() @@ -79,6 +83,7 @@ func sampleNames(t *testing.T) []string { if err != nil { t.Fatal(err) } + sort.Strings(samples) return samples } @@ -175,8 +180,29 @@ func TestRescale(t *testing.T) { smallIm := smallStraightFImage(t) + gotB, wantB := rescaledIm.Bounds(), smallIm.Bounds() + if !gotB.Eq(wantB) { + t.Errorf("(scale) %v bounds not equal, got %v want %v", name, gotB, wantB) + } if !equals(rescaledIm, smallIm) { - t.Fatalf("%v not properly rescaled", name) + t.Errorf("(scale) %v pixels not equal", name) + } + + _, err = f.Seek(0, os.SEEK_SET) + if err != nil { + t.Fatal(err) + } + + rescaledIm, _, err = Decode(f, &DecodeOpts{MaxWidth: 2000, MaxHeight: 40}) + if err != nil { + t.Fatal(err) + } + gotB = rescaledIm.Bounds() + if !gotB.Eq(wantB) { + t.Errorf("(max) %v bounds not equal, got %v want %v", name, gotB, wantB) + } + if !equals(rescaledIm, smallIm) { + t.Errorf("(max) %v pixels not equal", name) } } @@ -203,8 +229,29 @@ func TestRescaleEXIF(t *testing.T) { t.Fatal(err) } + gotB, wantB := rescaledIm.Bounds(), smallStraightF.Bounds() + if !gotB.Eq(wantB) { + t.Errorf("(scale) %v bounds not equal, got %v want %v", name, gotB, wantB) + } if !equals(rescaledIm, smallStraightF) { - t.Fatalf("%v not properly rescaled", name) + t.Errorf("(scale) %v pixels not equal", name) + } + + _, err = f.Seek(0, os.SEEK_SET) + if err != nil { + t.Fatal(err) + } + rescaledIm, _, err = Decode(f, &DecodeOpts{MaxWidth: 2000, MaxHeight: 40}) + if err != nil { + t.Fatal(err) + } + + gotB = rescaledIm.Bounds() + if !gotB.Eq(wantB) { + t.Errorf("(max) %v bounds not equal, got %v want %v", name, gotB, wantB) + } + if !equals(rescaledIm, smallStraightF) { + t.Errorf("(max) %v pixels not equal", name) } } } diff --git a/pkg/index/receive.go b/pkg/index/receive.go index 37c901f6d..d2faa847f 100644 --- a/pkg/index/receive.go +++ b/pkg/index/receive.go @@ -264,7 +264,10 @@ func (ix *Index) populateFile(b *schema.Blob, mm *mutationMap) (err error) { var copyDest io.Writer = sha1 var imageBuf *keepFirstN // or nil if strings.HasPrefix(mime, "image/") { - imageBuf = &keepFirstN{N: 256 << 10} + // Emperically derived 1MiB assuming CR2 images require more than any + // other filetype we support: + // https://gist.github.com/wathiede/7982372 + imageBuf = &keepFirstN{N: 1 << 20} copyDest = io.MultiWriter(copyDest, imageBuf) } size, err := io.Copy(copyDest, reader)