From 73c41ef808464b4f6c7c329a3c73a62fd370ca6c Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Fri, 9 Oct 2020 08:47:20 +0200 Subject: [PATCH] [MM-28638] Improve image thumbnail generation logic (#15534) * Improve image thumbnail generation logic * Improve naming --- api4/file_test.go | 24 +++++++++++++ app/file.go | 43 +++--------------------- app/image.go | 50 ++++++++++++++++++++++++++++ tests/10000x1.png | Bin 0 -> 283 bytes tests/10000x1_expected_preview.jpeg | Bin 0 -> 1076 bytes tests/10000x1_expected_thumb.jpeg | Bin 0 -> 628 bytes tests/1x10000.png | Bin 0 -> 307 bytes tests/1x10000_expected_preview.jpeg | Bin 0 -> 3096 bytes tests/1x10000_expected_thumb.jpeg | Bin 0 -> 624 bytes 9 files changed, 78 insertions(+), 39 deletions(-) create mode 100644 app/image.go create mode 100644 tests/10000x1.png create mode 100644 tests/10000x1_expected_preview.jpeg create mode 100644 tests/10000x1_expected_thumb.jpeg create mode 100644 tests/1x10000.png create mode 100644 tests/1x10000_expected_preview.jpeg create mode 100644 tests/1x10000_expected_thumb.jpeg diff --git a/api4/file_test.go b/api4/file_test.go index 9cb271aaa0..19423351d4 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -380,6 +380,30 @@ func TestUploadFiles(t *testing.T) { expectedImageMiniPreview: []bool{true}, expectedCreatorId: th.BasicUser.Id, }, + // Extremely wide image test + { + title: "Happy image thumbnail/preview 10", + names: []string{"10000x1.png"}, + expectedImageThumbnailNames: []string{"10000x1_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"10000x1_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{10000}, + expectedImageHeights: []int{1}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, + // Extremely high image test + { + title: "Happy image thumbnail/preview 11", + names: []string{"1x10000.png"}, + expectedImageThumbnailNames: []string{"1x10000_expected_thumb.jpeg"}, + expectedImagePreviewNames: []string{"1x10000_expected_preview.jpeg"}, + expectImage: true, + expectedImageWidths: []int{1}, + expectedImageHeights: []int{10000}, + expectedImageHasPreview: []bool{true}, + expectedCreatorId: th.BasicUser.Id, + }, { title: "Happy admin", client: th.SystemAdminClient, diff --git a/app/file.go b/app/file.go index 98c45df7c0..87c439e45e 100644 --- a/app/file.go +++ b/app/file.go @@ -838,31 +838,16 @@ func (t *UploadFileTask) postprocessImage() { } } - w := decoded.Bounds().Dx() - h := decoded.Bounds().Dy() - var wg sync.WaitGroup wg.Add(3) go func() { defer wg.Done() - thumb := decoded - if h > ImageThumbnailHeight || w > ImageThumbnailWidth { - if float64(h)/float64(w) < ImageThumbnailRatio { - thumb = imaging.Resize(decoded, 0, ImageThumbnailHeight, imaging.Lanczos) - } else { - thumb = imaging.Resize(decoded, ImageThumbnailWidth, 0, imaging.Lanczos) - } - } - writeJPEG(thumb, t.fileinfo.ThumbnailPath) + writeJPEG(genThumbnail(decoded), t.fileinfo.ThumbnailPath) }() go func() { defer wg.Done() - preview := decoded - if w > ImagePreviewWidth { - preview = imaging.Resize(decoded, ImagePreviewWidth, 0, imaging.Lanczos) - } - writeJPEG(preview, t.fileinfo.PreviewPath) + writeJPEG(genPreview(decoded), t.fileinfo.PreviewPath) }() go func() { @@ -1083,22 +1068,8 @@ func getImageOrientation(input io.Reader) (int, error) { } func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string, width int, height int) { - thumbWidth := float64(IMAGE_THUMBNAIL_PIXEL_WIDTH) - thumbHeight := float64(IMAGE_THUMBNAIL_PIXEL_HEIGHT) - imgWidth := float64(width) - imgHeight := float64(height) - - var thumbnail image.Image - if imgHeight < IMAGE_THUMBNAIL_PIXEL_HEIGHT && imgWidth < thumbWidth { - thumbnail = img - } else if imgHeight/imgWidth < thumbHeight/thumbWidth { - thumbnail = imaging.Resize(img, 0, IMAGE_THUMBNAIL_PIXEL_HEIGHT, imaging.Lanczos) - } else { - thumbnail = imaging.Resize(img, IMAGE_THUMBNAIL_PIXEL_WIDTH, 0, imaging.Lanczos) - } - buf := new(bytes.Buffer) - if err := jpeg.Encode(buf, thumbnail, &jpeg.Options{Quality: 90}); err != nil { + if err := jpeg.Encode(buf, genThumbnail(img), &jpeg.Options{Quality: 90}); err != nil { mlog.Error("Unable to encode image as jpeg", mlog.String("path", thumbnailPath), mlog.Err(err)) return } @@ -1110,13 +1081,7 @@ func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string, widt } func (a *App) generatePreviewImage(img image.Image, previewPath string, width int) { - var preview image.Image - - if width > IMAGE_PREVIEW_PIXEL_WIDTH { - preview = imaging.Resize(img, IMAGE_PREVIEW_PIXEL_WIDTH, 0, imaging.Lanczos) - } else { - preview = img - } + preview := genPreview(img) buf := new(bytes.Buffer) diff --git a/app/image.go b/app/image.go new file mode 100644 index 0000000000..a2f8ea10b2 --- /dev/null +++ b/app/image.go @@ -0,0 +1,50 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "image" + + "github.com/disintegration/imaging" +) + +func genThumbnail(img image.Image) image.Image { + thumb := img + w := img.Bounds().Dx() + h := img.Bounds().Dy() + + if h > ImageThumbnailHeight || w > ImageThumbnailWidth { + ratio := float64(h) / float64(w) + if ratio < ImageThumbnailRatio { + // we pre-calculate the thumbnail's width to make sure we are not upscaling. + targetWidth := int(float64(ImageThumbnailHeight) * float64(w) / float64(h)) + if targetWidth <= w { + thumb = imaging.Resize(img, 0, ImageThumbnailHeight, imaging.Lanczos) + } else { + thumb = imaging.Resize(img, ImageThumbnailWidth, 0, imaging.Lanczos) + } + } else { + // we pre-calculate the thumbnail's height to make sure we are not upscaling. + targetHeight := int(float64(ImageThumbnailWidth) * float64(h) / float64(w)) + if targetHeight <= h { + thumb = imaging.Resize(img, ImageThumbnailWidth, 0, imaging.Lanczos) + } else { + thumb = imaging.Resize(img, 0, ImageThumbnailHeight, imaging.Lanczos) + } + } + } + + return thumb +} + +func genPreview(img image.Image) image.Image { + preview := img + w := img.Bounds().Dx() + + if w > ImagePreviewWidth { + preview = imaging.Resize(img, ImagePreviewWidth, 0, imaging.Lanczos) + } + + return preview +} diff --git a/tests/10000x1.png b/tests/10000x1.png new file mode 100644 index 0000000000000000000000000000000000000000..2f02142202e231133f444768828c21490421c773 GIT binary patch literal 283 zcmeAS@N?(olHy`uVBq!ia0y~yP!|BQ85x;@q?3S}2asY(cl32+VA$Bt{U?zX$X7`A z2=ZlMs8VBKXlP+z_yrVdc)`F>YQVtoDuIE)Y6b&?c)^@qfi^%1wg8_H*Z&L*|NsAY zlr<3qiZCX5ySp&{XVSd~UftDnm{r-UW|IwwVd literal 0 HcmV?d00001 diff --git a/tests/10000x1_expected_preview.jpeg b/tests/10000x1_expected_preview.jpeg new file mode 100644 index 0000000000000000000000000000000000000000..c547e21eefc9f60da050b4f7d8a8e0f352931f70 GIT binary patch literal 1076 zcmex=nP38T>!MAjrYM$lk!rsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T T{9k=E7IiU<;^7$v|8D{SFo9Ub literal 0 HcmV?d00001 diff --git a/tests/10000x1_expected_thumb.jpeg b/tests/10000x1_expected_thumb.jpeg new file mode 100644 index 0000000000000000000000000000000000000000..90f2925324ff095484083398f9111f8bf51a8324 GIT binary patch literal 628 zcmex=nP38T>!MAjrYM$WXz|sKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T Q{9k=E7IiV;!vAjq0Em{u=Kufz literal 0 HcmV?d00001 diff --git a/tests/1x10000.png b/tests/1x10000.png new file mode 100644 index 0000000000000000000000000000000000000000..0a3fab5044b40962dc13b5d0ea4491986c6ac1e4 GIT binary patch literal 307 zcmeAS@N?(olHy`uVBq!ia0vp^j0_Cw0*uT+)^qQMXF!T2-O<;Pfnj4m_n$;oAYUQb zBgmJ5p-PQ`p`nF=;TKS-;RORjsR0ASs{{rHs~HRo;stYd1=;{5*aCb)T>mpL{Qv*o zQPxBdD8iWJ?e4<(pGo%~ki%Z$>Fdh=gj0xHS;tKEml04%%hSa%q~g}wgN8tdFdSO& z)jyY)e*ut*4k`@%6c`wrPv{8&WmHRCBT7;dOH!?pi&B9UgOP!efv$n2uAxPUp`n$r rsg;R|wt<0_fkCFNKn;q9-29Zxv`X9>BobB}0NL&7>gTe~DWM4fB>YIC literal 0 HcmV?d00001 diff --git a/tests/1x10000_expected_preview.jpeg b/tests/1x10000_expected_preview.jpeg new file mode 100644 index 0000000000000000000000000000000000000000..e2511cb26fca3274a9d8617f7c2aab5eccc3adec GIT binary patch literal 3096 zcmex=nP38T>!MAjqLEz`)4NsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T o{9k=E7IiU<;?Xb|O#`E8U^ESkrh(BkFq#HN)4nP38T>!MAjrXx!obMPsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T Q{9k=E7IiUT!~bsr09t{