Improve handling of non-square avatars (#7025)

* Crop avatar before resizing (#1268)

Signed-off-by: Rob Watson <rfwatson@users.noreply.github.com>

* Fix spelling error

Signed-off-by: Rob Watson <rfwatson@users.noreply.github.com>

go.mod

@@ -90,6 +90,7 @@ require (
 	github.com/mschoch/smat v0.0.0-20160514031455-90eadee771ae // indirect
 	github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc
 	github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5
+	github.com/oliamb/cutter v0.2.2
 	github.com/philhofer/fwd v1.0.0 // indirect
 	github.com/pkg/errors v0.8.1 // indirect
 	github.com/pquerna/otp v0.0.0-20160912161815-54653902c20e

go.sum

@@ -244,6 +244,8 @@ github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc h1:z1PgdCCmYYVL0BoJT
 github.com/msteinert/pam v0.0.0-20151204160544-02ccfbfaf0cc/go.mod h1:np1wUFZ6tyoke22qDJZY40URn9Ae51gX7ljIWXN5TJs=
 github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5 h1:BvoENQQU+fZ9uukda/RzCAL/191HHwJA5b13R6diVlY=
 github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5/go.mod h1:jpp1/29i3P1S/RLdc7JQKbRpFeM1dOBd8T9ki5s+AY8=
+github.com/oliamb/cutter v0.2.2 h1:Lfwkya0HHNU1YLnGv2hTkzHfasrSMkgv4Dn+5rmlk3k=
+github.com/oliamb/cutter v0.2.2/go.mod h1:4BenG2/4GuRBDbVm/OPahDVqbrOemzpPiG5mi1iryBU=
 github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/ginkgo v1.7.0 h1:WSHQ+IS43OoUrWtD1/bbclrwK8TTH5hzp+umCiuxHgs=
 github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=

models/user.go

@@ -6,7 +6,6 @@
 package models
 
 import (
-	"bytes"
 	"container/list"
 	"crypto/md5"
 	"crypto/sha256"
@@ -14,7 +13,6 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"image"
 
 	// Needed for jpeg support
 	_ "image/jpeg"
@@ -39,7 +37,6 @@ import (
 	"github.com/go-xorm/builder"
 	"github.com/go-xorm/core"
 	"github.com/go-xorm/xorm"
-	"github.com/nfnt/resize"
 	"golang.org/x/crypto/pbkdf2"
 	"golang.org/x/crypto/ssh"
 )
@@ -457,24 +454,11 @@ func (u *User) IsPasswordSet() bool {
 // UploadAvatar saves custom avatar for user.
 // FIXME: split uploads to different subdirs in case we have massive users.
 func (u *User) UploadAvatar(data []byte) error {
-	imgCfg, _, err := image.DecodeConfig(bytes.NewReader(data))
+	m, err := avatar.Prepare(data)
 	if err != nil {
-		return fmt.Errorf("DecodeConfig: %v", err)
+		return err
-	}
-	if imgCfg.Width > setting.AvatarMaxWidth {
-		return fmt.Errorf("Image width is to large: %d > %d", imgCfg.Width, setting.AvatarMaxWidth)
-	}
-	if imgCfg.Height > setting.AvatarMaxHeight {
-		return fmt.Errorf("Image height is to large: %d > %d", imgCfg.Height, setting.AvatarMaxHeight)
-	}
-
-	img, _, err := image.Decode(bytes.NewReader(data))
-	if err != nil {
-		return fmt.Errorf("Decode: %v", err)
 	}
 
-	m := resize.Resize(avatar.AvatarSize, avatar.AvatarSize, img, resize.NearestNeighbor)
-
 	sess := x.NewSession()
 	defer sess.Close()
 	if err = sess.Begin(); err != nil {
@@ -497,7 +481,7 @@ func (u *User) UploadAvatar(data []byte) error {
 	}
 	defer fw.Close()
 
-	if err = png.Encode(fw, m); err != nil {
+	if err = png.Encode(fw, *m); err != nil {
 		return fmt.Errorf("Encode: %v", err)
 	}
 

modules/avatar/avatar.go

@@ -5,13 +5,20 @@
 package avatar
 
 import (
+	"bytes"
 	"fmt"
 	"image"
 	"image/color/palette"
+	// Enable PNG support:
+	_ "image/png"
 	"math/rand"
 	"time"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/issue9/identicon"
+	"github.com/nfnt/resize"
+	"github.com/oliamb/cutter"
 )
 
 // AvatarSize returns avatar's size
@@ -42,3 +49,46 @@ func RandomImageSize(size int, data []byte) (image.Image, error) {
 func RandomImage(data []byte) (image.Image, error) {
 	return RandomImageSize(AvatarSize, data)
 }
+
+// Prepare accepts a byte slice as input, validates it contains an image of an
+// acceptable format, and crops and resizes it appropriately.
+func Prepare(data []byte) (*image.Image, error) {
+	imgCfg, _, err := image.DecodeConfig(bytes.NewReader(data))
+	if err != nil {
+		return nil, fmt.Errorf("DecodeConfig: %v", err)
+	}
+	if imgCfg.Width > setting.AvatarMaxWidth {
+		return nil, fmt.Errorf("Image width is too large: %d > %d", imgCfg.Width, setting.AvatarMaxWidth)
+	}
+	if imgCfg.Height > setting.AvatarMaxHeight {
+		return nil, fmt.Errorf("Image height is too large: %d > %d", imgCfg.Height, setting.AvatarMaxHeight)
+	}
+
+	img, _, err := image.Decode(bytes.NewReader(data))
+	if err != nil {
+		return nil, fmt.Errorf("Decode: %v", err)
+	}
+
+	if imgCfg.Width != imgCfg.Height {
+		var newSize, ax, ay int
+		if imgCfg.Width > imgCfg.Height {
+			newSize = imgCfg.Height
+			ax = (imgCfg.Width - imgCfg.Height) / 2
+		} else {
+			newSize = imgCfg.Width
+			ay = (imgCfg.Height - imgCfg.Width) / 2
+		}
+
+		img, err = cutter.Crop(img, cutter.Config{
+			Width:  newSize,
+			Height: newSize,
+			Anchor: image.Point{ax, ay},
+		})
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	img = resize.Resize(AvatarSize, AvatarSize, img, resize.NearestNeighbor)
+	return &img, nil
+}

modules/avatar/avatar_test.go

@@ -5,8 +5,11 @@
 package avatar
 
 import (
+	"io/ioutil"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -17,3 +20,49 @@ func Test_RandomImage(t *testing.T) {
 	_, err = RandomImageSize(0, []byte("gogs@local"))
 	assert.Error(t, err)
 }
+
+func Test_PrepareWithPNG(t *testing.T) {
+	setting.AvatarMaxWidth = 4096
+	setting.AvatarMaxHeight = 4096
+
+	data, err := ioutil.ReadFile("testdata/avatar.png")
+	assert.NoError(t, err)
+
+	imgPtr, err := Prepare(data)
+	assert.NoError(t, err)
+
+	assert.Equal(t, 290, (*imgPtr).Bounds().Max.X)
+	assert.Equal(t, 290, (*imgPtr).Bounds().Max.Y)
+}
+
+func Test_PrepareWithJPEG(t *testing.T) {
+	setting.AvatarMaxWidth = 4096
+	setting.AvatarMaxHeight = 4096
+
+	data, err := ioutil.ReadFile("testdata/avatar.jpeg")
+	assert.NoError(t, err)
+
+	imgPtr, err := Prepare(data)
+	assert.NoError(t, err)
+
+	assert.Equal(t, 290, (*imgPtr).Bounds().Max.X)
+	assert.Equal(t, 290, (*imgPtr).Bounds().Max.Y)
+}
+
+func Test_PrepareWithInvalidImage(t *testing.T) {
+	setting.AvatarMaxWidth = 5
+	setting.AvatarMaxHeight = 5
+
+	_, err := Prepare([]byte{})
+	assert.EqualError(t, err, "DecodeConfig: image: unknown format")
+}
+func Test_PrepareWithInvalidImageSize(t *testing.T) {
+	setting.AvatarMaxWidth = 5
+	setting.AvatarMaxHeight = 5
+
+	data, err := ioutil.ReadFile("testdata/avatar.png")
+	assert.NoError(t, err)
+
+	_, err = Prepare(data)
+	assert.EqualError(t, err, "Image width is too large: 10 > 5")
+}

vendor/github.com/oliamb/cutter/.gitignore

@@ -0,0 +1,22 @@
+# Compiled Object files, Static and Dynamic libs (Shared Objects)
+*.o
+*.a
+*.so
+
+# Folders
+_obj
+_test
+
+# Architecture specific extensions/prefixes
+*.[568vq]
+[568vq].out
+
+*.cgo1.go
+*.cgo2.c
+_cgo_defun.c
+_cgo_gotypes.go
+_cgo_export.*
+
+_testmain.go
+
+*.exe

vendor/github.com/oliamb/cutter/.travis.yml

@@ -0,0 +1,6 @@
+language: go
+
+go:
+  - 1.0
+  - 1.1
+  - tip

vendor/github.com/oliamb/cutter/LICENSE

@@ -0,0 +1,20 @@
+The MIT License (MIT)
+
+Copyright (c) 2014 Olivier Amblet
+
+Permission is hereby granted, free of charge, to any person obtaining a copy of
+this software and associated documentation files (the "Software"), to deal in
+the Software without restriction, including without limitation the rights to
+use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
+the Software, and to permit persons to whom the Software is furnished to do so,
+subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
+IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.

vendor/github.com/oliamb/cutter/README.md

@@ -0,0 +1,107 @@
+Cutter
+======
+
+A Go library to crop images.
+
+[![Build Status](https://travis-ci.org/oliamb/cutter.png?branch=master)](https://travis-ci.org/oliamb/cutter)
+[![GoDoc](https://godoc.org/github.com/oliamb/cutter?status.png)](https://godoc.org/github.com/oliamb/cutter)
+
+Cutter was initially developped to be able
+to crop image resized using github.com/nfnt/resize.
+
+Usage
+-----
+
+Read the doc on https://godoc.org/github.com/oliamb/cutter
+
+Import package with
+
+```go
+import "github.com/oliamb/cutter"
+```
+
+Package cutter provides a function to crop image.
+
+By default, the original image will be cropped at the
+given size from the top left corner.
+
+```go
+croppedImg, err := cutter.Crop(img, cutter.Config{
+  Width:  250,
+  Height: 500,
+})
+```
+
+Most of the time, the cropped image will share some memory
+with the original, so it should be used read only. You must
+ask explicitely for a copy if nedded.
+
+```go
+croppedImg, err := cutter.Crop(img, cutter.Config{
+  Width:  250,
+  Height: 500,
+  Options: cutter.Copy,
+})
+```
+
+It is possible to specify the top left position:
+
+```go
+croppedImg, err := cutter.Crop(img, cutter.Config{
+  Width:  250,
+  Height: 500,
+  Anchor: image.Point{100, 100},
+  Mode:   cutter.TopLeft, // optional, default value
+})
+```
+
+The Anchor property can represents the center of the cropped image
+instead of the top left corner:
+
+```go
+croppedImg, err := cutter.Crop(img, cutter.Config{
+  Width: 250,
+  Height: 500,
+  Mode: cutter.Centered,
+})
+```
+
+The default crop use the specified dimension, but it is possible
+to use Width and Heigth as a ratio instead. In this case,
+the resulting image will be as big as possible to fit the asked ratio
+from the anchor position.
+
+```go
+croppedImg, err := cutter.Crop(baseImage, cutter.Config{
+  Width: 4,
+  Height: 3,
+  Mode: cutter.Centered,
+  Options: cutter.Ratio&cutter.Copy, // Copy is useless here
+})
+```
+
+About resize
+------------
+This lib only manage crop and won't resize image, but it works great in combination with [github.com/nfnt/resize](https://github.com/nfnt/resize)
+
+Contributing
+------------
+I'd love to see your contributions to Cutter. If you'd like to hack on it: 
+
+- fork the project,
+- hack on it,
+- ensure tests pass,
+- make a pull request
+
+If you plan to modify the API, let's disscuss it first.
+
+Licensing
+---------
+MIT License, Please see the file called LICENSE.
+
+Credits
+-------
+Test Picture: Gopher picture from Heidi Schuyt, http://www.flickr.com/photos/hschuyt/7674222278/,
+© copyright Creative Commons(http://creativecommons.org/licenses/by-nc-sa/2.0/)
+
+Thanks to Urturn(http://www.urturn.com) for the time allocated to develop the library.

vendor/github.com/oliamb/cutter/cutter.go

@@ -0,0 +1,192 @@
+/*
+Package cutter provides a function to crop image.
+
+By default, the original image will be cropped at the
+given size from the top left corner.
+
+		croppedImg, err := cutter.Crop(img, cutter.Config{
+		  Width:  250,
+		  Height: 500,
+		})
+
+Most of the time, the cropped image will share some memory
+with the original, so it should be used read only. You must
+ask explicitely for a copy if nedded.
+
+    croppedImg, err := cutter.Crop(img, cutter.Config{
+      Width:  250,
+      Height: 500,
+      Options: Copy,
+    })
+
+It is possible to specify the top left position:
+
+		croppedImg, err := cutter.Crop(img, cutter.Config{
+		  Width:  250,
+		  Height: 500,
+		  Anchor: image.Point{100, 100},
+		  Mode:   TopLeft, // optional, default value
+		})
+
+The Anchor property can represents the center of the cropped image
+instead of the top left corner:
+
+
+		croppedImg, err := cutter.Crop(img, cutter.Config{
+		  Width: 250,
+		  Height: 500,
+		  Mode: Centered,
+		})
+
+The default crop use the specified dimension, but it is possible
+to use Width and Heigth as a ratio instead. In this case,
+the resulting image will be as big as possible to fit the asked ratio
+from the anchor position.
+
+		croppedImg, err := cutter.Crop(baseImage, cutter.Config{
+		  Width: 4,
+		  Height: 3,
+		  Mode: Centered,
+		  Options: Ratio,
+		})
+*/
+package cutter
+
+import (
+	"image"
+	"image/draw"
+)
+
+// Config is used to defined
+// the way the crop should be realized.
+type Config struct {
+	Width, Height int
+	Anchor        image.Point // The Anchor Point in the source image
+	Mode          AnchorMode  // Which point in the resulting image the Anchor Point is referring to
+	Options       Option
+}
+
+// AnchorMode is an enumeration of the position an anchor can represent.
+type AnchorMode int
+
+const (
+	// TopLeft defines the Anchor Point
+	// as the top left of the cropped picture.
+	TopLeft AnchorMode = iota
+	// Centered defines the Anchor Point
+	// as the center of the cropped picture.
+	Centered = iota
+)
+
+// Option flags to modify the way the crop is done.
+type Option int
+
+const (
+	// Ratio flag is use when Width and Height
+	// must be used to compute a ratio rather
+	// than absolute size in pixels.
+	Ratio Option = 1 << iota
+	// Copy flag is used to enforce the function
+	// to retrieve a copy of the selected pixels.
+	// This disable the use of SubImage method
+	// to compute the result.
+	Copy = 1 << iota
+)
+
+// An interface that is
+// image.Image + SubImage method.
+type subImageSupported interface {
+	SubImage(r image.Rectangle) image.Image
+}
+
+// Crop retrieves an image that is a
+// cropped copy of the original img.
+//
+// The crop is made given the informations provided in config.
+func Crop(img image.Image, c Config) (image.Image, error) {
+	maxBounds := c.maxBounds(img.Bounds())
+	size := c.computeSize(maxBounds, image.Point{c.Width, c.Height})
+	cr := c.computedCropArea(img.Bounds(), size)
+	cr = img.Bounds().Intersect(cr)
+
+	if c.Options&Copy == Copy {
+		return cropWithCopy(img, cr)
+	}
+	if dImg, ok := img.(subImageSupported); ok {
+		return dImg.SubImage(cr), nil
+	}
+	return cropWithCopy(img, cr)
+}
+
+func cropWithCopy(img image.Image, cr image.Rectangle) (image.Image, error) {
+	result := image.NewRGBA(cr)
+	draw.Draw(result, cr, img, cr.Min, draw.Src)
+	return result, nil
+}
+
+func (c Config) maxBounds(bounds image.Rectangle) (r image.Rectangle) {
+	if c.Mode == Centered {
+		anchor := c.centeredMin(bounds)
+		w := min(anchor.X-bounds.Min.X, bounds.Max.X-anchor.X)
+		h := min(anchor.Y-bounds.Min.Y, bounds.Max.Y-anchor.Y)
+		r = image.Rect(anchor.X-w, anchor.Y-h, anchor.X+w, anchor.Y+h)
+	} else {
+		r = image.Rect(c.Anchor.X, c.Anchor.Y, bounds.Max.X, bounds.Max.Y)
+	}
+	return
+}
+
+// computeSize retrieve the effective size of the cropped image.
+// It is defined by Height, Width, and Ratio option.
+func (c Config) computeSize(bounds image.Rectangle, ratio image.Point) (p image.Point) {
+	if c.Options&Ratio == Ratio {
+		// Ratio option is on, so we take the biggest size available that fit the given ratio.
+		if float64(ratio.X)/float64(bounds.Dx()) > float64(ratio.Y)/float64(bounds.Dy()) {
+			p = image.Point{bounds.Dx(), (bounds.Dx() / ratio.X) * ratio.Y}
+		} else {
+			p = image.Point{(bounds.Dy() / ratio.Y) * ratio.X, bounds.Dy()}
+		}
+	} else {
+		p = image.Point{ratio.X, ratio.Y}
+	}
+	return
+}
+
+// computedCropArea retrieve the theorical crop area.
+// It is defined by Height, Width, Mode and
+func (c Config) computedCropArea(bounds image.Rectangle, size image.Point) (r image.Rectangle) {
+	min := bounds.Min
+	switch c.Mode {
+	case Centered:
+		rMin := c.centeredMin(bounds)
+		r = image.Rect(rMin.X-size.X/2, rMin.Y-size.Y/2, rMin.X-size.X/2+size.X, rMin.Y-size.Y/2+size.Y)
+	default: // TopLeft
+		rMin := image.Point{min.X + c.Anchor.X, min.Y + c.Anchor.Y}
+		r = image.Rect(rMin.X, rMin.Y, rMin.X+size.X, rMin.Y+size.Y)
+	}
+	return
+}
+
+func (c *Config) centeredMin(bounds image.Rectangle) (rMin image.Point) {
+	if c.Anchor.X == 0 && c.Anchor.Y == 0 {
+		rMin = image.Point{
+			X: bounds.Dx() / 2,
+			Y: bounds.Dy() / 2,
+		}
+	} else {
+		rMin = image.Point{
+			X: c.Anchor.X,
+			Y: c.Anchor.Y,
+		}
+	}
+	return
+}
+
+func min(a, b int) (r int) {
+	if a < b {
+		r = a
+	} else {
+		r = b
+	}
+	return
+}

vendor/modules.txt

@@ -261,6 +261,8 @@ github.com/mschoch/smat
 github.com/msteinert/pam
 # github.com/nfnt/resize v0.0.0-20160724205520-891127d8d1b5
 github.com/nfnt/resize
+# github.com/oliamb/cutter v0.2.2
+github.com/oliamb/cutter
 # github.com/pelletier/go-buffruneio v0.2.0
 github.com/pelletier/go-buffruneio
 # github.com/philhofer/fwd v1.0.0