Repository: gohugoio/hugo (language: go)

Issue:
Avoid processing all GIF frames if targetFormat != GIF
This can potentially be a very costly operation.


Discussion / hints:

Fix patch:
diff --git a/resources/images/config.go b/resources/images/config.go
index 62b5c72d8cf..6f562ff8ece 100644
--- a/resources/images/config.go
+++ b/resources/images/config.go
@@ -60,6 +60,7 @@ var (
 	imageFormatsVersions = map[Format]int{
 		PNG:  3, // Fix transparency issue with 32 bit images.
 		WEBP: 2, // Fix transparency issue with 32 bit images.
+		GIF:  1, // Fix resize issue with animated GIFs when target != GIF.
 	}
 
 	// Increment to mark all processed images as stale. Only use when absolutely needed.
diff --git a/resources/images/image.go b/resources/images/image.go
index 4ffbaa2295a..9dc8ed408c3 100644
--- a/resources/images/image.go
+++ b/resources/images/image.go
@@ -247,7 +247,7 @@ func (p *ImageProcessor) ApplyFiltersFromConfig(src image.Image, conf ImageConfi
 		return nil, fmt.Errorf("unsupported action: %q", conf.Action)
 	}
 
-	img, err := p.Filter(src, filters...)
+	img, err := p.doFilter(src, conf.TargetFormat, filters...)
 	if err != nil {
 		return nil, err
 	}
@@ -256,25 +256,34 @@ func (p *ImageProcessor) ApplyFiltersFromConfig(src image.Image, conf ImageConfi
 }
 
 func (p *ImageProcessor) Filter(src image.Image, filters ...gift.Filter) (image.Image, error) {
+	return p.doFilter(src, 0, filters...)
+}
+
+func (p *ImageProcessor) doFilter(src image.Image, targetFormat Format, filters ...gift.Filter) (image.Image, error) {
 
 	filter := gift.New(filters...)
 
-	if giph, ok := src.(Giphy); ok && len(giph.GIF().Image) > 1 {
+	if giph, ok := src.(Giphy); ok {
 		g := giph.GIF()
-		var bounds image.Rectangle
-		firstFrame := g.Image[0]
-		tmp := image.NewNRGBA(firstFrame.Bounds())
-		for i := range g.Image {
-			gift.New().DrawAt(tmp, g.Image[i], g.Image[i].Bounds().Min, gift.OverOperator)
-			bounds = filter.Bounds(tmp.Bounds())
-			dst := image.NewPaletted(bounds, g.Image[i].Palette)
-			filter.Draw(dst, tmp)
-			g.Image[i] = dst
+		if len(g.Image) < 2 || (targetFormat == 0 || targetFormat != GIF) {
+			src = g.Image[0]
+		} else {
+			var bounds image.Rectangle
+			firstFrame := g.Image[0]
+			tmp := image.NewNRGBA(firstFrame.Bounds())
+			for i := range g.Image {
+				gift.New().DrawAt(tmp, g.Image[i], g.Image[i].Bounds().Min, gift.OverOperator)
+				bounds = filter.Bounds(tmp.Bounds())
+				dst := image.NewPaletted(bounds, g.Image[i].Palette)
+				filter.Draw(dst, tmp)
+				g.Image[i] = dst
+			}
+			g.Config.Width = bounds.Dx()
+			g.Config.Height = bounds.Dy()
+
+			return giph, nil
 		}
-		g.Config.Width = bounds.Dx()
-		g.Config.Height = bounds.Dy()
 
-		return giph, nil
 	}
 
 	bounds := filter.Bounds(src.Bounds())


Test patch:
diff --git a/resources/image_test.go b/resources/image_test.go
index 111ce66db2b..75a20f156b6 100644
--- a/resources/image_test.go
+++ b/resources/image_test.go
@@ -620,6 +620,8 @@ func TestImageOperationsGolden(t *testing.T) {
 
 	// Note, if you're enabling this on a MacOS M1 (ARM) you need to run the test with GOARCH=amd64.
 	// GOARCH=amd64 go test -timeout 30s -run "^TestImageOperationsGolden$" ./resources -v
+	// The above will print out a folder.
+	// Replace testdata/golden with resources/_gen/images in that folder.
 	devMode := false
 
 	testImages := []string{"sunset.jpg", "gohugoio8.png", "gohugoio24.png"}
@@ -663,7 +665,7 @@ func TestImageOperationsGolden(t *testing.T) {
 
 	// Animated GIF
 	orig = fetchImageForSpec(spec, c, "giphy.gif")
-	for _, resizeSpec := range []string{"200x", "512x"} {
+	for _, resizeSpec := range []string{"200x", "512x", "100x jpg"} {
 		resized, err := orig.Resize(resizeSpec)
 		c.Assert(err, qt.IsNil)
 		rel := resized.RelPermalink()
diff --git a/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_100x0_resize_q75_bgffffff_box_1.jpg b/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_100x0_resize_q75_bgffffff_box_1.jpg
new file mode 100644
index 00000000000..c55e5d3adbd
Binary files /dev/null and b/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_100x0_resize_q75_bgffffff_box_1.jpg differ
diff --git a/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_200x0_resize_box.gif b/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_200x0_resize_box_1.gif
similarity index 100%
rename from resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_200x0_resize_box.gif
rename to resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_200x0_resize_box_1.gif
diff --git a/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_512x0_resize_box.gif b/resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_512x0_resize_box_1.gif
similarity index 100%
rename from resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_512x0_resize_box.gif
rename to resources/testdata/golden/giphy_hu3eafc418e52414ace6236bf1d31f82e1_52213_512x0_resize_box_1.gif
diff --git a/resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_100x0_resize_box.gif b/resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_100x0_resize_box_1.gif
similarity index 100%
rename from resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_100x0_resize_box.gif
rename to resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_100x0_resize_box_1.gif
diff --git a/resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_220x0_resize_box.gif b/resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_220x0_resize_box_1.gif
similarity index 100%
rename from resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_220x0_resize_box.gif
rename to resources/testdata/golden/gohugoio-card_hu4d09f75255d3942fd4680641110a1a73_10820_220x0_resize_box_1.gif