From 5af045ebab109d3e5501b8b6d9fd448840c96c9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 15 Jun 2021 18:22:05 +0200 Subject: [PATCH] resources/image: Fix fill with smartcrop sometimes returning 0 bytes images Fixes #7955 --- resources/image.go | 19 ++++++- resources/image_test.go | 94 ++++++++++++++++++++++++----------- resources/images/smartcrop.go | 31 ++++++++++++ 3 files changed, 113 insertions(+), 31 deletions(-) diff --git a/resources/image.go b/resources/image.go index edf05639..282f008e 100644 --- a/resources/image.go +++ b/resources/image.go @@ -201,9 +201,26 @@ func (i *imageResource) Fill(spec string) (resource.Image, error) { return nil, err } - return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) { + img, err := i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) { return i.Proc.ApplyFiltersFromConfig(src, conf) }) + + if err != nil { + return nil, err + } + + if conf.Anchor == 0 && img.Width() == 0 || img.Height() == 0 { + // See https://github.com/gohugoio/hugo/issues/7955 + // Smartcrop fails silently in some rare cases. + // Fall back to a center fill. + conf.Anchor = gift.CenterAnchor + conf.AnchorStr = "center" + return i.doWithImageConfig(conf, func(src image.Image) (image.Image, error) { + return i.Proc.ApplyFiltersFromConfig(src, conf) + }) + } + + return img, err } func (i *imageResource) Filter(filters ...interface{}) (resource.Image, error) { diff --git a/resources/image_test.go b/resources/image_test.go index 8c69591c..cca961ee 100644 --- a/resources/image_test.go +++ b/resources/image_test.go @@ -175,36 +175,6 @@ func TestImageTransformFormat(t *testing.T) { assertFileCache(c, fileCache, path.Base(imageGif.RelPermalink()), 225, 141) } -// https://github.com/gohugoio/hugo/issues/4261 -func TestImageTransformLongFilename(t *testing.T) { - c := qt.New(t) - - image := fetchImage(c, "1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph.jpg") - c.Assert(image, qt.Not(qt.IsNil)) - - resized, err := image.Resize("200x") - c.Assert(err, qt.IsNil) - c.Assert(resized, qt.Not(qt.IsNil)) - c.Assert(resized.Width(), qt.Equals, 200) - c.Assert(resized.RelPermalink(), qt.Equals, "/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_65b757a6e14debeae720fe8831f0a9bc.jpg") - resized, err = resized.Resize("100x") - c.Assert(err, qt.IsNil) - c.Assert(resized, qt.Not(qt.IsNil)) - c.Assert(resized.Width(), qt.Equals, 100) - c.Assert(resized.RelPermalink(), qt.Equals, "/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_c876768085288f41211f768147ba2647.jpg") -} - -// Issue 6137 -func TestImageTransformUppercaseExt(t *testing.T) { - c := qt.New(t) - image := fetchImage(c, "sunrise.JPG") - - resized, err := image.Resize("200x") - c.Assert(err, qt.IsNil) - c.Assert(resized, qt.Not(qt.IsNil)) - c.Assert(resized.Width(), qt.Equals, 200) -} - // https://github.com/gohugoio/hugo/issues/5730 func TestImagePermalinkPublishOrder(t *testing.T) { for _, checkOriginalFirst := range []bool{true, false} { @@ -250,6 +220,70 @@ func TestImagePermalinkPublishOrder(t *testing.T) { } } +func TestImageBugs(t *testing.T) { + c := qt.New(t) + + // Issue #4261 + c.Run("Transform long filename", func(c *qt.C) { + image := fetchImage(c, "1234567890qwertyuiopasdfghjklzxcvbnm5to6eeeeee7via8eleph.jpg") + c.Assert(image, qt.Not(qt.IsNil)) + + resized, err := image.Resize("200x") + c.Assert(err, qt.IsNil) + c.Assert(resized, qt.Not(qt.IsNil)) + c.Assert(resized.Width(), qt.Equals, 200) + c.Assert(resized.RelPermalink(), qt.Equals, "/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_65b757a6e14debeae720fe8831f0a9bc.jpg") + resized, err = resized.Resize("100x") + c.Assert(err, qt.IsNil) + c.Assert(resized, qt.Not(qt.IsNil)) + c.Assert(resized.Width(), qt.Equals, 100) + c.Assert(resized.RelPermalink(), qt.Equals, "/a/_hu59e56ffff1bc1d8d122b1403d34e039f_90587_c876768085288f41211f768147ba2647.jpg") + + }) + + // Issue #6137 + c.Run("Transform upper case extension", func(c *qt.C) { + image := fetchImage(c, "sunrise.JPG") + + resized, err := image.Resize("200x") + c.Assert(err, qt.IsNil) + c.Assert(resized, qt.Not(qt.IsNil)) + c.Assert(resized.Width(), qt.Equals, 200) + + }) + + // Issue #7955 + c.Run("Fill with smartcrop", func(c *qt.C) { + sunset := fetchImage(c, "sunset.jpg") + + for _, test := range []struct { + originalDimensions string + targetWH int + }{ + {"408x403", 400}, + {"425x403", 400}, + {"459x429", 400}, + {"476x442", 400}, + {"544x403", 400}, + {"476x468", 400}, + {"578x585", 550}, + {"578x598", 550}, + } { + c.Run(test.originalDimensions, func(c *qt.C) { + image, err := sunset.Resize(test.originalDimensions) + c.Assert(err, qt.IsNil) + resized, err := image.Fill(fmt.Sprintf("%dx%d smart", test.targetWH, test.targetWH)) + c.Assert(err, qt.IsNil) + c.Assert(resized, qt.Not(qt.IsNil)) + c.Assert(resized.Width(), qt.Equals, test.targetWH) + c.Assert(resized.Height(), qt.Equals, test.targetWH) + }) + + } + + }) +} + func TestImageTransformConcurrent(t *testing.T) { var wg sync.WaitGroup diff --git a/resources/images/smartcrop.go b/resources/images/smartcrop.go index 76f547ef..864c6de0 100644 --- a/resources/images/smartcrop.go +++ b/resources/images/smartcrop.go @@ -15,6 +15,7 @@ package images import ( "image" + "math" "github.com/disintegration/gift" @@ -41,6 +42,14 @@ type imagingResizer struct { } func (r imagingResizer) Resize(img image.Image, width, height uint) image.Image { + // See https://github.com/gohugoio/hugo/issues/7955#issuecomment-861710681 + scaleX, scaleY := calcFactorsNfnt(width, height, float64(img.Bounds().Dx()), float64(img.Bounds().Dy())) + if width == 0 { + width = uint(math.Ceil(float64(img.Bounds().Dx()) / scaleX)) + } + if height == 0 { + height = uint(math.Ceil(float64(img.Bounds().Dy()) / scaleY)) + } result, _ := r.p.Filter(img, gift.Resize(int(width), int(height), r.filter)) return result } @@ -71,3 +80,25 @@ func (p *ImageProcessor) smartCrop(img image.Image, width, height int, filter gi return img.Bounds().Intersect(rect), nil } + +// Calculates scaling factors using old and new image dimensions. +// Code borrowed from https://github.com/nfnt/resize/blob/83c6a9932646f83e3267f353373d47347b6036b2/resize.go#L593 +func calcFactorsNfnt(width, height uint, oldWidth, oldHeight float64) (scaleX, scaleY float64) { + if width == 0 { + if height == 0 { + scaleX = 1.0 + scaleY = 1.0 + } else { + scaleY = oldHeight / float64(height) + scaleX = scaleY + } + } else { + scaleX = oldWidth / float64(width) + if height == 0 { + scaleY = scaleX + } else { + scaleY = oldHeight / float64(height) + } + } + return +}