Merge pull request #8 from adalberht/master

Rename concurrent opacity config and use own isOpaque method when built in implementation is not fast

pkg/config/config.go

@@ -13,7 +13,7 @@ type config struct {
 	port                            int
 	cacheTime                       int
 	source                          source
-	enableConcurrentImageProcessing bool
+	enableConcurrentOpacityChecking bool
 }
 
 var instance *config
@@ -59,7 +59,7 @@ func newConfig() *config {
 		port:                            port,
 		cacheTime:                       v.GetInt("cache.time"),
 		source:                          s,
-		enableConcurrentImageProcessing: v.GetBool("enableConcurrentImageProcessing"),
+		enableConcurrentOpacityChecking: v.GetBool("enableConcurrentOpacityChecking"),
 	}
 }
 
@@ -108,7 +108,7 @@ 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
+// ConcurrentOpacityCheckingEnabled returns true if we want to process image using multiple cores (checking isOpaque)
+func ConcurrentOpacityCheckingEnabled() bool {
+	return getConfig().enableConcurrentOpacityChecking
 }

pkg/processor/native/utils.go

@@ -7,14 +7,28 @@ import (
 	"image"
 )
 
+func hasFastIsOpaque(im image.Image) bool {
+	if _, ok := im.(*image.Gray); ok {
+		return true
+	}
+	if _, ok := im.(*image.Gray16); ok {
+		return true
+	}
+	if _, ok := im.(*image.CMYK); ok {
+		return true
+	}
+	return false
+}
+
 func isOpaque(im image.Image) bool {
-	// Check if image has Opaque() method:
-	if oim, ok := im.(interface {
-		Opaque() bool
-	}); ok {
-		return oim.Opaque() // It does, call it and return its result!
+	// Check if image has fast Opaque checking method
+	if hasFastIsOpaque(im) {
+		oim, _ := im.(interface {
+			Opaque() bool
+		})
+		return oim.Opaque()
 	}
-	// No Opaque() method, we need to loop through all pixels and check manually:
+	// No fast Opaque() method, we need to loop through all pixels and check manually:
 	rect := im.Bounds()
 	isOpaque := true
 	f := func(start, end int) {
@@ -26,12 +40,12 @@ func isOpaque(im image.Image) bool {
 			}
 		}
 	}
-	if config.ConcurrentImageProcessingEnabled() {
+	if config.ConcurrentOpacityCheckingEnabled() {
 		parallel.Line(rect.Dy(), f)
 	} else {
 		f(rect.Min.Y, rect.Max.Y)
 	}
-	return isOpaque // All pixels are opaque, so is the image
+	return isOpaque
 }
 
 // rw: required width, rh: required height, aw: actual width, ah: actual height

pkg/processor/native/utils_test.go

@@ -1,6 +1,7 @@
 package native
 
 import (
+	"github.com/gojek/darkroom/pkg/config"
 	"github.com/gojek/darkroom/pkg/processor"
 	"github.com/stretchr/testify/assert"
 	"image"
@@ -100,33 +101,57 @@ func TestGetStartingPointForCrop(t *testing.T) {
 	assert.Equal(t, 0, y)
 }
 
-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_isOpaqueWithFastOpaqueMethod(t *testing.T) {
+	r := image.Rect(0, 0, 640, 480)
+	gray, gray16, cmyk := image.NewGray(r), image.NewGray16(r), image.NewCMYK(r)
+	assert.True(t, isOpaque(gray))
+	assert.True(t, isOpaque(gray16))
+	assert.True(t, isOpaque(cmyk))
 }
 
-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)
+func Test_isOpaqueWithoutFastOpaqueMethodShouldReturnTrue(t *testing.T) {
+	isOpaqueShouldReturnTrue := func() {
+		img := NewMockImage(image.Rect(0, 0, 640, 480))
+		draw.Draw(img, img.Bounds(), image.Opaque, image.ZP, draw.Src)
 		val := isOpaque(img)
-		assert.False(t, val)
-		img.Set(x, y, image.Opaque.C)
+		assert.True(t, val)
+	}
+	v := config.Viper()
+	v.Set("enableConcurrentOpacityChecking", true)
+	config.Update()
+	isOpaqueShouldReturnTrue()
+	v.Set("enableConcurrentOpacityChecking", false)
+	isOpaqueShouldReturnTrue()
+}
+
+func Test_isOpaqueWithoutFastOpaqueMethodShouldReturnFalse(t *testing.T) {
+	isOpaqueShouldReturnFalse := func() {
+		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)
+		}
 	}
+	v := config.Viper()
+	v.Set("enableConcurrentOpacityChecking", true)
+	config.Update()
+	isOpaqueShouldReturnFalse()
+	v.Set("enableConcurrentOpacityChecking", false)
+	isOpaqueShouldReturnFalse()
 }
 
 type mockImage struct {