Merge d773e5e into 1426ed3

gojek · Jul 16, 2019 · 32a37c0 · 32a37c0
2 parents 1426ed3 + d773e5e
commit 32a37c0
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 44 deletions.
diff --git a/config.yaml.example b/config.yaml.example
@@ -28,3 +28,5 @@ port: 3000
 
 cache:
   time: 31536000    # One year
+
+enableConcurrentImageProcessing: true
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -7,12 +7,13 @@ import (
 )
 
 type config struct {
-	logLevel  string
-	app       app
-	debugMode bool
-	port      int
-	cacheTime int
-	source    source
+	logLevel                        string
+	app                             app
+	debugMode                       bool
+	port                            int
+	cacheTime                       int
+	source                          source
+	enableConcurrentImageProcessing bool
 }
 
 var instance *config
@@ -54,10 +55,11 @@ func newConfig() *config {
 			version:     v.GetString("app.version"),
 			description: v.GetString("app.description"),
 		},
-		debugMode: v.GetBool("debug"),
-		port:      port,
-		cacheTime: v.GetInt("cache.time"),
-		source:    s,
+		debugMode:                       v.GetBool("debug"),
+		port:                            port,
+		cacheTime:                       v.GetInt("cache.time"),
+		source:                          s,
+		enableConcurrentImageProcessing: v.GetBool("enableConcurrentImageProcessing"),
 	}
 }
 
@@ -105,3 +107,8 @@ func CacheTime() int {
 func Source() *source {
 	return &getConfig().source
 }
+
+// ConcurrentImageProcessingEnabled returns true if we want to process image using multiple cores (checking isOpaque)
+func ConcurrentImageProcessingEnabled() bool {
+	return getConfig().enableConcurrentImageProcessing
+}
diff --git a/pkg/processor/native/encoder_test.go b/pkg/processor/native/encoder_test.go
@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/suite"
 	"image"
+	"image/draw"
 	"image/jpeg"
 	"image/png"
 	"io/ioutil"
@@ -14,8 +15,10 @@ import (
 
 type EncoderSuite struct {
 	suite.Suite
-	srcImage  image.Image
-	processor processor.Processor
+	srcImage         image.Image
+	processor        processor.Processor
+	opaqueImage      image.Image
+	transparentImage image.Image
 }
 
 func (s *EncoderSuite) SetupSuite() {
@@ -28,6 +31,13 @@ func (s *EncoderSuite) SetupSuite() {
 	if err != nil {
 		panic(err)
 	}
+	opaque := image.NewRGBA(image.Rect(0, 0, 640, 480))
+	draw.Draw(opaque, opaque.Bounds(), image.Opaque, image.ZP, draw.Src)
+	s.opaqueImage = opaque
+
+	transparent := image.NewRGBA(image.Rect(0, 0, 640, 480))
+	draw.Draw(transparent, transparent.Bounds(), image.Transparent, image.ZP, draw.Src)
+	s.transparentImage = transparent
 }
 
 func TestNewEncoders(t *testing.T) {
@@ -36,13 +46,13 @@ func TestNewEncoders(t *testing.T) {
 
 func (s *EncoderSuite) TestEncoders_GetEncoder() {
 	encoders := NewEncoders(DefaultCompressionOptions)
-	_, ok := (encoders.GetEncoder(image.Opaque, "jpg")).(*JpegEncoder)
+	_, ok := (encoders.GetEncoder(s.opaqueImage, "jpg")).(*JpegEncoder)
 	assert.True(s.T(), ok)
-	_, ok = (encoders.GetEncoder(image.Opaque, "jpeg")).(*JpegEncoder)
+	_, ok = (encoders.GetEncoder(s.opaqueImage, "jpeg")).(*JpegEncoder)
 	assert.True(s.T(), ok)
-	_, ok = (encoders.GetEncoder(image.Opaque, "png")).(*JpegEncoder)
+	_, ok = (encoders.GetEncoder(s.opaqueImage, "png")).(*JpegEncoder)
 	assert.True(s.T(), ok)
-	_, ok = (encoders.GetEncoder(image.Transparent, "png")).(*PngEncoder)
+	_, ok = (encoders.GetEncoder(s.transparentImage, "png")).(*PngEncoder)
 	assert.True(s.T(), ok)
 	_, ok = (encoders.GetEncoder(image.Black, "unknown")).(*NopEncoder)
 	assert.True(s.T(), ok)

diff --git a/pkg/processor/native/utils.go b/pkg/processor/native/utils.go
@@ -1,6 +1,8 @@
 package native
 
 import (
+	"github.com/anthonynsimon/bild/parallel"
+	"github.com/gojek/darkroom/pkg/config"
 	"github.com/gojek/darkroom/pkg/processor"
 	"image"
 )
@@ -14,14 +16,22 @@ func isOpaque(im image.Image) bool {
 	}
 	// No Opaque() method, we need to loop through all pixels and check manually:
 	rect := im.Bounds()
-	for y := rect.Min.Y; y < rect.Max.Y; y++ {
-		for x := rect.Min.X; x < rect.Max.X; x++ {
-			if _, _, _, a := im.At(x, y).RGBA(); a != 0xffff {
-				return false // Found a non-opaque pixel: image is non-opaque
+	isOpaque := true
+	f := func(start, end int) {
+		for y := rect.Min.Y + start; isOpaque && y < rect.Min.Y+end; y++ {
+			for x := rect.Min.X; isOpaque && x < rect.Max.X; x++ {
+				if _, _, _, a := im.At(x, y).RGBA(); a != 0xffff {
+					isOpaque = false // Found a non-opaque pixel: image is non-opaque
+				}
 			}
 		}
 	}
-	return true // All pixels are opaque, so is the image
+	if config.ConcurrentImageProcessingEnabled() {
+		parallel.Line(rect.Dy(), f)
+	} else {
+		f(rect.Min.Y, rect.Max.Y)
+	}
+	return isOpaque // All pixels are opaque, so is the image
 }
 
 // rw: required width, rh: required height, aw: actual width, ah: actual height

diff --git a/pkg/processor/native/utils_test.go b/pkg/processor/native/utils_test.go
@@ -5,6 +5,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"image"
 	"image/color"
+	"image/draw"
 	"testing"
 )
 
@@ -99,42 +100,64 @@ func TestGetStartingPointForCrop(t *testing.T) {
 	assert.Equal(t, 0, y)
 }
 
-func Test_isOpaqueWithoutOpaqueMethod(t *testing.T) {
-	im := &mockImage{opaque: false}
-	val := isOpaque(im)
-	assert.False(t, val)
-
-	im = &mockImage{opaque: true}
-	val = isOpaque(im)
+func Test_isOpaqueWithoutOpaqueMethodShouldReturnTrue(t *testing.T) {
+	img := NewMockImage(image.Rect(0, 0, 640, 480))
+	draw.Draw(img, img.Bounds(), image.Opaque, image.ZP, draw.Src)
+	val := isOpaque(img)
 	assert.True(t, val)
 }
 
+func Test_isOpaqueWithoutOpaqueMethodShouldReturnFalse(t *testing.T) {
+	w, h := 640, 480
+	img := NewMockImage(image.Rect(0, 0, w, h))
+	draw.Draw(img, img.Bounds(), image.Opaque, image.ZP, draw.Src)
+
+	cases := []struct {
+		x, y int
+	}{
+		{x: 0, y: 0},
+		{x: w / 2, y: h / 2},
+		{x: w - 1, y: h - 1},
+	}
+	for _, c := range cases {
+		// Flip only 1 bit to be transparent for each test case
+		x, y := c.x, c.y
+		img.Set(x, y, image.Transparent.C)
+		val := isOpaque(img)
+		assert.False(t, val)
+		img.Set(x, y, image.Opaque.C)
+	}
+}
+
 type mockImage struct {
-	opaque bool
+	rect   image.Rectangle
+	points [][]color.Color
+}
+
+func NewMockImage(rect image.Rectangle) *mockImage {
+	mockImg := &mockImage{
+		rect: rect,
+	}
+	points := make([][]color.Color, rect.Dy())
+	for i := range points {
+		points[i] = make([]color.Color, rect.Dx())
+	}
+	mockImg.points = points
+	return mockImg
 }
 
 func (im *mockImage) ColorModel() color.Model {
-	panic("implement me")
+	return color.RGBAModel
 }
 
 func (im *mockImage) Bounds() image.Rectangle {
-	return image.Rectangle{
-		Min: image.Point{X: 5, Y: 5},
-		Max: image.Point{X: 10, Y: 10},
-	}
+	return im.rect
 }
 
 func (im *mockImage) At(x, y int) color.Color {
-	return &mockColor{opaque: im.opaque}
+	return im.points[y][x]
 }
 
-type mockColor struct {
-	opaque bool
-}
-
-func (m *mockColor) RGBA() (r, g, b, a uint32) {
-	if m.opaque {
-		return 0x0fff, 0xf0ff, 0xff0f, 0xffff
-	}
-	return 0x0fff, 0xf0ff, 0xff0f, 0xfff0
+func (im *mockImage) Set(x, y int, c color.Color) {
+	im.points[y][x] = c
 }